Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[order] Sort imports by import kinds #2544

Merged
merged 1 commit into from Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
85 changes: 60 additions & 25 deletions src/rules/order.js
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,63 @@ 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;
}
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 +336,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 +576,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 +650,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
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`',
}],
}),
],
});