From 7f8f543ebd95dae1bd258321ae1e6be8e4f61cfc Mon Sep 17 00:00:00 2001 From: Brian Suh <bsuh@users.noreply.github.com> Date: Wed, 21 Feb 2018 16:33:29 -0800 Subject: [PATCH 01/26] Fix eslint-import-resolver-webpack with pnpm (#968) Updated resolve to ^1.4.0, where it introduced option to not preserve symlinks when resolving, matching node's behavior. --- resolvers/webpack/index.js | 4 ++-- resolvers/webpack/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/resolvers/webpack/index.js b/resolvers/webpack/index.js index 42f9e9828..4273490b6 100644 --- a/resolvers/webpack/index.js +++ b/resolvers/webpack/index.js @@ -122,8 +122,8 @@ function createResolveSync(configPath, webpackConfig) { } try { - var webpackFilename = resolve.sync('webpack', { basedir }) - var webpackResolveOpts = { basedir: path.dirname(webpackFilename) } + var webpackFilename = resolve.sync('webpack', { basedir, preserveSymlinks: false }) + var webpackResolveOpts = { basedir: path.dirname(webpackFilename), preserveSymlinks: false } webpackRequire = function (id) { return require(resolve.sync(id, webpackResolveOpts)) diff --git a/resolvers/webpack/package.json b/resolvers/webpack/package.json index bcacdb07a..c4e87f316 100644 --- a/resolvers/webpack/package.json +++ b/resolvers/webpack/package.json @@ -38,7 +38,7 @@ "is-absolute": "^0.2.3", "lodash.get": "^4.4.2", "node-libs-browser": "^1.0.0 || ^2.0.0", - "resolve": "^1.2.0", + "resolve": "^1.4.0", "semver": "^5.3.0" }, "peerDependencies": { From 5be3f4a734fb448e8b6edb19366cdf9a9467311b Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Wed, 21 Feb 2018 19:35:25 -0500 Subject: [PATCH 02/26] changelog note for #968 --- resolvers/webpack/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/resolvers/webpack/CHANGELOG.md b/resolvers/webpack/CHANGELOG.md index 2d38ff458..1ebc0d351 100644 --- a/resolvers/webpack/CHANGELOG.md +++ b/resolvers/webpack/CHANGELOG.md @@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## Unreleased +### Breaking (?) +- Fix with `pnpm` ([#968]) ## 0.8.4 - 2018-01-05 ### Changed @@ -93,6 +95,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel Thanks to [@gausie] for the initial PR ([#164], ages ago! 😅) and [@jquense] for tests ([#278]). [#969]: https://github.com/benmosher/eslint-plugin-import/pull/969 +[#968]: https://github.com/benmosher/eslint-plugin-import/pull/968 [#683]: https://github.com/benmosher/eslint-plugin-import/pull/683 [#572]: https://github.com/benmosher/eslint-plugin-import/pull/572 [#569]: https://github.com/benmosher/eslint-plugin-import/pull/569 From 1f4ef022989aa71db379f6f1cdaa521377a53b89 Mon Sep 17 00:00:00 2001 From: Thomas Grainger <tagrain@gmail.com> Date: Thu, 22 Feb 2018 01:20:59 +0000 Subject: [PATCH 03/26] add changelog for no-useless-path-segments --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50b742d42..068bffea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - Add [`group-exports`] rule: style-guide rule to report use of multiple named exports ([#721], thanks [@robertrossmann]) - Add [`no-self-import`] rule: forbids a module from importing itself. ([#727], [#449], [#447], thanks [@giodamelio]). - Add [`no-default-export`] rule ([#889], thanks [@isiahmeadows]) +- Add [`no-useless-path-segments`] rule ([#912], thanks [@graingert] and [@danny-andrews]) - ... and more! check the commits for v[2.9.0] ## [2.8.0] - 2017-10-18 @@ -440,6 +441,7 @@ for info on changes for earlier releases. [`group-exports`]: ./docs/rules/group-exports.md [`no-self-import`]: ./docs/rules/no-self-import.md [`no-default-export`]: ./docs/rules/no-default-export.md +[`no-useless-path-segments`]: ./docs/rules/no-useless-path-segments.md [`memo-parser`]: ./memo-parser/README.md @@ -512,6 +514,7 @@ for info on changes for earlier releases. [#164]: https://github.com/benmosher/eslint-plugin-import/pull/164 [#157]: https://github.com/benmosher/eslint-plugin-import/pull/157 [#314]: https://github.com/benmosher/eslint-plugin-import/pull/314 +[#912]: https://github.com/benmosher/eslint-plugin-import/pull/912 [#886]: https://github.com/benmosher/eslint-plugin-import/issues/886 [#863]: https://github.com/benmosher/eslint-plugin-import/issues/863 @@ -678,3 +681,5 @@ for info on changes for earlier releases. [@alexgorbatchev]: https://github.com/alexgorbatchev [@robertrossmann]: https://github.com/robertrossmann [@isiahmeadows]: https://github.com/isiahmeadows +[@graingert]: https://github.com/graingert +[@danny-andrews]: https://github.com/dany-andrews From 402c60a3377fa2bbbb7980b2efa7f4afd62ebcd8 Mon Sep 17 00:00:00 2001 From: Jordan Harband <ljharb@gmail.com> Date: Thu, 22 Feb 2018 13:59:55 -0800 Subject: [PATCH 04/26] [Fix] `group-exports`: use module.exports, not export default Fixes #1031. --- src/rules/group-exports.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/group-exports.js b/src/rules/group-exports.js index ccfb8d2bd..96fff24fe 100644 --- a/src/rules/group-exports.js +++ b/src/rules/group-exports.js @@ -98,7 +98,7 @@ function create(context) { } } -export default { +module.exports = { meta, create, } From 59ea30e60bd44b83185945618fd4917dc1a4ef62 Mon Sep 17 00:00:00 2001 From: Ian MacLeod <ian@nevir.net> Date: Sun, 4 Mar 2018 15:32:33 -0800 Subject: [PATCH 05/26] Header-ify rule categories for easy linking It's often pretty helpful to link to rule categories in eslint configuration files. This lets one link to specific categories, too --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 9aeee0a9e..528e58db5 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a ## Rules -**Static analysis:** +### Static analysis * Ensure imports point to a file/module that can be resolved. ([`no-unresolved`]) * Ensure named imports correspond to a named export in the remote file. ([`named`]) @@ -36,7 +36,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`no-webpack-loader-syntax`]: ./docs/rules/no-webpack-loader-syntax.md [`no-self-import`]: ./docs/rules/no-self-import.md -**Helpful warnings:** +### Helpful warnings * Report any invalid exports, i.e. re-export of the same name ([`export`]) @@ -53,7 +53,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`no-extraneous-dependencies`]: ./docs/rules/no-extraneous-dependencies.md [`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md -**Module systems:** +### Module systems * Report potentially ambiguous parse goal (`script` vs. `module`) ([`unambiguous`]) * Report CommonJS `require` calls and `module.exports` or `exports.*`. ([`no-commonjs`]) @@ -66,7 +66,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md -**Style guide:** +### Style guide * Ensure all imports appear before other statements ([`first`]) * Ensure all exports appear after other statements ([`exports-last`]) From f12f2a708d4bc11649c69189a05355c0e2dd18e0 Mon Sep 17 00:00:00 2001 From: Plusb Preco <plusb21@gmail.com> Date: Tue, 6 Mar 2018 17:18:34 +0900 Subject: [PATCH 06/26] Fixes #656 - Should handle object-rest properties in `namespace` --- src/rules/namespace.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rules/namespace.js b/src/rules/namespace.js index 1e0b3eb86..71dd57db8 100644 --- a/src/rules/namespace.js +++ b/src/rules/namespace.js @@ -161,6 +161,9 @@ module.exports = { if (pattern.type !== 'ObjectPattern') return for (let property of pattern.properties) { + if (property.type === 'ExperimentalRestProperty') { + continue + } if (property.key.type !== 'Identifier') { context.report({ From 1a084cc975c0f0a3da60822197e47c718402d694 Mon Sep 17 00:00:00 2001 From: Plusb Preco <plusb21@gmail.com> Date: Tue, 6 Mar 2018 17:20:01 +0900 Subject: [PATCH 07/26] Add tests --- tests/src/rules/namespace.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/src/rules/namespace.js b/tests/src/rules/namespace.js index 96eebcc0e..19a69a8d9 100644 --- a/tests/src/rules/namespace.js +++ b/tests/src/rules/namespace.js @@ -92,6 +92,20 @@ const valid = [ options: [{ allowComputed: true }], }), + // #656: should handle object-rest properties + test({ + code: `import * as names from './named-exports'; const {a, b, ...rest} = names;`, + parserOptions: { + ecmaFeatures: { + experimentalObjectRestSpread: true, + }, + }, + }), + test({ + code: `import * as names from './named-exports'; const {a, b, ...rest} = names;`, + parser: 'babel-eslint', + }), + ...SYNTAX_CASES, ] From 084464558d11509254d496688d9957649f95ebe3 Mon Sep 17 00:00:00 2001 From: Kamal Bennani <kamalbouchboy1@hotmail.fr> Date: Mon, 12 Mar 2018 17:09:52 +0100 Subject: [PATCH 08/26] Add missing env variable for webpack config --- resolvers/webpack/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/resolvers/webpack/index.js b/resolvers/webpack/index.js index 42f9e9828..83456761b 100644 --- a/resolvers/webpack/index.js +++ b/resolvers/webpack/index.js @@ -47,6 +47,7 @@ exports.resolve = function (source, file, settings) { var configPath = get(settings, 'config') , configIndex = get(settings, 'config-index') + , env = get(settings, 'env') , packageDir log('Config path from settings:', configPath) @@ -82,7 +83,7 @@ exports.resolve = function (source, file, settings) { } if (typeof webpackConfig === 'function') { - webpackConfig = webpackConfig() + webpackConfig = webpackConfig(env) } if (Array.isArray(webpackConfig)) { From 4b311ac6faf93c6f6fc9a186de6f44da3bd70aaa Mon Sep 17 00:00:00 2001 From: Kamal Bennani <kamalbouchboy1@hotmail.fr> Date: Tue, 13 Mar 2018 10:25:38 +0100 Subject: [PATCH 09/26] Add Unit test using env option --- resolvers/webpack/test/config.js | 12 ++++++++++++ resolvers/webpack/test/files/some/goofy/path/bar.js | 0 .../webpack/test/files/webpack.function.config.js | 3 ++- 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 resolvers/webpack/test/files/some/goofy/path/bar.js diff --git a/resolvers/webpack/test/config.js b/resolvers/webpack/test/config.js index 42c5eca1f..2519daf8a 100644 --- a/resolvers/webpack/test/config.js +++ b/resolvers/webpack/test/config.js @@ -91,4 +91,16 @@ describe("config", function () { .and.equal(path.join(__dirname, 'files', 'some', 'goofy', 'path', 'foo.js')) }) + it('finds the config at option env when config is a function', function() { + var settings = { + config: require(path.join(__dirname, './files/webpack.function.config.js')), + env: { + dummy: true, + }, + } + + expect(resolve('bar', file, settings)).to.have.property('path') + .and.equal(path.join(__dirname, 'files', 'some', 'goofy', 'path', 'bar.js')) + }) + }) diff --git a/resolvers/webpack/test/files/some/goofy/path/bar.js b/resolvers/webpack/test/files/some/goofy/path/bar.js new file mode 100644 index 000000000..e69de29bb diff --git a/resolvers/webpack/test/files/webpack.function.config.js b/resolvers/webpack/test/files/webpack.function.config.js index 7f07afda6..ce87dd1b1 100644 --- a/resolvers/webpack/test/files/webpack.function.config.js +++ b/resolvers/webpack/test/files/webpack.function.config.js @@ -1,11 +1,12 @@ var path = require('path') var pluginsTest = require('webpack-resolver-plugin-test') -module.exports = function() { +module.exports = function(env) { return { resolve: { alias: { 'foo': path.join(__dirname, 'some', 'goofy', 'path', 'foo.js'), + 'bar': env ? path.join(__dirname, 'some', 'goofy', 'path', 'bar.js') : undefined, 'some-alias': path.join(__dirname, 'some'), }, modules: [ From efa1723ff9fd31a6b4b1945845dc6db4246746e1 Mon Sep 17 00:00:00 2001 From: Pete Gleeson <pgleeson@atlassian.com> Date: Wed, 14 Mar 2018 17:12:04 +1100 Subject: [PATCH 10/26] adds more examples to the import/extensions rule docs --- docs/rules/extensions.md | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/docs/rules/extensions.md b/docs/rules/extensions.md index e520ef5b5..e2b68e950 100644 --- a/docs/rules/extensions.md +++ b/docs/rules/extensions.md @@ -8,9 +8,33 @@ In order to provide a consistent use of file extensions across your code base, t 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. +```json +"import/extensions": [<severity>, "never" | "always" | "ignorePackages"] +``` + +By providing an object you can configure each extension separately. + +```json +"import/extensions": [<severity>, { + <extension>: "never" | "always" | "ignorePackages" +}] +``` + + For example `{ "js": "always", "json": "never" }` would always enforce the use of the `.js` extension but never allow the use of the `.json` extension. + +By providing both a string and an object, the string will set the default setting for all extensions, and the object can be used to set granular overrides for specific extensions. + +```json +"import/extensions": [ + <severity>, + "never" | "always" | "ignorePackages", + { + <extension>: "never" | "always" | "ignorePackages" + } +] +``` -By providing both a string and an object, the string will set the default setting for all extensions, and the object can be used to set granular overrides for specific extensions. For example, `[<enabled>, "never", { "svg": "always" }]` would require that all extensions are omitted, except for "svg". +For example, `["error", "never", { "svg": "always" }]` would require that all extensions are omitted, except for "svg". ### Exception @@ -110,7 +134,7 @@ import express from 'express'; ``` -The following patterns are not considered problems when configuration set to `[ 'always', {ignorePackages: true} ]`: +The following patterns are not considered problems when configuration set to `['error', 'always', {ignorePackages: true} ]`: ```js import Component from './Component.jsx'; From 84b34e89b06c65caeb0d9e6ceba5709c13a9fc63 Mon Sep 17 00:00:00 2001 From: Pete Gleeson <pgleeson@atlassian.com> Date: Thu, 15 Mar 2018 10:09:06 +1100 Subject: [PATCH 11/26] [Docs] fixes problem with weird highlighting --- docs/rules/extensions.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rules/extensions.md b/docs/rules/extensions.md index e2b68e950..6e1d2b50a 100644 --- a/docs/rules/extensions.md +++ b/docs/rules/extensions.md @@ -8,13 +8,13 @@ In order to provide a consistent use of file extensions across your code base, t 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. -```json +``` "import/extensions": [<severity>, "never" | "always" | "ignorePackages"] ``` By providing an object you can configure each extension separately. -```json +``` "import/extensions": [<severity>, { <extension>: "never" | "always" | "ignorePackages" }] @@ -24,7 +24,7 @@ By providing an object you can configure each extension separately. By providing both a string and an object, the string will set the default setting for all extensions, and the object can be used to set granular overrides for specific extensions. -```json +``` "import/extensions": [ <severity>, "never" | "always" | "ignorePackages", From 5fa2851adc2d27c01d5b4ce1f4f3af10999d775b Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Mon, 19 Mar 2018 07:33:51 -0400 Subject: [PATCH 12/26] wip: no-cycle support with general dependency "imports" map in ExportMap --- src/ExportMap.js | 38 ++++++++++++++++++++------ src/index.js | 1 + src/rules/no-cycle.js | 62 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 src/rules/no-cycle.js diff --git a/src/ExportMap.js b/src/ExportMap.js index aefec2ba3..1a35d6923 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -21,7 +21,16 @@ export default class ExportMap { this.namespace = new Map() // todo: restructure to key on path, value is resolver + map of names this.reexports = new Map() - this.dependencies = new Map() + /** + * star-exports + * @type {Set} of () => ExportMap + */ + this.dependencies = new Set() + /** + * dependencies of this module that are not explicitly re-exported + * @type {Map} from path = () => ExportMap + */ + this.imports = new Map() this.errors = [] } @@ -46,7 +55,7 @@ export default class ExportMap { // default exports must be explicitly re-exported (#328) if (name !== 'default') { - for (let dep of this.dependencies.values()) { + for (let dep of this.dependencies) { let innerMap = dep() // todo: report as unresolved? @@ -88,7 +97,7 @@ export default class ExportMap { // default exports must be explicitly re-exported (#328) if (name !== 'default') { - for (let dep of this.dependencies.values()) { + for (let dep of this.dependencies) { let innerMap = dep() // todo: report as unresolved? if (!innerMap) continue @@ -125,7 +134,7 @@ export default class ExportMap { // default exports must be explicitly re-exported (#328) if (name !== 'default') { - for (let dep of this.dependencies.values()) { + for (let dep of this.dependencies) { let innerMap = dep() // todo: report as unresolved? if (!innerMap) continue @@ -373,6 +382,18 @@ ExportMap.parse = function (path, content, context) { return object } + function captureDependency(declaration) { + if (declaration.source == null) return + + const p = remotePath(declaration) + if (p == null) return + if (m.imports.has(p)) return + + const getter = () => ExportMap.for(p, context) + m.imports.set(p, getter) + return getter + } + ast.body.forEach(function (n) { @@ -386,14 +407,14 @@ ExportMap.parse = function (path, content, context) { } if (n.type === 'ExportAllDeclaration') { - let remoteMap = remotePath(n) - if (remoteMap == null) return - m.dependencies.set(remoteMap, () => ExportMap.for(remoteMap, context)) + const getter = captureDependency(n) + if (getter) m.dependencies.add(getter) return } // capture namespaces in case of later export if (n.type === 'ImportDeclaration') { + captureDependency(n) let ns if (n.specifiers.some(s => s.type === 'ImportNamespaceSpecifier' && (ns = s))) { namespaces.set(ns.local.name, n) @@ -401,7 +422,8 @@ ExportMap.parse = function (path, content, context) { return } - if (n.type === 'ExportNamedDeclaration'){ + if (n.type === 'ExportNamedDeclaration') { + captureDependency(n) // capture declaration if (n.declaration != null) { switch (n.declaration.type) { diff --git a/src/index.js b/src/index.js index 6c5e11252..2d6352b83 100644 --- a/src/index.js +++ b/src/index.js @@ -12,6 +12,7 @@ export const rules = { 'group-exports': require('./rules/group-exports'), 'no-self-import': require('./rules/no-self-import'), + 'no-cycle': require('./rules/no-cycle'), 'no-named-default': require('./rules/no-named-default'), 'no-named-as-default': require('./rules/no-named-as-default'), 'no-named-as-default-member': require('./rules/no-named-as-default-member'), diff --git a/src/rules/no-cycle.js b/src/rules/no-cycle.js new file mode 100644 index 000000000..394c94e2e --- /dev/null +++ b/src/rules/no-cycle.js @@ -0,0 +1,62 @@ +/** + * @fileOverview Ensures that no imported module imports the linted module. + * @author Ben Mosher + */ + +import Exports from '../ExportMap' +import moduleVisitor, { optionsSchema } from 'eslint-module-utils/moduleVisitor' +import docsUrl from '../docsUrl' + +// todo: cache cycles / deep relationships for faster repeat evaluation +module.exports = { + meta: { + docs: { + url: docsUrl('no-cycle'), + }, + + schema: [optionsSchema], + }, + + create: function (context) { + + const myPath = context.getFilename() + + function checkSourceValue(source) { + const imported = Exports.get(source.value, context) + + if (imported === undefined) { + return // no-unresolved territory + } + + if (imported.path === myPath) { + // todo: report direct self import? + return + } + + const untraversed = [imported] + const traversed = new Set() + function detectCycle(m) { + if (traversed.has(m.path)) return + traversed.add(m.path) + + for (let [path, getter] of m.imports) { + if (path === context) return true + if (traversed.has(path)) continue + untraversed.push(getter()) + } + } + + while (untraversed.length > 0) { + if (detectCycle(untraversed.pop())) { + // todo: report + + // todo: cache + + return + } + } + } + + return moduleVisitor(checkSourceValue, context.options[0]) + }, +} From 0c21c4e0f6e8248f56561ad66dfcfa7d21ba31a1 Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Mon, 19 Mar 2018 07:44:11 -0400 Subject: [PATCH 13/26] sublime-linter project tweaks --- import.sublime-project | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/import.sublime-project b/import.sublime-project index d2ff5f73a..3b7feb346 100644 --- a/import.sublime-project +++ b/import.sublime-project @@ -16,8 +16,12 @@ }, "eslint_d": { + "disable": true, "chdir": "${project}" } + }, + "paths": { + "osx": ["${project}/node_modules/.bin"] } } } From f7c48b5e819ffb2dd310bc107a987c605db9305c Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Tue, 20 Mar 2018 21:32:34 -0400 Subject: [PATCH 14/26] no-cycle: real rule! first draft, perf is likely atrocious --- src/ExportMap.js | 3 +- src/rules/no-cycle.js | 49 ++++++++++--------- tests/files/cycles/depth-one.js | 2 + tests/files/cycles/depth-three-indirect.js | 5 ++ tests/files/cycles/depth-three-star.js | 2 + tests/files/cycles/depth-two.js | 2 + tests/files/cycles/depth-zero.js | 1 + tests/src/rules/no-cycle.js | 55 ++++++++++++++++++++++ utils/moduleVisitor.js | 10 ++-- utils/parse.js | 3 ++ 10 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 tests/files/cycles/depth-one.js create mode 100644 tests/files/cycles/depth-three-indirect.js create mode 100644 tests/files/cycles/depth-three-star.js create mode 100644 tests/files/cycles/depth-two.js create mode 100644 tests/files/cycles/depth-zero.js create mode 100644 tests/src/rules/no-cycle.js diff --git a/src/ExportMap.js b/src/ExportMap.js index 1a35d6923..4325de5e0 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -390,7 +390,7 @@ ExportMap.parse = function (path, content, context) { if (m.imports.has(p)) return const getter = () => ExportMap.for(p, context) - m.imports.set(p, getter) + m.imports.set(p, { getter, source: declaration.source }) return getter } @@ -423,7 +423,6 @@ ExportMap.parse = function (path, content, context) { } if (n.type === 'ExportNamedDeclaration') { - captureDependency(n) // capture declaration if (n.declaration != null) { switch (n.declaration.type) { diff --git a/src/rules/no-cycle.js b/src/rules/no-cycle.js index 394c94e2e..c9148b7fa 100644 --- a/src/rules/no-cycle.js +++ b/src/rules/no-cycle.js @@ -10,48 +10,51 @@ import docsUrl from '../docsUrl' // todo: cache cycles / deep relationships for faster repeat evaluation module.exports = { meta: { - docs: { - url: docsUrl('no-cycle'), - }, - + docs: { url: docsUrl('no-cycle') }, schema: [optionsSchema], }, create: function (context) { - const myPath = context.getFilename() + if (myPath === '<text>') return // can't cycle-check a non-file - function checkSourceValue(source) { - const imported = Exports.get(source.value, context) + function checkSourceValue(sourceNode, importer) { + const imported = Exports.get(sourceNode.value, context) - if (imported === undefined) { - return // no-unresolved territory + if (imported == null) { + return // no-unresolved territory } if (imported.path === myPath) { - // todo: report direct self import? - return + return // no-self-import territory } - const untraversed = [imported] + const untraversed = [{imported, route:[]}] const traversed = new Set() - function detectCycle(m) { + function detectCycle({imported: m, route}) { if (traversed.has(m.path)) return traversed.add(m.path) - for (let [path, getter] of m.imports) { - if (path === context) return true + for (let [path, { getter, source }] of m.imports) { + if (path === myPath) return true if (traversed.has(path)) continue - untraversed.push(getter()) + const deeper = getter() + if (deeper != null) { + untraversed.push({ + imported: deeper, + route: route.concat(source), + }) + } } } while (untraversed.length > 0) { - if (detectCycle(untraversed.pop())) { - // todo: report - - // todo: cache - + const next = untraversed.shift() // bfs! + if (detectCycle(next)) { + const message = (next.route.length > 0 + ? `Dependency cycle via ${routeString(next.route)}` + : 'Dependency cycle detected.') + context.report(importer, message) return } } @@ -60,3 +63,7 @@ module.exports = { return moduleVisitor(checkSourceValue, context.options[0]) }, } + +function routeString(route) { + return route.map(s => `${s.value}:${s.loc.start.line}`).join('=>') +} diff --git a/tests/files/cycles/depth-one.js b/tests/files/cycles/depth-one.js new file mode 100644 index 000000000..748f65f84 --- /dev/null +++ b/tests/files/cycles/depth-one.js @@ -0,0 +1,2 @@ +import foo from "./depth-zero" +export { foo } diff --git a/tests/files/cycles/depth-three-indirect.js b/tests/files/cycles/depth-three-indirect.js new file mode 100644 index 000000000..814562f44 --- /dev/null +++ b/tests/files/cycles/depth-three-indirect.js @@ -0,0 +1,5 @@ +import './depth-two' + +export function bar() { + return "side effects???" +} \ No newline at end of file diff --git a/tests/files/cycles/depth-three-star.js b/tests/files/cycles/depth-three-star.js new file mode 100644 index 000000000..3f680bcb0 --- /dev/null +++ b/tests/files/cycles/depth-three-star.js @@ -0,0 +1,2 @@ +import * as two from "./depth-two" +export { two } diff --git a/tests/files/cycles/depth-two.js b/tests/files/cycles/depth-two.js new file mode 100644 index 000000000..3f38d78a5 --- /dev/null +++ b/tests/files/cycles/depth-two.js @@ -0,0 +1,2 @@ +import { foo } from "./depth-one" +export { foo } diff --git a/tests/files/cycles/depth-zero.js b/tests/files/cycles/depth-zero.js new file mode 100644 index 000000000..c9f23e9ca --- /dev/null +++ b/tests/files/cycles/depth-zero.js @@ -0,0 +1 @@ +// export function foo() {} diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js new file mode 100644 index 000000000..b05a64100 --- /dev/null +++ b/tests/src/rules/no-cycle.js @@ -0,0 +1,55 @@ +import { test as _test, testFilePath } from '../utils' + +import { RuleTester } from 'eslint' + +const ruleTester = new RuleTester() + , rule = require('rules/no-cycle') + +const error = message => ({ ruleId: 'no-cycle', message }) + +const test = def => _test(Object.assign(def, { + filename: testFilePath("./cycles/depth-zero.js") +})) + +// describe.only("no-cycle", () => { +ruleTester.run('no-cycle', rule, { + valid: [ + // this rule doesn't care if the cycle length is 0 + test({ code: 'import foo from "./foo.js"'}), + + test({ code: 'import _ from "lodash"' }), + test({ code: 'import foo from "@scope/foo"' }), + test({ code: 'var _ = require("lodash")' }), + test({ code: 'var find = require("lodash.find")' }), + test({ code: 'var foo = require("./foo")' }), + test({ code: 'var foo = require("../foo")' }), + test({ code: 'var foo = require("foo")' }), + test({ code: 'var foo = require("./")' }), + test({ code: 'var foo = require("@scope/foo")' }), + test({ code: 'var bar = require("./bar/index")' }), + test({ code: 'var bar = require("./bar")' }), + test({ + code: 'var bar = require("./bar")', + filename: '<text>', + }), + ], + invalid: [ + test({ + code: 'import { foo } from "./depth-one"', + errors: [error("Dependency cycle detected.")] + }), + test({ + code: 'import { foo } from "./depth-two"', + errors: [error("Dependency cycle via ./depth-one:1")] + }), + test({ + code: 'import { two } from "./depth-three-star"', + errors: [error("Dependency cycle via ./depth-two:1=>./depth-one:1")] + }), + test({ + code: 'import { bar } from "./depth-three-indirect"', + errors: [error("Dependency cycle via ./depth-two:1=>./depth-one:1")] + }), + ], +}) +// }) diff --git a/utils/moduleVisitor.js b/utils/moduleVisitor.js index 4248317b6..6d087a163 100644 --- a/utils/moduleVisitor.js +++ b/utils/moduleVisitor.js @@ -19,19 +19,19 @@ exports.default = function visitModules(visitor, options) { ignoreRegExps = options.ignore.map(p => new RegExp(p)) } - function checkSourceValue(source) { + function checkSourceValue(source, importer) { if (source == null) return //? // handle ignore if (ignoreRegExps.some(re => re.test(source.value))) return // fire visitor - visitor(source) + visitor(source, importer) } // for import-y declarations function checkSource(node) { - checkSourceValue(node.source) + checkSourceValue(node.source, node) } // for CommonJS `require` calls @@ -45,7 +45,7 @@ exports.default = function visitModules(visitor, options) { if (modulePath.type !== 'Literal') return if (typeof modulePath.value !== 'string') return - checkSourceValue(modulePath) + checkSourceValue(modulePath, call) } function checkAMD(call) { @@ -64,7 +64,7 @@ exports.default = function visitModules(visitor, options) { if (element.value === 'require' || element.value === 'exports') continue // magic modules: http://git.io/vByan - checkSourceValue(element) + checkSourceValue(element, call) } } diff --git a/utils/parse.js b/utils/parse.js index b921f8778..5bafdba49 100644 --- a/utils/parse.js +++ b/utils/parse.js @@ -23,6 +23,9 @@ exports.default = function parse(path, content, context) { parserOptions.comment = true parserOptions.attachComment = true + // attach node locations + parserOptions.loc = true + // provide the `filePath` like eslint itself does, in `parserOptions` // https://github.com/eslint/eslint/blob/3ec436ee/lib/linter.js#L637 parserOptions.filePath = path From 314c0b771787b8808b40d8fe82809b0c63102986 Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Wed, 21 Mar 2018 21:55:43 -0400 Subject: [PATCH 15/26] fix issue (and add conspicuously absent test) with 'export *' --- src/ExportMap.js | 11 +++++------ tests/files/export-all.js | 1 + tests/src/rules/named.js | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/ExportMap.js b/src/ExportMap.js index 4325de5e0..11354a37e 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -383,15 +383,14 @@ ExportMap.parse = function (path, content, context) { } function captureDependency(declaration) { - if (declaration.source == null) return + if (declaration.source == null) return null const p = remotePath(declaration) - if (p == null) return - if (m.imports.has(p)) return + if (p == null || m.imports.has(p)) return p const getter = () => ExportMap.for(p, context) m.imports.set(p, { getter, source: declaration.source }) - return getter + return p } @@ -407,8 +406,8 @@ ExportMap.parse = function (path, content, context) { } if (n.type === 'ExportAllDeclaration') { - const getter = captureDependency(n) - if (getter) m.dependencies.add(getter) + const p = captureDependency(n) + if (p) m.dependencies.add(m.imports.get(p).getter) return } diff --git a/tests/files/export-all.js b/tests/files/export-all.js index 8839f6bb9..cfe060b7b 100644 --- a/tests/files/export-all.js +++ b/tests/files/export-all.js @@ -1 +1,2 @@ +import { foo } from './sibling-with-names' // ensure importing exported name doesn't block export * from './sibling-with-names' diff --git a/tests/src/rules/named.js b/tests/src/rules/named.js index 2364c74ed..8bd78f6eb 100644 --- a/tests/src/rules/named.js +++ b/tests/src/rules/named.js @@ -329,3 +329,18 @@ if (!CASE_SENSITIVE_FS) { ], }) } + +// export-all +ruleTester.run('named (export *)', rule, { + valid: [ + test({ + code: 'import { foo } from "./export-all"', + }), + ], + invalid: [ + test({ + code: 'import { bar } from "./export-all"', + errors: [`bar not found in './export-all'`], + }), + ], +}) From 864dbcff8e0b0f98f8093f217952ad4b45e2f9af Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Thu, 22 Mar 2018 06:14:37 -0400 Subject: [PATCH 16/26] no-cycle: explicit CJS/AMD tests (even though moduleVisitor handles it) --- tests/src/rules/no-cycle.js | 30 +++++++++++++++++++++++++----- utils/moduleVisitor.js | 4 ++-- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index b05a64100..178e9438d 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -8,7 +8,7 @@ const ruleTester = new RuleTester() const error = message => ({ ruleId: 'no-cycle', message }) const test = def => _test(Object.assign(def, { - filename: testFilePath("./cycles/depth-zero.js") + filename: testFilePath('./cycles/depth-zero.js'), })) // describe.only("no-cycle", () => { @@ -36,19 +36,39 @@ ruleTester.run('no-cycle', rule, { invalid: [ test({ code: 'import { foo } from "./depth-one"', - errors: [error("Dependency cycle detected.")] + errors: [error(`Dependency cycle detected.`)], + }), + test({ + code: 'const { foo } = require("./depth-one")', + errors: [error(`Dependency cycle detected.`)], + options: [{ commonjs: true }], + }), + test({ + code: 'require(["./depth-one"], d1 => {})', + errors: [error(`Dependency cycle detected.`)], + options: [{ amd: true }], + }), + test({ + code: 'define(["./depth-one"], d1 => {})', + errors: [error(`Dependency cycle detected.`)], + options: [{ amd: true }], }), test({ code: 'import { foo } from "./depth-two"', - errors: [error("Dependency cycle via ./depth-one:1")] + errors: [error(`Dependency cycle via ./depth-one:1`)], + }), + test({ + code: 'const { foo } = require("./depth-two")', + errors: [error(`Dependency cycle via ./depth-one:1`)], + options: [{ commonjs: true }], }), test({ code: 'import { two } from "./depth-three-star"', - errors: [error("Dependency cycle via ./depth-two:1=>./depth-one:1")] + errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], }), test({ code: 'import { bar } from "./depth-three-indirect"', - errors: [error("Dependency cycle via ./depth-two:1=>./depth-one:1")] + errors: [error(`Dependency cycle via ./depth-two:1=>./depth-one:1`)], }), ], }) diff --git a/utils/moduleVisitor.js b/utils/moduleVisitor.js index 6d087a163..2abcc8345 100644 --- a/utils/moduleVisitor.js +++ b/utils/moduleVisitor.js @@ -1,4 +1,4 @@ -"use strict" +'use strict' exports.__esModule = true /** @@ -64,7 +64,7 @@ exports.default = function visitModules(visitor, options) { if (element.value === 'require' || element.value === 'exports') continue // magic modules: http://git.io/vByan - checkSourceValue(element, call) + checkSourceValue(element, element) } } From 6933fa4e33e76218a92fc968b6e6541a19902f7d Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Sun, 25 Mar 2018 13:43:55 -0400 Subject: [PATCH 17/26] no-cycle: initial docs + maxDepth option --- docs/rules/no-cycle.md | 37 +++++++++++++++++++++++++++++++++++++ src/rules/no-cycle.js | 24 +++++++++++++++++------- utils/moduleVisitor.js | 3 --- 3 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 docs/rules/no-cycle.md diff --git a/docs/rules/no-cycle.md b/docs/rules/no-cycle.md new file mode 100644 index 000000000..2c2a8e8ef --- /dev/null +++ b/docs/rules/no-cycle.md @@ -0,0 +1,37 @@ +# import/no-cycle + +Ensures that there is no resolvable path back to this module via its dependencies. + +By default, this rule only detects cycles for ES6 imports, but see the [`no-unresolved` options](./no-unresolved.md#options) as this rule also supports the same `commonjs` and `amd` flags. However, these flags only impact which import types are _linted_; the +import/export infrastructure only registers `import` statements in dependencies, so +cycles created by `require` within imported modules may not be detected. + +This includes cycles of depth 1 (imported module imports me) to `Infinity`. + +```js +// dep-b.js +import './dep-a.js' + +export function b() { /* ... */ } + +// dep-a.js +import { b } from './dep-b.js' // reported: Dependency cycle detected. +``` + +This rule does _not_ detect imports that resolve directly to the linted module; +for that, see [`no-self-import`]. + + +## Rule Details + +## When Not To Use It + +This rule is computationally expensive. If you are pressed for lint time, or don't +think you have an issue with dependency cycles, you may not want this rule enabled. + +## Further Reading + +- [Original inspiring issue](https://github.com/benmosher/eslint-plugin-import/issues/941) +- Rule to detect that module imports itself: [`no-self-import`] + +[`no-self-import`]: ./no-self-import.md diff --git a/src/rules/no-cycle.js b/src/rules/no-cycle.js index c9148b7fa..b1bf7b3e9 100644 --- a/src/rules/no-cycle.js +++ b/src/rules/no-cycle.js @@ -4,20 +4,29 @@ */ import Exports from '../ExportMap' -import moduleVisitor, { optionsSchema } from 'eslint-module-utils/moduleVisitor' +import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor' import docsUrl from '../docsUrl' // todo: cache cycles / deep relationships for faster repeat evaluation module.exports = { meta: { docs: { url: docsUrl('no-cycle') }, - schema: [optionsSchema], + schema: [makeOptionsSchema({ + maxDepth:{ + description: 'maximum dependency depth to traverse', + type: 'integer', + minimum: 1, + }, + })], }, create: function (context) { const myPath = context.getFilename() if (myPath === '<text>') return // can't cycle-check a non-file + const options = context.options[0] || {} + const maxDepth = options.maxDepth || Infinity + function checkSourceValue(sourceNode, importer) { const imported = Exports.get(sourceNode.value, context) @@ -29,19 +38,20 @@ module.exports = { return // no-self-import territory } - const untraversed = [{imported, route:[]}] + const untraversed = [{mget: () => imported, route:[]}] const traversed = new Set() - function detectCycle({imported: m, route}) { + function detectCycle({mget, route}) { + const m = mget() + if (m == null) return if (traversed.has(m.path)) return traversed.add(m.path) for (let [path, { getter, source }] of m.imports) { if (path === myPath) return true if (traversed.has(path)) continue - const deeper = getter() - if (deeper != null) { + if (route.length + 1 < maxDepth) { untraversed.push({ - imported: deeper, + mget: getter, route: route.concat(source), }) } diff --git a/utils/moduleVisitor.js b/utils/moduleVisitor.js index 2abcc8345..7bb980e45 100644 --- a/utils/moduleVisitor.js +++ b/utils/moduleVisitor.js @@ -90,9 +90,6 @@ exports.default = function visitModules(visitor, options) { /** * make an options schema for the module visitor, optionally * adding extra fields. - - * @param {[type]} additionalProperties [description] - * @return {[type]} [description] */ function makeOptionsSchema(additionalProperties) { const base = { From d81f48a2506182738409805f5272eff4d77c9348 Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Mon, 26 Mar 2018 07:04:09 -0400 Subject: [PATCH 18/26] no-cycle: maxDepth tests + docs --- docs/rules/no-cycle.md | 40 ++++++++++++++++++++++++++++++------- tests/src/rules/no-cycle.js | 18 +++++++++++++++-- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/docs/rules/no-cycle.md b/docs/rules/no-cycle.md index 2c2a8e8ef..aa28fda0a 100644 --- a/docs/rules/no-cycle.md +++ b/docs/rules/no-cycle.md @@ -2,11 +2,8 @@ Ensures that there is no resolvable path back to this module via its dependencies. -By default, this rule only detects cycles for ES6 imports, but see the [`no-unresolved` options](./no-unresolved.md#options) as this rule also supports the same `commonjs` and `amd` flags. However, these flags only impact which import types are _linted_; the -import/export infrastructure only registers `import` statements in dependencies, so -cycles created by `require` within imported modules may not be detected. - -This includes cycles of depth 1 (imported module imports me) to `Infinity`. +This includes cycles of depth 1 (imported module imports me) to `Infinity`, if the +[`maxDepth`](#maxdepth) option is not set. ```js // dep-b.js @@ -24,10 +21,39 @@ for that, see [`no-self-import`]. ## Rule Details +### Options + +By default, this rule only detects cycles for ES6 imports, but see the [`no-unresolved` options](./no-unresolved.md#options) as this rule also supports the same `commonjs` and `amd` flags. However, these flags only impact which import types are _linted_; the +import/export infrastructure only registers `import` statements in dependencies, so +cycles created by `require` within imported modules may not be detected. + +#### `maxDepth` + +There is a `maxDepth` option available to prevent full expansion of very deep dependency trees: + +```js +/*eslint import/no-unresolved: [2, { maxDepth: 1 }]*/ + +// dep-c.js +import './dep-a.js' + +// dep-b.js +import './dep-c.js' + +export function b() { /* ... */ } + +// dep-a.js +import { b } from './dep-b.js' // not reported as the cycle is at depth 2 +``` + +This is not necessarily recommended, but available as a cost/benefit tradeoff mechanism +for reducing total project lint time, if needed. + ## When Not To Use It -This rule is computationally expensive. If you are pressed for lint time, or don't -think you have an issue with dependency cycles, you may not want this rule enabled. +This rule is comparatively computationally expensive. If you are pressed for lint +time, or don't think you have an issue with dependency cycles, you may not want +this rule enabled. ## Further Reading diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index 178e9438d..22b3682ac 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -11,7 +11,7 @@ const test = def => _test(Object.assign(def, { filename: testFilePath('./cycles/depth-zero.js'), })) -// describe.only("no-cycle", () => { +describe.only("no-cycle", () => { ruleTester.run('no-cycle', rule, { valid: [ // this rule doesn't care if the cycle length is 0 @@ -32,12 +32,21 @@ ruleTester.run('no-cycle', rule, { code: 'var bar = require("./bar")', filename: '<text>', }), + test({ + code: 'import { foo } from "./depth-two"', + options: [{ maxDepth: 1 }], + }), ], invalid: [ test({ code: 'import { foo } from "./depth-one"', errors: [error(`Dependency cycle detected.`)], }), + test({ + code: 'import { foo } from "./depth-one"', + options: [{ maxDepth: 1 }], + errors: [error(`Dependency cycle detected.`)], + }), test({ code: 'const { foo } = require("./depth-one")', errors: [error(`Dependency cycle detected.`)], @@ -57,6 +66,11 @@ ruleTester.run('no-cycle', rule, { code: 'import { foo } from "./depth-two"', errors: [error(`Dependency cycle via ./depth-one:1`)], }), + test({ + code: 'import { foo } from "./depth-two"', + options: [{ maxDepth: 2 }], + errors: [error(`Dependency cycle via ./depth-one:1`)], + }), test({ code: 'const { foo } = require("./depth-two")', errors: [error(`Dependency cycle via ./depth-one:1`)], @@ -72,4 +86,4 @@ ruleTester.run('no-cycle', rule, { }), ], }) -// }) +}) From ad66aea712554a50a000ade565b81b0956ca1cfa Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Tue, 27 Mar 2018 20:06:32 -0400 Subject: [PATCH 19/26] smh. --- tests/files/cycles/depth-three-indirect.js | 2 +- tests/src/rules/no-cycle.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/files/cycles/depth-three-indirect.js b/tests/files/cycles/depth-three-indirect.js index 814562f44..f93ed00db 100644 --- a/tests/files/cycles/depth-three-indirect.js +++ b/tests/files/cycles/depth-three-indirect.js @@ -2,4 +2,4 @@ import './depth-two' export function bar() { return "side effects???" -} \ No newline at end of file +} diff --git a/tests/src/rules/no-cycle.js b/tests/src/rules/no-cycle.js index 22b3682ac..ae45ba36e 100644 --- a/tests/src/rules/no-cycle.js +++ b/tests/src/rules/no-cycle.js @@ -11,7 +11,7 @@ const test = def => _test(Object.assign(def, { filename: testFilePath('./cycles/depth-zero.js'), })) -describe.only("no-cycle", () => { +// describe.only("no-cycle", () => { ruleTester.run('no-cycle', rule, { valid: [ // this rule doesn't care if the cycle length is 0 @@ -86,4 +86,4 @@ ruleTester.run('no-cycle', rule, { }), ], }) -}) +// }) From 231874c6162f1eb5ca877f46bab55170f58f9599 Mon Sep 17 00:00:00 2001 From: Thomas Grainger <tagrain@gmail.com> Date: Wed, 28 Mar 2018 01:53:06 +0100 Subject: [PATCH 20/26] update eslint-import-resolver-webpack homepage to the source of the package (#997) --- resolvers/webpack/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolvers/webpack/package.json b/resolvers/webpack/package.json index c4e87f316..01b8ff7f5 100644 --- a/resolvers/webpack/package.json +++ b/resolvers/webpack/package.json @@ -27,7 +27,7 @@ "bugs": { "url": "https://github.com/benmosher/eslint-plugin-import/issues" }, - "homepage": "https://github.com/benmosher/eslint-plugin-import#readme", + "homepage": "https://github.com/benmosher/eslint-plugin-import/tree/master/resolvers/webpack", "dependencies": { "array-find": "^1.0.0", "debug": "^2.6.8", From e215b61193bae8c610157a72bd26596288cec2de Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Tue, 27 Mar 2018 21:14:44 -0400 Subject: [PATCH 21/26] try solution from appveyor/ci#650 --- appveyor.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/appveyor.yml b/appveyor.yml index 406099f66..b2e2a2d31 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -21,6 +21,10 @@ install: } - npm install + # fix symlinks + - cmd: git config core.symlinks true + - cmd: git reset --hard + # todo: learn how to do this for all .\resolvers\* on Windows - cd .\resolvers\webpack && npm install && cd ..\.. - cd .\resolvers\node && npm install && cd ..\.. From b34d9ff9f2c1ab45ff4a5e840f802b16be111da2 Mon Sep 17 00:00:00 2001 From: Eugene Tihonov <tihonov.ea@gmail.com> Date: Wed, 28 Mar 2018 20:54:53 +0500 Subject: [PATCH 22/26] Add autofixer for order rule (#908) --- CHANGELOG.md | 2 + docs/rules/order.md | 4 +- src/rules/order.js | 247 ++++++++++++++++++-- tests/src/rules/order.js | 490 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 717 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 068bffea2..a88cc01aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] +- Autofixer for [`order`] rule ([#711], thanks [@tihonove]) ## [2.9.0] - 2018-02-21 ### Added @@ -679,6 +680,7 @@ for info on changes for earlier releases. [@mplewis]: https://github.com/mplewis [@rosswarren]: https://github.com/rosswarren [@alexgorbatchev]: https://github.com/alexgorbatchev +[@tihonove]: https://github.com/tihonove [@robertrossmann]: https://github.com/robertrossmann [@isiahmeadows]: https://github.com/isiahmeadows [@graingert]: https://github.com/graingert diff --git a/docs/rules/order.md b/docs/rules/order.md index f51152cf5..45bde6acc 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -1,6 +1,8 @@ # import/order: Enforce a convention in module import order -Enforce a convention in the order of `require()` / `import` statements. The order is as shown in the following example: +Enforce a convention in the order of `require()` / `import` statements. ++(fixable) The `--fix` option on the [command line] automatically fixes problems reported by this rule. +The order is as shown in the following example: ```js // 1. node "builtin" modules diff --git a/src/rules/order.js b/src/rules/order.js index cdda60358..81babd7fd 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -6,7 +6,7 @@ import docsUrl from '../docsUrl' const defaultGroups = ['builtin', 'external', 'parent', 'sibling', 'index'] -// REPORTING +// REPORTING AND FIXING function reverse(array) { return array.map(function (v) { @@ -18,6 +18,60 @@ function reverse(array) { }).reverse() } +function getTokensOrCommentsAfter(sourceCode, node, count) { + let currentNodeOrToken = node + const result = [] + for (let i = 0; i < count; i++) { + currentNodeOrToken = sourceCode.getTokenOrCommentAfter(currentNodeOrToken) + if (currentNodeOrToken == null) { + break + } + result.push(currentNodeOrToken) + } + return result +} + +function getTokensOrCommentsBefore(sourceCode, node, count) { + let currentNodeOrToken = node + const result = [] + for (let i = 0; i < count; i++) { + currentNodeOrToken = sourceCode.getTokenOrCommentBefore(currentNodeOrToken) + if (currentNodeOrToken == null) { + break + } + result.push(currentNodeOrToken) + } + return result.reverse() +} + +function takeTokensAfterWhile(sourceCode, node, condition) { + const tokens = getTokensOrCommentsAfter(sourceCode, node, 100) + const result = [] + for (let i = 0; i < tokens.length; i++) { + if (condition(tokens[i])) { + result.push(tokens[i]) + } + else { + break + } + } + return result +} + +function takeTokensBeforeWhile(sourceCode, node, condition) { + const tokens = getTokensOrCommentsBefore(sourceCode, node, 100) + const result = [] + for (let i = tokens.length - 1; i >= 0; i--) { + if (condition(tokens[i])) { + result.push(tokens[i]) + } + else { + break + } + } + return result.reverse() +} + function findOutOfOrder(imported) { if (imported.length === 0) { return [] @@ -32,13 +86,141 @@ function findOutOfOrder(imported) { }) } +function findRootNode(node) { + let parent = node + while (parent.parent != null && parent.parent.body == null) { + parent = parent.parent + } + return parent +} + +function findEndOfLineWithComments(sourceCode, node) { + const tokensToEndOfLine = takeTokensAfterWhile(sourceCode, node, commentOnSameLineAs(node)) + let endOfTokens = tokensToEndOfLine.length > 0 + ? tokensToEndOfLine[tokensToEndOfLine.length - 1].end + : node.end + let result = endOfTokens + for (let i = endOfTokens; i < sourceCode.text.length; i++) { + if (sourceCode.text[i] === '\n') { + result = i + 1 + break + } + if (sourceCode.text[i] !== ' ' && sourceCode.text[i] !== '\t' && sourceCode.text[i] !== '\r') { + break + } + result = i + 1 + } + return result +} + +function commentOnSameLineAs(node) { + return token => (token.type === 'Block' || token.type === 'Line') && + token.loc.start.line === token.loc.end.line && + token.loc.end.line === node.loc.end.line +} + +function findStartOfLineWithComments(sourceCode, node) { + const tokensToEndOfLine = takeTokensBeforeWhile(sourceCode, node, commentOnSameLineAs(node)) + let startOfTokens = tokensToEndOfLine.length > 0 ? tokensToEndOfLine[0].start : node.start + let result = startOfTokens + for (let i = startOfTokens - 1; i > 0; i--) { + if (sourceCode.text[i] !== ' ' && sourceCode.text[i] !== '\t') { + break + } + result = i + } + return result +} + +function isPlainRequireModule(node) { + if (node.type !== 'VariableDeclaration') { + return false + } + if (node.declarations.length !== 1) { + return false + } + const decl = node.declarations[0] + const result = (decl.id != null && decl.id.type === 'Identifier') && + decl.init != null && + decl.init.type === 'CallExpression' && + decl.init.callee != null && + decl.init.callee.name === 'require' && + decl.init.arguments != null && + decl.init.arguments.length === 1 && + decl.init.arguments[0].type === 'Literal' + return result +} + +function isPlainImportModule(node) { + return node.type === 'ImportDeclaration' && node.specifiers != null && node.specifiers.length > 0 +} + +function canCrossNodeWhileReorder(node) { + return isPlainRequireModule(node) || isPlainImportModule(node) +} + +function canReorderItems(firstNode, secondNode) { + const parent = firstNode.parent + const firstIndex = parent.body.indexOf(firstNode) + const secondIndex = parent.body.indexOf(secondNode) + const nodesBetween = parent.body.slice(firstIndex, secondIndex + 1) + for (var nodeBetween of nodesBetween) { + if (!canCrossNodeWhileReorder(nodeBetween)) { + return false + } + } + return true +} + +function fixOutOfOrder(context, firstNode, secondNode, order) { + const sourceCode = context.getSourceCode() + + const firstRoot = findRootNode(firstNode.node) + let firstRootStart = findStartOfLineWithComments(sourceCode, firstRoot) + const firstRootEnd = findEndOfLineWithComments(sourceCode, firstRoot) + + const secondRoot = findRootNode(secondNode.node) + let secondRootStart = findStartOfLineWithComments(sourceCode, secondRoot) + let secondRootEnd = findEndOfLineWithComments(sourceCode, secondRoot) + const canFix = canReorderItems(firstRoot, secondRoot) + + let newCode = sourceCode.text.substring(secondRootStart, secondRootEnd) + if (newCode[newCode.length - 1] !== '\n') { + newCode = newCode + '\n' + } + + const message = '`' + secondNode.name + '` import should occur ' + order + + ' import of `' + firstNode.name + '`' + + if (order === 'before') { + context.report({ + node: secondNode.node, + message: message, + fix: canFix && (fixer => + fixer.replaceTextRange( + [firstRootStart, secondRootEnd], + newCode + sourceCode.text.substring(firstRootStart, secondRootStart) + )), + }) + } else if (order === 'after') { + context.report({ + node: secondNode.node, + message: message, + fix: canFix && (fixer => + fixer.replaceTextRange( + [secondRootStart, firstRootEnd], + sourceCode.text.substring(secondRootEnd, firstRootEnd) + newCode + )), + }) + } +} + function reportOutOfOrder(context, imported, outOfOrder, order) { outOfOrder.forEach(function (imp) { const found = imported.find(function hasHigherRank(importedItem) { return importedItem.rank > imp.rank }) - context.report(imp.node, '`' + imp.name + '` import should occur ' + order + - ' import of `' + found.name + '`') + fixOutOfOrder(context, found, imp, order) }) } @@ -109,6 +291,32 @@ function convertGroupsToRanks(groups) { }, rankObject) } +function fixNewLineAfterImport(context, previousImport) { + const prevRoot = findRootNode(previousImport.node) + const tokensToEndOfLine = takeTokensAfterWhile( + context.getSourceCode(), prevRoot, commentOnSameLineAs(prevRoot)) + + let endOfLine = prevRoot.end + if (tokensToEndOfLine.length > 0) { + endOfLine = tokensToEndOfLine[tokensToEndOfLine.length - 1].end + } + return (fixer) => fixer.insertTextAfterRange([prevRoot.start, endOfLine], '\n') +} + +function removeNewLineAfterImport(context, currentImport, previousImport) { + const sourceCode = context.getSourceCode() + const prevRoot = findRootNode(previousImport.node) + const currRoot = findRootNode(currentImport.node) + const rangeToRemove = [ + findEndOfLineWithComments(sourceCode, prevRoot), + findStartOfLineWithComments(sourceCode, currRoot), + ] + if (/^\s*$/.test(sourceCode.text.substring(rangeToRemove[0], rangeToRemove[1]))) { + return (fixer) => fixer.removeRange(rangeToRemove) + } + return undefined +} + function makeNewlinesBetweenReport (context, imported, newlinesBetweenImports) { const getNumberOfEmptyLinesBetween = (currentImport, previousImport) => { const linesBetweenImports = context.getSourceCode().lines.slice( @@ -125,23 +333,27 @@ function makeNewlinesBetweenReport (context, imported, newlinesBetweenImports) { if (newlinesBetweenImports === 'always' || newlinesBetweenImports === 'always-and-inside-groups') { - if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) - { - context.report( - previousImport.node, 'There should be at least one empty line between import groups' - ) + if (currentImport.rank !== previousImport.rank && emptyLinesBetween === 0) { + context.report({ + node: previousImport.node, + message: 'There should be at least one empty line between import groups', + fix: fixNewLineAfterImport(context, previousImport, currentImport), + }) } else if (currentImport.rank === previousImport.rank && emptyLinesBetween > 0 - && newlinesBetweenImports !== 'always-and-inside-groups') - { - context.report( - previousImport.node, 'There should be no empty line within import group' - ) - } - } else { - if (emptyLinesBetween > 0) { - context.report(previousImport.node, 'There should be no empty line between import groups') + && newlinesBetweenImports !== 'always-and-inside-groups') { + context.report({ + node: previousImport.node, + message: 'There should be no empty line within import group', + fix: removeNewLineAfterImport(context, currentImport, previousImport), + }) } + } else if (emptyLinesBetween > 0) { + context.report({ + node: previousImport.node, + message: 'There should be no empty line between import groups', + fix: removeNewLineAfterImport(context, currentImport, previousImport), + }) } previousImport = currentImport @@ -154,6 +366,7 @@ module.exports = { url: docsUrl('order'), }, + fixable: 'code', schema: [ { type: 'object', diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index c87508573..fb3b78844 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -5,6 +5,10 @@ import { RuleTester } from 'eslint' const ruleTester = new RuleTester() , rule = require('rules/order') +function withoutAutofixOutput(test) { + return Object.assign({}, test, { output: test.code }) +} + ruleTester.run('order', rule, { valid: [ // Default order using require @@ -410,6 +414,135 @@ ruleTester.run('order', rule, { var async = require('async'); var fs = require('fs'); `, + output: ` + var fs = require('fs'); + var async = require('async'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order with spaces on the end of line + test({ + code: ` + var async = require('async'); + var fs = require('fs');${' '} + `, + output: ` + var fs = require('fs');${' '} + var async = require('async'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order with comment on the end of line + test({ + code: ` + var async = require('async'); + var fs = require('fs'); /* comment */ + `, + output: ` + var fs = require('fs'); /* comment */ + var async = require('async'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order with comments at the end and start of line + test({ + code: ` + /* comment1 */ var async = require('async'); /* comment2 */ + /* comment3 */ var fs = require('fs'); /* comment4 */ + `, + output: ` + /* comment3 */ var fs = require('fs'); /* comment4 */ + /* comment1 */ var async = require('async'); /* comment2 */ + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order with few comments at the end and start of line + test({ + code: ` + /* comment0 */ /* comment1 */ var async = require('async'); /* comment2 */ + /* comment3 */ var fs = require('fs'); /* comment4 */ + `, + output: ` + /* comment3 */ var fs = require('fs'); /* comment4 */ + /* comment0 */ /* comment1 */ var async = require('async'); /* comment2 */ + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order with windows end of lines + test({ + code: + `/* comment0 */ /* comment1 */ var async = require('async'); /* comment2 */` + `\r\n` + + `/* comment3 */ var fs = require('fs'); /* comment4 */` + `\r\n` + , + output: + `/* comment3 */ var fs = require('fs'); /* comment4 */` + `\r\n` + + `/* comment0 */ /* comment1 */ var async = require('async'); /* comment2 */` + `\r\n` + , + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order with multilines comments at the end and start of line + test({ + code: ` + /* multiline1 + comment1 */ var async = require('async'); /* multiline2 + comment2 */ var fs = require('fs'); /* multiline3 + comment3 */ + `, + output: ` + /* multiline1 + comment1 */ var fs = require('fs');` + ' ' + ` + var async = require('async'); /* multiline2 + comment2 *//* multiline3 + comment3 */ + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order of multile import + test({ + code: ` + var async = require('async'); + var fs = + require('fs'); + `, + output: ` + var fs = + require('fs'); + var async = require('async'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + }), + // fix order at the end of file + test({ + code: ` + var async = require('async'); + var fs = require('fs');`, + output: ` + var fs = require('fs'); + var async = require('async');` + '\n', errors: [{ ruleId: 'order', message: '`fs` import should occur before import of `async`', @@ -421,6 +554,10 @@ ruleTester.run('order', rule, { import async from 'async'; import fs from 'fs'; `, + output: ` + import fs from 'fs'; + import async from 'async'; + `, errors: [{ ruleId: 'order', message: '`fs` import should occur before import of `async`', @@ -432,6 +569,10 @@ ruleTester.run('order', rule, { var async = require('async'); import fs from 'fs'; `, + output: ` + import fs from 'fs'; + var async = require('async'); + `, errors: [{ ruleId: 'order', message: '`fs` import should occur before import of `async`', @@ -443,6 +584,10 @@ ruleTester.run('order', rule, { var parent = require('../parent'); var async = require('async'); `, + output: ` + var async = require('async'); + var parent = require('../parent'); + `, errors: [{ ruleId: 'order', message: '`async` import should occur before import of `../parent`', @@ -454,6 +599,10 @@ ruleTester.run('order', rule, { var sibling = require('./sibling'); var parent = require('../parent'); `, + output: ` + var parent = require('../parent'); + var sibling = require('./sibling'); + `, errors: [{ ruleId: 'order', message: '`../parent` import should occur before import of `./sibling`', @@ -465,6 +614,10 @@ ruleTester.run('order', rule, { var index = require('./'); var sibling = require('./sibling'); `, + output: ` + var sibling = require('./sibling'); + var index = require('./'); + `, errors: [{ ruleId: 'order', message: '`./sibling` import should occur before import of `./`', @@ -495,6 +648,14 @@ ruleTester.run('order', rule, { var foo = require('foo'); var bar = require('bar'); `, + output: ` + var fs = require('fs'); + var path = require('path'); + var _ = require('lodash'); + var foo = require('foo'); + var bar = require('bar'); + var index = require('./'); + `, errors: [{ ruleId: 'order', message: '`./` import should occur after import of `bar`', @@ -506,6 +667,10 @@ ruleTester.run('order', rule, { var fs = require('fs'); var index = require('./'); `, + output: ` + var index = require('./'); + var fs = require('fs'); + `, options: [{groups: ['index', 'sibling', 'parent', 'external', 'builtin']}], errors: [{ ruleId: 'order', @@ -513,7 +678,7 @@ ruleTester.run('order', rule, { }], }), // member expression of require - test({ + test(withoutAutofixOutput({ code: ` var foo = require('./foo').bar; var fs = require('fs'); @@ -522,9 +687,9 @@ ruleTester.run('order', rule, { ruleId: 'order', message: '`fs` import should occur before import of `./foo`', }], - }), + })), // nested member expression of require - test({ + test(withoutAutofixOutput({ code: ` var foo = require('./foo').bar.bar.bar; var fs = require('fs'); @@ -533,7 +698,33 @@ ruleTester.run('order', rule, { ruleId: 'order', message: '`fs` import should occur before import of `./foo`', }], - }), + })), + // fix near nested member expression of require with newlines + test(withoutAutofixOutput({ + code: ` + var foo = require('./foo').bar + .bar + .bar; + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `./foo`', + }], + })), + // fix nested member expression of require with newlines + test(withoutAutofixOutput({ + code: ` + var foo = require('./foo'); + var fs = require('fs').bar + .bar + .bar; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `./foo`', + }], + })), // Grouping import types test({ code: ` @@ -542,6 +733,12 @@ ruleTester.run('order', rule, { var sibling = require('./foo'); var path = require('path'); `, + output: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + var sibling = require('./foo'); + `, options: [{groups: [ ['builtin', 'index'], ['sibling', 'parent', 'external'], @@ -557,6 +754,10 @@ ruleTester.run('order', rule, { var path = require('path'); var async = require('async'); `, + output: ` + var async = require('async'); + var path = require('path'); + `, options: [{groups: [ 'index', ['sibling', 'parent', 'external', 'internal'], @@ -639,6 +840,15 @@ ruleTester.run('order', rule, { import sibling, {foo3} from './foo'; var index = require('./'); `, + output: ` + import async, {foo1} from 'async'; + import relParent2, {foo2} from '../foo/bar'; + import sibling, {foo3} from './foo'; + var fs = require('fs'); + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var index = require('./'); + `, errors: [{ ruleId: 'order', message: '`./foo` import should occur before import of `fs`', @@ -650,6 +860,11 @@ ruleTester.run('order', rule, { import async, {foo1} from 'async'; import relParent2, {foo2} from '../foo/bar'; `, + output: ` + import async, {foo1} from 'async'; + import relParent2, {foo2} from '../foo/bar'; + var fs = require('fs'); + `, errors: [{ ruleId: 'order', message: '`fs` import should occur after import of `../foo/bar`', @@ -668,6 +883,15 @@ ruleTester.run('order', rule, { var relParent3 = require('../'); var async = require('async'); `, + output: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + var sibling = require('./foo'); + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, options: [ { groups: [ @@ -689,7 +913,58 @@ ruleTester.run('order', rule, { }, ], }), - // // Option newlines-between: 'always' - should report lack of newline between groups + // Fix newlines-between with comments after + test({ + code: ` + var fs = require('fs'); /* comment */ + + var index = require('./'); + `, + output: ` + var fs = require('fs'); /* comment */ + var index = require('./'); + `, + options: [ + { + groups: [['builtin'], ['index']], + 'newlines-between': 'never', + }, + ], + errors: [ + { + line: 2, + message: 'There should be no empty line between import groups', + }, + ], + }), + // Cannot fix newlines-between with multiline comment after + test({ + code: ` + var fs = require('fs'); /* multiline + comment */ + + var index = require('./'); + `, + output: ` + var fs = require('fs'); /* multiline + comment */ + + var index = require('./'); + `, + options: [ + { + groups: [['builtin'], ['index']], + 'newlines-between': 'never', + }, + ], + errors: [ + { + line: 2, + message: 'There should be no empty line between import groups', + }, + ], + }), + // Option newlines-between: 'always' - should report lack of newline between groups test({ code: ` var fs = require('fs'); @@ -700,6 +975,17 @@ ruleTester.run('order', rule, { var relParent3 = require('../'); var async = require('async'); `, + output: ` + var fs = require('fs'); + var index = require('./'); + var path = require('path'); + + var sibling = require('./foo'); + + var relParent1 = require('../foo'); + var relParent3 = require('../'); + var async = require('async'); + `, options: [ { groups: [ @@ -721,7 +1007,7 @@ ruleTester.run('order', rule, { }, ], }), - //Option newlines-between: 'always' should report unnecessary empty lines space between import groups + // Option newlines-between: 'always' should report unnecessary empty lines space between import groups test({ code: ` var fs = require('fs'); @@ -733,11 +1019,19 @@ ruleTester.run('order', rule, { var async = require('async'); `, + output: ` + var fs = require('fs'); + var path = require('path'); + var index = require('./'); + + var sibling = require('./foo'); + var async = require('async'); + `, options: [ { groups: [ ['builtin', 'index'], - ['sibling', 'parent', 'external'] + ['sibling', 'parent', 'external'], ], 'newlines-between': 'always', }, @@ -753,7 +1047,7 @@ ruleTester.run('order', rule, { }, ], }), - // Option newlines-between: 'never' should report unnecessary empty lines when using not assigned imports + // Option newlines-between: 'never' cannot fix if there are other statements between imports test({ code: ` import path from 'path'; @@ -762,6 +1056,13 @@ ruleTester.run('order', rule, { import 'something-else'; import _ from 'lodash'; `, + output: ` + import path from 'path'; + import 'loud-rejection'; + + import 'something-else'; + import _ from 'lodash'; + `, options: [{ 'newlines-between': 'never' }], errors: [ { @@ -778,6 +1079,72 @@ ruleTester.run('order', rule, { import 'something-else'; import _ from 'lodash'; `, + output: ` + import path from 'path'; + + import 'loud-rejection'; + import 'something-else'; + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'always' }], + errors: [ + { + line: 2, + message: 'There should be at least one empty line between import groups', + }, + ], + }), + // fix missing empty lines with single line comment after + test({ + code: ` + import path from 'path'; // comment + import _ from 'lodash'; + `, + output: ` + import path from 'path'; // comment + + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'always' }], + errors: [ + { + line: 2, + message: 'There should be at least one empty line between import groups', + }, + ], + }), + // fix missing empty lines with few line block comment after + test({ + code: ` + import path from 'path'; /* comment */ /* comment */ + import _ from 'lodash'; + `, + output: ` + import path from 'path'; /* comment */ /* comment */ + + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'always' }], + errors: [ + { + line: 2, + message: 'There should be at least one empty line between import groups', + }, + ], + }), + // fix missing empty lines with single line block comment after + test({ + code: ` + import path from 'path'; /* 1 + 2 */ + import _ from 'lodash'; + `, + output: ` + import path from 'path'; + /* 1 + 2 */ + import _ from 'lodash'; + `, options: [{ 'newlines-between': 'always' }], errors: [ { @@ -786,5 +1153,112 @@ ruleTester.run('order', rule, { }, ], }), + + // reorder fix cannot cross non import or require + test(withoutAutofixOutput({ + code: ` + var async = require('async'); + fn_call(); + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // reorder cannot cross non plain requires + test(withoutAutofixOutput({ + code: ` + var async = require('async'); + var a = require('./value.js')(a); + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // reorder fixes cannot be applied to non plain requires #1 + test(withoutAutofixOutput({ + code: ` + var async = require('async'); + var fs = require('fs')(a); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // reorder fixes cannot be applied to non plain requires #2 + test(withoutAutofixOutput({ + code: ` + var async = require('async')(a); + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // cannot require in case of not assignement require + test(withoutAutofixOutput({ + code: ` + var async = require('async'); + require('./aa'); + var fs = require('fs'); + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // reorder cannot cross function call (import statement) + test(withoutAutofixOutput({ + code: ` + import async from 'async'; + fn_call(); + import fs from 'fs'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // reorder cannot cross variable assignemet (import statement) + test(withoutAutofixOutput({ + code: ` + import async from 'async'; + var a = 1; + import fs from 'fs'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // reorder cannot cross non plain requires (import statement) + test(withoutAutofixOutput({ + code: ` + import async from 'async'; + var a = require('./value.js')(a); + import fs from 'fs'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), + // cannot reorder in case of not assignement import + test(withoutAutofixOutput({ + code: ` + import async from 'async'; + import './aa'; + import fs from 'fs'; + `, + errors: [{ + ruleId: 'order', + message: '`fs` import should occur before import of `async`', + }], + })), ], }) From ab44320f8f99972f7e07f31804616652d0d79c98 Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Thu, 29 Mar 2018 20:21:58 -0400 Subject: [PATCH 23/26] changelog notes --- CHANGELOG.md | 3 +++ utils/CHANGELOG.md | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50b742d42..028d2a413 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] +### Added +- Add [`no-cycle`] rule: reports import cycles. ## [2.9.0] - 2018-02-21 ### Added @@ -440,6 +442,7 @@ for info on changes for earlier releases. [`group-exports`]: ./docs/rules/group-exports.md [`no-self-import`]: ./docs/rules/no-self-import.md [`no-default-export`]: ./docs/rules/no-default-export.md +[`no-cycle`]: ./docs/rules/no-cycle.md [`memo-parser`]: ./memo-parser/README.md diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index cc9bc051b..5aaa3cc76 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -4,7 +4,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## Unreleased - +### Changed +- `parse`: attach node locations by default. +- `moduleVisitor`: visitor now gets the full `import` statement node as a second + argument, so rules may report against the full statement / `require` call instead + of only the string literal node. ## v2.1.1 - 2017-06-22 From 82f67e69adb9b6850312cafb50c61cd23e49e87c Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Thu, 29 Mar 2018 20:45:12 -0400 Subject: [PATCH 24/26] bump utils to v2.2.0 --- utils/CHANGELOG.md | 2 ++ utils/package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/utils/CHANGELOG.md b/utils/CHANGELOG.md index 5aaa3cc76..018fd3066 100644 --- a/utils/CHANGELOG.md +++ b/utils/CHANGELOG.md @@ -4,6 +4,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## Unreleased + +## v2.2.0 - 2018-03-29 ### Changed - `parse`: attach node locations by default. - `moduleVisitor`: visitor now gets the full `import` statement node as a second diff --git a/utils/package.json b/utils/package.json index b961179db..d955c5368 100644 --- a/utils/package.json +++ b/utils/package.json @@ -1,6 +1,6 @@ { "name": "eslint-module-utils", - "version": "2.1.1", + "version": "2.2.0", "description": "Core utilities to support eslint-plugin-import and other module-related plugins.", "engines": { "node": ">=4" From 6356a78aa6fc2a2b294832fdf6e5b31630952def Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Thu, 29 Mar 2018 20:56:41 -0400 Subject: [PATCH 25/26] bump to v2.10.0 --- CHANGELOG.md | 6 +++++- README.md | 2 ++ package.json | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0af90c8d8..c3a940732 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] + + +## [2.10.0] - 2018-03-29 ### Added - Autofixer for [`order`] rule ([#711], thanks [@tihonove]) - Add [`no-cycle`] rule: reports import cycles. @@ -586,7 +589,8 @@ for info on changes for earlier releases. [#119]: https://github.com/benmosher/eslint-plugin-import/issues/119 [#89]: https://github.com/benmosher/eslint-plugin-import/issues/89 -[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v2.9.0...HEAD +[Unreleased]: https://github.com/benmosher/eslint-plugin-import/compare/v2.10.0...HEAD +[2.10.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.9.0...v2.10.0 [2.9.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.8.0...v2.9.0 [2.8.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.7.0...v2.8.0 [2.7.0]: https://github.com/benmosher/eslint-plugin-import/compare/v2.6.1...v2.7.0 diff --git a/README.md b/README.md index 528e58db5..821005689 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a * Prevent importing the submodules of other modules ([`no-internal-modules`]) * Forbid Webpack loader syntax in imports ([`no-webpack-loader-syntax`]) * Forbid a module from importing itself ([`no-self-import`]) +* Forbid a module from importing a module with a dependency path back to itself ([`no-cycle`]) [`no-unresolved`]: ./docs/rules/no-unresolved.md [`named`]: ./docs/rules/named.md @@ -35,6 +36,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a [`no-internal-modules`]: ./docs/rules/no-internal-modules.md [`no-webpack-loader-syntax`]: ./docs/rules/no-webpack-loader-syntax.md [`no-self-import`]: ./docs/rules/no-self-import.md +[`no-cycle`]: ./docs/rules/no-cycle.md ### Helpful warnings diff --git a/package.json b/package.json index 6282f6c2b..39768ca58 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "eslint-plugin-import", - "version": "2.9.0", + "version": "2.10.0", "description": "Import with sanity.", "engines": { "node": ">=4" @@ -84,7 +84,7 @@ "debug": "^2.6.8", "doctrine": "1.5.0", "eslint-import-resolver-node": "^0.3.1", - "eslint-module-utils": "^2.1.1", + "eslint-module-utils": "^2.2.0", "has": "^1.0.1", "lodash": "^4.17.4", "minimatch": "^3.0.3", From 47ac30fcee9556a1b8d6f0a4626463b7d3eb472c Mon Sep 17 00:00:00 2001 From: Ben Mosher <me@benmosher.com> Date: Thu, 29 Mar 2018 21:03:57 -0400 Subject: [PATCH 26/26] bump webpack resolver to v0.9.0 --- resolvers/webpack/CHANGELOG.md | 7 +++++-- resolvers/webpack/package.json | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/resolvers/webpack/CHANGELOG.md b/resolvers/webpack/CHANGELOG.md index 1ebc0d351..0abd1e8bf 100644 --- a/resolvers/webpack/CHANGELOG.md +++ b/resolvers/webpack/CHANGELOG.md @@ -4,8 +4,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## Unreleased -### Breaking (?) -- Fix with `pnpm` ([#968]) + + +## 0.9.0 - 2018-03-29 +### Breaking +- Fix with `pnpm` by bumping `resolve` ([#968]) ## 0.8.4 - 2018-01-05 ### Changed diff --git a/resolvers/webpack/package.json b/resolvers/webpack/package.json index 01b8ff7f5..9cbce0d47 100644 --- a/resolvers/webpack/package.json +++ b/resolvers/webpack/package.json @@ -1,6 +1,6 @@ { "name": "eslint-import-resolver-webpack", - "version": "0.8.4", + "version": "0.9.0", "description": "Resolve paths to dependencies, given a webpack.config.js. Plugin for eslint-plugin-import.", "main": "index.js", "scripts": {