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

Fixes #1230.

Author: ljqx

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)
+      }
+    },
+  })
+}

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' ] }
@@ -121,12 +122,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
@@ -162,9 +158,6 @@ module.exports = {
       }
     }
 
-    return {
-      ImportDeclaration: checkFileExtension,
-      ExportNamedDeclaration: checkFileExtension,
-    }
+    return addForGenericImport({}, checkFileExtension)
   },
 }

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
@@ -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 addForGenericImport({
       'Program:exit': function () {
         countDependencies(dependencies, lastNode, context)
       },
-    }
+    }, (source) => {
+      dependencies.add(source.value)
+      lastNode = source
+    })
   },
 }

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 = {}) {
@@ -189,15 +189,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)
+    })
   },
 }

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 = {
@@ -87,16 +87,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)
+    })
   },
 }

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) {
@@ -20,15 +20,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)
+    })
   },
 }

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 = {
@@ -65,17 +65,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)
+    })
   },
 }

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) {
@@ -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 addForGenericImport({}, (source, node) => {
+      isImportingSelf(context, node, source.value)
+    })
   },
 }

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) {
@@ -18,15 +18,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)
+    })
   },
 }

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,
+        },
+      ],
+    }),
   ],
 })