Skip to content

Commit

Permalink
[Fix] import/order: fix autofix to not move imports across fn calls
Browse files Browse the repository at this point in the history
Fixes #1252.

 - Reordering import statement to line below ignores uncrossable statements
 - Add more tests for ordering around function call
  • Loading branch information
tihonove authored and ljharb committed Jan 3, 2019
1 parent c14c9bd commit e62011f
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 4 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Expand Up @@ -8,14 +8,13 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
### Added
- [`group-exports`]: make aggregate module exports valid ([#1472], thanks [@atikenny])
- [`no-namespace`]: Make rule fixable ([#1401], thanks [@TrevorBurnham])

### Added
- support `parseForESLint` from custom parser ([#1435], thanks [@JounQin])
- [`no-extraneous-dependencies`]: Implement support for [bundledDependencies](https://npm.github.io/using-pkgs-docs/package-json/types/bundleddependencies.html) ([#1436], thanks [@schmidsi]))

### Fixed
- `default`: make error message less confusing ([#1470], thanks [@golopot])
- Support export of a merged typescript namespace declaration ([#1495], thanks [@benmunro])
- [`import/order`]: fix autofix to not move imports across fn calls ([#1253], thanks [@tihonove])

## [2.18.2] - 2019-07-19
- Skip warning on type interfaces ([#1425], thanks [@lencioni])
Expand Down Expand Up @@ -651,6 +650,7 @@ for info on changes for earlier releases.
[#1290]: https://github.com/benmosher/eslint-plugin-import/pull/1290
[#1277]: https://github.com/benmosher/eslint-plugin-import/pull/1277
[#1257]: https://github.com/benmosher/eslint-plugin-import/pull/1257
[#1253]: https://github.com/benmosher/eslint-plugin-import/pull/1253
[#1235]: https://github.com/benmosher/eslint-plugin-import/pull/1235
[#1234]: https://github.com/benmosher/eslint-plugin-import/pull/1234
[#1232]: https://github.com/benmosher/eslint-plugin-import/pull/1232
Expand Down Expand Up @@ -995,3 +995,4 @@ for info on changes for earlier releases.
[@schmidsi]: https://github.com/schmidsi
[@TrevorBurnham]: https://github.com/TrevorBurnham
[@benmunro]: https://github.com/benmunro
[@tihonove]: https://github.com/tihonove
6 changes: 4 additions & 2 deletions src/rules/order.js
Expand Up @@ -162,8 +162,10 @@ function canCrossNodeWhileReorder(node) {

function canReorderItems(firstNode, secondNode) {
const parent = firstNode.parent
const firstIndex = parent.body.indexOf(firstNode)
const secondIndex = parent.body.indexOf(secondNode)
const [firstIndex, secondIndex] = [
parent.body.indexOf(firstNode),
parent.body.indexOf(secondNode),
].sort()
const nodesBetween = parent.body.slice(firstIndex, secondIndex + 1)
for (var nodeBetween of nodesBetween) {
if (!canCrossNodeWhileReorder(nodeBetween)) {
Expand Down
157 changes: 157 additions & 0 deletions tests/src/rules/order.js
Expand Up @@ -1265,7 +1265,137 @@ ruleTester.run('order', rule, {
},
],
}),
// reorder fix cannot cross function call on moving below #1
test({
code: `
const local = require('./local');
fn_call();
const global1 = require('global1');
const global2 = require('global2');
fn_call();
`,
output: `
const local = require('./local');
fn_call();
const global1 = require('global1');
const global2 = require('global2');
fn_call();
`,
errors: [{
ruleId: 'order',
message: '`./local` import should occur after import of `global2`',
}],
}),
// reorder fix cannot cross function call on moving below #2
test({
code: `
const local = require('./local');
fn_call();
const global1 = require('global1');
const global2 = require('global2');
fn_call();
`,
output: `
const local = require('./local');
fn_call();
const global1 = require('global1');
const global2 = require('global2');
fn_call();
`,
errors: [{
ruleId: 'order',
message: '`./local` import should occur after import of `global2`',
}],
}),
// reorder fix cannot cross function call on moving below #3
test({
code: `
const local1 = require('./local1');
const local2 = require('./local2');
const local3 = require('./local3');
const local4 = require('./local4');
fn_call();
const global1 = require('global1');
const global2 = require('global2');
const global3 = require('global3');
const global4 = require('global4');
const global5 = require('global5');
fn_call();
`,
output: `
const local1 = require('./local1');
const local2 = require('./local2');
const local3 = require('./local3');
const local4 = require('./local4');
fn_call();
const global1 = require('global1');
const global2 = require('global2');
const global3 = require('global3');
const global4 = require('global4');
const global5 = require('global5');
fn_call();
`,
errors: [
'`./local1` import should occur after import of `global5`',
'`./local2` import should occur after import of `global5`',
'`./local3` import should occur after import of `global5`',
'`./local4` import should occur after import of `global5`',
],
}),
// reorder fix cannot cross function call on moving below
test(withoutAutofixOutput({
code: `
const local = require('./local');
const global1 = require('global1');
const global2 = require('global2');
fn_call();
const global3 = require('global3');
fn_call();
`,
errors: [{
ruleId: 'order',
message: '`./local` import should occur after import of `global3`',
}],
})),
// reorder fix cannot cross function call on moving below
// fix imports that not crosses function call only
test({
code: `
const local1 = require('./local1');
const global1 = require('global1');
const global2 = require('global2');
fn_call();
const local2 = require('./local2');
const global3 = require('global3');
const global4 = require('global4');
fn_call();
`,
output: `
const local1 = require('./local1');
const global1 = require('global1');
const global2 = require('global2');
fn_call();
const global3 = require('global3');
const global4 = require('global4');
const local2 = require('./local2');
fn_call();
`,
errors: [
'`./local1` import should occur after import of `global4`',
'`./local2` import should occur after import of `global4`',
],
}),
// reorder fix cannot cross non import or require
test(withoutAutofixOutput({
code: `
Expand All @@ -1278,6 +1408,33 @@ ruleTester.run('order', rule, {
message: '`fs` import should occur before import of `async`',
}],
})),
// reorder fix cannot cross function call on moving below (from #1252)
test({
code: `
const env = require('./config');
Object.keys(env);
const http = require('http');
const express = require('express');
http.createServer(express());
`,
output: `
const env = require('./config');
Object.keys(env);
const http = require('http');
const express = require('express');
http.createServer(express());
`,
errors: [{
ruleId: 'order',
message: '`./config` import should occur after import of `express`',
}],
}),
// reorder cannot cross non plain requires
test(withoutAutofixOutput({
code: `
Expand Down

0 comments on commit e62011f

Please sign in to comment.