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.

Sign up for GitHub

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

[Fix] order: codify invariants from docs into config schema #3152

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[Fix] order: codify invariants from docs into config schema #3152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9900388
fix: prohibit nonsensical configurations using the schema
Xunnamius Jan 26, 2025
a8ab646
test: fix erroneously invalid options in "valid" tests identified by …
Xunnamius Jan 26, 2025
30b4dcf
refactor(src): undo `pathGroupsExcludedImportTypes` schema tightening…
Xunnamius Feb 1, 2025
a73a2f2
refactor: move the "groups" duplicates/existence validation logic up …
Xunnamius Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 73 additions & 18 deletions src/rules/order.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,18 +590,14 @@ function getRequireBlock(node) {

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

// Creates an object with type-rank pairs.
// 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
/**
* Creates an object with type-rank pairs.
*
* Example: { index: 0, sibling: 1, parent: 1, external: 1, builtin: 2, internal: 2 }
*/
function convertGroupsToRanks(groups) {
const rankObject = groups.reduce(function (res, group, index) {
[].concat(group).forEach(function (groupItem) {
if (types.indexOf(groupItem) === -1) {
throw new Error(`Incorrect configuration of the rule: Unknown type \`${JSON.stringify(groupItem)}\``);
}
if (res[groupItem] !== undefined) {
throw new Error(`Incorrect configuration of the rule: \`${groupItem}\` is duplicated`);
}
res[groupItem] = index * 2;
});
return res;
Expand Down Expand Up @@ -858,6 +854,17 @@ module.exports = {
properties: {
groups: {
type: 'array',
uniqueItems: true,
items: {
oneOf: [
{ enum: types },
{
type: 'array',
uniqueItems: true,
items: { enum: types },
},
],
},
},
pathGroupsExcludedImportTypes: {
type: 'array',
Expand Down Expand Up @@ -965,24 +972,72 @@ module.exports = {
},
additionalProperties: false,
dependencies: {
sortTypesGroup: {
oneOf: [
{
// When sortTypesGroup is true, groups must NOT be an array that does not contain 'type'
properties: {
sortTypesGroup: { enum: [true] },
groups: {
not: {
type: 'array',
uniqueItems: true,
items: {
oneOf: [
{ enum: types.filter((t) => t !== 'type') },
{
type: 'array',
uniqueItems: true,
items: { enum: types.filter((t) => t !== 'type') },
},
],
},
},
},
},
required: ['groups'],
},
{
properties: {
sortTypesGroup: { enum: [false] },
},
},
],
},
'newlines-between-types': {
properties: {
sortTypesGroup: { enum: [true] },
},
required: ['sortTypesGroup'],
},
consolidateIslands: {
anyOf: [{
properties: {
'newlines-between': { enum: ['always-and-inside-groups'] },
oneOf: [
{
properties: {
consolidateIslands: { enum: ['inside-groups'] },
},
anyOf: [
{
properties: {
'newlines-between': { enum: ['always-and-inside-groups'] },
},
required: ['newlines-between'],
},
{
properties: {
'newlines-between-types': { enum: ['always-and-inside-groups'] },
},
required: ['newlines-between-types'],
},
],
},
required: ['newlines-between'],
}, {
properties: {
'newlines-between-types': { enum: ['always-and-inside-groups'] },
{
properties: {
consolidateIslands: { enum: ['never'] },
},
},
required: ['newlines-between-types'],
}] },
],
},
},
},
],
Expand Down
93 changes: 2 additions & 91 deletions tests/src/rules/order.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,63 +1643,6 @@ ruleTester.run('order', rule, {
message: '`async` import should occur before import of `path`',
}],
}),
// Setting the order for an unknown type
// should make the rule trigger an error and do nothing else
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{ groups: [
'index',
['sibling', 'parent', 'UNKNOWN', 'internal'],
] }],
errors: [{
message: 'Incorrect configuration of the rule: Unknown type `"UNKNOWN"`',
}],
}),
// Type in an array can't be another array, too much nesting
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{ groups: [
'index',
['sibling', 'parent', ['builtin'], 'internal'],
] }],
errors: [{
message: 'Incorrect configuration of the rule: Unknown type `["builtin"]`',
}],
}),
// No numbers
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{ groups: [
'index',
['sibling', 'parent', 2, 'internal'],
] }],
errors: [{
message: 'Incorrect configuration of the rule: Unknown type `2`',
}],
}),
// Duplicate
test({
code: `
var async = require('async');
var index = require('./');
`,
options: [{ groups: [
'index',
['sibling', 'parent', 'parent', 'internal'],
] }],
errors: [{
message: 'Incorrect configuration of the rule: `parent` is duplicated',
}],
}),
// Mixing require and import should have import up top
test({
code: `
Expand Down Expand Up @@ -2511,7 +2454,7 @@ ruleTester.run('order', rule, {
{ pattern: '@namespace', group: 'external', position: 'after' },
{ pattern: '@namespace/**', group: 'external', position: 'after' },
],
pathGroupsExcludedImportTypes: ['@namespace'],
pathGroupsExcludedImportTypes: [],
},
],
errors: [
Expand Down Expand Up @@ -3554,38 +3497,6 @@ context('TypeScript', function () {
},
],
}),
// Option sortTypesGroup: true and 'type' omitted from groups
test({
code: `
import c from 'Bar';
import type { AA } from 'abc';
import a from 'foo';
import type { A } from 'foo';

import type { C } from 'dirA/Bar';
import b from 'dirA/bar';
import type { D } from 'dirA/bar';

import index from './';
`,
...parserConfig,
options: [
{
alphabetize: { order: 'asc' },
groups: ['external', 'internal', 'index'],
pathGroups: [
{
pattern: 'dirA/**',
group: 'internal',
},
],
'newlines-between': 'always',
pathGroupsExcludedImportTypes: [],
// Becomes a no-op without "type" in groups
sortTypesGroup: true,
},
],
}),
test({
code: `
import c from 'Bar';
Expand Down Expand Up @@ -6877,7 +6788,7 @@ flowRuleTester.run('order', rule, {
},
},
],
pathGroupsExcludedImportTypes: ['react'],
pathGroupsExcludedImportTypes: [],
alphabetize: {
order: 'asc',
},
Expand Down