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

Fixes #1639

Author: Michael Hayes

Diff for: 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])
@@ -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
@@ -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

Diff for: 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 for: 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 for: 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 }],
+          }),
         ],
       });
     });