fix: fix sorting class members with same names

azat-io · Aug 15, 2024 · f1f875e · f1f875e
1 parent 4e19b94
commit f1f875e
Show file tree

Hide file tree

Showing 3 changed files with 174 additions and 8 deletions.
diff --git a/rules/sort-classes-utils.ts b/rules/sort-classes-utils.ts
@@ -1,3 +1,5 @@
+import type { TSESTree } from '@typescript-eslint/utils'
+
 import type { Modifier, Selector } from './sort-classes'
 
 /**
@@ -95,3 +97,44 @@ const getPermutations = (elements: string[]): string[][] => {
 
   return result
 }
+
+/**
+ * Returns a list of groups of overload signatures.
+ */
+export const getOverloadSignatureGroups = (
+  members: TSESTree.ClassElement[],
+): TSESTree.ClassElement[][] => {
+  let methods = members
+    .filter(
+      member =>
+        member.type === 'MethodDefinition' ||
+        member.type === 'TSAbstractMethodDefinition',
+    )
+    .filter(member => member.kind === 'method')
+  // Static and non-static overload signatures can coexist with the same name
+  let staticOverloadSignaturesByName = new Map<
+    string,
+    TSESTree.ClassElement[]
+  >()
+  let overloadSignaturesByName = new Map<string, TSESTree.ClassElement[]>()
+  for (let method of methods) {
+    if (method.key.type !== 'Identifier') {
+      continue
+    }
+    let { name } = method.key
+    let mapToUse = method.static
+      ? staticOverloadSignaturesByName
+      : overloadSignaturesByName
+    let signatureOverloadsGroup = mapToUse.get(name)
+    if (!signatureOverloadsGroup) {
+      signatureOverloadsGroup = []
+      mapToUse.set(name, signatureOverloadsGroup)
+    }
+    signatureOverloadsGroup.push(method)
+  }
+  // Ignore groups that only have one method
+  return [
+    ...overloadSignaturesByName.values(),
+    ...staticOverloadSignaturesByName.values(),
+  ].filter(group => group.length > 1)
+}
diff --git a/rules/sort-classes.ts b/rules/sort-classes.ts
@@ -3,10 +3,13 @@ import type { TSESLint } from '@typescript-eslint/utils'
 
 import type { SortingNode } from '../typings'
 
+import {
+  getOverloadSignatureGroups,
+  generateOfficialGroups,
+} from './sort-classes-utils'
 import { isPartitionComment } from '../utils/is-partition-comment'
 import { getCommentBefore } from '../utils/get-comment-before'
 import { createEslintRule } from '../utils/create-eslint-rule'
-import { generateOfficialGroups } from './sort-classes-utils'
 import { getGroupNumber } from '../utils/get-group-number'
 import { getSourceCode } from '../utils/get-source-code'
 import { toSingleLine } from '../utils/to-single-line'
@@ -319,6 +322,8 @@ export default createEslintRule<Options, MESSAGE_ID>({
           return dependencies
         }
 
+        let overloadSignatureGroups = getOverloadSignatureGroups(node.body)
+
         let formattedNodes: SortingNode[][] = node.body.reduce(
           (accumulator: SortingNode[][], member) => {
             let comment = getCommentBefore(member, sourceCode)
@@ -503,8 +508,16 @@ export default createEslintRule<Options, MESSAGE_ID>({
               dependencies = extractDependencies(member.value)
             }
 
-            let value = {
-              size: rangeToDiff(member.range),
+            // Members belonging to the same overload signature group should have the same size in order to keep line-length sorting between them consistent.
+            // It is unclear what should be considered the size of an overload signature group. Take the size of the implementation by default.
+            let overloadSignatureGroupMember = overloadSignatureGroups
+              .find(overloadSignatures => overloadSignatures.includes(member))
+              ?.at(-1)
+
+            let value: SortingNode = {
+              size: overloadSignatureGroupMember
+                ? rangeToDiff(overloadSignatureGroupMember.range)
+                : rangeToDiff(member.range),
               group: getGroup(),
               node: member,
               dependencies,
@@ -524,10 +537,9 @@ export default createEslintRule<Options, MESSAGE_ID>({
             let rightNum = getGroupNumber(options.groups, right)
 
             if (
-              left.name !== right.name &&
-              (leftNum > rightNum ||
-                (leftNum === rightNum &&
-                  isPositive(compare(left, right, options))))
+              leftNum > rightNum ||
+              (leftNum === rightNum &&
+                isPositive(compare(left, right, options)))
             ) {
               context.report({
                 messageId:

diff --git a/test/sort-classes.test.ts b/test/sort-classes.test.ts
@@ -1556,6 +1556,49 @@ describe(ruleName, () => {
       },
     )
 
+    ruleTester.run(
+      `${ruleName}(${type}): sorts class with attributes having the same name`,
+      rule,
+      {
+        valid: [],
+        invalid: [
+          {
+            code: dedent`
+              class Class {
+                static a;
+
+                a;
+              }
+            `,
+            output: dedent`
+              class Class {
+                a;
+
+                static a;
+              }
+            `,
+            options: [
+              {
+                ...options,
+                groups: ['property', 'static-property'],
+              },
+            ],
+            errors: [
+              {
+                messageId: 'unexpectedClassesGroupOrder',
+                data: {
+                  left: 'a',
+                  leftGroup: 'static-property',
+                  right: 'a',
+                  rightGroup: 'property',
+                },
+              },
+            ],
+          },
+        ],
+      },
+    )
+
     ruleTester.run(
       `${ruleName}(${type}): sorts class with ts index signatures`,
       rule,
@@ -4379,7 +4422,75 @@ describe(ruleName, () => {
             ],
           },
         ],
-        invalid: [],
+        invalid: [
+          {
+            code: dedent`
+              class Decorations {
+
+                setBackground(r: number, g: number, b: number, a?: number): this
+                setBackground(color: number, hexFlag: boolean): this
+                setBackground(color: Color | string | CSSColor): this
+                setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
+                  /* ... */
+                }
+
+                static setBackground(r: number, g: number, b: number, a?: number): this
+                static setBackground(color: number, hexFlag: boolean): this
+                static setBackground(color: Color | string | CSSColor): this
+                static setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
+                  /* ... */
+                }
+
+                a
+              }
+            `,
+            output: dedent`
+              class Decorations {
+
+                a
+                static setBackground(r: number, g: number, b: number, a?: number): this
+                static setBackground(color: number, hexFlag: boolean): this
+                static setBackground(color: Color | string | CSSColor): this
+
+                static setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
+                  /* ... */
+                }
+                setBackground(r: number, g: number, b: number, a?: number): this
+                setBackground(color: number, hexFlag: boolean): this
+                setBackground(color: Color | string | CSSColor): this
+
+                setBackground(color: ColorArgument, arg1?: boolean | number, arg2?: number, arg3?: number): this {
+                  /* ... */
+                }
+              }
+            `,
+            options: [
+              {
+                ...options,
+              },
+            ],
+            errors: [
+              {
+                messageId: 'unexpectedClassesGroupOrder',
+                data: {
+                  left: 'setBackground',
+                  leftGroup: 'method',
+                  right: 'setBackground',
+                  rightGroup: 'static-method',
+                },
+              },
+              {
+                messageId: 'unexpectedClassesGroupOrder',
+                data: {
+                  left: 'setBackground',
+                  leftGroup: 'static-method',
+                  right: 'a',
+                  rightGroup: 'property',
+                },
+              },
+            ],
+          },
+        ],
       },
     )