Skip to content

Commit

Permalink
fix: prioritize dependencies over partitions by comment and partition…
Browse files Browse the repository at this point in the history
…s by line
  • Loading branch information
hugop95 committed Sep 21, 2024
1 parent efa2750 commit 199ab39
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 119 deletions.
94 changes: 47 additions & 47 deletions rules/sort-classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,7 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
[[]],
)

let sortedNodes: SortingNodeWithDependencies[] = []
for (let nodes of formattedNodes) {
let nodesByNonIgnoredGroupNumber: {
[key: number]: SortingNodeWithDependencies[]
Expand All @@ -650,7 +651,6 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
nodesByNonIgnoredGroupNumber[groupNum].push(sortingNode)
}

let sortedNodes: SortingNodeWithDependencies[] = []
for (let groupNumber of Object.keys(
nodesByNonIgnoredGroupNumber,
).sort((a, b) => Number(a) - Number(b))) {
Expand All @@ -674,54 +674,54 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
for (let ignoredIndex of ignoredNodeIndices) {
sortedNodes.splice(ignoredIndex, 0, nodes[ignoredIndex])
}
}

sortedNodes = sortNodesByDependencies(sortedNodes)

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

let indexOfLeft = sortedNodes.indexOf(left)
let indexOfRight = sortedNodes.indexOf(right)
let firstUnorderedNodeDependentOnRight =
getFirstUnorderedNodeDependentOn(right, nodes)
if (
firstUnorderedNodeDependentOnRight ||
(!isLeftOrRightIgnored && indexOfLeft > indexOfRight)
) {
let messageId: MESSAGE_ID
if (firstUnorderedNodeDependentOnRight) {
messageId = 'unexpectedClassesDependencyOrder'
} else {
messageId =
leftNum !== rightNum
? 'unexpectedClassesGroupOrder'
: 'unexpectedClassesOrder'
}
context.report({
messageId,
data: {
left: toSingleLine(left.name),
leftGroup: left.group,
right: toSingleLine(right.name),
rightGroup: right.group,
nodeDependentOnRight:
firstUnorderedNodeDependentOnRight?.name,
},
node: right.node,
fix: (fixer: TSESLint.RuleFixer) =>
makeFixes(fixer, nodes, sortedNodes, sourceCode, {
partitionComment: options.partitionByComment,
}),
})
sortedNodes = sortNodesByDependencies(sortedNodes)
let nodes = formattedNodes.flat()

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

let indexOfLeft = sortedNodes.indexOf(left)
let indexOfRight = sortedNodes.indexOf(right)
let firstUnorderedNodeDependentOnRight =
getFirstUnorderedNodeDependentOn(right, nodes)
if (
firstUnorderedNodeDependentOnRight ||
(!isLeftOrRightIgnored && indexOfLeft > indexOfRight)
) {
let messageId: MESSAGE_ID
if (firstUnorderedNodeDependentOnRight) {
messageId = 'unexpectedClassesDependencyOrder'
} else {
messageId =
leftNum !== rightNum
? 'unexpectedClassesGroupOrder'
: 'unexpectedClassesOrder'
}
})
}
context.report({
messageId,
data: {
left: toSingleLine(left.name),
leftGroup: left.group,
right: toSingleLine(right.name),
rightGroup: right.group,
nodeDependentOnRight: firstUnorderedNodeDependentOnRight?.name,
},
node: right.node,
fix: (fixer: TSESLint.RuleFixer) =>
makeFixes(fixer, nodes, sortedNodes, sourceCode, {
partitionComment: options.partitionByComment,
}),
})
}
})
}
},
}),
Expand Down
58 changes: 29 additions & 29 deletions rules/sort-enums.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,35 +242,35 @@ export default createEslintRule<Options, MESSAGE_ID>({
}
: undefined,
}
for (let nodes of formattedMembers) {
let sortedNodes = sortNodesByDependencies(
sortNodes(nodes, compareOptions),
)
pairwise(nodes, (left, right) => {
let indexOfLeft = sortedNodes.indexOf(left)
let indexOfRight = sortedNodes.indexOf(right)
if (indexOfLeft > indexOfRight) {
let firstUnorderedNodeDependentOnRight =
getFirstUnorderedNodeDependentOn(right, nodes)
context.report({
messageId: firstUnorderedNodeDependentOnRight
? 'unexpectedEnumsDependencyOrder'
: 'unexpectedEnumsOrder',
data: {
left: toSingleLine(left.name),
right: toSingleLine(right.name),
nodeDependentOnRight:
firstUnorderedNodeDependentOnRight?.name,
},
node: right.node,
fix: fixer =>
makeFixes(fixer, nodes, sortedNodes, sourceCode, {
partitionComment,
}),
})
}
})
}
let sortedNodes = sortNodesByDependencies(
formattedMembers
.map(nodes => sortNodes(nodes, compareOptions))
.flat(),
)
let nodes = formattedMembers.flat()
pairwise(nodes, (left, right) => {
let indexOfLeft = sortedNodes.indexOf(left)
let indexOfRight = sortedNodes.indexOf(right)
if (indexOfLeft > indexOfRight) {
let firstUnorderedNodeDependentOnRight =
getFirstUnorderedNodeDependentOn(right, nodes)
context.report({
messageId: firstUnorderedNodeDependentOnRight
? 'unexpectedEnumsDependencyOrder'
: 'unexpectedEnumsOrder',
data: {
left: toSingleLine(left.name),
right: toSingleLine(right.name),
nodeDependentOnRight: firstUnorderedNodeDependentOnRight?.name,
},
node: right.node,
fix: fixer =>
makeFixes(fixer, nodes, sortedNodes, sourceCode, {
partitionComment,
}),
})
}
})
}
},
}),
Expand Down
87 changes: 44 additions & 43 deletions rules/sort-objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,10 @@ export default createEslintRule<Options, MESSAGE_ID>({
[[]],
)

for (let nodes of formatProperties(node.properties)) {
let sortedNodes: SortingNodeWithDependencies[] = []

let formattedNodes = formatProperties(node.properties)
for (let nodes of formattedNodes) {
let grouped: {
[key: string]: SortingNodeWithDependencies[]
} = {}
Expand All @@ -450,56 +453,54 @@ export default createEslintRule<Options, MESSAGE_ID>({
}
}

let sortedNodes: SortingNodeWithDependencies[] = []

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

sortedNodes = sortNodesByDependencies(sortedNodes)

pairwise(nodes, (left, right) => {
let indexOfLeft = sortedNodes.indexOf(left)
let indexOfRight = sortedNodes.indexOf(right)

if (indexOfLeft > indexOfRight) {
let firstUnorderedNodeDependentOnRight =
getFirstUnorderedNodeDependentOn(right, nodes)
let fix:
| ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix[])
| undefined = fixer =>
makeFixes(fixer, nodes, sortedNodes, sourceCode, {
partitionComment: options.partitionByComment,
})
let leftNum = getGroupNumber(options.groups, left)
let rightNum = getGroupNumber(options.groups, right)
let messageId: MESSAGE_ID
if (firstUnorderedNodeDependentOnRight) {
messageId = 'unexpectedObjectsDependencyOrder'
} else {
messageId =
leftNum !== rightNum
? 'unexpectedObjectsGroupOrder'
: 'unexpectedObjectsOrder'
}
context.report({
messageId,
data: {
left: toSingleLine(left.name),
leftGroup: left.group,
right: toSingleLine(right.name),
rightGroup: right.group,
nodeDependentOnRight:
firstUnorderedNodeDependentOnRight?.name,
},
node: right.node,
fix,
sortedNodes = sortNodesByDependencies(sortedNodes)
let nodes = formattedNodes.flat()

pairwise(nodes, (left, right) => {
let indexOfLeft = sortedNodes.indexOf(left)
let indexOfRight = sortedNodes.indexOf(right)

if (indexOfLeft > indexOfRight) {
let firstUnorderedNodeDependentOnRight =
getFirstUnorderedNodeDependentOn(right, nodes)
let fix:
| ((fixer: TSESLint.RuleFixer) => TSESLint.RuleFix[])
| undefined = fixer =>
makeFixes(fixer, nodes, sortedNodes, sourceCode, {
partitionComment: options.partitionByComment,
})
let leftNum = getGroupNumber(options.groups, left)
let rightNum = getGroupNumber(options.groups, right)
let messageId: MESSAGE_ID
if (firstUnorderedNodeDependentOnRight) {
messageId = 'unexpectedObjectsDependencyOrder'
} else {
messageId =
leftNum !== rightNum
? 'unexpectedObjectsGroupOrder'
: 'unexpectedObjectsOrder'
}
})
}
context.report({
messageId,
data: {
left: toSingleLine(left.name),
leftGroup: left.group,
right: toSingleLine(right.name),
rightGroup: right.group,
nodeDependentOnRight: firstUnorderedNodeDependentOnRight?.name,
},
node: right.node,
fix,
})
}
})
}
}

Expand Down
41 changes: 41 additions & 0 deletions test/sort-classes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3778,6 +3778,47 @@ describe(ruleName, () => {
},
)

ruleTester.run(
`${ruleName}(${type}): prioritizes dependencies over partitionByComment`,
rule,
{
valid: [],
invalid: [
{
code: dedent`
class Class {
b = this.a
// Part1
a
}
`,
output: dedent`
class Class {
a
// Part1
b = this.a
}
`,
options: [
{
...options,
partitionByComment: 'Part*',
},
],
errors: [
{
messageId: 'unexpectedClassesDependencyOrder',
data: {
right: 'a',
nodeDependentOnRight: 'b',
},
},
],
},
],
},
)

ruleTester.run(
`${ruleName}(${type}): works with left and right dependencies`,
rule,
Expand Down
41 changes: 41 additions & 0 deletions test/sort-enums.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1934,6 +1934,47 @@ describe(ruleName, () => {
},
],
})

ruleTester.run(
`${ruleName}: prioritizes dependencies over partitionByComment`,
rule,
{
valid: [],
invalid: [
{
code: dedent`
enum Enum {
B = A,
// Part: 1
A = 'A',
}
`,
output: dedent`
enum Enum {
A = 'A',
// Part: 1
B = A,
}
`,
options: [
{
type: 'alphabetical',
partitionByComment: 'Part**',
},
],
errors: [
{
messageId: 'unexpectedEnumsDependencyOrder',
data: {
right: 'A',
nodeDependentOnRight: 'B',
},
},
],
},
],
},
)
})

describe('handles complex comment cases', () => {
Expand Down
Loading

0 comments on commit 199ab39

Please sign in to comment.