From 42fb05db0fcade4ad6d16b8c9a6615ac1e6c10d2 Mon Sep 17 00:00:00 2001 From: Naveen Jain Date: Tue, 10 Mar 2020 19:42:34 +0530 Subject: [PATCH] Added @internal Js comments to internal API functions Created ESLint rule to check whether internal API functions are properly annotated or not Added support to get list of puvlic API functions (map.json created during linting) Signed-off-by: Naveen Jain --- package.json | 4 +- resources/babel-plugins/extract-exports.js | 117 +++++++++++++++++++++ resources/babel-plugins/helper/index.js | 33 ++++++ resources/eslint-rules/internal-func.js | 49 +++++++++ resources/generate-exported.js | 26 +++++ src/language/ast.js | 2 + src/validation/ValidationContext.js | 4 + 7 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 resources/babel-plugins/extract-exports.js create mode 100644 resources/babel-plugins/helper/index.js create mode 100644 resources/eslint-rules/internal-func.js create mode 100644 resources/generate-exported.js diff --git a/package.json b/package.json index 8618d5c1ac..bfd88c4986 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,9 @@ "test:ci": "yarn check --integrity && npm run prettier:check && npm run lint -- --no-cache && npm run check && npm run testonly:cover && npm run check:ts && npm run check:spelling && npm run build", "testonly": "mocha --full-trace src/**/__tests__/**/*-test.js", "testonly:cover": "nyc npm run testonly", - "lint": "eslint --rulesdir './resources/eslint-rules' --rule 'no-dir-import: error' --cache --ext .js,.ts src resources", + "prelint": "node resources/generate-exported", + "lint": "eslint --rulesdir './resources/eslint-rules' --rule 'no-dir-import: error' --cache --ext .js,.ts src resources && eslint --rulesdir ./resources/eslint-rules/ --rule 'internal-func: 1' src --ignore-pattern 'src/jsutils/*' --ignore-pattern 'src/polyfills/*' --ignore-pattern 'src/**/__tests__/**' --ignore-pattern 'src/__tests__/*'", + "postlint": "rm resources/babel-plugins/map.json", "benchmark": "node --noconcurrent_sweeping --expose-gc --predictable ./resources/benchmark.js", "prettier": "prettier --ignore-path .gitignore --write --list-different \"**/*.{js,ts,md,json,yml}\"", "prettier:check": "prettier --ignore-path .gitignore --check \"**/*.{js,ts,md,json,yml}\"", diff --git a/resources/babel-plugins/extract-exports.js b/resources/babel-plugins/extract-exports.js new file mode 100644 index 0000000000..d3e5f61aa4 --- /dev/null +++ b/resources/babel-plugins/extract-exports.js @@ -0,0 +1,117 @@ +// @flow strict + +'use strict'; + +// @noflow + +const path = require('path'); +const fs = require('fs'); + +const { HashMap } = require('./helper'); + +// Create a new Hashmap to store file names and declarations +const mp = new HashMap(); + +// All the rules that are required by plugin +const ExportNamedDeclaration = { + enter(babelPath, state) { + if (babelPath.node.declaration) { + return handleDeclaration(babelPath, state); + } + + if (babelPath.node.specifiers) { + return handleNodeSpecifiers( + babelPath.node.specifiers, + state, + babelPath.node.source ? babelPath.node.source.value : undefined, + ); + } + }, +}; + +const ExportAllDeclaration = { + enter(babelPath, state) { + mp.add( + '*', + state, + babelPath.node.source ? babelPath.node.source.value : undefined, + ); + }, +}; + +module.exports = function() { + return { + visitor: { + ExportNamedDeclaration, + ExportAllDeclaration, + ExportDefaultDeclaration: ExportNamedDeclaration, + Program: { + exit() { + return writeToJSON(); + }, + }, + }, + }; +}; + +// Helper functions for the rules +function handleDeclaration(babelPath, state) { + switch (babelPath.node.declaration.type) { + case 'VariableDeclaration': + return handleVariableDeclarations( + babelPath.node.declaration, + state, + babelPath.node.source ? babelPath.node.source.value : undefined, + ); + case 'FunctionDeclaration': + case 'ClassDeclaration': + return handleFunctionDeclarations( + babelPath.node.declaration, + state, + babelPath.node.source ? babelPath.node.source.value : undefined, + ); + } +} + +function handleNodeSpecifiers(specifiers, state, source) { + return specifiers.forEach(specifier => { + switch (specifier.type) { + case 'ExportSpecifier': + mp.add(specifier.local.name, state, source); + break; + case 'ExportNamespaceSpecifier': + mp.add('*', state, source); + break; + } + }); +} + +function handleVariableDeclarations(variableDeclaration, state, source) { + variableDeclaration.declarations.forEach(declaration => + mp.add(declaration.id.name, state, source), + ); +} + +function handleFunctionDeclarations(declaration, state, source) { + return mp.add(declaration.id.name, state, source); +} + +// To write final result to JSON file +function writeToJSON() { + if (!fs.existsSync(path.join(__dirname, '/map.json'))) { + fs.writeFileSync(path.join(__dirname, '/map.json'), JSON.stringify({})); + } + const exportedValues = require(path.join(__dirname, '/map.json')); + for (const key of mp.keys()) { + exportedValues[key] = exportedValues[key] || []; + + exportedValues[key] = exportedValues[key].concat(Array.from(mp.get(key))); + + exportedValues[key] = Array.from(new Set(exportedValues[key])); + } + + fs.writeFileSync( + path.join(__dirname, '/map.json'), + JSON.stringify(exportedValues), + ); +} diff --git a/resources/babel-plugins/helper/index.js b/resources/babel-plugins/helper/index.js new file mode 100644 index 0000000000..c08604e37c --- /dev/null +++ b/resources/babel-plugins/helper/index.js @@ -0,0 +1,33 @@ +// @flow strict + +'use strict'; +const path = require('path'); + +class HashMap { + constructor() { + this._map = new Map(); + } + + add(value, state, source) { + let filepath = path.resolve(state.file.opts.filename); + if (source) { + const pathArray = state.file.opts.filename.split('/'); + const directoryPath = pathArray.slice(0, pathArray.length - 1).join('/'); + filepath = require.resolve(path.resolve(directoryPath, source)); + } + if (!this._map.has(filepath)) { + this._map.set(filepath, new Set()); + } + this._map.get(filepath).add(value); + } + + get(key) { + return this._map.get(key); + } + + keys() { + return this._map.keys(); + } +} + +module.exports = { HashMap }; diff --git a/resources/eslint-rules/internal-func.js b/resources/eslint-rules/internal-func.js new file mode 100644 index 0000000000..021e1c2171 --- /dev/null +++ b/resources/eslint-rules/internal-func.js @@ -0,0 +1,49 @@ +// @flow strict + +// @noflow + +'use strict'; +const path = require('path'); + +const listOfExports = require(path.join( + process.cwd(), + '/resources/babel-plugins/', + 'map.json', +)); +module.exports = { + create(context) { + function isExportedLocallyOnly(name) { + if (!listOfExports[context.getFilename()]) { + return true; + } + return !listOfExports[context.getFilename()].find( + value => value === name || value === '*', + ); + } + + const source = context.getSourceCode(); + /** + * + */ + return { + 'ExportNamedDeclaration > :matches(FunctionDeclaration,ClassDeclaration)'( + node, + ) { + if (isExportedLocallyOnly(node.id.name)) { + if (!source.getJSDocComment(node)) { + return context.report({ + node, + message: 'Please enter JSDoC internal functions using @internal', + }); + } + if (!source.getJSDocComment(node).value.includes('@internal')) { + context.report({ + node, + message: 'Please annotate internal functions using @internal', + }); + } + } + }, + }; + }, +}; diff --git a/resources/generate-exported.js b/resources/generate-exported.js new file mode 100644 index 0000000000..094096d05e --- /dev/null +++ b/resources/generate-exported.js @@ -0,0 +1,26 @@ +// @noflow + +'use strict'; + +const babel = require('@babel/core'); +const flowTypesPlugin = require('@babel/plugin-transform-flow-strip-types'); + +const extractExportPlugin = require('./babel-plugins/extract-exports'); + +const directoriesToScan = [ + '/src', + '/src/error', + '/src/type', + '/src/language', + '/src/validation', + '/src/utilities', + '/src/execution', + '/src/subscription', +]; + +directoriesToScan.forEach(path => + babel.transformFileSync(process.cwd() + path + '/index.js', { + babelrc: false, + plugins: [flowTypesPlugin, extractExportPlugin], + }), +); diff --git a/src/language/ast.js b/src/language/ast.js index 9d5df64d86..83a75e0a2e 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -8,6 +8,7 @@ import { type TokenKindEnum } from './tokenKind'; /** * Contains a range of UTF-8 character offsets and token references that * identify the region of the source from which the AST derived. + * @internal */ export class Location { /** @@ -52,6 +53,7 @@ defineToJSON(Location, function() { /** * Represents a range of characters represented by a lexical token * within a Source. + * @internal */ export class Token { /** diff --git a/src/validation/ValidationContext.js b/src/validation/ValidationContext.js index a8f720a05e..3c68cc2dbc 100644 --- a/src/validation/ValidationContext.js +++ b/src/validation/ValidationContext.js @@ -38,6 +38,7 @@ type VariableUsage = {| * An instance of this class is passed as the "this" context to all validators, * allowing access to commonly useful contextual information from within a * validation rule. + * @internal */ export class ASTValidationContext { _ast: DocumentNode; @@ -133,6 +134,9 @@ export class ASTValidationContext { export type ASTValidationRule = ASTValidationContext => ASTVisitor; +/** + * @internal + */ export class SDLValidationContext extends ASTValidationContext { _schema: ?GraphQLSchema;