fix(sort-classes): fix new lines inside always for signature overloads

Author: hugop95

rules/sort-classes.ts

@@ -6,6 +6,7 @@ import type {
   Selector,
 } from './sort-classes/types'
 import type { SortingNodeWithDependencies } from '../utils/sort-nodes-by-dependencies'
+import type { NewlinesBetweenOption } from '../types/common-options'
 
 import {
   buildCustomGroupsArrayJsonSchema,
@@ -101,6 +102,11 @@ let defaultOptions: Required<SortClassesOptions[0]> = {
   order: 'asc',
 }
 
+interface SortClassSortingNodes
+  extends SortingNodeWithDependencies<TSESTree.ClassElement> {
+  overloadSignaturesGroupId: number | null
+}
+
 export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
   create: context => ({
     ClassBody: node => {
@@ -287,8 +293,8 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
         return dependencies
       }
       let overloadSignatureGroups = getOverloadSignatureGroups(node.body)
-      let formattedNodes: SortingNodeWithDependencies[][] = node.body.reduce(
-        (accumulator: SortingNodeWithDependencies[][], member) => {
+      let formattedNodes: SortClassSortingNodes[][] = node.body.reduce(
+        (accumulator: SortClassSortingNodes[][], member) => {
           let name: string
           let dependencies: string[] = []
           let { defineGroup, getGroup } = useGroups(options)
@@ -553,26 +559,32 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
            * 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 overloadSignatureGroupMemberIndex =
+            overloadSignatureGroups.findIndex(overloadSignatures =>
+              overloadSignatures.includes(member),
+            )
+          let overloadSignatureGroupMember =
+            overloadSignatureGroups[overloadSignatureGroupMemberIndex]?.at(-1)
 
-          let sortingNode: SortingNodeWithDependencies = {
+          let sortingNode: SortClassSortingNodes = {
             dependencyName: getDependencyName({
               nodeNameWithoutStartingHash: name.startsWith('#')
                 ? name.slice(1)
                 : name,
               isStatic: modifiers.includes('static'),
               isPrivateHash,
             }),
+            overloadSignaturesGroupId:
+              overloadSignatureGroupMemberIndex === -1
+                ? null
+                : overloadSignatureGroupMemberIndex,
             size: overloadSignatureGroupMember
               ? rangeToDiff(overloadSignatureGroupMember, sourceCode)
               : rangeToDiff(member, sourceCode),
             isEslintDisabled: isNodeEslintDisabled(member, eslintDisabledLines),
             addSafetySemicolonWhenInline,
             group: getGroup(),
             node: member,
-
             dependencies,
             name,
           }
@@ -599,7 +611,7 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
 
       let sortNodesExcludingEslintDisabled = (
         ignoreEslintDisabledNodes: boolean,
-      ): SortingNodeWithDependencies[] =>
+      ): SortClassSortingNodes[] =>
         sortNodesByDependencies(
           formattedNodes.flatMap(nodes =>
             sortNodesByGroups(nodes, options, {
@@ -618,7 +630,20 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({
 
       let nodes = formattedNodes.flat()
 
-      reportAllErrors<MESSAGE_ID>({
+      reportAllErrors<MESSAGE_ID, SortClassSortingNodes>({
+        newlinesBetweenValueGetter: ({
+          computedNewlinesBetween,
+          right,
+          left,
+        }): NewlinesBetweenOption => {
+          if (
+            left.overloadSignaturesGroupId !== null &&
+            left.overloadSignaturesGroupId === right.overloadSignaturesGroupId
+          ) {
+            return 'never'
+          }
+          return computedNewlinesBetween
+        },
         availableMessageIds: {
           missedSpacingBetweenMembers: 'missedSpacingBetweenClassMembers',
           extraSpacingBetweenMembers: 'extraSpacingBetweenClassMembers',

test/rules/sort-classes.test.ts

@@ -4434,6 +4434,57 @@ describe(ruleName, () => {
         },
       )
 
+      for (let newlinesBetween of ['always', 'ignore', 'never'] as const) {
+        ruleTester.run(
+          `${ruleName}: enforces no newline between overload signatures when newlinesBetween is "${newlinesBetween}"`,
+          rule,
+          {
+            invalid: [
+              {
+                errors: [
+                  {
+                    data: {
+                      right: 'method',
+                      left: 'method',
+                    },
+                    messageId: 'extraSpacingBetweenClassMembers',
+                  },
+                  {
+                    data: {
+                      right: 'method',
+                      left: 'method',
+                    },
+                    messageId: 'extraSpacingBetweenClassMembers',
+                  },
+                ],
+                output: dedent`
+                  class Class {
+                    method(a: string): void {}
+                    method(a: number): void {}
+                    method(a: string | number): void {}
+                  }
+                `,
+                code: dedent`
+                  class Class {
+                    method(a: string): void {}
+
+                    method(a: number): void {}
+
+                    method(a: string | number): void {}
+                  }
+                `,
+                options: [
+                  {
+                    newlinesBetween,
+                  },
+                ],
+              },
+            ],
+            valid: [],
+          },
+        )
+      }
+
       describe(`${ruleName}(${type}): "newlinesBetween" inside groups`, () => {
         ruleTester.run(
           `${ruleName}(${type}): handles "newlinesBetween" between consecutive groups`,
@@ -6841,14 +6892,12 @@ describe(ruleName, () => {
                 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 {
                   /* ... */
                 }
@@ -8181,6 +8230,64 @@ describe(ruleName, () => {
         ],
         valid: [],
       })
+
+      for (let newlinesInside of ['always', 'never'] as const) {
+        ruleTester.run(
+          `${ruleName}: enforces no newline between overload signatures when newlinesBetween is "${newlinesInside}"`,
+          rule,
+          {
+            invalid: [
+              {
+                errors: [
+                  {
+                    data: {
+                      right: 'method',
+                      left: 'method',
+                    },
+                    messageId: 'extraSpacingBetweenClassMembers',
+                  },
+                  {
+                    data: {
+                      right: 'method',
+                      left: 'method',
+                    },
+                    messageId: 'extraSpacingBetweenClassMembers',
+                  },
+                ],
+                options: [
+                  {
+                    customGroups: [
+                      {
+                        groupName: 'methods',
+                        selector: 'method',
+                        newlinesInside,
+                      },
+                    ],
+                    groups: ['methods'],
+                  },
+                ],
+                output: dedent`
+                  class Class {
+                    method(a: string): void {}
+                    method(a: number): void {}
+                    method(a: string | number): void {}
+                  }
+                `,
+                code: dedent`
+                  class Class {
+                    method(a: string): void {}
+
+                    method(a: number): void {}
+
+                    method(a: string | number): void {}
+                  }
+                `,
+              },
+            ],
+            valid: [],
+          },
+        )
+      }
     })
   })
 

utils/get-newlines-errors.ts

@@ -11,22 +11,36 @@ import type { SortingNode } from '../types/sorting-node'
 import { getNewlinesBetweenOption } from './get-newlines-between-option'
 import { getLinesBetween } from './get-lines-between'
 
-interface GetNewlinesErrorsParameters<T extends string> {
+export type NewlinesBetweenValueGetter<T extends SortingNode> = (props: {
+  computedNewlinesBetween: NewlinesBetweenOption
+  right: T
+  left: T
+}) => NewlinesBetweenOption
+
+interface GetNewlinesErrorsParameters<
+  MessageIds extends string,
+  T extends SortingNode,
+> {
   options: {
     customGroups?: DeprecatedCustomGroupsOption | CustomGroupsOption
     newlinesBetween: NewlinesBetweenOption
     groups: GroupsOptions<string>
   }
+  newlinesBetweenValueGetter?: NewlinesBetweenValueGetter<T>
   sourceCode: TSESLint.SourceCode
-  missedSpacingError: T
-  extraSpacingError: T
-  right: SortingNode
-  left: SortingNode
+  missedSpacingError: MessageIds
+  extraSpacingError: MessageIds
   rightNum: number
   leftNum: number
+  right: T
+  left: T
 }
 
-export let getNewlinesErrors = <T extends string>({
+export let getNewlinesErrors = <
+  MessageIds extends string,
+  T extends SortingNode,
+>({
+  newlinesBetweenValueGetter,
   missedSpacingError,
   extraSpacingError,
   sourceCode,
@@ -35,12 +49,18 @@ export let getNewlinesErrors = <T extends string>({
   options,
   right,
   left,
-}: GetNewlinesErrorsParameters<T>): T[] => {
+}: GetNewlinesErrorsParameters<MessageIds, T>): MessageIds[] => {
   let newlinesBetween = getNewlinesBetweenOption({
     nextSortingNode: right,
     sortingNode: left,
     options,
   })
+  newlinesBetween =
+    newlinesBetweenValueGetter?.({
+      computedNewlinesBetween: newlinesBetween,
+      right,
+      left,
+    }) ?? newlinesBetween
   if (leftNum > rightNum) {
     return []
   }

utils/make-fixes.ts

@@ -7,34 +7,37 @@ import type {
   CustomGroupsOption,
   GroupsOptions,
 } from '../types/common-options'
+import type { NewlinesBetweenValueGetter } from './get-newlines-errors'
 import type { SortingNode } from '../types/sorting-node'
 
 import { makeCommentAfterFixes } from './make-comment-after-fixes'
 import { makeNewlinesFixes } from './make-newlines-fixes'
 import { makeOrderFixes } from './make-order-fixes'
 
-export interface MakeFixesParameters {
+export interface MakeFixesParameters<T extends SortingNode> {
   options?: {
     customGroups?: DeprecatedCustomGroupsOption | CustomGroupsOption
     partitionByComment?: PartitionByCommentOption
     newlinesBetween?: NewlinesBetweenOption
     groups?: GroupsOptions<string>
   }
+  newlinesBetweenValueGetter?: NewlinesBetweenValueGetter<T>
   ignoreFirstNodeHighestBlockComment?: boolean
   sourceCode: TSESLint.SourceCode
-  sortedNodes: SortingNode[]
   fixer: TSESLint.RuleFixer
-  nodes: SortingNode[]
+  sortedNodes: T[]
+  nodes: T[]
 }
 
-export let makeFixes = ({
+export let makeFixes = <T extends SortingNode>({
   ignoreFirstNodeHighestBlockComment,
+  newlinesBetweenValueGetter,
   sortedNodes,
   sourceCode,
   options,
   fixer,
   nodes,
-}: MakeFixesParameters): TSESLint.RuleFix[] => {
+}: MakeFixesParameters<T>): TSESLint.RuleFix[] => {
   let orderFixes = makeOrderFixes({
     ignoreFirstNodeHighestBlockComment,
     sortedNodes,
@@ -64,6 +67,7 @@ export let makeFixes = ({
       newlinesBetween: options.newlinesBetween,
       groups: options.groups,
     },
+    newlinesBetweenValueGetter,
     sortedNodes,
     sourceCode,
     fixer,

utils/make-newlines-fixes.ts

@@ -6,31 +6,34 @@ import type {
   CustomGroupsOption,
   GroupsOptions,
 } from '../types/common-options'
+import type { NewlinesBetweenValueGetter } from './get-newlines-errors'
 import type { SortingNode } from '../types/sorting-node'
 
 import { getNewlinesBetweenOption } from './get-newlines-between-option'
 import { getLinesBetween } from './get-lines-between'
 import { getNodeRange } from './get-node-range'
 
-interface MakeNewlinesFixesParameters {
+interface MakeNewlinesFixesParameters<T extends SortingNode> {
   options: {
     customGroups?: DeprecatedCustomGroupsOption | CustomGroupsOption
     newlinesBetween: NewlinesBetweenOption
     groups: GroupsOptions<string>
   }
+  newlinesBetweenValueGetter?: NewlinesBetweenValueGetter<T>
   sourceCode: TSESLint.SourceCode
-  sortedNodes: SortingNode[]
   fixer: TSESLint.RuleFixer
-  nodes: SortingNode[]
+  sortedNodes: T[]
+  nodes: T[]
 }
 
-export let makeNewlinesFixes = ({
+export let makeNewlinesFixes = <T extends SortingNode>({
+  newlinesBetweenValueGetter,
   sortedNodes,
   sourceCode,
   options,
   fixer,
   nodes,
-}: MakeNewlinesFixesParameters): TSESLint.RuleFix[] => {
+}: MakeNewlinesFixesParameters<T>): TSESLint.RuleFix[] => {
   let fixes: TSESLint.RuleFix[] = []
 
   for (let i = 0; i < sortedNodes.length - 1; i++) {
@@ -44,6 +47,12 @@ export let makeNewlinesFixes = ({
       sortingNode: sortedSortingNode,
       options,
     })
+    newlinesBetween =
+      newlinesBetweenValueGetter?.({
+        computedNewlinesBetween: newlinesBetween,
+        right: nextSortedSortingNode,
+        left: sortedSortingNode,
+      }) ?? newlinesBetween
 
     if (newlinesBetween === 'ignore') {
       continue

utils/report-all-errors.ts

@@ -1,6 +1,7 @@
 import type { TSESLint } from '@typescript-eslint/utils'
 
 import type { SortingNodeWithDependencies } from './sort-nodes-by-dependencies'
+import type { NewlinesBetweenValueGetter } from './get-newlines-errors'
 import type { GroupsOptions } from '../types/common-options'
 import type { SortingNode } from '../types/sorting-node'
 import type { MakeFixesParameters } from './make-fixes'
@@ -12,35 +13,41 @@ import { getGroupNumber } from './get-group-number'
 import { reportErrors } from './report-errors'
 import { pairwise } from './pairwise'
 
-interface ReportAllErrorsParameters<MessageIds extends string> {
+interface ReportAllErrorsParameters<
+  MessageIds extends string,
+  T extends SortingNode,
+> {
   availableMessageIds: {
     missedSpacingBetweenMembers?: MessageIds
     extraSpacingBetweenMembers?: MessageIds
     unexpectedDependencyOrder?: MessageIds
     unexpectedGroupOrder?: MessageIds
     unexpectedOrder: MessageIds
   }
-  sortNodesExcludingEslintDisabled(
-    ignoreEslintDisabledNodes: boolean,
-  ): SortingNode[]
   options: {
     groups?: GroupsOptions<string>
-  } & MakeFixesParameters['options']
+  } & MakeFixesParameters<T>['options']
+  sortNodesExcludingEslintDisabled(ignoreEslintDisabledNodes: boolean): T[]
+  newlinesBetweenValueGetter?: NewlinesBetweenValueGetter<T>
   context: TSESLint.RuleContext<MessageIds, unknown[]>
   ignoreFirstNodeHighestBlockComment?: boolean
   sourceCode: TSESLint.SourceCode
-  nodes: SortingNode[]
+  nodes: T[]
 }
 
-export let reportAllErrors = <MessageIds extends string>({
+export let reportAllErrors = <
+  MessageIds extends string,
+  T extends SortingNode = SortingNode,
+>({
   ignoreFirstNodeHighestBlockComment,
   sortNodesExcludingEslintDisabled,
+  newlinesBetweenValueGetter,
   availableMessageIds,
   sourceCode,
   context,
   options,
   nodes,
-}: ReportAllErrorsParameters<MessageIds>): void => {
+}: ReportAllErrorsParameters<MessageIds, T>): void => {
   let sortedNodes = sortNodesExcludingEslintDisabled(false)
   let sortedNodesExcludingEslintDisabled =
     sortNodesExcludingEslintDisabled(true)
@@ -58,14 +65,12 @@ export let reportAllErrors = <MessageIds extends string>({
 
     let messageIds: MessageIds[] = []
 
-    let firstUnorderedNodeDependentOnRight:
-      | SortingNodeWithDependencies
-      | undefined
+    let firstUnorderedNodeDependentOnRight: undefined | T
     if (availableMessageIds.unexpectedDependencyOrder) {
       firstUnorderedNodeDependentOnRight = getFirstUnorderedNodeDependentOn(
-        right as SortingNodeWithDependencies,
-        nodes as SortingNodeWithDependencies[],
-      )
+        right as unknown as SortingNodeWithDependencies,
+        nodes as unknown as SortingNodeWithDependencies[],
+      ) as unknown as T
     }
 
     if (
@@ -101,6 +106,7 @@ export let reportAllErrors = <MessageIds extends string>({
           },
           missedSpacingError: availableMessageIds.missedSpacingBetweenMembers,
           extraSpacingError: availableMessageIds.extraSpacingBetweenMembers,
+          newlinesBetweenValueGetter,
           rightNum: rightNumber,
           leftNum: leftNumber,
           sourceCode,
@@ -114,6 +120,7 @@ export let reportAllErrors = <MessageIds extends string>({
       sortedNodes: sortedNodesExcludingEslintDisabled,
       ignoreFirstNodeHighestBlockComment,
       firstUnorderedNodeDependentOnRight,
+      newlinesBetweenValueGetter,
       messageIds,
       sourceCode,
       options,

utils/report-errors.ts

@@ -1,5 +1,6 @@
 import type { TSESLint } from '@typescript-eslint/utils'
 
+import type { NewlinesBetweenValueGetter } from './get-newlines-errors'
 import type { SortingNode } from '../types/sorting-node'
 import type { MakeFixesParameters } from './make-fixes'
 
@@ -23,22 +24,27 @@ export const EXTRA_SPACING_ERROR =
 export const MISSED_SPACING_ERROR =
   `Missed spacing between "{{${LEFT}}}" and "{{${RIGHT}}}".` as const
 
-interface ReportErrorsParameters<MessageIds extends string> {
+interface ReportErrorsParameters<
+  MessageIds extends string,
+  T extends SortingNode,
+> {
+  newlinesBetweenValueGetter?: NewlinesBetweenValueGetter<T>
   context: TSESLint.RuleContext<MessageIds, unknown[]>
-  firstUnorderedNodeDependentOnRight?: SortingNode
   ignoreFirstNodeHighestBlockComment?: boolean
-  options?: MakeFixesParameters['options']
+  options?: MakeFixesParameters<T>['options']
+  firstUnorderedNodeDependentOnRight?: T
   sourceCode: TSESLint.SourceCode
-  sortedNodes: SortingNode[]
   messageIds: MessageIds[]
-  nodes: SortingNode[]
-  right: SortingNode
-  left: SortingNode
+  sortedNodes: T[]
+  nodes: T[]
+  right: T
+  left: T
 }
 
-export let reportErrors = <MessageIds extends string>({
+export let reportErrors = <MessageIds extends string, T extends SortingNode>({
   firstUnorderedNodeDependentOnRight,
   ignoreFirstNodeHighestBlockComment,
+  newlinesBetweenValueGetter,
   sortedNodes,
   messageIds,
   sourceCode,
@@ -47,25 +53,26 @@ export let reportErrors = <MessageIds extends string>({
   nodes,
   right,
   left,
-}: ReportErrorsParameters<MessageIds>): void => {
+}: ReportErrorsParameters<MessageIds, T>): void => {
   for (let messageId of messageIds) {
     context.report({
-      data: {
-        [NODE_DEPENDENT_ON_RIGHT]: firstUnorderedNodeDependentOnRight?.name,
-        [RIGHT]: toSingleLine(right.name),
-        [LEFT]: toSingleLine(left.name),
-        [RIGHT_GROUP]: right.group,
-        [LEFT_GROUP]: left.group,
-      },
       fix: (fixer: TSESLint.RuleFixer) =>
         makeFixes({
           ignoreFirstNodeHighestBlockComment,
+          newlinesBetweenValueGetter,
           sortedNodes,
           sourceCode,
           options,
           fixer,
           nodes,
         }),
+      data: {
+        [NODE_DEPENDENT_ON_RIGHT]: firstUnorderedNodeDependentOnRight?.name,
+        [RIGHT]: toSingleLine(right.name),
+        [LEFT]: toSingleLine(left.name),
+        [RIGHT_GROUP]: right.group,
+        [LEFT_GROUP]: left.group,
+      },
       node: right.node,
       messageId,
     })