Skip to content

Commit

Permalink
[New]: Add option to enable warnings for out of order unassigned imports
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael Hayes committed Feb 12, 2021
1 parent f2db74a commit d93eb29
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 8 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-internal-modules`]: Add `forbid` option ([#1846], thanks [@guillaumewuip])
- add [`no-relative-packages`] ([#1860], [#966], thanks [@tapayne88] [@panrafal])
- add [`no-import-module-exports`] rule: report import declarations with CommonJS exports ([#804], thanks [@kentcdodds] and [@ttmarek])
- [`order`]: Add `warnOnUnassignedImports` option to enable warnings for out of order unassigned imports

### Fixed
- [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella])
Expand Down Expand Up @@ -1340,4 +1341,4 @@ for info on changes for earlier releases.
[@panrafal]: https://github.com/panrafal
[@ttmarek]: https://github.com/ttmarek
[@christianvuerings]: https://github.com/christianvuerings
[@devongovett]: https://github.com/devongovett
[@devongovett]: https://github.com/devongovett
27 changes: 27 additions & 0 deletions docs/rules/order.md
Expand Up @@ -253,6 +253,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
Expand Down
11 changes: 8 additions & 3 deletions src/rules/order.js
Expand Up @@ -337,7 +337,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)
) {
Expand All @@ -346,7 +346,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'
);
}

Expand Down Expand Up @@ -566,6 +566,10 @@ module.exports = {
},
additionalProperties: false,
},
warnOnUnassignedImports: {
type: 'boolean',
default: false,
},
},
additionalProperties: false,
},
Expand Down Expand Up @@ -598,7 +602,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,
Expand Down
102 changes: 98 additions & 4 deletions tests/src/rules/order.js
Expand Up @@ -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`',
}],
Expand Down Expand Up @@ -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';
Expand All @@ -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: [
{
Expand Down Expand Up @@ -1764,7 +1812,6 @@ ruleTester.run('order', rule, {
'`./local2` import should occur after import of `global4`',
],
}),

// pathGroup with position 'after'
test({
code: `
Expand Down Expand Up @@ -2299,6 +2346,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 }],
}),
],
});
});
Expand Down

0 comments on commit d93eb29

Please sign in to comment.