diff --git a/packages/eslint-plugin/docs/rules/member-ordering.md b/packages/eslint-plugin/docs/rules/member-ordering.md index 9a2a4cdee8f..eda1c573c8e 100644 --- a/packages/eslint-plugin/docs/rules/member-ordering.md +++ b/packages/eslint-plugin/docs/rules/member-ordering.md @@ -62,6 +62,9 @@ There are multiple ways to specify the member types. The most explicit and granu "public-static-field", "protected-static-field", "private-static-field", + "public-decorated-field", + "protected-decorated-field", + "private-decorated-field", "public-instance-field", "protected-instance-field", "private-instance-field", @@ -78,6 +81,9 @@ There are multiple ways to specify the member types. The most explicit and granu "public-static-method", "protected-static-method", "private-static-method", + "public-decorated-method", + "protected-decorated-method", + "private-decorated-method", "public-instance-method", "protected-instance-method", "private-instance-method", @@ -99,17 +105,45 @@ It is also possible to group member types by their accessibility (`static`, `ins // No accessibility for index signature. See above. // Fields - "public-field", // = ["public-static-field", "public-instance-field"]) - "protected-field", // = ["protected-static-field", "protected-instance-field"]) - "private-field", // = ["private-static-field", "private-instance-field"]) + "public-field", // = ["public-static-field", "public-instance-field"] + "protected-field", // = ["protected-static-field", "protected-instance-field"] + "private-field", // = ["private-static-field", "private-instance-field"] // Constructors // Only the accessibility of constructors is configurable. See below. // Methods - "public-method", // = ["public-static-method", "public-instance-method"]) - "protected-method", // = ["protected-static-method", "protected-instance-method"]) - "private-method" // = ["private-static-method", "private-instance-method"]) + "public-method", // = ["public-static-method", "public-instance-method"] + "protected-method", // = ["protected-static-method", "protected-instance-method"] + "private-method" // = ["private-static-method", "private-instance-method"] +] +``` + +### Member group types (with accessibility and a decorator) + +It is also possible to group methods or fields with a decorator separately, optionally specifying +their accessibility. + +```jsonc +[ + // Index signature + // No decorators for index signature. + + // Fields + "public-decorated-field", + "protected-decorated-field", + "private-decorated-field", + + "decorated-field", // = ["public-decorated-field", "protected-decorated-field", "private-decorated-field"] + + // Constructors + // There are no decorators for constructors. + + "public-decorated-method", + "protected-decorated-method", + "private-decorated-method", + + "decorated-method" // = ["public-decorated-method", "protected-decorated-method", "private-decorated-method"] ] ``` @@ -123,17 +157,17 @@ Another option is to group the member types by their scope (`public`, `protected // No scope for index signature. See above. // Fields - "static-field", // = ["public-static-field", "protected-static-field", "private-static-field"]) - "instance-field", // = ["public-instance-field", "protected-instance-field", "private-instance-field"]) - "abstract-field", // = ["public-abstract-field", "protected-abstract-field", "private-abstract-field"]) + "static-field", // = ["public-static-field", "protected-static-field", "private-static-field"] + "instance-field", // = ["public-instance-field", "protected-instance-field", "private-instance-field"] + "abstract-field", // = ["public-abstract-field", "protected-abstract-field", "private-abstract-field"] // Constructors - "constructor", // = ["public-constructor", "protected-constructor", "private-constructor"]) + "constructor", // = ["public-constructor", "protected-constructor", "private-constructor"] // Methods - "static-method", // = ["public-static-method", "protected-static-method", "private-static-method"]) - "instance-method", // = ["public-instance-method", "protected-instance-method", "private-instance-method"]) - "abstract-method" // = ["public-abstract-method", "protected-abstract-method", "private-abstract-method"]) + "static-method", // = ["public-static-method", "protected-static-method", "private-static-method"] + "instance-method", // = ["public-instance-method", "protected-instance-method", "private-instance-method"] + "abstract-method" // = ["public-abstract-method", "protected-abstract-method", "private-abstract-method"] ] ``` @@ -148,14 +182,14 @@ The third grouping option is to ignore both scope and accessibility. // Fields "field", // = ["public-static-field", "protected-static-field", "private-static-field", "public-instance-field", "protected-instance-field", "private-instance-field", - // "public-abstract-field", "protected-abstract-field", private-abstract-field"]) + // "public-abstract-field", "protected-abstract-field", private-abstract-field"] // Constructors // Only the accessibility of constructors is configurable. See above. // Methods "method" // = ["public-static-method", "protected-static-method", "private-static-method", "public-instance-method", "protected-instance-method", "private-instance-method", - // "public-abstract-method", "protected-abstract-method", "private-abstract-method"]) + // "public-abstract-method", "protected-abstract-method", "private-abstract-method"] ] ``` @@ -174,6 +208,10 @@ The default configuration looks as follows: "protected-static-field", "private-static-field", + "public-decorated-field", + "protected-decorated-field", + "private-decorated-field", + "public-instance-field", "protected-instance-field", "private-instance-field", @@ -190,6 +228,8 @@ The default configuration looks as follows: "instance-field", "abstract-field", + "decorated-field", + "field", // Constructors @@ -204,6 +244,10 @@ The default configuration looks as follows: "protected-static-method", "private-static-method", + "public-decorated-method", + "protected-decorated-method", + "private-decorated-method", + "public-instance-method", "protected-instance-method", "private-instance-method", @@ -220,6 +264,8 @@ The default configuration looks as follows: "instance-method", "abstract-method", + "decorated-method", + "method" ] } @@ -749,7 +795,7 @@ type Foo = { It is possible to sort all members within a group alphabetically. -#### Configuration: `{ default: { memberTypes: , order: "alphabetically" } }` +#### Configuration: `{ "default": { "memberTypes": , "order": "alphabetically" } }` This will apply the default order (see above) and enforce an alphabetic order within each group. @@ -795,7 +841,7 @@ interface Foo { It is also possible to sort all members and ignore the member groups completely. -#### Configuration: `{ default: { memberTypes: "never", order: "alphabetically" } }` +#### Configuration: `{ "default": { "memberTypes": "never", "order": "alphabetically" } }` ##### Incorrect example diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 14ddcc236a9..411823c8de7 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -60,6 +60,10 @@ export const defaultOrder = [ 'protected-static-field', 'private-static-field', + 'public-decorated-field', + 'protected-decorated-field', + 'private-decorated-field', + 'public-instance-field', 'protected-instance-field', 'private-instance-field', @@ -76,6 +80,8 @@ export const defaultOrder = [ 'instance-field', 'abstract-field', + 'decorated-field', + 'field', // Constructors @@ -90,6 +96,10 @@ export const defaultOrder = [ 'protected-static-method', 'private-static-method', + 'public-decorated-method', + 'protected-decorated-method', + 'private-decorated-method', + 'public-instance-method', 'protected-instance-method', 'private-instance-method', @@ -106,6 +116,8 @@ export const defaultOrder = [ 'instance-method', 'abstract-method', + 'decorated-method', + 'method', ]; @@ -119,6 +131,18 @@ const allMemberTypes = ['signature', 'field', 'method', 'constructor'].reduce< all.push(`${accessibility}-${type}`); // e.g. `public-field` } + // Only class instance fields and methods can have decorators attached to them + if (type === 'field' || type === 'method') { + const decoratedMemberType = `${accessibility}-decorated-${type}`; + const decoratedMemberTypeNoAccessibility = `decorated-${type}`; + if (!all.includes(decoratedMemberType)) { + all.push(decoratedMemberType); + } + if (!all.includes(decoratedMemberTypeNoAccessibility)) { + all.push(decoratedMemberTypeNoAccessibility); + } + } + if (type !== 'constructor' && type !== 'signature') { // There is no `static-constructor` or `instance-constructor` or `abstract-constructor` ['static', 'instance', 'abstract'].forEach(scope => { @@ -258,6 +282,12 @@ function getRank( const memberGroups = []; if (supportsModifiers) { + const decorated = 'decorators' in node && node.decorators!.length > 0; + if (decorated && (type === 'field' || type === 'method')) { + memberGroups.push(`${accessibility}-decorated-${type}`); + memberGroups.push(`decorated-${type}`); + } + if (type !== 'constructor') { // Constructors have no scope memberGroups.push(`${accessibility}-${scope}-${type}`); @@ -396,27 +426,29 @@ export default util.createRule({ const name = getMemberName(member, context.getSourceCode()); const rankLastMember = previousRanks[previousRanks.length - 1]; - if (rank !== -1) { - // Works for 1st item because x < undefined === false for any x (typeof string) - if (rank < rankLastMember) { - context.report({ - node: member, - messageId: 'incorrectGroupOrder', - data: { - name, - rank: getLowestRank(previousRanks, rank, groupOrder), - }, - }); + if (rank === -1) { + return; + } - isCorrectlySorted = false; - } else if (rank === rankLastMember) { - // Same member group --> Push to existing member group array - memberGroups[memberGroups.length - 1].push(member); - } else { - // New member group --> Create new member group array - previousRanks.push(rank); - memberGroups.push([member]); - } + // Works for 1st item because x < undefined === false for any x (typeof string) + if (rank < rankLastMember) { + context.report({ + node: member, + messageId: 'incorrectGroupOrder', + data: { + name, + rank: getLowestRank(previousRanks, rank, groupOrder), + }, + }); + + isCorrectlySorted = false; + } else if (rank === rankLastMember) { + // Same member group --> Push to existing member group array + memberGroups[memberGroups.length - 1].push(member); + } else { + // New member group --> Create new member group array + previousRanks.push(rank); + memberGroups.push([member]); } }); @@ -472,36 +504,34 @@ export default util.createRule({ orderConfig: OrderConfig, supportsModifiers: boolean, ): void { - if (orderConfig !== 'never') { - // Standardize config - let order = null; - let memberTypes; + if (orderConfig === 'never') { + return; + } - if (Array.isArray(orderConfig)) { - memberTypes = orderConfig; - } else { - order = orderConfig.order; - memberTypes = orderConfig.memberTypes; - } + // Standardize config + let order = null; + let memberTypes; - // Check order - if (Array.isArray(memberTypes)) { - const grouped = checkGroupSort( - members, - memberTypes, - supportsModifiers, - ); + if (Array.isArray(orderConfig)) { + memberTypes = orderConfig; + } else { + order = orderConfig.order; + memberTypes = orderConfig.memberTypes; + } - if (grouped === null) { - return; - } + // Check order + if (Array.isArray(memberTypes)) { + const grouped = checkGroupSort(members, memberTypes, supportsModifiers); - if (order === 'alphabetically') { - grouped.some(groupMember => !checkAlphaSort(groupMember)); - } - } else if (order === 'alphabetically') { - checkAlphaSort(members); + if (grouped === null) { + return; + } + + if (order === 'alphabetically') { + grouped.some(groupMember => !checkAlphaSort(groupMember)); } + } else if (order === 'alphabetically') { + checkAlphaSort(members); } } diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index c64c57c4591..5134d591ffa 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -1317,6 +1317,101 @@ abstract class Foo { `, options: [{ classes: ['signature', 'field', 'constructor', 'method'] }], }, + { + code: ` +class Foo { + @Dec() B: string; + @Dec() A: string; + constructor() {} + D: string; + C: string; + E(): void; + F(): void; +} `, + options: [{ default: ['decorated-field', 'field'] }], + }, + { + code: ` +class Foo { + A: string; + B: string; + @Dec() private C: string; + private D: string; +} `, + options: [ + { + default: ['public-field', 'private-decorated-field', 'private-field'], + }, + ], + }, + { + code: ` +class Foo { + constructor() {} + @Dec() public A(): void; + @Dec() private B: string; + private C(): void; + private D: string; +} `, + options: [ + { + default: [ + 'decorated-method', + 'private-decorated-field', + 'private-method', + ], + }, + ], + }, + { + code: ` +class Foo { + @Dec() private A(): void; + @Dec() private B: string; + constructor() {} + private C(): void; + private D: string; +} `, + options: [ + { + default: [ + 'private-decorated-method', + 'private-decorated-field', + 'constructor', + 'private-field', + ], + }, + ], + }, + { + code: ` +class Foo { + public A: string; + @Dec() private B: string; +} `, + options: [ + { + default: ['private-decorated-field', 'public-instance-field'], + classes: ['public-instance-field', 'private-decorated-field'], + }, + ], + }, + // class + ignore decorator + { + code: ` +class Foo { + public A(): string; + @Dec() public B(): string; + public C(): string; + + d: string; +} `, + options: [ + { + default: ['public-method', 'field'], + }, + ], + }, ], invalid: [ { @@ -3614,6 +3709,96 @@ abstract class Foo { }, ], }, + { + code: ` +// no accessibility === public +class Foo { + B: string; + @Dec() A: string = ""; + C: string = ""; + constructor() {} + D() {} + E() {} +} `, + options: [{ default: ['decorated-field', 'field'] }], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'A', + rank: 'field', + }, + line: 5, + column: 5, + }, + ], + }, + { + code: ` +class Foo { + A() {} + + @Decorator() + B() {} +} `, + options: [{ default: ['decorated-method', 'method'] }], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'B', + rank: 'method', + }, + line: 5, // Symbol starts at the line with decorator + column: 5, + }, + ], + }, + { + code: ` +class Foo { + @Decorator() C() {} + A() {} +} `, + options: [{ default: ['public-method', 'decorated-method'] }], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'A', + rank: 'decorated method', + }, + line: 4, + column: 5, + }, + ], + }, + { + code: ` +class Foo { + A(): void; + B(): void; + private C() {} + constructor() {} + @Dec() private D() {} +} `, + options: [ + { + classes: ['public-method', 'decorated-method', 'private-method'], + }, + ], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'D', + rank: 'private method', + }, + line: 7, + column: 5, + }, + ], + }, ], }; @@ -3702,9 +3887,11 @@ class Foo { protected static b : string = ""; private static c : string = ""; constructor() {} - public d : string = ""; - protected e : string = ""; - private f : string = ""; + @Dec() d: string; + public e : string = ""; + @Dec() f : string = ""; + protected g : string = ""; + private h : string = ""; } `, options: [{ default: { memberTypes: 'never', order: 'alphabetically' } }], @@ -3765,6 +3952,18 @@ const foo = class Foo { const foo = class Foo { public static a1 : string; public static aa : string; +} + `, + options: [{ default: { memberTypes: 'never', order: 'alphabetically' } }], + }, + + // default option + class + decorators + { + code: ` +class Foo { + public static a : string; + @Dec() static b : string; + public static c : string; } `, options: [{ default: { memberTypes: 'never', order: 'alphabetically' } }], @@ -4064,10 +4263,11 @@ type Foo = { class Foo { public static a : string; protected static b : string = ""; - private static c : string = ""; + @Dec() private static c : string = ""; constructor() {} public d : string = ""; protected e : string = ""; + @Dec() private f : string = ""; } `, @@ -5019,6 +5219,29 @@ class Foo { protected e: string = ""; private f: string = ""; + constructor() {} +} + `, + options: [ + { default: { memberTypes: defaultOrder, order: 'alphabetically' } }, + ], + }, + // default option + class + decorators + default order + alphabetically + { + code: ` +class Foo { + public static a: string; + protected static b: string = ""; + private static c: string = ""; + + @Dec() public d: string; + @Dec() protected e: string; + @Dec() private f: string; + + public g: string = ""; + protected h: string = ""; + private i: string = ""; + constructor() {} } `, @@ -5217,6 +5440,55 @@ const foo = class Foo { }, ], }, + // default option + class + decorators + custom order + wrong order within group and wrong group order + alphabetically + { + code: ` +class Foo { + @Dec() a1: string; + @Dec() + a3: string; + @Dec() + a2: string; + + constructor() {} + + b1: string; + b2: string; + + public c(): void; + @Dec() d(): void +} + `, + options: [ + { + default: { + memberTypes: [ + 'decorated-field', + 'field', + 'constructor', + 'decorated-method', + ], + order: 'alphabetically', + }, + }, + ], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'b1', + rank: 'constructor', + }, + }, + { + messageId: 'incorrectGroupOrder', + data: { + name: 'b2', + rank: 'constructor', + }, + }, + ], + }, ], };