From a133e5f28d93c06ba48acf455ddc6078630bd344 Mon Sep 17 00:00:00 2001 From: Aaron Adams Date: Fri, 4 Mar 2022 20:59:08 +0000 Subject: [PATCH] [New] `order`: new `alphabetize.orderImportKind` option to sort imports with same path based on their kind (`type`, `typeof`) Fixes #2339 Co-authored-by: Aaron Adams Co-authored-by: stropho <3704482+stropho@users.noreply.github.com> --- CHANGELOG.md | 1 + docs/rules/order.md | 3 +- src/rules/order.js | 84 +++++++++++++++++++-------- tests/src/rules/order.js | 122 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 176 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3351937a98..89ed22ac03 100644 --- a/CHANGELOG.md +++ b/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]) diff --git a/docs/rules/order.md b/docs/rules/order.md index 53faff1530..c525dfbbac 100644 --- a/docs/rules/order.md +++ b/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: diff --git a/src/rules/order.js b/src/rules/order.js index 95311c0bcf..ed8070da5a 100644 --- a/src/rules/order.js +++ b/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,20 +265,35 @@ 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('/'); @@ -274,13 +301,8 @@ function getSorter(ascending) { 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) { @@ -288,7 +310,17 @@ function getSorter(ascending) { } } - 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, }, diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index d143e54f9e..07511ee4de 100644 --- a/tests/src/rules/order.js +++ b/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`', + }], + }), + ], +});