Skip to content

Commit

Permalink
[New] order: new alphabetize.orderImportKind option to sort impor…
Browse files Browse the repository at this point in the history 
…ts with same path based on their kind (`type`, `typeof`)

Fixes #2339

Co-authored-by: Aaron Adams <aaron@aaronadams.ca>
Co-authored-by: stropho <3704482+stropho@users.noreply.github.com>
  • Loading branch information
@aaronadamsCA @stropho @ljharb
2 people authored and ljharb committed Mar 4, 2022
1 parent 395e26b commit a133e5f
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 34 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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])
Expand Down
3 changes: 2 additions & 1 deletion docs/rules/order.md
View file
Expand Up @@ -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:
Expand Down
84 changes: 59 additions & 25 deletions src/rules/order.js
View file
Expand Up @@ -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();

Expand All @@ -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({
Expand Down Expand Up @@ -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;
};
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -619,6 +649,10 @@ module.exports = {
enum: ['ignore', 'asc', 'desc'],
default: 'ignore',
},
orderImportKind: {
enum: ['ignore', 'asc', 'desc'],
default: 'ignore',
},
},
additionalProperties: false,
},
Expand Down
122 changes: 114 additions & 8 deletions tests/src/rules/order.js
View file
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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`)/,
},
],
}),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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: [
Expand All @@ -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`',
}],
}),
],
});

0 comments on commit a133e5f

Please sign in to comment.