Skip to content

Commit

Permalink
feat: don't sort unknown elements if unknown is not referenced in groups
Browse files Browse the repository at this point in the history
  • Loading branch information
@hugop95
hugop95 authored Aug 21, 2024
1 parent 0c724e0 commit 0086427
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 36 deletions.
3 changes: 2 additions & 1 deletion docs/content/rules/sort-classes.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ Elements that are not `protected` nor `private` will be matched with the `public


##### The `unknown` group
Members that don’t fit into any group entered by the user will be placed in the `unknown` group.
Members that don’t fit into any group entered by the user will be placed in the `unknown` group. If the `unknown` group is not specified in `groups`,
the members will remain in their original order.

##### Behavior when multiple groups match an element

Expand Down
65 changes: 36 additions & 29 deletions rules/sort-classes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,11 +537,17 @@ export default createEslintRule<Options, MESSAGE_ID>({
pairwise(nodes, (left, right) => {
let leftNum = getGroupNumber(options.groups, left)
let rightNum = getGroupNumber(options.groups, right)
// Ignore nodes belonging to `unknown` group when that group is not referenced in the
// `groups` option.
let isLeftOrRightIgnored =
leftNum === options.groups.length ||
rightNum === options.groups.length

if (
leftNum > rightNum ||
(leftNum === rightNum &&
isPositive(compare(left, right, options)))
!isLeftOrRightIgnored &&
(leftNum > rightNum ||
(leftNum === rightNum &&
isPositive(compare(left, right, options))))
) {
context.report({
messageId:
Expand All @@ -556,35 +562,36 @@ export default createEslintRule<Options, MESSAGE_ID>({
},
node: right.node,
fix: (fixer: TSESLint.RuleFixer) => {
let grouped = nodes.reduce(
(
accumulator: {
[key: string]: SortingNode[]
},
sortingNode,
) => {
let groupNum = getGroupNumber(options.groups, sortingNode)

if (!(groupNum in accumulator)) {
accumulator[groupNum] = [sortingNode]
} else {
accumulator[groupNum] = sortNodes(
[...accumulator[groupNum], sortingNode],
options,
)
}

return accumulator
},
{},
)
let nodesByNonIgnoredGroupNumber: {
[key: number]: SortingNode[]
} = {}
let ignoredNodeIndices: number[] = []
for (let [index, sortingNode] of nodes.entries()) {
let groupNum = getGroupNumber(options.groups, sortingNode)
if (groupNum === options.groups.length) {
ignoredNodeIndices.push(index)
continue
}
nodesByNonIgnoredGroupNumber[groupNum] =
nodesByNonIgnoredGroupNumber[groupNum] ?? []
nodesByNonIgnoredGroupNumber[groupNum].push(sortingNode)
}

let sortedNodes: SortingNode[] = []
for (let groupNumber of Object.keys(
nodesByNonIgnoredGroupNumber,
).sort((a, b) => Number(a) - Number(b))) {
sortedNodes.push(
...sortNodes(
nodesByNonIgnoredGroupNumber[Number(groupNumber)],
options,
),
)
}

for (let group of Object.keys(grouped).sort(
(a, b) => Number(a) - Number(b),
)) {
sortedNodes.push(...sortNodes(grouped[group], options))
// Add ignored nodes at the same position as they were before linting
for (let ignoredIndex of ignoredNodeIndices) {
sortedNodes.splice(ignoredIndex, 0, nodes[ignoredIndex])
}

return makeFixes(fixer, nodes, sortedNodes, sourceCode, {
Expand Down
159 changes: 153 additions & 6 deletions test/sort-classes.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1650,7 +1650,7 @@ describe(ruleName, () => {
...options,
groups: [
['static-property', 'private-property', 'property'],
'constructor',
'index-signature',
],
},
],
Expand All @@ -1659,7 +1659,7 @@ describe(ruleName, () => {
messageId: 'unexpectedClassesGroupOrder',
data: {
left: '[k: string];',
leftGroup: 'unknown',
leftGroup: 'index-signature',
right: 'a',
rightGroup: 'static-property',
},
Expand Down Expand Up @@ -2736,6 +2736,55 @@ describe(ruleName, () => {
},
],
})

ruleTester.run(`${ruleName}(${type}): should ignore unknown group`, rule, {
valid: [],
invalid: [
{
code: dedent`
class Class {
public i = 'i';
private z() {}
public method3() {}
private y() {}
public method4() {}
public method1() {}
private x() {}
}
`,
output: dedent`
class Class {
private x() {}
private y() {}
public method3() {}
private z() {}
public method4() {}
public method1() {}
public i = 'i';
}
`,
options: [
{
...options,
groups: ['private-method', 'property'],
},
],
errors: [
{
messageId: 'unexpectedClassesGroupOrder',
data: {
left: 'i',
leftGroup: 'property',
right: 'z',
rightGroup: 'private-method',
},
},
],
},
],
})
})

describe(`${ruleName}: sorting by natural order`, () => {
Expand Down Expand Up @@ -3028,7 +3077,7 @@ describe(ruleName, () => {
...options,
groups: [
['static-property', 'private-property', 'property'],
'constructor',
'index-signature',
],
},
],
Expand All @@ -3037,7 +3086,7 @@ describe(ruleName, () => {
messageId: 'unexpectedClassesGroupOrder',
data: {
left: '[k: string];',
leftGroup: 'unknown',
leftGroup: 'index-signature',
right: 'a',
rightGroup: 'static-property',
},
Expand Down Expand Up @@ -4114,6 +4163,55 @@ describe(ruleName, () => {
},
],
})

ruleTester.run(`${ruleName}(${type}): should ignore unknown group`, rule, {
valid: [],
invalid: [
{
code: dedent`
class Class {
public i = 'i';
private z() {}
public method3() {}
private y() {}
public method4() {}
public method1() {}
private x() {}
}
`,
output: dedent`
class Class {
private x() {}
private y() {}
public method3() {}
private z() {}
public method4() {}
public method1() {}
public i = 'i';
}
`,
options: [
{
...options,
groups: ['private-method', 'property'],
},
],
errors: [
{
messageId: 'unexpectedClassesGroupOrder',
data: {
left: 'i',
leftGroup: 'property',
right: 'z',
rightGroup: 'private-method',
},
},
],
},
],
})
})

describe(`${ruleName}: sorting by line length`, () => {
Expand Down Expand Up @@ -4375,7 +4473,7 @@ describe(ruleName, () => {
...options,
groups: [
['static-property', 'private-property', 'property'],
'constructor',
'index-signature',
],
},
],
Expand All @@ -4384,7 +4482,7 @@ describe(ruleName, () => {
messageId: 'unexpectedClassesGroupOrder',
data: {
left: '[k: string];',
leftGroup: 'unknown',
leftGroup: 'index-signature',
right: 'a',
rightGroup: 'static-property',
},
Expand Down Expand Up @@ -4900,6 +4998,55 @@ describe(ruleName, () => {
},
],
})

ruleTester.run(`${ruleName}(${type}): should ignore unknown group`, rule, {
valid: [],
invalid: [
{
code: dedent`
class Class {
public i = 'i';
private z() {}
public method3() {}
private y() {}
public method4() {}
public method1() {}
private x() {}
}
`,
output: dedent`
class Class {
private z() {}
private y() {}
public method3() {}
private x() {}
public method4() {}
public method1() {}
public i = 'i';
}
`,
options: [
{
...options,
groups: ['private-method', 'property'],
},
],
errors: [
{
messageId: 'unexpectedClassesGroupOrder',
data: {
left: 'i',
leftGroup: 'property',
right: 'z',
rightGroup: 'private-method',
},
},
],
},
],
})
})

describe(`${ruleName}: misc`, () => {
Expand Down

0 comments on commit 0086427

Please sign in to comment.