diff --git a/CHANGELOG.md b/CHANGELOG.md index ea766cc340..2ce9ff7d89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel - add [`no-import-module-exports`] rule: report import declarations with CommonJS exports ([#804], thanks [@kentcdodds] and [@ttmarek]) - [`no-unused-modules`]: Support destructuring assignment for `export`. ([#1997], thanks [@s-h-a-d-o-w]) - [`order`]: support type imports ([#2021], thanks [@grit96]) +- [`order`]: Add `warnOnUnassignedImports` option to enable warnings for out of order unassigned imports ([#1990], thanks [@hayes]) ### Fixed - [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella]) @@ -769,6 +770,7 @@ for info on changes for earlier releases. [#2021]: https://github.com/benmosher/eslint-plugin-import/pull/2021 [#1997]: https://github.com/benmosher/eslint-plugin-import/pull/1997 [#1993]: https://github.com/benmosher/eslint-plugin-import/pull/1993 +[#1990]: https://github.com/benmosher/eslint-plugin-import/pull/1990 [#1985]: https://github.com/benmosher/eslint-plugin-import/pull/1985 [#1983]: https://github.com/benmosher/eslint-plugin-import/pull/1983 [#1974]: https://github.com/benmosher/eslint-plugin-import/pull/1974 @@ -1357,3 +1359,4 @@ for info on changes for earlier releases. [@grit96]: https://github.com/grit96 [@lilling]: https://github.com/lilling [@silviogutierrez]: https://github.com/silviogutierrez +[@hayes]: https://github.com/hayes \ No newline at end of file diff --git a/docs/rules/order.md b/docs/rules/order.md index c2358edc13..848c91ddef 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -256,6 +256,33 @@ import React, { PureComponent } from 'react'; import { compose, apply } from 'xcompose'; ``` +### `warnOnUnassignedImports: true|false`: + +* default: `false` + +Warns when unassigned imports are out of order. These warning will not be fixed +with `--fix` because unassigned imports are used for side-effects and changing the +import of order of modules with side effects can not be done automatically in a +way that is safe. + +This will fail the rule check: + +```js +/* eslint import/order: ["error", {"warnOnUnassignedImports": true}] */ +import fs from 'fs'; +import './styles.css'; +import path from 'path'; +``` + +While this will pass: + +```js +/* eslint import/order: ["error", {"warnOnUnassignedImports": true}] */ +import fs from 'fs'; +import path from 'path'; +import './styles.css'; +``` + ## Related - [`import/external-module-folders`] setting diff --git a/src/rules/order.js b/src/rules/order.js index 3aabbc74c7..34cc992e8e 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -339,7 +339,7 @@ function isModuleLevelRequire(node) { let n = node; // Handle cases like `const baz = require('foo').bar.baz` // and `const foo = require('foo')()` - while ( + while ( (n.parent.type === 'MemberExpression' && n.parent.object === n) || (n.parent.type === 'CallExpression' && n.parent.callee === n) ) { @@ -348,7 +348,7 @@ function isModuleLevelRequire(node) { return ( n.parent.type === 'VariableDeclarator' && n.parent.parent.type === 'VariableDeclaration' && - n.parent.parent.parent.type === 'Program' + n.parent.parent.parent.type === 'Program' ); } @@ -568,6 +568,10 @@ module.exports = { }, additionalProperties: false, }, + warnOnUnassignedImports: { + type: 'boolean', + default: false, + }, }, additionalProperties: false, }, @@ -600,7 +604,8 @@ module.exports = { return { ImportDeclaration: function handleImports(node) { - if (node.specifiers.length) { // Ignoring unassigned imports + // Ignoring unassigned imports unless warnOnUnassignedImports is set + if (node.specifiers.length || options.warnOnUnassignedImports) { const name = node.source.value; registerNode( context, diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 4fcbb29d73..9b4103127a 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -854,10 +854,10 @@ ruleTester.run('order', rule, { test({ code: `/* comment0 */ /* comment1 */ var async = require('async'); /* comment2 */` + `\r\n` + - `/* comment3 */ var fs = require('fs'); /* comment4 */` + `\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`, + `/* comment0 */ /* comment1 */ var async = require('async'); /* comment2 */` + `\r\n`, errors: [{ message: '`fs` import should occur before import of `async`', }], @@ -1530,7 +1530,8 @@ ruleTester.run('order', rule, { }, ], }), - // Option newlines-between: 'never' cannot fix if there are other statements between imports + // Option newlines-between: 'never' with unassigned imports and warnOnUnassignedImports disabled + // newline is preserved to match existing behavior test({ code: ` import path from 'path'; @@ -1546,6 +1547,53 @@ ruleTester.run('order', rule, { import 'something-else'; import _ from 'lodash'; `, + options: [{ 'newlines-between': 'never', warnOnUnassignedImports: false }], + errors: [ + { + line: 2, + message: 'There should be no empty line between import groups', + }, + ], + }), + // Option newlines-between: 'never' with unassigned imports and warnOnUnassignedImports enabled + test({ + code: ` + import path from 'path'; + import 'loud-rejection'; + + import 'something-else'; + import _ from 'lodash'; + `, + output: ` + import path from 'path'; + import 'loud-rejection'; + import 'something-else'; + import _ from 'lodash'; + `, + options: [{ 'newlines-between': 'never', warnOnUnassignedImports: true }], + errors: [ + { + line: 3, + message: 'There should be no empty line between import groups', + }, + ], + }), + // Option newlines-between: 'never' cannot fix if there are other statements between imports + test({ + code: ` + import path from 'path'; + export const abc = 123; + + import 'something-else'; + import _ from 'lodash'; + `, + output: ` + import path from 'path'; + export const abc = 123; + + import 'something-else'; + import _ from 'lodash'; + `, options: [{ 'newlines-between': 'never' }], errors: [ { @@ -1764,7 +1812,6 @@ ruleTester.run('order', rule, { '`./local2` import should occur after import of `global4`', ], }), - // pathGroup with position 'after' test({ code: ` @@ -2309,6 +2356,53 @@ context('TypeScript', function () { }, parserConfig, ), + // warns for out of order unassigned imports (warnOnUnassignedImports enabled) + test({ + code: ` + import './local1'; + import global from 'global1'; + import local from './local2'; + import 'global2'; + `, + output: ` + import './local1'; + import global from 'global1'; + import local from './local2'; + import 'global2'; + `, + errors: [ + { + message: '`global1` import should occur before import of `./local1`', + }, + { + message: '`global2` import should occur before import of `./local1`', + }, + ], + options: [{ warnOnUnassignedImports: true }], + }), + // fix cannot move below unassigned import (warnOnUnassignedImports enabled) + test({ + code: ` + import local from './local'; + + import 'global1'; + + import global2 from 'global2'; + import global3 from 'global3'; + `, + output: ` + import local from './local'; + + import 'global1'; + + import global2 from 'global2'; + import global3 from 'global3'; + `, + errors: [{ + message: '`./local` import should occur after import of `global3`', + }], + options: [{ warnOnUnassignedImports: true }], + }), ], }); });