[New] order: new alphabetize.orderImportKind option to sort imports with same path based on their kind (type, typeof)

Fixes import-js#2339

Co-authored-by: Aaron Adams <[email protected]>
Co-authored-by: stropho <[email protected]>

Author: 2 people

CHANGELOG.md

@@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 - [`order`]: Add `distinctGroup` option ([#2395], thanks [@hyperupcall])
 - [`no-extraneous-dependencies`]: Add `includeInternal` option ([#2541], thanks [@bdwain])
 - [`no-extraneous-dependencies`]: Add `includeTypes` option ([#2543], thanks [@bdwain])
+- [`order`]: new `alphabetize.orderImportKind` option to sort imports with same path based on their kind (`type`, `typeof`) ([#2544], thanks [@stropho])
 
 ### Fixed
 - [`order`]: move nested imports closer to main import entry ([#2396], thanks [@pri1311])

docs/rules/order.md

@@ -267,11 +267,12 @@ import index from './';
 import sibling from './foo';
 ```
 
-### `alphabetize: {order: asc|desc|ignore, caseInsensitive: true|false}`:
+### `alphabetize: {order: asc|desc|ignore, orderImportKind: asc|desc|ignore, caseInsensitive: true|false}`:
 
 Sort the order within each group in alphabetical manner based on **import path**:
 
 - `order`: use `asc` to sort in ascending order, and `desc` to sort in descending order (default: `ignore`).
+- `orderImportKind`: use `asc` to sort in ascending order various import kinds, e.g. imports prefixed with `type` or `typeof`, with same import path. Use `desc` to sort in descending order (default: `ignore`).
 - `caseInsensitive`: use `true` to ignore case, and `false` to consider case (default: `false`).
 
 Example setting:

src/rules/order.js

@@ -187,6 +187,16 @@ function canReorderItems(firstNode, secondNode) {
   return true;
 }
 
+function makeImportDescription(node) {
+  if (node.node.importKind === 'type') {
+    return 'type import';
+  }
+  if (node.node.importKind === 'typeof') {
+    return 'typeof import';
+  }
+  return 'import';
+}
+
 function fixOutOfOrder(context, firstNode, secondNode, order) {
   const sourceCode = context.getSourceCode();
 
@@ -204,7 +214,9 @@ function fixOutOfOrder(context, firstNode, secondNode, order) {
     newCode = newCode + '\n';
   }
 
-  const message = `\`${secondNode.displayName}\` import should occur ${order} import of \`${firstNode.displayName}\``;
+  const firstImport = `${makeImportDescription(firstNode)} of \`${firstNode.displayName}\``;
+  const secondImport = `\`${secondNode.displayName}\` ${makeImportDescription(secondNode)}`;
+  const message = `${secondImport} should occur ${order} ${firstImport}`;
 
   if (order === 'before') {
     context.report({
@@ -253,42 +265,62 @@ function makeOutOfOrderReport(context, imported) {
   reportOutOfOrder(context, imported, outOfOrder, 'before');
 }
 
-function getSorter(ascending) {
-  const multiplier = ascending ? 1 : -1;
+const compareString = (a, b) => {
+  if (a < b) {
+    return -1;
+  } else if (a > b) {
+    return 1;
+  }
+  return 0;
+};
+
+/** Some parsers (languages without types) don't provide ImportKind */
+const DEAFULT_IMPORT_KIND = 'value';
+const getNormalizedValue = (node, toLowerCase) => {
+  const value = node.value;
+  return toLowerCase ? String(value).toLowerCase() : value;
+};
+
+function getSorter(alphabetizeOptions) {
+  const multiplier = alphabetizeOptions.order === 'asc' ? 1 : -1;
+  const orderImportKind = alphabetizeOptions.orderImportKind;
+  const multiplierImportKind = orderImportKind !== 'ignore' &&
+    (alphabetizeOptions.orderImportKind === 'asc' ? 1 : -1);
 
-  return function importsSorter(importA, importB) {
+  return function importsSorter(nodeA, nodeB) {
+    const importA = getNormalizedValue(nodeA, alphabetizeOptions.caseInsensitive);
+    const importB = getNormalizedValue(nodeB, alphabetizeOptions.caseInsensitive);
     let result = 0;
 
     if (!includes(importA, '/') && !includes(importB, '/')) {
-      if (importA < importB) {
-        result = -1;
-      } else if (importA > importB) {
-        result = 1;
-      } else {
-        result = 0;
-      }
+      result = compareString(importA, importB);
     } else {
       const A = importA.split('/');
       const B = importB.split('/');
       const a = A.length;
       const b = B.length;
 
       for (let i = 0; i < Math.min(a, b); i++) {
-        if (A[i] < B[i]) {
-          result = -1;
-          break;
-        } else if (A[i] > B[i]) {
-          result = 1;
-          break;
-        }
+        result = compareString(A[i], B[i]);
+        if (result) break;
       }
 
       if (!result && a !== b) {
         result = a < b ? -1 : 1;
       }
     }
 
-    return result * multiplier;
+    result = result * multiplier;
+
+    // In case the paths are equal (result === 0), sort them by importKind
+    if (!result && multiplierImportKind) {
+      result = multiplierImportKind * compareString(
+        nodeA.node.importKind || DEAFULT_IMPORT_KIND,
+        nodeB.node.importKind || DEAFULT_IMPORT_KIND,
+      );
+    }
+
+    return result;
   };
 }
 
@@ -303,14 +335,11 @@ function mutateRanksToAlphabetize(imported, alphabetizeOptions) {
 
   const groupRanks = Object.keys(groupedByRanks);
 
-  const sorterFn = getSorter(alphabetizeOptions.order === 'asc');
-  const comparator = alphabetizeOptions.caseInsensitive
-    ? (a, b) => sorterFn(String(a.value).toLowerCase(), String(b.value).toLowerCase())
-    : (a, b) => sorterFn(a.value, b.value);
+  const sorterFn = getSorter(alphabetizeOptions);
 
   // sort imports locally within their group
   groupRanks.forEach(function (groupRank) {
-    groupedByRanks[groupRank].sort(comparator);
+    groupedByRanks[groupRank].sort(sorterFn);
   });
 
   // assign globally unique rank to each import
@@ -546,9 +575,10 @@ function makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, di
 function getAlphabetizeConfig(options) {
   const alphabetize = options.alphabetize || {};
   const order = alphabetize.order || 'ignore';
+  const orderImportKind = alphabetize.orderImportKind || 'ignore';
   const caseInsensitive = alphabetize.caseInsensitive || false;
 
-  return { order, caseInsensitive };
+  return { order, orderImportKind, caseInsensitive };
 }
 
 // TODO, semver-major: Change the default of "distinctGroup" from true to false
@@ -619,6 +649,10 @@ module.exports = {
                 enum: ['ignore', 'asc', 'desc'],
                 default: 'ignore',
               },
+              orderImportKind: {
+                enum: ['ignore', 'asc', 'desc'],
+                default: 'ignore',
+              },
             },
             additionalProperties: false,
           },

tests/src/rules/order.js

@@ -4,8 +4,21 @@ import { RuleTester } from 'eslint';
 import eslintPkg from 'eslint/package.json';
 import semver from 'semver';
 import flatMap from 'array.prototype.flatmap';
+import { resolve } from 'path';
+import { default as babelPresetFlow } from 'babel-preset-flow';
+
 
 const ruleTester = new RuleTester();
+const flowRuleTester = new RuleTester({
+  parser: resolve(__dirname, '../../../node_modules/babel-eslint'),
+  parserOptions: {
+    babelOptions: {
+      configFile: false,
+      babelrc: false,
+      presets: [babelPresetFlow],
+    },
+  },
+});
 const rule = require('rules/order');
 
 function withoutAutofixOutput(test) {
@@ -1080,6 +1093,19 @@ ruleTester.run('order', rule, {
         },
       ],
     }),
+    // orderImportKind option that is not used
+    test({
+      code: `
+            import B from './B';
+            import b from './b';
+          `,
+      options: [
+        {
+          'alphabetize': { order: 'asc', orderImportKind: 'asc', 'caseInsensitive': true },
+        },
+      ],
+    }),
+
   ],
   invalid: [
     // builtin before external module (require)
@@ -2931,8 +2957,8 @@ context('TypeScript', function () {
             errors: [
               {
                 message: semver.satisfies(eslintPkg.version, '< 3')
-                  ? '`bar` import should occur after import of `Bar`'
-                  : /(`bar` import should occur after import of `Bar`)|(`Bar` import should occur before import of `bar`)/,
+                  ? '`bar` import should occur after type import of `Bar`'
+                  : /(`bar` import should occur after type import of `Bar`)|(`Bar` type import should occur before import of `bar`)/,
               },
             ],
           }),
@@ -3002,10 +3028,10 @@ context('TypeScript', function () {
             ],
             errors: semver.satisfies(eslintPkg.version, '< 3') ? [
               { message: '`Bar` import should occur before import of `bar`' },
-              { message: '`Bar` import should occur before import of `foo`' },
+              { message: '`Bar` type import should occur before type import of `foo`' },
             ] : [
               { message: /(`Bar` import should occur before import of `bar`)|(`bar` import should occur after import of `Bar`)/ },
-              { message: /(`Bar` import should occur before import of `foo`)|(`foo` import should occur after import of `Bar`)/ },
+              { message: /(`Bar` type import should occur before type import of `foo`)|(`foo` type import should occur after type import of `Bar`)/ },
             ],
           }),
           // Option alphabetize: {order: 'desc'} with type group
@@ -3039,10 +3065,10 @@ context('TypeScript', function () {
             ],
             errors: semver.satisfies(eslintPkg.version, '< 3') ? [
               { message: '`bar` import should occur before import of `Bar`' },
-              { message: '`foo` import should occur before import of `Bar`' },
+              { message: '`foo` type import should occur before type import of `Bar`' },
             ] : [
               { message: /(`bar` import should occur before import of `Bar`)|(`Bar` import should occur after import of `bar`)/ },
-              { message: /(`foo` import should occur before import of `Bar`)|(`Bar` import should occur after import of `foo`)/ },
+              { message: /(`foo` type import should occur before type import of `Bar`)|(`Bar` type import should occur after import of type `foo`)/ },
             ],
           }),
           // warns for out of order unassigned imports (warnOnUnassignedImports enabled)
@@ -3113,9 +3139,9 @@ context('TypeScript', function () {
               }
             `,
             errors: [{
-              message: '`fs` import should occur before import of `path`',
+              message: '`fs` type import should occur before type import of `path`',
             },{
-              message: '`fs` import should occur before import of `path`',
+              message: '`fs` type import should occur before type import of `path`',
             }],
             ...parserConfig,
             options: [
@@ -3128,3 +3154,83 @@ context('TypeScript', function () {
       });
     });
 });
+
+flowRuleTester.run('order', rule, {
+  valid: [
+    test({
+      options: [
+        {
+          alphabetize: { order: 'asc', orderImportKind: 'asc' },
+        },
+      ],
+      code: `
+        import type {Bar} from 'common';
+        import typeof {foo} from 'common';
+        import {bar} from 'common';
+      `,
+    })],
+  invalid: [
+    test({
+      options: [
+        {
+          alphabetize: { order: 'asc', orderImportKind: 'asc' },
+        },
+      ],
+      code: `
+        import type {Bar} from 'common';
+        import {bar} from 'common';
+        import typeof {foo} from 'common';
+      `,
+      output: `
+        import type {Bar} from 'common';
+        import typeof {foo} from 'common';
+        import {bar} from 'common';
+      `,
+      errors: [{
+        message: '`common` typeof import should occur before import of `common`',
+      }],
+    }),
+    test({
+      options: [
+        {
+          alphabetize: { order: 'asc', orderImportKind: 'desc' },
+        },
+      ],
+      code: `
+        import type {Bar} from 'common';
+        import {bar} from 'common';
+        import typeof {foo} from 'common';
+      `,
+      output: `
+        import {bar} from 'common';
+        import typeof {foo} from 'common';
+        import type {Bar} from 'common';
+      `,
+      errors: [{
+        message: '`common` type import should occur after typeof import of `common`',
+      }],
+    }),
+    test({
+      options: [
+        {
+          alphabetize: { order: 'asc', orderImportKind: 'asc' },
+        },
+      ],
+      code: `
+        import type {Bar} from './local/sub';
+        import {bar} from './local/sub';
+        import {baz} from './local-sub';
+        import typeof {foo} from './local/sub';
+      `,
+      output: `
+        import type {Bar} from './local/sub';
+        import typeof {foo} from './local/sub';
+        import {bar} from './local/sub';
+        import {baz} from './local-sub';
+      `,
+      errors: [{
+        message: '`./local/sub` typeof import should occur before import of `./local/sub`',
+      }],
+    }),
+  ],
+});