Merge pull request #827 from collinsauve/issue414

[new] import/extensions should have a ignorePackages option.

Fixes #414

Author: ljharb

docs/rules/extensions.md

@@ -6,7 +6,7 @@ In order to provide a consistent use of file extensions across your code base, t
 
 ## Rule Details
 
-This rule either takes one string option, one object option, or a string and an object option. If it is the string `"never"` (the default value), then the rule forbids the use for any extension. If it is the string `"always"`, then the rule enforces the use of extensions for all import statements.
+This rule either takes one string option, one object option, or a string and an object option. If it is the string `"never"` (the default value), then the rule forbids the use for any extension. If it is the string `"always"`, then the rule enforces the use of extensions for all import statements. If it is the string `"ignorePackages"`, then the rule enforces the use of extensions for all import statements except package imports.
 
 By providing an object you can configure each extension separately, so for example `{ "js": "always", "json": "never" }` would always enforce the use of the `.js` extension but never allow the use of the `.json` extension.
 
@@ -41,7 +41,7 @@ import foo from './foo.js';
 
 import bar from './bar.json';
 
-import Component from './Component.jsx'
+import Component from './Component.jsx';
 
 import express from 'express/index.js';
 ```
@@ -53,7 +53,7 @@ import foo from './foo';
 
 import bar from './bar';
 
-import Component from './Component'
+import Component from './Component';
 
 import express from 'express/index';
 
@@ -67,7 +67,7 @@ import foo from './foo';
 
 import bar from './bar';
 
-import Component from './Component'
+import Component from './Component';
 
 import express from 'express';
 ```
@@ -79,13 +79,48 @@ import foo from './foo.js';
 
 import bar from './bar.json';
 
-import Component from './Component.jsx'
+import Component from './Component.jsx';
 
 import express from 'express/index.js';
 
 import * as path from 'path';
 ```
 
+The following patterns are considered problems when configuration set to "ignorePackages":
+
+```js
+import foo from './foo';
+
+import bar from './bar';
+
+import Component from './Component';
+
+```
+
+The following patterns are not considered problems when configuration set to "ignorePackages":
+
+```js
+import foo from './foo.js';
+
+import bar from './bar.json';
+
+import Component from './Component.jsx';
+
+import express from 'express';
+
+```
+
+The following patterns are not considered problems when configuration set to `[ 'always', {ignorePackages: true} ]`:
+
+```js
+import Component from './Component.jsx';
+
+import baz from 'foo/baz.js';
+
+import express from 'express';
+
+```
+
 ## When Not To Use It
 
 If you are not concerned about a consistent usage of file extension.

src/core/importType.js

@@ -37,11 +37,21 @@ function isExternalModule(name, settings, path) {
   return externalModuleRegExp.test(name) && isExternalPath(path, name, settings)
 }
 
+const externalModuleMainRegExp = /^[\w]((?!\/).)*$/
+export function isExternalModuleMain(name, settings, path) {
+  return externalModuleMainRegExp.test(name) && isExternalPath(path, name, settings)
+}
+
 const scopedRegExp = /^@[^/]+\/[^/]+/
 function isScoped(name) {
   return scopedRegExp.test(name)
 }
 
+const scopedMainRegExp = /^@[^/]+\/?[^/]+$/
+export function isScopedMain(name) {
+  return scopedMainRegExp.test(name)
+}
+
 function isInternalModule(name, settings, path) {
   return externalModuleRegExp.test(name) && !isExternalPath(path, name, settings)
 }

src/rules/extensions.js

@@ -1,14 +1,56 @@
 import path from 'path'
-import has from 'has'
 
 import resolve from 'eslint-module-utils/resolve'
-import { isBuiltIn } from '../core/importType'
+import { isBuiltIn, isExternalModuleMain, isScopedMain } from '../core/importType'
 
-const enumValues = { enum: [ 'always', 'never' ] }
+const enumValues = { enum: [ 'always', 'ignorePackages', 'never' ] }
 const patternProperties = {
   type: 'object',
   patternProperties: { '.*': enumValues },
 }
+const properties = {
+  type: 'object',
+  properties: { 
+    'pattern': patternProperties,
+    'ignorePackages': { type: 'boolean' },
+  },
+}
+
+function buildProperties(context) {
+
+    const result = {
+      defaultConfig: 'never',
+      pattern: {},
+      ignorePackages: false,
+    }
+
+    context.options.forEach(obj => {
+
+      // If this is a string, set defaultConfig to its value
+      if (typeof obj === 'string') {
+        result.defaultConfig = obj
+        return
+      }
+
+      // If this is not the new structure, transfer all props to result.pattern
+      if (obj.pattern === undefined && obj.ignorePackages === undefined) {
+        Object.assign(result.pattern, obj)
+        return
+      }
+
+      // If pattern is provided, transfer all props
+      if (obj.pattern !== undefined) {
+        Object.assign(result.pattern, obj.pattern)
+      }
+
+      // If ignorePackages is provided, transfer it to result
+      if (obj.ignorePackages !== undefined) {
+        result.ignorePackages = obj.ignorePackages
+      }      
+    })
+
+    return result
+}
 
 module.exports = {
   meta: {
@@ -21,6 +63,19 @@ module.exports = {
           items: [enumValues],
           additionalItems: false,
         },
+        {
+          type: 'array',
+          items: [
+            enumValues, 
+            properties,
+          ],
+          additionalItems: false,
+        },
+        {
+          type: 'array',
+          items: [properties],
+          additionalItems: false,
+        },                
         {
           type: 'array',
           items: [patternProperties],
@@ -39,21 +94,19 @@ module.exports = {
   },
 
   create: function (context) {
-    const configuration = context.options[0] || 'never'
-    const defaultConfig = typeof configuration === 'string' ? configuration : null
-    const modifiers = Object.assign(
-      {},
-      typeof configuration === 'object' ? configuration : context.options[1]
-    )
-
-    function isUseOfExtensionRequired(extension) {
-      if (!has(modifiers, extension)) { modifiers[extension] = defaultConfig }
-      return modifiers[extension] === 'always'
+
+    const props = buildProperties(context)
+    
+    function getModifier(extension) {
+      return props.pattern[extension] || props.defaultConfig
+    }
+
+    function isUseOfExtensionRequired(extension, isPackageMain) {
+      return getModifier(extension) === 'always' && (!props.ignorePackages || !isPackageMain)
     }
 
     function isUseOfExtensionForbidden(extension) {
-      if (!has(modifiers, extension)) { modifiers[extension] = defaultConfig }
-      return modifiers[extension] === 'never'
+      return getModifier(extension) === 'never'
     }
 
     function isResolvableWithoutExtension(file) {
@@ -77,8 +130,15 @@ module.exports = {
       // for unresolved, use source value.
       const extension = path.extname(resolvedPath || importPath).substring(1)
 
+      // determine if this is a module
+      const isPackageMain = 
+        isExternalModuleMain(importPath, context.settings) || 
+        isScopedMain(importPath)
+
       if (!extension || !importPath.endsWith(extension)) {
-        if (isUseOfExtensionRequired(extension) && !isUseOfExtensionForbidden(extension)) {
+        const extensionRequired = isUseOfExtensionRequired(extension, isPackageMain)
+        const extensionForbidden = isUseOfExtensionForbidden(extension)
+        if (extensionRequired && !extensionForbidden) {
           context.report({
             node: source,
             message:

tests/src/rules/extensions.js

@@ -59,6 +59,37 @@ ruleTester.run('extensions', rule, {
     test({ code: 'import thing from "./fake-file.js"', options: [ 'always' ] }),
     test({ code: 'import thing from "non-package"', options: [ 'never' ] }),
 
+
+    test({
+      code: `
+        import foo from './foo.js'
+        import bar from './bar.json'
+        import Component from './Component'
+        import express from 'express'
+      `,
+      options: [ 'ignorePackages' ],
+    }),
+
+    test({
+      code: `
+        import foo from './foo.js'
+        import bar from './bar.json'
+        import Component from './Component.jsx'
+        import express from 'express'
+      `,
+      options: [ 'always', {ignorePackages: true} ],
+    }),
+
+    test({
+      code: `
+        import foo from './foo'
+        import bar from './bar'
+        import Component from './Component'
+        import express from 'express'
+      `,
+      options: [ 'never', {ignorePackages: true} ],
+    }),
+
   ],
 
   invalid: [
@@ -201,5 +232,49 @@ ruleTester.run('extensions', rule, {
       ],
     }),
 
+
+    test({
+      code: `
+        import foo from './foo.js'
+        import bar from './bar.json'
+        import Component from './Component'
+        import baz from 'foo/baz'
+        import express from 'express'
+      `,
+      options: [ 'always', {ignorePackages: true} ],
+      errors: [
+        {
+          message: 'Missing file extension for "./Component"',
+          line: 4,
+          column: 31,
+        }, {
+          message: 'Missing file extension for "foo/baz"',
+          line: 5,
+          column: 25,
+        },
+      ],
+    }),
+
+    test({
+      code: `
+        import foo from './foo.js'
+        import bar from './bar.json'
+        import Component from './Component.jsx'
+        import express from 'express'
+      `,
+      errors: [
+        {
+          message: 'Unexpected use of file extension "js" for "./foo.js"',
+          line: 2,
+          column: 25,
+        }, {
+          message: 'Unexpected use of file extension "jsx" for "./Component.jsx"',
+          line: 4,
+          column: 31,
+        },
+      ],
+      options: [ 'never', {ignorePackages: true} ],
+    }),
+
   ],
 })