Skip to content

Commit f1f875e

Browse files
hugop95hugop95
authored
fix: fix sorting class members with same names
1 parent 4e19b94 commit f1f875e

File tree

3 files changed

+174
-8
lines changed

3 files changed

+174
-8
lines changed

‎rules/sort-classes-utils.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import type { TSESTree } from '@typescript-eslint/utils'
2+
13
import type { Modifier, Selector } from './sort-classes'
24

35
/**
@@ -95,3 +97,44 @@ const getPermutations = (elements: string[]): string[][] => {
9597

9698
return result
9799
}
100+
101+
/**
102+
* Returns a list of groups of overload signatures.
103+
*/
104+
export const getOverloadSignatureGroups = (
105+
members: TSESTree.ClassElement[],
106+
): TSESTree.ClassElement[][] => {
107+
let methods = members
108+
.filter(
109+
member =>
110+
member.type === 'MethodDefinition' ||
111+
member.type === 'TSAbstractMethodDefinition',
112+
)
113+
.filter(member => member.kind === 'method')
114+
// Static and non-static overload signatures can coexist with the same name
115+
let staticOverloadSignaturesByName = new Map<
116+
string,
117+
TSESTree.ClassElement[]
118+
>()
119+
let overloadSignaturesByName = new Map<string, TSESTree.ClassElement[]>()
120+
for (let method of methods) {
121+
if (method.key.type !== 'Identifier') {
122+
continue
123+
}
124+
let { name } = method.key
125+
let mapToUse = method.static
126+
? staticOverloadSignaturesByName
127+
: overloadSignaturesByName
128+
let signatureOverloadsGroup = mapToUse.get(name)
129+
if (!signatureOverloadsGroup) {
130+
signatureOverloadsGroup = []
131+
mapToUse.set(name, signatureOverloadsGroup)
132+
}
133+
signatureOverloadsGroup.push(method)
134+
}
135+
// Ignore groups that only have one method
136+
return [
137+
...overloadSignaturesByName.values(),
138+
...staticOverloadSignaturesByName.values(),
139+
].filter(group => group.length > 1)
140+
}

‎rules/sort-classes.ts‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ import type { TSESLint } from '@typescript-eslint/utils'
33

44
import type { SortingNode } from '../typings'
55

6+
import {
7+
getOverloadSignatureGroups,
8+
generateOfficialGroups,
9+
} from './sort-classes-utils'
610
import { isPartitionComment } from '../utils/is-partition-comment'
711
import { getCommentBefore } from '../utils/get-comment-before'
812
import { createEslintRule } from '../utils/create-eslint-rule'
9-
import { generateOfficialGroups } from './sort-classes-utils'
1013
import { getGroupNumber } from '../utils/get-group-number'
1114
import { getSourceCode } from '../utils/get-source-code'
1215
import { toSingleLine } from '../utils/to-single-line'
@@ -319,6 +322,8 @@ export default createEslintRule<Options, MESSAGE_ID>({
319322
return dependencies
320323
}
321324

325+
let overloadSignatureGroups = getOverloadSignatureGroups(node.body)
326+
322327
let formattedNodes: SortingNode[][] = node.body.reduce(
323328
(accumulator: SortingNode[][], member) => {
324329
let comment = getCommentBefore(member, sourceCode)
@@ -503,8 +508,16 @@ export default createEslintRule<Options, MESSAGE_ID>({
503508
dependencies = extractDependencies(member.value)
504509
}
505510

506-
let value = {
507-
size: rangeToDiff(member.range),
511+
// Members belonging to the same overload signature group should have the same size in order to keep line-length sorting between them consistent.
512+
// It is unclear what should be considered the size of an overload signature group. Take the size of the implementation by default.
513+
let overloadSignatureGroupMember = overloadSignatureGroups
514+
.find(overloadSignatures => overloadSignatures.includes(member))
515+
?.at(-1)
516+
517+
let value: SortingNode = {
518+
size: overloadSignatureGroupMember
519+
? rangeToDiff(overloadSignatureGroupMember.range)
520+
: rangeToDiff(member.range),
508521
group: getGroup(),
509522
node: member,
510523
dependencies,
@@ -524,10 +537,9 @@ export default createEslintRule<Options, MESSAGE_ID>({
524537
let rightNum = getGroupNumber(options.groups, right)
525538

526539
if (
527-
left.name !== right.name &&
528-
(leftNum > rightNum ||
529-
(leftNum === rightNum &&
530-
isPositive(compare(left, right, options))))
540+
leftNum > rightNum ||
541+
(leftNum === rightNum &&
542+
isPositive(compare(left, right, options)))
531543
) {
532544
context.report({
533545
messageId:

‎test/sort-classes.test.ts‎

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1556,6 +1556,49 @@ describe(ruleName, () => {
15561556
},
15571557
)
15581558

1559+
ruleTester.run(
1560+
`${ruleName}(${type}): sorts class with attributes having the same name`,
1561+
rule,
1562+
{
1563+
valid: [],
1564+
invalid: [
1565+
{
1566+
code: dedent`
1567+
class Class {
1568+
static a;
1569+
1570+
a;
1571+
}
1572+
`,
1573+
output: dedent`
1574+
class Class {
1575+
a;
1576+
1577+
static a;
1578+
}
1579+
`,
1580+
options: [
1581+
{
1582+
...options,
1583+
groups: ['property', 'static-property'],
1584+
},
1585+
],
1586+
errors: [
1587+
{
1588+
messageId: 'unexpectedClassesGroupOrder',
1589+
data: {
1590+
left: 'a',
1591+
leftGroup: 'static-property',
1592+
right: 'a',
1593+
rightGroup: 'property',
1594+
},
1595+
},
1596+
],
1597+
},
1598+
],
1599+
},
1600+
)
1601+
15591602
ruleTester.run(
15601603
`${ruleName}(${type}): sorts class with ts index signatures`,
15611604
rule,
@@ -4379,7 +4422,75 @@ describe(ruleName, () => {
43794422
],
43804423
},
43814424
],
4382-
invalid: [],
4425+
invalid: [
4426+
{
4427+
code: dedent`
4428+
class Decorations {
4429+
4430+
setBackground(r: number, g: number, b: number, a?: number): this
4431+
setBackground(color: number, hexFlag: boolean): this
4432+
setBackground(color: Color | string | CSSColor): this
4433+
setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
4434+
/* ... */
4435+
}
4436+
4437+
static setBackground(r: number, g: number, b: number, a?: number): this
4438+
static setBackground(color: number, hexFlag: boolean): this
4439+
static setBackground(color: Color | string | CSSColor): this
4440+
static setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
4441+
/* ... */
4442+
}
4443+
4444+
a
4445+
}
4446+
`,
4447+
output: dedent`
4448+
class Decorations {
4449+
4450+
a
4451+
static setBackground(r: number, g: number, b: number, a?: number): this
4452+
static setBackground(color: number, hexFlag: boolean): this
4453+
static setBackground(color: Color | string | CSSColor): this
4454+
4455+
static setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
4456+
/* ... */
4457+
}
4458+
setBackground(r: number, g: number, b: number, a?: number): this
4459+
setBackground(color: number, hexFlag: boolean): this
4460+
setBackground(color: Color | string | CSSColor): this
4461+
4462+
setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
4463+
/* ... */
4464+
}
4465+
}
4466+
`,
4467+
options: [
4468+
{
4469+
...options,
4470+
},
4471+
],
4472+
errors: [
4473+
{
4474+
messageId: 'unexpectedClassesGroupOrder',
4475+
data: {
4476+
left: 'setBackground',
4477+
leftGroup: 'method',
4478+
right: 'setBackground',
4479+
rightGroup: 'static-method',
4480+
},
4481+
},
4482+
{
4483+
messageId: 'unexpectedClassesGroupOrder',
4484+
data: {
4485+
left: 'setBackground',
4486+
leftGroup: 'static-method',
4487+
right: 'a',
4488+
rightGroup: 'property',
4489+
},
4490+
},
4491+
],
4492+
},
4493+
],
43834494
},
43844495
)
43854496

0 commit comments

Comments
 (0)