[Fix] import/order: fix autofix to not move imports across fn calls

Fixes #1252. - Reordering import statement to line below ignores uncrossable statements - Add more tests for ordering around function call
import-js · Jan 3, 2019 · e62011f · e62011f
1 parent c14c9bd
commit e62011f
Show file tree

Hide file tree

Showing 3 changed files with 164 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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])
@@ -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
@@ -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
diff --git a/src/rules/order.js b/src/rules/order.js
@@ -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)) {

diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js
@@ -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: `
@@ -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: `