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

import-js · Nov 19, 2018 · ab3a1a1 · ab3a1a1
1 parent 798eed7
commit ab3a1a1
Show file tree

Hide file tree

Showing 10 changed files with 176 additions and 95 deletions.
diff --git a/src/core/addForGenericImport.js 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
+        },
+      ],
+    }),
   ],
 })