From f707f3a6d4736ad2d585f4a1f25e2f6268eaad89 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sun, 22 Dec 2024 02:12:38 +0900 Subject: [PATCH 1/2] fix(eslint-plugin): [member-ordering] ignore method overloading --- .../src/rules/member-ordering.ts | 25 +++++++++++- .../tests/rules/member-ordering.test.ts | 40 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 83b297152267..2646e2adcd89 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -826,7 +826,9 @@ export default createRule({ let isCorrectlySorted = true; // Find first member which isn't correctly sorted - for (const member of members) { + for (let index = 0; index < members.length; index++) { + const member = members[index]; + const prevMember = members.at(index - 1); const rank = getRank(member, groupOrder, supportsModifiers); const name = getMemberName(member, context.sourceCode); const rankLastMember = previousRanks[previousRanks.length - 1]; @@ -836,7 +838,10 @@ export default createRule({ } // Works for 1st item because x < undefined === false for any x (typeof string) - if (rank < rankLastMember) { + if ( + rank < rankLastMember && + !(prevMember && isMethodOverloaing(prevMember, member)) + ) { context.report({ node: member, messageId: 'incorrectGroupOrder', @@ -1111,3 +1116,19 @@ export default createRule({ /* eslint-enable @typescript-eslint/no-non-null-assertion */ }, }); + +function isMethodOverloaing(prev: Member, next: Member): boolean { + if ( + prev.type === AST_NODE_TYPES.MethodDefinition && + next.type === AST_NODE_TYPES.MethodDefinition && + prev.key.type === AST_NODE_TYPES.Identifier && + next.key.type === AST_NODE_TYPES.Identifier + ) { + return ( + prev.static === next.static && + prev.key.name === next.key.name && + prev.value.type === AST_NODE_TYPES.TSEmptyBodyFunctionExpression + ); + } + return false; +} diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index 3660a040b468..2a98ee2a0b6d 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -2148,6 +2148,23 @@ interface Foo { }, ], }, + { + code: ` +class Foo { + public bar(): void; + @Decorator() public bar() {} +} + `, + options: [ + { + default: [ + 'public-decorated-method', + 'public-instance-method', + 'public-method', + ], + }, + ], + }, ], invalid: [ { @@ -5267,6 +5284,29 @@ interface Foo { }, ], }, + { + code: ` +class Foo { + static foo() {} + foo(): void; + foo() {} +} + `, + errors: [ + { + column: 3, + data: { + name: 'foo', + rank: 'public static method', + }, + line: 4, + messageId: 'incorrectGroupOrder', + }, + ], + options: [ + { default: ['public-instance-method', 'public-static-method'] }, + ], + }, ], }; From bc569e4aba1f5e10ad647f990a20add47a2f0db4 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Sun, 22 Dec 2024 17:03:48 +0900 Subject: [PATCH 2/2] refactor --- .../src/rules/member-ordering.ts | 32 ++++-------- .../tests/rules/member-ordering.test.ts | 51 +++++++++++++++---- 2 files changed, 49 insertions(+), 34 deletions(-) diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 2646e2adcd89..1f5dee70984c 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -548,6 +548,13 @@ function getRank( ): number { const type = getNodeType(node); + if ( + node.type === AST_NODE_TYPES.MethodDefinition && + node.value.type === AST_NODE_TYPES.TSEmptyBodyFunctionExpression + ) { + return -1; + } + if (type == null) { // shouldn't happen but just in case, put it on the end return orderConfig.length - 1; @@ -826,9 +833,7 @@ export default createRule({ let isCorrectlySorted = true; // Find first member which isn't correctly sorted - for (let index = 0; index < members.length; index++) { - const member = members[index]; - const prevMember = members.at(index - 1); + for (const member of members) { const rank = getRank(member, groupOrder, supportsModifiers); const name = getMemberName(member, context.sourceCode); const rankLastMember = previousRanks[previousRanks.length - 1]; @@ -838,10 +843,7 @@ export default createRule({ } // Works for 1st item because x < undefined === false for any x (typeof string) - if ( - rank < rankLastMember && - !(prevMember && isMethodOverloaing(prevMember, member)) - ) { + if (rank < rankLastMember) { context.report({ node: member, messageId: 'incorrectGroupOrder', @@ -1116,19 +1118,3 @@ export default createRule({ /* eslint-enable @typescript-eslint/no-non-null-assertion */ }, }); - -function isMethodOverloaing(prev: Member, next: Member): boolean { - if ( - prev.type === AST_NODE_TYPES.MethodDefinition && - next.type === AST_NODE_TYPES.MethodDefinition && - prev.key.type === AST_NODE_TYPES.Identifier && - next.key.type === AST_NODE_TYPES.Identifier - ) { - return ( - prev.static === next.static && - prev.key.name === next.key.name && - prev.value.type === AST_NODE_TYPES.TSEmptyBodyFunctionExpression - ); - } - return false; -} diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index 2a98ee2a0b6d..d53fe04d6d22 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -2150,18 +2150,47 @@ interface Foo { }, { code: ` +class Foo { + public baz(): void; + @Decorator() public baz() {} + + @Decorator() bar() {} +} + `, + options: [ + { + default: ['public-decorated-method', 'public-instance-method'], + }, + ], + }, + { + code: ` class Foo { public bar(): void; - @Decorator() public bar() {} + @Decorator() bar() {} + + public baz(): void; + @Decorator() public baz() {} } `, options: [ { - default: [ - 'public-decorated-method', - 'public-instance-method', - 'public-method', - ], + default: ['public-instance-method', 'public-decorated-method'], + }, + ], + }, + { + code: ` +class Foo { + @Decorator() bar() {} + + public baz(): void; + @Decorator() public baz() {} +} + `, + options: [ + { + default: ['public-instance-method', 'public-decorated-method'], }, ], }, @@ -4456,7 +4485,7 @@ abstract class Foo { class Foo { C: number; [A: string]: number; - public static D(): {}; + public static D() {} static [B: string]: number; } `, @@ -4488,7 +4517,7 @@ class Foo { abstract class Foo { abstract B: string; abstract A(): void; - public C(): {}; + public C() {} } `, errors: [ @@ -4640,7 +4669,7 @@ class Foo { code: ` class Foo { A: string; - private C(): void; + private C() {} constructor() {} @Dec() private B: string; set D() {} @@ -4992,7 +5021,7 @@ class Foo { code: ` class Foo { A: string; - private C(): void; + private C() {} constructor() {} private readonly B: string; set D() {} @@ -5299,7 +5328,7 @@ class Foo { name: 'foo', rank: 'public static method', }, - line: 4, + line: 5, messageId: 'incorrectGroupOrder', }, ],