Skip to content

Commit 9e1ad6b

Browse files
Xunnamiusljharb
authored andcommitted
[Fix] order: codify invariants from docs into config schema
1 parent f017790 commit 9e1ad6b

File tree

3 files changed

+77
-109
lines changed

3 files changed

+77
-109
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
2020
- [`order`]: ensure arcane imports do not cause undefined behavior ([#3128], thanks [@Xunnamius])
2121
- [`order`]: resolve undefined property access issue when using `named` ordering ([#3166], thanks [@Xunnamius])
2222
- [`enforce-node-protocol-usage`]: avoid a crash with some TS code ([#3173], thanks [@ljharb])
23+
- [`order`]: codify invariants from docs into config schema ([#3152], thanks [@Xunnamius])
2324

2425
### Changed
2526
- [Docs] [`extensions`], [`order`]: improve documentation ([#3106], thanks [@Xunnamius])
@@ -1183,6 +1184,7 @@ for info on changes for earlier releases.
11831184
[#3172]: https://github.com/import-js/eslint-plugin-import/pull/3172
11841185
[#3167]: https://github.com/import-js/eslint-plugin-import/pull/3167
11851186
[#3166]: https://github.com/import-js/eslint-plugin-import/pull/3166
1187+
[#3152]: https://github.com/import-js/eslint-plugin-import/pull/3152
11861188
[#3151]: https://github.com/import-js/eslint-plugin-import/pull/3151
11871189
[#3138]: https://github.com/import-js/eslint-plugin-import/pull/3138
11881190
[#3129]: https://github.com/import-js/eslint-plugin-import/pull/3129

src/rules/order.js

Lines changed: 73 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -590,18 +590,14 @@ function getRequireBlock(node) {
590590

591591
const types = ['builtin', 'external', 'internal', 'unknown', 'parent', 'sibling', 'index', 'object', 'type'];
592592

593-
// Creates an object with type-rank pairs.
594-
// Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
595-
// Will throw an error if it contains a type that does not exist, or has a duplicate
593+
/**
594+
* Creates an object with type-rank pairs.
595+
*
596+
* Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
597+
*/
596598
function convertGroupsToRanks(groups) {
597599
const rankObject = groups.reduce(function (res, group, index) {
598600
[].concat(group).forEach(function (groupItem) {
599-
if (types.indexOf(groupItem) === -1) {
600-
throw new Error(`Incorrect configuration of the rule: Unknown type \`${JSON.stringify(groupItem)}\``);
601-
}
602-
if (res[groupItem] !== undefined) {
603-
throw new Error(`Incorrect configuration of the rule: \`${groupItem}\` is duplicated`);
604-
}
605601
res[groupItem] = index * 2;
606602
});
607603
return res;
@@ -858,6 +854,17 @@ module.exports = {
858854
properties: {
859855
groups: {
860856
type: 'array',
857+
uniqueItems: true,
858+
items: {
859+
oneOf: [
860+
{ enum: types },
861+
{
862+
type: 'array',
863+
uniqueItems: true,
864+
items: { enum: types },
865+
},
866+
],
867+
},
861868
},
862869
pathGroupsExcludedImportTypes: {
863870
type: 'array',
@@ -965,24 +972,72 @@ module.exports = {
965972
},
966973
additionalProperties: false,
967974
dependencies: {
975+
sortTypesGroup: {
976+
oneOf: [
977+
{
978+
// When sortTypesGroup is true, groups must NOT be an array that does not contain 'type'
979+
properties: {
980+
sortTypesGroup: { enum: [true] },
981+
groups: {
982+
not: {
983+
type: 'array',
984+
uniqueItems: true,
985+
items: {
986+
oneOf: [
987+
{ enum: types.filter((t) => t !== 'type') },
988+
{
989+
type: 'array',
990+
uniqueItems: true,
991+
items: { enum: types.filter((t) => t !== 'type') },
992+
},
993+
],
994+
},
995+
},
996+
},
997+
},
998+
required: ['groups'],
999+
},
1000+
{
1001+
properties: {
1002+
sortTypesGroup: { enum: [false] },
1003+
},
1004+
},
1005+
],
1006+
},
9681007
'newlines-between-types': {
9691008
properties: {
9701009
sortTypesGroup: { enum: [true] },
9711010
},
9721011
required: ['sortTypesGroup'],
9731012
},
9741013
consolidateIslands: {
975-
anyOf: [{
976-
properties: {
977-
'newlines-between': { enum: ['always-and-inside-groups'] },
1014+
oneOf: [
1015+
{
1016+
properties: {
1017+
consolidateIslands: { enum: ['inside-groups'] },
1018+
},
1019+
anyOf: [
1020+
{
1021+
properties: {
1022+
'newlines-between': { enum: ['always-and-inside-groups'] },
1023+
},
1024+
required: ['newlines-between'],
1025+
},
1026+
{
1027+
properties: {
1028+
'newlines-between-types': { enum: ['always-and-inside-groups'] },
1029+
},
1030+
required: ['newlines-between-types'],
1031+
},
1032+
],
9781033
},
979-
required: ['newlines-between'],
980-
}, {
981-
properties: {
982-
'newlines-between-types': { enum: ['always-and-inside-groups'] },
1034+
{
1035+
properties: {
1036+
consolidateIslands: { enum: ['never'] },
1037+
},
9831038
},
984-
required: ['newlines-between-types'],
985-
}] },
1039+
],
1040+
},
9861041
},
9871042
},
9881043
],

tests/src/rules/order.js

Lines changed: 2 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,63 +1643,6 @@ ruleTester.run('order', rule, {
16431643
message: '`async` import should occur before import of `path`',
16441644
}],
16451645
}),
1646-
// Setting the order for an unknown type
1647-
// should make the rule trigger an error and do nothing else
1648-
test({
1649-
code: `
1650-
var async = require('async');
1651-
var index = require('./');
1652-
`,
1653-
options: [{ groups: [
1654-
'index',
1655-
['sibling', 'parent', 'UNKNOWN', 'internal'],
1656-
] }],
1657-
errors: [{
1658-
message: 'Incorrect configuration of the rule: Unknown type `"UNKNOWN"`',
1659-
}],
1660-
}),
1661-
// Type in an array can't be another array, too much nesting
1662-
test({
1663-
code: `
1664-
var async = require('async');
1665-
var index = require('./');
1666-
`,
1667-
options: [{ groups: [
1668-
'index',
1669-
['sibling', 'parent', ['builtin'], 'internal'],
1670-
] }],
1671-
errors: [{
1672-
message: 'Incorrect configuration of the rule: Unknown type `["builtin"]`',
1673-
}],
1674-
}),
1675-
// No numbers
1676-
test({
1677-
code: `
1678-
var async = require('async');
1679-
var index = require('./');
1680-
`,
1681-
options: [{ groups: [
1682-
'index',
1683-
['sibling', 'parent', 2, 'internal'],
1684-
] }],
1685-
errors: [{
1686-
message: 'Incorrect configuration of the rule: Unknown type `2`',
1687-
}],
1688-
}),
1689-
// Duplicate
1690-
test({
1691-
code: `
1692-
var async = require('async');
1693-
var index = require('./');
1694-
`,
1695-
options: [{ groups: [
1696-
'index',
1697-
['sibling', 'parent', 'parent', 'internal'],
1698-
] }],
1699-
errors: [{
1700-
message: 'Incorrect configuration of the rule: `parent` is duplicated',
1701-
}],
1702-
}),
17031646
// Mixing require and import should have import up top
17041647
test({
17051648
code: `
@@ -2511,7 +2454,7 @@ ruleTester.run('order', rule, {
25112454
{ pattern: '@namespace', group: 'external', position: 'after' },
25122455
{ pattern: '@namespace/**', group: 'external', position: 'after' },
25132456
],
2514-
pathGroupsExcludedImportTypes: ['@namespace'],
2457+
pathGroupsExcludedImportTypes: [],
25152458
},
25162459
],
25172460
errors: [
@@ -3554,38 +3497,6 @@ context('TypeScript', function () {
35543497
},
35553498
],
35563499
}),
3557-
// Option sortTypesGroup: true and 'type' omitted from groups
3558-
test({
3559-
code: `
3560-
import c from 'Bar';
3561-
import type { AA } from 'abc';
3562-
import a from 'foo';
3563-
import type { A } from 'foo';
3564-
3565-
import type { C } from 'dirA/Bar';
3566-
import b from 'dirA/bar';
3567-
import type { D } from 'dirA/bar';
3568-
3569-
import index from './';
3570-
`,
3571-
...parserConfig,
3572-
options: [
3573-
{
3574-
alphabetize: { order: 'asc' },
3575-
groups: ['external', 'internal', 'index'],
3576-
pathGroups: [
3577-
{
3578-
pattern: 'dirA/**',
3579-
group: 'internal',
3580-
},
3581-
],
3582-
'newlines-between': 'always',
3583-
pathGroupsExcludedImportTypes: [],
3584-
// Becomes a no-op without "type" in groups
3585-
sortTypesGroup: true,
3586-
},
3587-
],
3588-
}),
35893500
test({
35903501
code: `
35913502
import c from 'Bar';
@@ -6889,7 +6800,7 @@ flowRuleTester.run('order', rule, {
68896800
},
68906801
},
68916802
],
6892-
pathGroupsExcludedImportTypes: ['react'],
6803+
pathGroupsExcludedImportTypes: [],
68936804
alphabetize: {
68946805
order: 'asc',
68956806
},

0 commit comments

Comments
 (0)