diff --git a/README.md b/README.md index d5613cea..53e0f379 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,7 @@ $ npm install --save-dev eslint eslint-plugin-node | Rule ID | Description | | |:--------|:------------|:--:| | [node/exports-style](./docs/rules/exports-style.md) | enforce either `module.exports` or `exports` | | +| [node/file-extension-in-import](./docs/rules/file-extension-in-import.md) | enforce the style of file extensions in `import` declarations | ✒️ | | [node/prefer-global/buffer](./docs/rules/prefer-global/buffer.md) | enforce either `Buffer` or `require("buffer").Buffer` | | | [node/prefer-global/console](./docs/rules/prefer-global/console.md) | enforce either `console` or `require("console")` | | | [node/prefer-global/process](./docs/rules/prefer-global/process.md) | enforce either `process` or `require("process")` | | diff --git a/docs/rules/file-extension-in-import.md b/docs/rules/file-extension-in-import.md new file mode 100644 index 00000000..fee33c1c --- /dev/null +++ b/docs/rules/file-extension-in-import.md @@ -0,0 +1,110 @@ +# enforce the style of file extensions in `import` declarations (file-extension-in-import) + +We can omit file extensions in `import`/`export` declarations. + +```js +import foo from "./path/to/a/file" // maybe it's resolved to 'file.js' or 'file.json' +export * from "./path/to/a/file" +``` + +However, [--experimental-modules](https://medium.com/@nodejs/announcing-a-new-experimental-modules-1be8d2d6c2ff) has declared to drop the file extension omition. + +Also, we can import a variety kind of files with bundlers such as Webpack. In the situation, probably explicit file extensions help us to understand code. + +## Rule Details + +This rule enforces the style of file extensions in `import`/`export` declarations. + +## Options + +This rule has a string option and an object option. + +```json +{ + "node/file-extension-in-import": [ + "error", + "always" or "never", + { + "tryExtensions": [".js", ".json", ".node"], + ".xxx": "always" or "never", + } + ] +} +``` + +- `"always"` (default) requires file extensions in `import`/`export` declarations. +- `"never"` disallows file extensions in `import`/`export` declarations. +- `tryExtensions` is the file extensions to resolve import paths. Default is `[".js", ".json", ".node"]`. +- `.xxx` is the overriding setting for specific file extensions. You can use arbitrary property names which start with `.`. + +### always + +Examples of :-1: **incorrect** code for the `"always"` option: + +```js +/*eslint node/file-extension-in-import: ["error", "always"]*/ + +import foo from "./path/to/a/file" +``` + +Examples of :+1: **correct** code for the `"always"` option: + +```js +/*eslint node/file-extension-in-import: ["error", "always"]*/ + +import eslint from "eslint" +import foo from "./path/to/a/file.js" +``` + +### never + +Examples of :-1: **incorrect** code for the `"never"` option: + +```js +/*eslint node/file-extension-in-import: ["error", "never"]*/ + +import foo from "./path/to/a/file.js" +``` + +Examples of :+1: **correct** code for the `"never"` option: + +```js +/*eslint node/file-extension-in-import: ["error", "never"]*/ + +import eslint from "eslint" +import foo from "./path/to/a/file" +``` + +### .xxx + +Examples of :+1: **correct** code for the `["always", { ".js": "never" }]` option: + +```js +/*eslint node/file-extension-in-import: ["error", "always", { ".js": "never" }]*/ + +import eslint from "eslint" +import script from "./script" +import styles from "./styles.css" +import logo from "./logo.png" +``` + +## Shared Settings + +The following options can be set by [shared settings](http://eslint.org/docs/user-guide/configuring.html#adding-shared-settings). +Several rules have the same option, but we can set this option at once. + +- `tryExtensions` + +```js +// .eslintrc.js +module.exports = { + "settings": { + "node": { + "tryExtensions": [".js", ".json", ".node"] + } + }, + "rules": { + "node/file-extension-in-import": "error" + } +} +``` diff --git a/lib/index.js b/lib/index.js index 8aec5dd8..cb057f45 100644 --- a/lib/index.js +++ b/lib/index.js @@ -11,6 +11,7 @@ module.exports = { }, rules: { "exports-style": require("./rules/exports-style"), + "file-extension-in-import": require("./rules/file-extension-in-import"), "no-deprecated-api": require("./rules/no-deprecated-api"), "no-extraneous-import": require("./rules/no-extraneous-import"), "no-extraneous-require": require("./rules/no-extraneous-require"), diff --git a/lib/rules/file-extension-in-import.js b/lib/rules/file-extension-in-import.js new file mode 100644 index 00000000..a46e944d --- /dev/null +++ b/lib/rules/file-extension-in-import.js @@ -0,0 +1,126 @@ +/** + * @author Toru Nagashima + * See LICENSE file in root directory for full license. + */ +"use strict" + +const path = require("path") +const fs = require("fs") +const getImportExportTargets = require("../util/get-import-export-targets") +const getTryExtensions = require("../util/get-try-extensions") + +/** + * Get all file extensions of the files which have the same basename. + * @param {string} filePath The path to the original file to check. + * @returns {string[]} File extensions. + */ +function getExistingExtensions(filePath) { + const basename = path.basename(filePath, path.extname(filePath)) + try { + return fs + .readdirSync(path.dirname(filePath)) + .filter( + filename => + path.basename(filename, path.extname(filename)) === basename + ) + .map(filename => path.extname(filename)) + } catch (_error) { + return [] + } +} + +module.exports = { + meta: { + docs: { + description: + "enforce the style of file extensions in `import` declarations", + category: "Stylistic Issues", + recommended: false, + url: + "https://github.com/mysticatea/eslint-plugin-node/blob/v8.0.1/docs/rules/file-extension-in-import.md", + }, + fixable: "code", + messages: { + requireExt: "require file extension '{{ext}}'.", + forbidExt: "forbid file extension '{{ext}}'.", + }, + schema: [ + { + enum: ["always", "never"], + }, + { + type: "object", + properties: { + tryExtensions: getTryExtensions.schema, + }, + additionalProperties: { + enum: ["always", "never"], + }, + }, + ], + type: "suggestion", + }, + create(context) { + if (context.getFilename().startsWith("<")) { + return {} + } + const defaultStyle = context.options[0] || "always" + const overrideStyle = context.options[1] || {} + + function verify({ filePath, name, node }) { + // Ignore if it's not resolved to a file or it's a bare module. + if (!filePath || !/[/\\]/u.test(name)) { + return + } + + // Get extension. + const originalExt = path.extname(name) + const resolvedExt = path.extname(filePath) + const existingExts = getExistingExtensions(filePath) + if (!resolvedExt && existingExts.length !== 1) { + // Ignore if the file extension could not be determined one. + return + } + const ext = resolvedExt || existingExts[0] + const style = overrideStyle[ext] || defaultStyle + + // Verify. + if (style === "always" && ext !== originalExt) { + context.report({ + node, + messageId: "requireExt", + data: { ext }, + fix(fixer) { + if (existingExts.length !== 1) { + return null + } + const index = node.range[1] - 1 + return fixer.insertTextBeforeRange([index, index], ext) + }, + }) + } else if (style === "never" && ext === originalExt) { + context.report({ + node, + messageId: "forbidExt", + data: { ext }, + fix(fixer) { + if (existingExts.length !== 1) { + return null + } + const index = name.lastIndexOf(ext) + const start = node.range[0] + 1 + index + const end = start + ext.length + return fixer.removeRange([start, end]) + }, + }) + } + } + + return { + "Program:exit"(node) { + const opts = { optionIndex: 1 } + getImportExportTargets(context, node, opts).forEach(verify) + }, + } + }, +} diff --git a/lib/rules/no-hide-core-modules.js b/lib/rules/no-hide-core-modules.js index 562f7a56..bdf7cd02 100644 --- a/lib/rules/no-hide-core-modules.js +++ b/lib/rules/no-hide-core-modules.js @@ -104,7 +104,9 @@ module.exports = { const targets = [] .concat( getRequireTargets(context, true), - getImportExportTargets(context, node, true) + getImportExportTargets(context, node, { + includeCore: true, + }) ) .filter(t => CORE_MODULES.has(t.moduleName)) diff --git a/lib/util/get-import-export-targets.js b/lib/util/get-import-export-targets.js index 38d09769..08ac36f2 100644 --- a/lib/util/get-import-export-targets.js +++ b/lib/util/get-import-export-targets.js @@ -20,18 +20,20 @@ const MODULE_TYPE = /^(?:Import|Export(?:Named|Default|All))Declaration$/u * * @param {RuleContext} context - The rule context. * @param {ASTNode} programNode - The node of Program. - * @param {boolean} includeCore - The flag to include core modules. + * @param {Object} [options] - The flag to include core modules. + * @param {boolean} [options.includeCore] - The flag to include core modules. + * @param {number} [options.optionIndex] - The index of rule options. * @returns {ImportTarget[]} A list of found target's information. */ module.exports = function getImportExportTargets( context, programNode, - includeCore + { includeCore = false, optionIndex = 0 } = {} ) { const retv = [] const basedir = path.dirname(path.resolve(context.getFilename())) - const paths = getResolvePaths(context) - const extensions = getTryExtensions(context) + const paths = getResolvePaths(context, optionIndex) + const extensions = getTryExtensions(context, optionIndex) const options = { basedir, paths, extensions } for (const statement of programNode.body) { diff --git a/lib/util/get-resolve-paths.js b/lib/util/get-resolve-paths.js index f2377a3f..14e553fc 100644 --- a/lib/util/get-resolve-paths.js +++ b/lib/util/get-resolve-paths.js @@ -29,9 +29,9 @@ function get(option) { * @param {RuleContext} context - The rule context. * @returns {string[]} A list of extensions. */ -module.exports = function getResolvePaths(context) { +module.exports = function getResolvePaths(context, optionIndex = 0) { return ( - get(context.options && context.options[0]) || + get(context.options && context.options[optionIndex]) || get(context.settings && context.settings.node) || DEFAULT_VALUE ) diff --git a/lib/util/get-try-extensions.js b/lib/util/get-try-extensions.js index 4b278f88..bed5f609 100644 --- a/lib/util/get-try-extensions.js +++ b/lib/util/get-try-extensions.js @@ -29,9 +29,9 @@ function get(option) { * @param {RuleContext} context - The rule context. * @returns {string[]} A list of extensions. */ -module.exports = function getTryExtensions(context) { +module.exports = function getTryExtensions(context, optionIndex = 0) { return ( - get(context.options && context.options[0]) || + get(context.options && context.options[optionIndex]) || get(context.settings && context.settings.node) || DEFAULT_VALUE ) diff --git a/tests/fixtures/file-extension-in-import/a.js b/tests/fixtures/file-extension-in-import/a.js new file mode 100644 index 00000000..e69de29b diff --git a/tests/fixtures/file-extension-in-import/b.json b/tests/fixtures/file-extension-in-import/b.json new file mode 100644 index 00000000..e69de29b diff --git a/tests/fixtures/file-extension-in-import/c.mjs b/tests/fixtures/file-extension-in-import/c.mjs new file mode 100644 index 00000000..e69de29b diff --git a/tests/fixtures/file-extension-in-import/multi.cjs b/tests/fixtures/file-extension-in-import/multi.cjs new file mode 100644 index 00000000..e69de29b diff --git a/tests/fixtures/file-extension-in-import/multi.mjs b/tests/fixtures/file-extension-in-import/multi.mjs new file mode 100644 index 00000000..e69de29b diff --git a/tests/lib/rules/file-extension-in-import.js b/tests/lib/rules/file-extension-in-import.js new file mode 100644 index 00000000..b7ab5a09 --- /dev/null +++ b/tests/lib/rules/file-extension-in-import.js @@ -0,0 +1,230 @@ +/** + * @author Toru Nagashima + * See LICENSE file in root directory for full license. + */ +"use strict" + +const path = require("path") +const RuleTester = require("eslint").RuleTester +const rule = require("../../../lib/rules/file-extension-in-import") + +function fixture(filename) { + return path.resolve( + __dirname, + "../../fixtures/file-extension-in-import", + filename + ) +} + +new RuleTester({ + parserOptions: { + ecmaVersion: 2015, + sourceType: "module", + }, + settings: { + node: { + tryExtensions: [".mjs", ".cjs", ".js", ".json", ".node"], + }, + }, +}).run("file-extension-in-import", rule, { + valid: [ + { + filename: fixture("test.js"), + code: "import 'eslint'", + }, + { + filename: fixture("test.js"), + code: "import 'xxx'", + }, + { + filename: fixture("test.js"), + code: "import './a.js'", + }, + { + filename: fixture("test.js"), + code: "import './b.json'", + }, + { + filename: fixture("test.js"), + code: "import './c.mjs'", + }, + { + filename: fixture("test.js"), + code: "import './a.js'", + options: ["always"], + }, + { + filename: fixture("test.js"), + code: "import './b.json'", + options: ["always"], + }, + { + filename: fixture("test.js"), + code: "import './c.mjs'", + options: ["always"], + }, + { + filename: fixture("test.js"), + code: "import './a'", + options: ["never"], + }, + { + filename: fixture("test.js"), + code: "import './b'", + options: ["never"], + }, + { + filename: fixture("test.js"), + code: "import './c'", + options: ["never"], + }, + { + filename: fixture("test.js"), + code: "import './a'", + options: ["always", { ".js": "never" }], + }, + { + filename: fixture("test.js"), + code: "import './b.json'", + options: ["always", { ".js": "never" }], + }, + { + filename: fixture("test.js"), + code: "import './c.mjs'", + options: ["always", { ".js": "never" }], + }, + { + filename: fixture("test.js"), + code: "import './a'", + options: ["never", { ".json": "always" }], + }, + { + filename: fixture("test.js"), + code: "import './b.json'", + options: ["never", { ".json": "always" }], + }, + { + filename: fixture("test.js"), + code: "import './c'", + options: ["never", { ".json": "always" }], + }, + ], + invalid: [ + { + filename: fixture("test.js"), + code: "import './a'", + output: "import './a.js'", + errors: [{ messageId: "requireExt", data: { ext: ".js" } }], + }, + { + filename: fixture("test.js"), + code: "import './b'", + output: "import './b.json'", + errors: [{ messageId: "requireExt", data: { ext: ".json" } }], + }, + { + filename: fixture("test.js"), + code: "import './c'", + output: "import './c.mjs'", + errors: [{ messageId: "requireExt", data: { ext: ".mjs" } }], + }, + { + filename: fixture("test.js"), + code: "import './a'", + output: "import './a.js'", + options: ["always"], + errors: [{ messageId: "requireExt", data: { ext: ".js" } }], + }, + { + filename: fixture("test.js"), + code: "import './b'", + output: "import './b.json'", + options: ["always"], + errors: [{ messageId: "requireExt", data: { ext: ".json" } }], + }, + { + filename: fixture("test.js"), + code: "import './c'", + output: "import './c.mjs'", + options: ["always"], + errors: [{ messageId: "requireExt", data: { ext: ".mjs" } }], + }, + { + filename: fixture("test.js"), + code: "import './a.js'", + output: "import './a'", + options: ["never"], + errors: [{ messageId: "forbidExt", data: { ext: ".js" } }], + }, + { + filename: fixture("test.js"), + code: "import './b.json'", + output: "import './b'", + options: ["never"], + errors: [{ messageId: "forbidExt", data: { ext: ".json" } }], + }, + { + filename: fixture("test.js"), + code: "import './c.mjs'", + output: "import './c'", + options: ["never"], + errors: [{ messageId: "forbidExt", data: { ext: ".mjs" } }], + }, + { + filename: fixture("test.js"), + code: "import './a.js'", + output: "import './a'", + options: ["always", { ".js": "never" }], + errors: [{ messageId: "forbidExt", data: { ext: ".js" } }], + }, + { + filename: fixture("test.js"), + code: "import './b'", + output: "import './b.json'", + options: ["always", { ".js": "never" }], + errors: [{ messageId: "requireExt", data: { ext: ".json" } }], + }, + { + filename: fixture("test.js"), + code: "import './c'", + output: "import './c.mjs'", + options: ["always", { ".js": "never" }], + errors: [{ messageId: "requireExt", data: { ext: ".mjs" } }], + }, + { + filename: fixture("test.js"), + code: "import './a.js'", + output: "import './a'", + options: ["never", { ".json": "always" }], + errors: [{ messageId: "forbidExt", data: { ext: ".js" } }], + }, + { + filename: fixture("test.js"), + code: "import './b'", + output: "import './b.json'", + options: ["never", { ".json": "always" }], + errors: [{ messageId: "requireExt", data: { ext: ".json" } }], + }, + { + filename: fixture("test.js"), + code: "import './c.mjs'", + output: "import './c'", + options: ["never", { ".json": "always" }], + errors: [{ messageId: "forbidExt", data: { ext: ".mjs" } }], + }, + { + filename: fixture("test.js"), + code: "import './multi'", + output: null, + options: ["always"], + errors: [{ messageId: "requireExt", data: { ext: ".mjs" } }], + }, + { + filename: fixture("test.js"), + code: "import './multi.cjs'", + output: null, + options: ["never"], + errors: [{ messageId: "forbidExt", data: { ext: ".cjs" } }], + }, + ], +})