From e780a6b9c36d60e35fb4ff8c5c0f70ff9b1cf445 Mon Sep 17 00:00:00 2001 From: Pearce Date: Sat, 21 Jan 2023 20:07:16 -0800 Subject: [PATCH 1/4] Add intra group ordering --- src/rules/order.js | 200 +++++++++++++++++++++++++++++---------- tests/src/rules/order.js | 90 +++++++++++++++++- 2 files changed, 238 insertions(+), 52 deletions(-) diff --git a/src/rules/order.js b/src/rules/order.js index bdead9d40..035ebea8b 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -3,7 +3,7 @@ import minimatch from 'minimatch'; import includes from 'array-includes'; -import importType from '../core/importType'; +import resolveImportType from '../core/importType'; import isStaticRequire from '../core/staticRequire'; import docsUrl from '../docsUrl'; @@ -325,81 +325,169 @@ function getSorter(alphabetizeOptions) { }; } -function mutateRanksToAlphabetize(imported, alphabetizeOptions) { +function mutateRanksForIntraGroupOrdering(computedContext, imported, groups, intraGroupOrdering, alphabetizeOptions) { + const { omittedTypes } = computedContext; + + const nonTypeGroups = new Set(groups.flat()); + if (intraGroupOrdering) { + nonTypeGroups.delete('type'); + + if (omittedTypes.size) { + nonTypeGroups.add('unknown'); + } + } + + let groupIndexMap = {}; + if (intraGroupOrdering) { + // map of indices of each import type in the original groups array + groupIndexMap = groups.reduce(function (acc, group, idx) { + if (typeof group === 'string') { + acc[group] = idx; + } else if (Array.isArray(group)) { + for (const groupItem of group) { + acc[groupItem] = idx; + } + } + + return acc; + }, groupIndexMap); + } + const groupedByRanks = imported.reduce(function (acc, importedItem) { - if (!Array.isArray(acc[importedItem.rank])) { - acc[importedItem.rank] = []; + const { rank, groupType, pathType } = importedItem; + + if ( + !intraGroupOrdering || + (intraGroupOrdering && (omittedTypes.has(groupType) || !includes(types, pathType))) + ) { + if (!Array.isArray(acc[rank])) { + acc[rank] = [[]]; + } + + acc[rank][0].push(importedItem); + } else if (intraGroupOrdering) { + // index of the import type in the original groups array + const groupsIndex = groupIndexMap[groupType]; + let group = Array.isArray(groups[groupsIndex]) ? groups[groupsIndex] : [groups[groupsIndex]]; + + if (groupType === 'type') { + if (omittedTypes.size) { + nonTypeGroups.add('unknown'); + } + + // sort type imports in the same order groups are sorted in but without newlines + group = Array.from(nonTypeGroups); + } + + if (!Array.isArray(acc[rank])) { + acc[rank] = Array.from({ length: group.length }, function () { + return []; + }); + } + + let groupIdx; + if (groupType === 'type') { + groupIdx = group.indexOf(pathType); + + if (groupIdx === -1) { + // if a type isn't specified, put in in the `unknown` group + groupIdx = group.length - 1; + } + } else { + groupIdx = group.indexOf(groupType); + } + + acc[rank][groupIdx].push(importedItem); } - acc[importedItem.rank].push(importedItem); + return acc; }, {}); - const sorterFn = getSorter(alphabetizeOptions); // sort group keys so that they can be iterated on in order const groupRanks = Object.keys(groupedByRanks).sort(function (a, b) { return a - b; }); - // sort imports locally within their group - groupRanks.forEach(function (groupRank) { - groupedByRanks[groupRank].sort(sorterFn); - }); + if (alphabetizeOptions.order !== 'ignore') { + const sorterFn = getSorter(alphabetizeOptions); + + // sort imports locally within their group + for (const groupRank of groupRanks) { + for (const importedGroup of groupedByRanks[groupRank]) { + importedGroup.sort(sorterFn); + } + } + } // assign globally unique rank to each import let newRank = 0; - const alphabetizedRanks = groupRanks.reduce(function (acc, groupRank) { - groupedByRanks[groupRank].forEach(function (importedItem) { - acc[`${importedItem.value}|${importedItem.node.importKind}`] = parseInt(groupRank, 10) + newRank; - newRank += 1; - }); + const mutatedRanks = groupRanks.reduce(function (acc, groupRank) { + for (const importedGroup of groupedByRanks[groupRank]) { + for (const importedItem of importedGroup) { + acc[`${importedItem.value}|${importedItem.node.importKind}`] = parseInt(groupRank, 10) + newRank; + newRank += 1; + } + } return acc; }, {}); // mutate the original group-rank with alphabetized-rank imported.forEach(function (importedItem) { - importedItem.rank = alphabetizedRanks[`${importedItem.value}|${importedItem.node.importKind}`]; + importedItem.rank = mutatedRanks[`${importedItem.value}|${importedItem.node.importKind}`]; }); } // DETECTING -function computePathRank(ranks, pathGroups, path, maxPosition) { +function computePathGroupRank(ranks, pathGroups, path, maxPosition) { for (let i = 0, l = pathGroups.length; i < l; i++) { const { pattern, patternOptions, group, position = 1 } = pathGroups[i]; if (minimatch(path, pattern, patternOptions || { nocomment: true })) { - return ranks[group] + (position / maxPosition); + return [ranks[group] + (position / maxPosition), group]; } } } -function computeRank(context, ranks, importEntry, excludedImportTypes) { - let impType; +function computeRank(context, computedContext, importEntry, excludedImportTypes) { let rank; + let groupType; // the assigned group type of the import + let pathType; // the computed type of the import's path with no other modifiers (used for sorting type imports) + if (importEntry.type === 'import:object') { - impType = 'object'; - } else if (importEntry.node.importKind === 'type' && ranks.omittedTypes.indexOf('type') === -1) { - impType = 'type'; + groupType = 'object'; } else { - impType = importType(importEntry.value, context); + groupType = resolveImportType(importEntry.value, context); + pathType = groupType; + } + + if (importEntry.node.importKind === 'type' && !computedContext.omittedTypes.has('type')) { + groupType = 'type'; } - if (!excludedImportTypes.has(impType)) { - rank = computePathRank(ranks.groups, ranks.pathGroups, importEntry.value, ranks.maxPosition); + + if (!excludedImportTypes.has(groupType)) { + const pathGroupRank = computePathGroupRank(computedContext.ranks, computedContext.pathGroups, importEntry.value, computedContext.maxPosition); + if (pathGroupRank) { + [rank, groupType] = pathGroupRank; + pathType = groupType; + } } + if (typeof rank === 'undefined') { - rank = ranks.groups[impType]; + rank = computedContext.ranks[groupType]; } + if (importEntry.type !== 'import' && !importEntry.type.startsWith('import:')) { rank += 100; } - return rank; + return { rank, groupType, pathType }; } -function registerNode(context, importEntry, ranks, imported, excludedImportTypes) { - const rank = computeRank(context, ranks, importEntry, excludedImportTypes); - if (rank !== -1) { - imported.push(Object.assign({}, importEntry, { rank })); +function registerNode(context, importEntry, computedContext, imported, excludedImportTypes) { + const computedRank = computeRank(context, computedContext, importEntry, excludedImportTypes); + if (computedRank.rank !== -1) { + imported.push(Object.assign({}, importEntry, computedRank)); } } @@ -428,10 +516,11 @@ const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling' // Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 } // Will throw an error if it contains a type that does not exist, or has a duplicate function convertGroupsToRanks(groups) { - const rankObject = groups.reduce(function (res, group, index) { + const ranks = groups.reduce(function (res, group, index) { if (typeof group === 'string') { group = [group]; } + group.forEach(function (groupItem) { if (types.indexOf(groupItem) === -1) { throw new Error('Incorrect configuration of the rule: Unknown type `' + @@ -445,16 +534,15 @@ function convertGroupsToRanks(groups) { return res; }, {}); - const omittedTypes = types.filter(function (type) { - return rankObject[type] === undefined; - }); + const omittedTypes = new Set(types.filter(function (type) { + return ranks[type] === undefined; + })); - const ranks = omittedTypes.reduce(function (res, type) { - res[type] = groups.length * 2; - return res; - }, rankObject); + for (const omittedType of omittedTypes) { + ranks[omittedType] = groups.length * 2; + } - return { groups: ranks, omittedTypes }; + return { ranks, omittedTypes }; } function convertPathGroupsForRanks(pathGroups) { @@ -588,6 +676,9 @@ function getAlphabetizeConfig(options) { // TODO, semver-major: Change the default of "distinctGroup" from true to false const defaultDistinctGroup = true; +// TODO, semver-major: Remove the "intraGroupOrdering" option and make it's truthy case the default implementation of this rule +const defaultIntraGroupOrdering = false; + module.exports = { meta: { type: 'suggestion', @@ -605,6 +696,10 @@ module.exports = { groups: { type: 'array', }, + intraGroupOrdering: { + type: 'boolean', + default: defaultIntraGroupOrdering, + }, pathGroupsExcludedImportTypes: { type: 'array', }, @@ -674,17 +769,20 @@ module.exports = { create: function importOrderRule(context) { const options = context.options[0] || {}; + const groups = options.groups || defaultGroups; const newlinesBetweenImports = options['newlines-between'] || 'ignore'; const pathGroupsExcludedImportTypes = new Set(options['pathGroupsExcludedImportTypes'] || ['builtin', 'external', 'object']); const alphabetize = getAlphabetizeConfig(options); const distinctGroup = options.distinctGroup == null ? defaultDistinctGroup : !!options.distinctGroup; - let ranks; - + const intraGroupOrdering = options.intraGroupOrdering == null ? defaultIntraGroupOrdering : !!options.intraGroupOrdering; + + let computedContext; try { const { pathGroups, maxPosition } = convertPathGroupsForRanks(options.pathGroups || []); - const { groups, omittedTypes } = convertGroupsToRanks(options.groups || defaultGroups); - ranks = { - groups, + const { ranks, omittedTypes } = convertGroupsToRanks(groups); + + computedContext = { + ranks, omittedTypes, pathGroups, maxPosition, @@ -719,7 +817,7 @@ module.exports = { displayName: name, type: 'import', }, - ranks, + computedContext, getBlockImports(node.parent), pathGroupsExcludedImportTypes, ); @@ -750,7 +848,7 @@ module.exports = { displayName, type, }, - ranks, + computedContext, getBlockImports(node.parent), pathGroupsExcludedImportTypes, ); @@ -772,7 +870,7 @@ module.exports = { displayName: name, type: 'require', }, - ranks, + computedContext, getBlockImports(block), pathGroupsExcludedImportTypes, ); @@ -783,8 +881,8 @@ module.exports = { makeNewlinesBetweenReport(context, imported, newlinesBetweenImports, distinctGroup); } - if (alphabetize.order !== 'ignore') { - mutateRanksToAlphabetize(imported, alphabetize); + if (intraGroupOrdering || alphabetize.order !== 'ignore') { + mutateRanksForIntraGroupOrdering(computedContext, imported, groups, intraGroupOrdering, alphabetize); } makeOutOfOrderReport(context, imported); diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 291dae33b..114386d27 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -1105,6 +1105,31 @@ ruleTester.run('order', rule, { }, ], }), + + // Option intraGroupOrdering: false (should sort imports in order of appearance) + test({ + code: ` + import { foo } from './bar'; + import { fooz } from '../baz';`, + options: [ + { + groups: [['parent', 'sibling']], + }, + ], + }), + + // Option intraGroupOrdering: true (should sort imports in order of nested group) + test({ + code: ` + import { fooz } from '../baz'; + import { foo } from './bar';`, + options: [ + { + groups: [['parent', 'sibling']], + intraGroupOrdering: true, + }, + ], + }), ], invalid: [ // builtin before external module (require) @@ -2718,6 +2743,28 @@ ruleTester.run('order', rule, { message: 'There should be no empty line within import group', }], }), + + // Option intraGroupOrdering: true (should sort imports in order of nested group) + test({ + code: ` + import { foo } from './bar'; + import { fooz } from '../baz'; + `, + output: ` + import { fooz } from '../baz'; + import { foo } from './bar'; + `, + options: [ + { + groups: [['parent', 'sibling']], + intraGroupOrdering: true, + }, + ], + errors: [{ + message: '`../baz` import should occur before import of `./bar`', + }], + }), + // Alphabetize with require ...semver.satisfies(eslintPkg.version, '< 3.0.0') ? [] : [ test({ @@ -2747,7 +2794,7 @@ ruleTester.run('order', rule, { context('TypeScript', function () { getNonDefaultParsers() // Type-only imports were added in TypeScript ESTree 2.23.0 - .filter((parser) => parser !== parsers.TS_OLD) + .filter((parser) => parser !== parsers.TS_OLD).slice(0, 1) .forEach((parser) => { const parserConfig = { parser, @@ -2967,6 +3014,25 @@ context('TypeScript', function () { }, ], }), + + // Option intraGroupOrdering: true (should sort type imports in remaining group order) + test({ + name: 'pearce', + code: ` + import type { Dirent } from 'fs'; + + import type { Fooz } from '../baz'; + import type { Foo } from './bar'; + `, + ...parserConfig, + options: [ + { + groups: ['type', 'external' ['parent', 'sibling']], + intraGroupOrdering: true, + 'newlines-between': 'always', + }, + ], + }), ], invalid: [ // Option alphabetize: {order: 'asc'} @@ -3192,6 +3258,28 @@ context('TypeScript', function () { }, ], }), + + // Option intraGroupOrdering: true (should sort type imports in order of nested group) + test({ + code: ` + import type { Foo } from './bar'; + import type { Fooz } from '../baz'; + `, + output: ` + import type { Fooz } from '../baz'; + import type { Foo } from './bar'; + `, + ...parserConfig, + options: [ + { + groups: ['type', ['parent', 'sibling']], + intraGroupOrdering: true, + }, + ], + errors: [{ + message: '`../baz` type import should occur before type import of `./bar`', + }], + }), ], }); }); From f78217fdf8f94c8ce29cd0888074704a59194b07 Mon Sep 17 00:00:00 2001 From: Pearce Date: Thu, 16 Feb 2023 13:15:02 -0800 Subject: [PATCH 2/4] Fix tests --- src/rules/order.js | 7 ++++--- tests/src/rules/order.js | 19 ------------------- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/src/rules/order.js b/src/rules/order.js index 035ebea8b..c15f2c357 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -517,11 +517,12 @@ const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling' // Will throw an error if it contains a type that does not exist, or has a duplicate function convertGroupsToRanks(groups) { const ranks = groups.reduce(function (res, group, index) { + let groupItems = group; if (typeof group === 'string') { - group = [group]; + groupItems = [group]; } - group.forEach(function (groupItem) { + groupItems.forEach(function (groupItem) { if (types.indexOf(groupItem) === -1) { throw new Error('Incorrect configuration of the rule: Unknown type `' + JSON.stringify(groupItem) + '`'); @@ -780,7 +781,7 @@ module.exports = { try { const { pathGroups, maxPosition } = convertPathGroupsForRanks(options.pathGroups || []); const { ranks, omittedTypes } = convertGroupsToRanks(groups); - + computedContext = { ranks, omittedTypes, diff --git a/tests/src/rules/order.js b/tests/src/rules/order.js index 114386d27..55e88c040 100644 --- a/tests/src/rules/order.js +++ b/tests/src/rules/order.js @@ -3014,25 +3014,6 @@ context('TypeScript', function () { }, ], }), - - // Option intraGroupOrdering: true (should sort type imports in remaining group order) - test({ - name: 'pearce', - code: ` - import type { Dirent } from 'fs'; - - import type { Fooz } from '../baz'; - import type { Foo } from './bar'; - `, - ...parserConfig, - options: [ - { - groups: ['type', 'external' ['parent', 'sibling']], - intraGroupOrdering: true, - 'newlines-between': 'always', - }, - ], - }), ], invalid: [ // Option alphabetize: {order: 'asc'} From e0eab08adbb828cecbec5a98f63c43a78dc323af Mon Sep 17 00:00:00 2001 From: Pearce Date: Fri, 17 Feb 2023 09:40:42 -0800 Subject: [PATCH 3/4] Add docs --- docs/rules/order.md | 71 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/docs/rules/order.md b/docs/rules/order.md index e3deacaf2..ba053dd36 100644 --- a/docs/rules/order.md +++ b/docs/rules/order.md @@ -88,7 +88,9 @@ This rule supports the following options: How groups are defined, and the order to respect. `groups` must be an array of `string` or [`string`]. The only allowed `string`s are: `"builtin"`, `"external"`, `"internal"`, `"unknown"`, `"parent"`, `"sibling"`, `"index"`, `"object"`, `"type"`. -The enforced order is the same as the order of each element in a group. Omitted types are implicitly grouped together as the last element. Example: +By default, the order of elements in a group is ignored and can be mingled +together (see `intraGroupOrder`). Omitted types are implicitly grouped together as the last element. +Example: ```ts [ 'builtin', // Built-in types are first @@ -323,6 +325,73 @@ import React, { PureComponent } from 'react'; import { compose, apply } from 'xcompose'; ``` +### `intraGroupOrdering: true|false`: + +* default: `false` +* Will be changed to `true` in the next major release. + +Enforces or forbids the ordering of imports within a group. This option applies +slightly differently depending on whether the import is a `type` import or a +regular import. + +#### Regular imports +When nested groups have been defined in `groups` (ie. `['builtin', ['parent', 'sibling']]`), +regular imports are sorted in the order of the elements in the nested group. So +in the case of `['builtin', ['parent', 'sibling']]`, parents would be sorted +above sibling imports but no newline would separate them. + +When `intraGroupOrdering` is set to `true`, the following will be invalid: + +```ts +/* eslint import/order: ["error", {"intraGroupOrdering": true, groups: ['builtin', ['parent', 'sibling']]}] */ +import { fs } from 'fs'; +import { Select } from './Select'; +import { Button } from '../Button'; +``` + +While this will be valid: + +```ts +/* eslint import/order: ["error", {"intraGroupOrdering": true, groups: ['builtin', ['parent', 'sibling']]}] */ +import { fs } from 'fs'; +import { Button } from '../Button'; +import { Select } from './Select'; +``` + +When alphabetization is enabled, imports with will be sorted in alphabetical +order within each sub group before the sub group is sorted relative to the order +of the nested group. + +#### Type imports +When `intraGroupOrdering` is set to `true`, type imports are sorted in the order +of the original groups array but flattened. So in the case of +`['builtin', ['parent', 'sibling']]`, the order would be [`builtin`, `parent`, `sibling`]. + +When `intraGroupOrdering` is set to `true`, the following will be invalid: + +```ts +/* eslint import/order: ["error", {"intraGroupOrdering": true, groups: ['type', 'builtin', ['parent', 'sibling']]}] */ +import type { SelectProps } from './Select'; +import type { ButtonProps } from '../Button'; +import type { Dirent } from 'fs'; + +import fs from 'fs'; +import { Button } from '../Button'; +import { Select } from './Select'; +``` + +While this will be valid: +```ts +/* eslint import/order: ["error", {"intraGroupOrdering": true, groups: ['type', 'builtin', ['parent', 'sibling']]}] */ +import type { Dirent } from 'fs'; +import type { ButtonProps } from '../Button'; +import type { SelectProps } from './Select'; + +import fs from 'fs'; +import { Button } from '../Button'; +import { Select } from './Select'; +``` + ### `warnOnUnassignedImports: true|false`: * default: `false` From de0e9f4e34795cbd7c74c6b3f7364bd99d6e542a Mon Sep 17 00:00:00 2001 From: Pearce Date: Fri, 17 Feb 2023 15:20:24 -0800 Subject: [PATCH 4/4] Fix backwards compat --- src/rules/order.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rules/order.js b/src/rules/order.js index c15f2c357..72f8a2d0a 100644 --- a/src/rules/order.js +++ b/src/rules/order.js @@ -2,6 +2,7 @@ import minimatch from 'minimatch'; import includes from 'array-includes'; +import flat from 'array.prototype.flat'; import resolveImportType from '../core/importType'; import isStaticRequire from '../core/staticRequire'; @@ -328,7 +329,7 @@ function getSorter(alphabetizeOptions) { function mutateRanksForIntraGroupOrdering(computedContext, imported, groups, intraGroupOrdering, alphabetizeOptions) { const { omittedTypes } = computedContext; - const nonTypeGroups = new Set(groups.flat()); + const nonTypeGroups = new Set(flat(groups)); if (intraGroupOrdering) { nonTypeGroups.delete('type');