[Fix] [Generic Import Callback] Make callback for all import once in rules

ljqx · ljqx · commit d8fd2ba5d9e0 · 2020-12-11T16:46:51.000+08:00
Fixes #1230.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
+        },
+      ],
+    }),
   ],
 })

Original file line number	Diff line number	Diff line change
`@@ -200,8 +200,8 @@ module.exports = {`
`200`	`200`	`allowBundledDeps: testConfig(options.bundledDependencies, filename) !== false,`
`201`	`201`	`}`
`202`	`202`
`203`		`- return moduleVisitor(node => {`
`204`		`- reportIfMissing(context, deps, depsOptions, node, node.value)`
	`203`	`+ return moduleVisitor((source, node) => {`
	`204`	`+ reportIfMissing(context, deps, depsOptions, node, source.value)`
`205`	`205`	`}, {commonjs: true})`
`206`	`206`	`},`
`207`	`207`	`}`