From ab3a1a1b836cd850b9a5f4d082ccf5dc50eabf93 Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Mon, 19 Nov 2018 13:52:41 +0800 Subject: [PATCH] [Generic Import Callback] Make callback for all import once in rules --- src/core/addForGenericImport.js | 37 +++++++++ src/rules/extensions.js | 13 +-- src/rules/max-dependencies.js | 22 ++--- src/rules/no-extraneous-dependencies.js | 15 +--- src/rules/no-internal-modules.js | 16 +--- src/rules/no-nodejs-modules.js | 15 +--- src/rules/no-restricted-paths.js | 17 +--- src/rules/no-self-import.js | 15 +--- src/rules/no-webpack-loader-syntax.js | 15 +--- tests/src/rules/extensions.js | 106 ++++++++++++++++++++++++ 10 files changed, 176 insertions(+), 95 deletions(-) create mode 100644 src/core/addForGenericImport.js diff --git a/src/core/addForGenericImport.js b/src/core/addForGenericImport.js new file mode 100644 index 0000000000..66d458f58d --- /dev/null +++ b/src/core/addForGenericImport.js @@ -0,0 +1,37 @@ +import isStaticRequire from './staticRequire' + +function noop() {} + +export default function addForGenericImport(allCallbacks, genericImportCallback) { + const { + ImportDeclaration = noop, + ExportNamedDeclaration = noop, + ExportAllDeclaration = noop, + CallExpression = noop, + } = allCallbacks + + return Object.assign({}, allCallbacks, { + ImportDeclaration(node) { + ImportDeclaration(node) + genericImportCallback(node.source, node) + }, + ExportNamedDeclaration(node) { + ExportNamedDeclaration(node) + const { source } = node + // bail if the declaration doesn't have a source, e.g. "export { foo };" + if (source) { + genericImportCallback(source, node) + } + }, + ExportAllDeclaration(node) { + ExportAllDeclaration(node) + genericImportCallback(node.source, node) + }, + CallExpression(node) { + CallExpression(node) + if (isStaticRequire(node)) { + genericImportCallback(node.arguments[0], node) + } + }, + }) +} diff --git a/src/rules/extensions.js b/src/rules/extensions.js index d50bd0ce8b..145223b72a 100644 --- a/src/rules/extensions.js +++ b/src/rules/extensions.js @@ -2,6 +2,7 @@ import path from 'path' import resolve from 'eslint-module-utils/resolve' import { isBuiltIn, isExternalModuleMain, isScopedMain } from '../core/importType' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' const enumValues = { enum: [ 'always', 'ignorePackages', 'never' ] } @@ -120,12 +121,7 @@ module.exports = { return resolvedFileWithoutExtension === resolve(file, context) } - function checkFileExtension(node) { - const { source } = node - - // bail if the declaration doesn't have a source, e.g. "export { foo };" - if (!source) return - + function checkFileExtension(source) { const importPath = source.value // don't enforce anything on builtins @@ -161,9 +157,6 @@ module.exports = { } } - return { - ImportDeclaration: checkFileExtension, - ExportNamedDeclaration: checkFileExtension, - } + return addForGenericImport({}, checkFileExtension) }, } diff --git a/src/rules/max-dependencies.js b/src/rules/max-dependencies.js index 9af8f7912e..1efef2de4f 100644 --- a/src/rules/max-dependencies.js +++ b/src/rules/max-dependencies.js @@ -1,4 +1,4 @@ -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' const DEFAULT_MAX = 10 @@ -35,23 +35,13 @@ module.exports = { const dependencies = new Set() // keep track of dependencies let lastNode // keep track of the last node to report on - return { - ImportDeclaration(node) { - dependencies.add(node.source.value) - lastNode = node.source - }, - - CallExpression(node) { - if (isStaticRequire(node)) { - const [ requirePath ] = node.arguments - dependencies.add(requirePath.value) - lastNode = node - } - }, - + return addForGenericImport({ 'Program:exit': function () { countDependencies(dependencies, lastNode, context) }, - } + }, (source) => { + dependencies.add(source.value) + lastNode = source + }) }, } diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 528bb827ba..7e17daa57d 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -5,7 +5,7 @@ import readPkgUp from 'read-pkg-up' import minimatch from 'minimatch' import resolve from 'eslint-module-utils/resolve' import importType from '../core/importType' -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' function hasKeys(obj = {}) { @@ -188,15 +188,8 @@ module.exports = { } // todo: use module visitor from module-utils core - return { - ImportDeclaration: function (node) { - reportIfMissing(context, deps, depsOptions, node, node.source.value) - }, - CallExpression: function handleRequires(node) { - if (isStaticRequire(node)) { - reportIfMissing(context, deps, depsOptions, node, node.arguments[0].value) - } - }, - } + return addForGenericImport({}, (source, node) => { + reportIfMissing(context, deps, depsOptions, node, source.value) + }) }, } diff --git a/src/rules/no-internal-modules.js b/src/rules/no-internal-modules.js index 3e28554faa..0c38554d40 100644 --- a/src/rules/no-internal-modules.js +++ b/src/rules/no-internal-modules.js @@ -2,7 +2,7 @@ import minimatch from 'minimatch' import resolve from 'eslint-module-utils/resolve' import importType from '../core/importType' -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' module.exports = { @@ -86,16 +86,8 @@ module.exports = { } } - return { - ImportDeclaration(node) { - checkImportForReaching(node.source.value, node.source) - }, - CallExpression(node) { - if (isStaticRequire(node)) { - const [ firstArgument ] = node.arguments - checkImportForReaching(firstArgument.value, firstArgument) - } - }, - } + return addForGenericImport({}, (source) => { + checkImportForReaching(source.value, source) + }) }, } diff --git a/src/rules/no-nodejs-modules.js b/src/rules/no-nodejs-modules.js index e73ed379d0..5b13e975b7 100644 --- a/src/rules/no-nodejs-modules.js +++ b/src/rules/no-nodejs-modules.js @@ -1,5 +1,5 @@ import importType from '../core/importType' -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' function reportIfMissing(context, node, allowed, name) { @@ -19,15 +19,8 @@ module.exports = { const options = context.options[0] || {} const allowed = options.allow || [] - return { - ImportDeclaration: function handleImports(node) { - reportIfMissing(context, node, allowed, node.source.value) - }, - CallExpression: function handleRequires(node) { - if (isStaticRequire(node)) { - reportIfMissing(context, node, allowed, node.arguments[0].value) - } - }, - } + return addForGenericImport({}, (source, node) => { + reportIfMissing(context, node, allowed, source.value) + }) }, } diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index 5b20c40d84..bf560d3891 100644 --- a/src/rules/no-restricted-paths.js +++ b/src/rules/no-restricted-paths.js @@ -2,7 +2,7 @@ import containsPath from 'contains-path' import path from 'path' import resolve from 'eslint-module-utils/resolve' -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' module.exports = { @@ -64,17 +64,8 @@ module.exports = { }) } - return { - ImportDeclaration(node) { - checkForRestrictedImportPath(node.source.value, node.source) - }, - CallExpression(node) { - if (isStaticRequire(node)) { - const [ firstArgument ] = node.arguments - - checkForRestrictedImportPath(firstArgument.value, firstArgument) - } - }, - } + return addForGenericImport({}, (source) => { + checkForRestrictedImportPath(source.value, source) + }) }, } diff --git a/src/rules/no-self-import.js b/src/rules/no-self-import.js index 8a8620c9ae..3d6ad48812 100644 --- a/src/rules/no-self-import.js +++ b/src/rules/no-self-import.js @@ -4,7 +4,7 @@ */ import resolve from 'eslint-module-utils/resolve' -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' function isImportingSelf(context, node, requireName) { @@ -30,15 +30,8 @@ module.exports = { schema: [], }, create: function (context) { - return { - ImportDeclaration(node) { - isImportingSelf(context, node, node.source.value) - }, - CallExpression(node) { - if (isStaticRequire(node)) { - isImportingSelf(context, node, node.arguments[0].value) - } - }, - } + return addForGenericImport({}, (source, node) => { + isImportingSelf(context, node, source.value) + }) }, } diff --git a/src/rules/no-webpack-loader-syntax.js b/src/rules/no-webpack-loader-syntax.js index e89fc9c35c..4c12dec99d 100644 --- a/src/rules/no-webpack-loader-syntax.js +++ b/src/rules/no-webpack-loader-syntax.js @@ -1,4 +1,4 @@ -import isStaticRequire from '../core/staticRequire' +import addForGenericImport from '../core/addForGenericImport' import docsUrl from '../docsUrl' function reportIfNonStandard(context, node, name) { @@ -17,15 +17,8 @@ module.exports = { }, create: function (context) { - return { - ImportDeclaration: function handleImports(node) { - reportIfNonStandard(context, node, node.source.value) - }, - CallExpression: function handleRequires(node) { - if (isStaticRequire(node)) { - reportIfNonStandard(context, node, node.arguments[0].value) - } - }, - } + return addForGenericImport({}, (source, node) => { + reportIfNonStandard(context, node, source.value) + }) }, } diff --git a/tests/src/rules/extensions.js b/tests/src/rules/extensions.js index 8b816daa9e..02b2a20f7c 100644 --- a/tests/src/rules/extensions.js +++ b/tests/src/rules/extensions.js @@ -330,6 +330,22 @@ ruleTester.run('extensions', rule, { options: [ 'never', {ignorePackages: true} ], }), + test({ + code: ` + import foo from './foo.js' + import bar from './bar.json' + import Component from './Component.jsx' + `, + errors: [ + { + message: 'Unexpected use of file extension "jsx" for "./Component.jsx"', + line: 4, + column: 31, + }, + ], + options: [ 'always', {pattern: {jsx: 'never'}} ], + }), + // export (#964) test({ code: [ @@ -359,5 +375,95 @@ ruleTester.run('extensions', rule, { }, ], }), + + // require (#1230) + test({ + code: [ + 'const { foo } = require("./foo")', + 'export { bar }', + ].join('\n'), + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "./foo"', + line: 1, + column: 25, + }, + ], + }), + test({ + code: [ + 'const { foo } = require("./foo.js")', + 'export { bar }', + ].join('\n'), + options: [ 'never' ], + errors: [ + { + message: 'Unexpected use of file extension "js" for "./foo.js"', + line: 1, + column: 25, + }, + ], + }), + + // export { } from + test({ + code: [ + 'export { foo } from "./foo"', + 'export { bar }', + ].join('\n'), + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "./foo"', + line: 1, + column: 21, + }, + ], + }), + test({ + code: [ + 'export { foo } from "./foo.js"', + 'export { bar }', + ].join('\n'), + options: [ 'never' ], + errors: [ + { + message: 'Unexpected use of file extension "js" for "./foo.js"', + line: 1, + column: 21, + }, + ], + }), + + // export * from + test({ + code: [ + 'export * from "./foo"', + 'export { bar }', + ].join('\n'), + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "./foo"', + line: 1, + column: 15, + }, + ], + }), + test({ + code: [ + 'export * from "./foo.js"', + 'export { bar }', + ].join('\n'), + options: [ 'never' ], + errors: [ + { + message: 'Unexpected use of file extension "js" for "./foo.js"', + line: 1, + column: 15, + }, + ], + }), ], })