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

Fixes #1230.

Author: ljqx

Diff for: 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,10 @@ module.exports = {
       return false;
     }
 
-    function checkFileExtension(node) {
-      const { source } = node;
-
+    function checkFileExtension(source) {
       // bail if the declaration doesn't have a source, e.g. "export { foo };"
       if (!source) return;
-
+      
       const importPathWithQueryString = source.value;
 
       // don't enforce anything on builtins
@@ -181,9 +180,6 @@ module.exports = {
       }
     }
 
-    return {
-      ImportDeclaration: checkFileExtension,
-      ExportNamedDeclaration: checkFileExtension,
-    };
+    return moduleVisitor(checkFileExtension, { commonjs: true });
   },
 };

Diff for: 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 for: src/rules/no-extraneous-dependencies.js

@@ -207,8 +207,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 for: 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 for: 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 for: 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 for: 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 for: 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 for: 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,
+        },
+      ],
+    }),
   ],
 });