From 8def087527ff4e392a45f09b7267031e9c7db857 Mon Sep 17 00:00:00 2001 From: Collin Sauve Date: Tue, 9 May 2017 21:11:08 -0400 Subject: [PATCH 1/6] import/extensions should have a ignorePackages option. Fixes #414 --- docs/rules/extensions.md | 26 +++++++++++- src/core/importType.js | 10 +++++ src/rules/extensions.js | 92 +++++++++++++++++++++++++++++++++------- 3 files changed, 111 insertions(+), 17 deletions(-) diff --git a/docs/rules/extensions.md b/docs/rules/extensions.md index 6ca33bb44..26a464ce9 100644 --- a/docs/rules/extensions.md +++ b/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. @@ -86,6 +86,30 @@ 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'; + +``` + ## When Not To Use It If you are not concerned about a consistent usage of file extension. diff --git a/src/core/importType.js b/src/core/importType.js index d663a1a87..4009b872e 100644 --- a/src/core/importType.js +++ b/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) } diff --git a/src/rules/extensions.js b/src/rules/extensions.js index 2036ba055..fef898b5a 100644 --- a/src/rules/extensions.js +++ b/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: From 62ff6e5ecf1f6084bd1ccb04099bd2df7883cfc9 Mon Sep 17 00:00:00 2001 From: Steve Miller Date: Mon, 30 Oct 2017 15:49:17 -0400 Subject: [PATCH 2/6] Fix lint no-useless-escape --- src/core/importType.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/importType.js b/src/core/importType.js index 4009b872e..3c4539e4f 100644 --- a/src/core/importType.js +++ b/src/core/importType.js @@ -47,7 +47,7 @@ function isScoped(name) { return scopedRegExp.test(name) } -const scopedMainRegExp = /^@[^\/]+\/?[^\/]+$/ +const scopedMainRegExp = /^@[^/]+\/?[^/]+$/ export function isScopedMain(name) { return scopedMainRegExp.test(name) } From fef0ab094a3c087568624f2e2f483e7ff6223063 Mon Sep 17 00:00:00 2001 From: Steve Miller Date: Mon, 30 Oct 2017 21:52:00 -0400 Subject: [PATCH 3/6] Add some test coverage for `ignorePackages` exts --- tests/src/rules/extensions.js | 70 +++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/src/rules/extensions.js b/tests/src/rules/extensions.js index 3ebd42c18..92b911de3 100644 --- a/tests/src/rules/extensions.js +++ b/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,44 @@ ruleTester.run('extensions', rule, { ], }), + + test({ + code: ` + import foo from './foo.js' + import bar from './bar.json' + import Component from './Component' + import express from 'express' + `, + options: [ 'always', {ignorePackages: true} ], + errors: [ + { + message: 'Missing file extension for "./Component"', + line: 4, + column: 31, + }, + ], + }), + + 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} ], + }), + ], }) From 2fc4577c88565bad15fb89e6fb9f8c856ea52cd8 Mon Sep 17 00:00:00 2001 From: Steve Miller Date: Tue, 31 Oct 2017 16:35:55 -0400 Subject: [PATCH 4/6] Should cover cherry-picked package methods ref: https://github.com/benmosher/eslint-plugin-import/pull/827/files/a8e8bfb664c560665fcdc92113c1b927c9981889#r115817771 --- tests/src/rules/extensions.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/src/rules/extensions.js b/tests/src/rules/extensions.js index 92b911de3..1b47066f7 100644 --- a/tests/src/rules/extensions.js +++ b/tests/src/rules/extensions.js @@ -238,6 +238,7 @@ ruleTester.run('extensions', rule, { 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} ], @@ -246,6 +247,10 @@ ruleTester.run('extensions', rule, { message: 'Missing file extension for "./Component"', line: 4, column: 31, + }, { + message: 'Missing file extension for "foo/baz"', + line: 5, + column: 25, }, ], }), From 7316dda08f1ab6086c0ebd985ab37c53db8edacd Mon Sep 17 00:00:00 2001 From: Steve Miller Date: Tue, 31 Oct 2017 16:51:44 -0400 Subject: [PATCH 5/6] Document cherry-picked package method ref: https://github.com/benmosher/eslint-plugin-import/pull/827/files/a8e8bfb664c560665fcdc92113c1b927c9981889#r115817771 --- docs/rules/extensions.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/rules/extensions.md b/docs/rules/extensions.md index 26a464ce9..5dd1b860f 100644 --- a/docs/rules/extensions.md +++ b/docs/rules/extensions.md @@ -110,6 +110,17 @@ 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. From 81d3b362c9821c01cfc78df3902c2b4cf63cdfbb Mon Sep 17 00:00:00 2001 From: Steve Miller Date: Tue, 31 Oct 2017 16:53:38 -0400 Subject: [PATCH 6/6] Typos --- docs/rules/extensions.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/rules/extensions.md b/docs/rules/extensions.md index 5dd1b860f..127439935 100644 --- a/docs/rules/extensions.md +++ b/docs/rules/extensions.md @@ -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,7 +79,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'; @@ -93,7 +93,7 @@ import foo from './foo'; import bar from './bar'; -import Component from './Component' +import Component from './Component'; ``` @@ -104,7 +104,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';