From 5368cb0d86b7172f5075288f30efba780585c8c1 Mon Sep 17 00:00:00 2001 From: Zhibin Liu Date: Mon, 19 Nov 2018 13:52:41 +0800 Subject: [PATCH] [Fix] [Generic Import Callback] Make callback for all import once in rules Fixes #1230. --- src/rules/extensions.js | 13 +--- src/rules/max-dependencies.js | 22 ++---- src/rules/no-extraneous-dependencies.js | 4 +- src/rules/no-internal-modules.js | 24 ++----- 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 | 93 +++++++++++++++++++++++++ 9 files changed, 124 insertions(+), 94 deletions(-) diff --git a/src/rules/extensions.js b/src/rules/extensions.js index fd9d177adf..4e6b5a2c61 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, isExternalModule, isScoped, isScopedModule } from '../core/importType' +import moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' const enumValues = { enum: [ 'always', 'ignorePackages', 'never' ] } @@ -134,12 +135,7 @@ module.exports = { return false } - 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 importPathWithQueryString = source.value // don't enforce anything on builtins @@ -181,9 +177,6 @@ module.exports = { } } - return { - ImportDeclaration: checkFileExtension, - ExportNamedDeclaration: checkFileExtension, - } + return moduleVisitor(checkFileExtension, { commonjs: true }) }, } diff --git a/src/rules/max-dependencies.js b/src/rules/max-dependencies.js index 7e1fdb1011..05793e613c 100644 --- a/src/rules/max-dependencies.js +++ b/src/rules/max-dependencies.js @@ -1,4 +1,4 @@ -import isStaticRequire from '../core/staticRequire' +import moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' const DEFAULT_MAX = 10 @@ -36,23 +36,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 Object.assign({ 'Program:exit': function () { countDependencies(dependencies, lastNode, context) }, - } + }, moduleVisitor((source) => { + dependencies.add(source.value) + lastNode = source + }, { commonjs: true })) }, } diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 366a684c40..b9a056b4f5 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -200,8 +200,8 @@ module.exports = { allowBundledDeps: testConfig(options.bundledDependencies, filename) !== false, } - return moduleVisitor(node => { - reportIfMissing(context, deps, depsOptions, node, node.value) + return moduleVisitor((source, node) => { + reportIfMissing(context, deps, depsOptions, node, source.value) }, {commonjs: true}) }, } diff --git a/src/rules/no-internal-modules.js b/src/rules/no-internal-modules.js index bd13ab07d0..e641ec0ccf 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 moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' module.exports = { @@ -87,24 +87,8 @@ module.exports = { } } - return { - ImportDeclaration(node) { - checkImportForReaching(node.source.value, node.source) - }, - ExportAllDeclaration(node) { - checkImportForReaching(node.source.value, node.source) - }, - ExportNamedDeclaration(node) { - if (node.source) { - checkImportForReaching(node.source.value, node.source) - } - }, - CallExpression(node) { - if (isStaticRequire(node)) { - const [ firstArgument ] = node.arguments - checkImportForReaching(firstArgument.value, firstArgument) - } - }, - } + return moduleVisitor((source) => { + checkImportForReaching(source.value, source) + }, { commonjs: true }) }, } diff --git a/src/rules/no-nodejs-modules.js b/src/rules/no-nodejs-modules.js index fb9bc23e28..0f3ed11c93 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 moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' function reportIfMissing(context, node, allowed, name) { @@ -35,15 +35,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 moduleVisitor((source, node) => { + reportIfMissing(context, node, allowed, source.value) + }, { commonjs: true }) }, } diff --git a/src/rules/no-restricted-paths.js b/src/rules/no-restricted-paths.js index a94b11ec16..8423e68a41 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 moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' import importType from '../core/importType' @@ -109,17 +109,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 moduleVisitor((source) => { + checkForRestrictedImportPath(source.value, source) + }, { commonjs: true }) }, } diff --git a/src/rules/no-self-import.js b/src/rules/no-self-import.js index b869d46e06..09a315e470 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 moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' function isImportingSelf(context, node, requireName) { @@ -31,15 +31,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 moduleVisitor((source, node) => { + isImportingSelf(context, node, source.value) + }, { commonjs: true }) }, } diff --git a/src/rules/no-webpack-loader-syntax.js b/src/rules/no-webpack-loader-syntax.js index 8075a6f9eb..18a2dc33da 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 moduleVisitor from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' function reportIfNonStandard(context, node, name) { @@ -19,15 +19,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 moduleVisitor((source, node) => { + reportIfNonStandard(context, node, source.value) + }, { commonjs: true }) }, } diff --git a/tests/src/rules/extensions.js b/tests/src/rules/extensions.js index 93860c16ab..364a7b647d 100644 --- a/tests/src/rules/extensions.js +++ b/tests/src/rules/extensions.js @@ -391,6 +391,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: [ @@ -444,6 +460,48 @@ ruleTester.run('extensions', rule, { }, ], }), + // require (#1230) + test({ + code: [ + 'const { foo } = require("./foo")', + 'export { foo }', + ].join('\n'), + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "./foo"', + line: 1, + column: 25, + }, + ], + }), + test({ + code: [ + 'const { foo } = require("./foo.js")', + 'export { foo }', + ].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"', + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "./foo"', + line: 1, + column: 21, + }, + ], + }), test({ code: 'import foo from "@/ImNotAScopedModule"', options: ['always'], @@ -454,5 +512,40 @@ ruleTester.run('extensions', rule, { }, ], }), + test({ + code: 'export { foo } from "./foo.js"', + options: [ 'never' ], + errors: [ + { + message: 'Unexpected use of file extension "js" for "./foo.js"', + line: 1, + column: 21, + }, + ], + }), + + // export * from + test({ + code: 'export * from "./foo"', + options: [ 'always' ], + errors: [ + { + message: 'Missing file extension for "./foo"', + line: 1, + column: 15, + }, + ], + }), + test({ + code: 'export * from "./foo.js"', + options: [ 'never' ], + errors: [ + { + message: 'Unexpected use of file extension "js" for "./foo.js"', + line: 1, + column: 15, + }, + ], + }), ], })