Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New] order: Add warnOnUnassignedImports option to enable warnings for out of order unassigned imports #1990

Merged
merged 1 commit into from May 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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])
Expand Down Expand Up @@ -773,6 +774,7 @@ for info on changes for earlier releases.
[#2012]: https://github.com/benmosher/eslint-plugin-import/pull/2012
[#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
Expand Down Expand Up @@ -1363,3 +1365,4 @@ for info on changes for earlier releases.
[@silviogutierrez]: https://github.com/silviogutierrez
[@aladdin-add]: https://github.com/aladdin-add
[@davidbonnet]: https://github.com/davidbonnet
[@hayes]: https://github.com/hayes
27 changes: 27 additions & 0 deletions docs/rules/order.md
Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions src/rules/order.js
Expand Up @@ -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)
) {
Expand All @@ -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'
);
}

Expand Down Expand Up @@ -568,6 +568,10 @@ module.exports = {
},
additionalProperties: false,
},
warnOnUnassignedImports: {
type: 'boolean',
default: false,
},
},
additionalProperties: false,
},
Expand Down Expand Up @@ -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,
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 @@ -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 }],
}),
],
});
});
Expand Down