Skip to content

Commit

Permalink
fix(sort-classes): fix new lines inside always for signature overloads
Browse files Browse the repository at this point in the history
  • Loading branch information
@hugop95
hugop95 authored Jan 31, 2025
1 parent 266910b commit f581714
Show file tree
Hide file tree
Showing 7 changed files with 237 additions and 58 deletions.
43 changes: 34 additions & 9 deletions rules/sort-classes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
}
Expand All @@ -599,7 +611,7 @@ export default createEslintRule<SortClassesOptions, MESSAGE_ID>({

let sortNodesExcludingEslintDisabled = (
ignoreEslintDisabledNodes: boolean,
): SortingNodeWithDependencies[] =>
): SortClassSortingNodes[] =>
sortNodesByDependencies(
formattedNodes.flatMap(nodes =>
sortNodesByGroups(nodes, options, {
Expand All @@ -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',
Expand Down
111 changes: 109 additions & 2 deletions test/rules/sort-classes.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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 {
/* ... */
}
Expand Down Expand Up @@ -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: [],
},
)
}
})
})

Expand Down
34 changes: 27 additions & 7 deletions utils/get-newlines-errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 []
}
Expand Down
14 changes: 9 additions & 5 deletions utils/make-fixes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -64,6 +67,7 @@ export let makeFixes = ({
newlinesBetween: options.newlinesBetween,
groups: options.groups,
},
newlinesBetweenValueGetter,
sortedNodes,
sourceCode,
fixer,
Expand Down
Loading

0 comments on commit f581714

Please sign in to comment.