From f02761af19848a84b8d1832bd00fd6c95d38fa0c Mon Sep 17 00:00:00 2001 From: Sviatoslav Zaytsev Date: Fri, 25 Nov 2022 04:09:46 +0300 Subject: [PATCH] fix(eslint-plugin): [member-ordering] support private fields (#5859) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Святослав Зайцев --- .../docs/rules/member-ordering.md | 50 +++- .../src/rules/member-ordering.ts | 102 ++++--- .../tests/rules/member-ordering.test.ts | 249 +++++++++++++++++- 3 files changed, 351 insertions(+), 50 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/member-ordering.md b/packages/eslint-plugin/docs/rules/member-ordering.md index 7adde7ba9a6..bfab8265260 100644 --- a/packages/eslint-plugin/docs/rules/member-ordering.md +++ b/packages/eslint-plugin/docs/rules/member-ordering.md @@ -54,7 +54,7 @@ The `OrderConfig` settings for each kind of construct may configure sorting on o You can define many different groups based on different attributes of members. The supported member attributes are, in order: -- **Accessibility** (`'public' | 'protected' | 'private'`) +- **Accessibility** (`'public' | 'protected' | 'private' | '#private'`) - **Decoration** (`'decorated'`): Whether the member has an explicit accessibility decorator - **Kind** (`'call-signature' | 'constructor' | 'field' | 'get' | 'method' | 'set' | 'signature'`) @@ -81,11 +81,13 @@ The default configuration looks as follows: "default": [ // Index signature "signature", + "call-signature", // Fields "public-static-field", "protected-static-field", "private-static-field", + "#private-static-field", "public-decorated-field", "protected-decorated-field", @@ -94,14 +96,15 @@ The default configuration looks as follows: "public-instance-field", "protected-instance-field", "private-instance-field", + "#private-instance-field", "public-abstract-field", "protected-abstract-field", - "private-abstract-field", "public-field", "protected-field", "private-field", + "#private-field", "static-field", "instance-field", @@ -125,6 +128,7 @@ The default configuration looks as follows: "public-static-get", "protected-static-get", "private-static-get", + "#private-static-get", "public-decorated-get", "protected-decorated-get", @@ -133,14 +137,15 @@ The default configuration looks as follows: "public-instance-get", "protected-instance-get", "private-instance-get", + "#private-instance-get", "public-abstract-get", "protected-abstract-get", - "private-abstract-get", "public-get", "protected-get", "private-get", + "#private-get", "static-get", "instance-get", @@ -154,6 +159,7 @@ The default configuration looks as follows: "public-static-set", "protected-static-set", "private-static-set", + "#private-static-set", "public-decorated-set", "protected-decorated-set", @@ -162,14 +168,15 @@ The default configuration looks as follows: "public-instance-set", "protected-instance-set", "private-instance-set", + "#private-instance-set", "public-abstract-set", "protected-abstract-set", - "private-abstract-set", "public-set", "protected-set", "private-set", + "#private-set", "static-set", "instance-set", @@ -183,6 +190,7 @@ The default configuration looks as follows: "public-static-method", "protected-static-method", "private-static-method", + "#private-static-method", "public-decorated-method", "protected-decorated-method", @@ -191,14 +199,15 @@ The default configuration looks as follows: "public-instance-method", "protected-instance-method", "private-instance-method", + "#private-instance-method", "public-abstract-method", "protected-abstract-method", - "private-abstract-method", "public-method", "protected-method", "private-method", + "#private-method", "static-method", "instance-method", @@ -918,15 +927,32 @@ The most explicit and granular form is the following: "public-static-field", "protected-static-field", "private-static-field", + "#private-static-field", + "public-decorated-field", "protected-decorated-field", "private-decorated-field", + "public-instance-field", "protected-instance-field", "private-instance-field", + "#private-instance-field", + "public-abstract-field", "protected-abstract-field", - "private-abstract-field", + + "public-field", + "protected-field", + "private-field", + "#private-field", + + "static-field", + "instance-field", + "abstract-field", + + "decorated-field", + + "field", // Static initialization "static-initialization", @@ -940,6 +966,7 @@ The most explicit and granular form is the following: "public-static-get", "protected-static-get", "private-static-get", + "#private-static-get", "public-decorated-get", "protected-decorated-get", @@ -948,14 +975,15 @@ The most explicit and granular form is the following: "public-instance-get", "protected-instance-get", "private-instance-get", + "#private-instance-get", "public-abstract-get", "protected-abstract-get", - "private-abstract-get", "public-get", "protected-get", "private-get", + "#private-get", "static-get", "instance-get", @@ -969,6 +997,7 @@ The most explicit and granular form is the following: "public-static-set", "protected-static-set", "private-static-set", + "#private-static-set", "public-decorated-set", "protected-decorated-set", @@ -977,10 +1006,10 @@ The most explicit and granular form is the following: "public-instance-set", "protected-instance-set", "private-instance-set", + "#private-instance-set", "public-abstract-set", "protected-abstract-set", - "private-abstract-set", "public-set", "protected-set", @@ -998,15 +1027,16 @@ The most explicit and granular form is the following: "public-static-method", "protected-static-method", "private-static-method", + "#private-static-method", "public-decorated-method", "protected-decorated-method", "private-decorated-method", "public-instance-method", "protected-instance-method", "private-instance-method", + "#private-instance-method", "public-abstract-method", - "protected-abstract-method", - "private-abstract-method" + "protected-abstract-method" ] ``` diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 3892c989bc9..c92af2650ce 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -22,15 +22,17 @@ type NonCallableMemberKind = Exclude; type MemberScope = 'static' | 'instance' | 'abstract'; +type Accessibility = TSESTree.Accessibility | '#private'; + type BaseMemberType = | MemberKind - | `${TSESTree.Accessibility}-${Exclude< + | `${Accessibility}-${Exclude< MemberKind, 'signature' | 'static-initialization' >}` - | `${TSESTree.Accessibility}-decorated-${DecoratedMemberKind}` + | `${Accessibility}-decorated-${DecoratedMemberKind}` | `decorated-${DecoratedMemberKind}` - | `${TSESTree.Accessibility}-${MemberScope}-${NonCallableMemberKind}` + | `${Accessibility}-${MemberScope}-${NonCallableMemberKind}` | `${MemberScope}-${NonCallableMemberKind}`; type MemberType = BaseMemberType | BaseMemberType[]; @@ -112,6 +114,7 @@ export const defaultOrder: MemberType[] = [ 'public-static-field', 'protected-static-field', 'private-static-field', + '#private-static-field', 'public-decorated-field', 'protected-decorated-field', @@ -120,14 +123,15 @@ export const defaultOrder: MemberType[] = [ 'public-instance-field', 'protected-instance-field', 'private-instance-field', + '#private-instance-field', 'public-abstract-field', 'protected-abstract-field', - 'private-abstract-field', 'public-field', 'protected-field', 'private-field', + '#private-field', 'static-field', 'instance-field', @@ -151,6 +155,7 @@ export const defaultOrder: MemberType[] = [ 'public-static-get', 'protected-static-get', 'private-static-get', + '#private-static-get', 'public-decorated-get', 'protected-decorated-get', @@ -159,14 +164,15 @@ export const defaultOrder: MemberType[] = [ 'public-instance-get', 'protected-instance-get', 'private-instance-get', + '#private-instance-get', 'public-abstract-get', 'protected-abstract-get', - 'private-abstract-get', 'public-get', 'protected-get', 'private-get', + '#private-get', 'static-get', 'instance-get', @@ -180,6 +186,7 @@ export const defaultOrder: MemberType[] = [ 'public-static-set', 'protected-static-set', 'private-static-set', + '#private-static-set', 'public-decorated-set', 'protected-decorated-set', @@ -188,14 +195,15 @@ export const defaultOrder: MemberType[] = [ 'public-instance-set', 'protected-instance-set', 'private-instance-set', + '#private-instance-set', 'public-abstract-set', 'protected-abstract-set', - 'private-abstract-set', 'public-set', 'protected-set', 'private-set', + '#private-set', 'static-set', 'instance-set', @@ -209,6 +217,7 @@ export const defaultOrder: MemberType[] = [ 'public-static-method', 'protected-static-method', 'private-static-method', + '#private-static-method', 'public-decorated-method', 'protected-decorated-method', @@ -217,14 +226,15 @@ export const defaultOrder: MemberType[] = [ 'public-instance-method', 'protected-instance-method', 'private-instance-method', + '#private-instance-method', 'public-abstract-method', 'protected-abstract-method', - 'private-abstract-method', 'public-method', 'protected-method', 'private-method', + '#private-method', 'static-method', 'instance-method', @@ -250,30 +260,49 @@ const allMemberTypes = Array.from( ).reduce>((all, type) => { all.add(type); - (['public', 'protected', 'private'] as const).forEach(accessibility => { - if (type !== 'signature' && type !== 'static-initialization') { - all.add(`${accessibility}-${type}`); // e.g. `public-field` - } + (['public', 'protected', 'private', '#private'] as const).forEach( + accessibility => { + if ( + type !== 'signature' && + type !== 'static-initialization' && + type !== 'call-signature' && + !(type === 'constructor' && accessibility === '#private') + ) { + all.add(`${accessibility}-${type}`); // e.g. `public-field` + } - // Only class instance fields, methods, get and set can have decorators attached to them - if ( - type === 'field' || - type === 'method' || - type === 'get' || - type === 'set' - ) { - all.add(`${accessibility}-decorated-${type}`); - all.add(`decorated-${type}`); - } + // Only class instance fields, methods, get and set can have decorators attached to them + if ( + accessibility !== '#private' && + (type === 'field' || + type === 'method' || + type === 'get' || + type === 'set') + ) { + all.add(`${accessibility}-decorated-${type}`); + all.add(`decorated-${type}`); + } - if (type !== 'constructor' && type !== 'signature') { - // There is no `static-constructor` or `instance-constructor` or `abstract-constructor` - (['static', 'instance', 'abstract'] as const).forEach(scope => { - all.add(`${scope}-${type}`); - all.add(`${accessibility}-${scope}-${type}`); - }); - } - }); + if ( + type !== 'constructor' && + type !== 'signature' && + type !== 'call-signature' + ) { + // There is no `static-constructor` or `instance-constructor` or `abstract-constructor` + if (accessibility === '#private' || accessibility === 'private') { + (['static', 'instance'] as const).forEach(scope => { + all.add(`${scope}-${type}`); + all.add(`${accessibility}-${scope}-${type}`); + }); + } else { + (['static', 'instance', 'abstract'] as const).forEach(scope => { + all.add(`${scope}-${type}`); + all.add(`${accessibility}-${scope}-${type}`); + }); + } + } + }, + ); return all; }, new Set()), @@ -407,6 +436,16 @@ function getRankOrder( return rank; } +function getAccessibility(node: Member): Accessibility { + if ('accessibility' in node && node.accessibility) { + return node.accessibility; + } + if ('key' in node && node.key?.type === AST_NODE_TYPES.PrivateIdentifier) { + return '#private'; + } + return 'public'; +} + /** * Gets the rank of the node given the order. * @param node the node to be evaluated. @@ -435,10 +474,7 @@ function getRank( : abstract ? 'abstract' : 'instance'; - const accessibility = - 'accessibility' in node && node.accessibility - ? node.accessibility - : 'public'; + const accessibility = getAccessibility(node); // Collect all existing member groups that apply to this node... // (e.g. 'public-instance-field', 'instance-field', 'public-field', 'constructor' etc.) diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index cfff3691476..3305bebfa8c 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -425,16 +425,20 @@ class Foo { public static A: string; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; constructor() {} public static G() {} protected static H() {} private static I() {} + static #I() {} public J() {} protected K() {} private L() {} + #L() {} } `, { @@ -444,16 +448,20 @@ class Foo { public static A: string; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; constructor() {} public static G() {} protected static H() {} private static I() {} + static #I() {} public J() {} protected K() {} private L() {} + #L() {} } `, options: [{ default: 'never' }], @@ -465,16 +473,20 @@ class Foo { public static A: string; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; constructor() {} public static G() {} protected static H() {} private static I() {} + static #I() {} public J() {} protected K() {} private L() {} + #L() {} } `, options: [{ default: ['signature', 'field', 'constructor', 'method'] }], @@ -487,15 +499,19 @@ class Foo { public static A: string; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; public static G() {} protected static H() {} private static I() {} + static #I() {} public J() {} protected K() {} private L() {} + #L() {} } `, options: [{ default: ['field', 'method'] }], @@ -527,16 +543,20 @@ class Foo { public static G() {} protected static H() {} private static I() {} + static #I() {} public J() {} protected K() {} private L() {} + #L() {} [Z: string]: any; public static A: string; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; constructor() {} } `, @@ -619,6 +639,8 @@ class Foo { public static A: string; private static C: string = ''; private F: string = ''; + static #M: string = ''; + #N: string = ''; protected static B: string = ''; protected E: string = ''; } @@ -631,6 +653,7 @@ class Foo { 'constructor', 'public-field', 'private-field', + '#private-field', 'protected-field', ], }, @@ -653,6 +676,7 @@ class Foo { protected E: string = ''; private static C: string = ''; private F: string = ''; + #M: string = ''; } `, options: [ @@ -667,6 +691,7 @@ class Foo { 'public-field', 'protected-field', 'private-field', + '#private-field', ], }, ], @@ -681,12 +706,16 @@ class Foo { constructor() {} protected K() {} private L() {} + #P() {} protected static H() {} private static I() {} + static #O() {} protected static B: string = ''; private static C: string = ''; + static #N: string = ''; protected E: string = ''; private F: string = ''; + #M: string = ''; [Z: string]: any; } `, @@ -710,15 +739,19 @@ class Foo { public static G() {} protected static H() {} private static I() {} + static #I() {} protected K() {} private L() {} + #L() {} constructor() {} [Z: string]: any; public static A: string; private F: string = ''; + #F: string = ''; protected static B: string = ''; public D: string = ''; private static C: string = ''; + static #C: string = ''; protected E: string = ''; } `, @@ -727,9 +760,10 @@ class Foo { classes: [ 'public-method', 'protected-static-method', - 'private-static-method', + '#private-static-method', 'protected-instance-method', 'private-instance-method', + '#private-instance-method', 'constructor', 'signature', 'field', @@ -764,6 +798,31 @@ class Foo { }, { code: ` +class Foo { + private L() {} + private static I() {} + static #H() {} + static #B: string = ''; + public static G() {} + public J() {} + #K() {} + private static C: string = ''; + private F: string = ''; + #E: string = ''; + public static A: string; + public D: string = ''; + constructor() {} + [Z: string]: any; +} + `, + options: [ + { + classes: ['private-instance-method', 'protected-static-field'], + }, + ], + }, + { + code: ` class Foo { private L() {} private static I() {} @@ -1535,6 +1594,60 @@ class Foo { }, ], }, + { + name: 'with private identifier', + code: ` +// no accessibility === public +class Foo { + imPublic() {} + #imPrivate() {} +} + `, + options: [ + { + default: { + memberTypes: ['public-method', '#private-method'], + order: 'alphabetically-case-insensitive', + }, + }, + ], + }, + { + name: 'private and #private member order', + code: ` +// no accessibility === public +class Foo { + private imPrivate() {} + #imPrivate() {} +} + `, + options: [ + { + default: { + memberTypes: ['private-method', '#private-method'], + order: 'alphabetically-case-insensitive', + }, + }, + ], + }, + { + name: '#private and private member order', + code: ` +// no accessibility === public +class Foo { + #imPrivate() {} + private imPrivate() {} +} + `, + options: [ + { + default: { + memberTypes: ['#private-method', 'private-method'], + order: 'alphabetically-case-insensitive', + }, + }, + ], + }, ], invalid: [ { @@ -2356,16 +2469,20 @@ class Foo { public static A: string = ''; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; constructor() {} public J() {} protected K() {} private L() {} + #L() {} public static G() {} protected static H() {} private static I() {} + static #I() {} } `, errors: [ @@ -2375,7 +2492,7 @@ class Foo { name: 'G', rank: 'public instance method', }, - line: 14, + line: 17, column: 3, }, { @@ -2384,7 +2501,7 @@ class Foo { name: 'H', rank: 'public instance method', }, - line: 15, + line: 18, column: 3, }, { @@ -2393,7 +2510,16 @@ class Foo { name: 'I', rank: 'public instance method', }, - line: 16, + line: 19, + column: 3, + }, + { + messageId: 'incorrectGroupOrder', + data: { + name: 'I', + rank: 'public instance method', + }, + line: 20, column: 3, }, ], @@ -2405,15 +2531,19 @@ class Foo { public static A: string = ''; protected static B: string = ''; private static C: string = ''; + static #C: string = ''; public D: string = ''; protected E: string = ''; private F: string = ''; + #F: string = ''; public J() {} protected K() {} private L() {} + #L() {} public static G() {} protected static H() {} private static I() {} + static #I() {} [Z: string]: any; } `, @@ -2449,7 +2579,7 @@ class Foo { { messageId: 'incorrectGroupOrder', data: { - name: 'D', + name: 'C', rank: 'constructor', }, line: 7, @@ -2458,7 +2588,7 @@ class Foo { { messageId: 'incorrectGroupOrder', data: { - name: 'E', + name: 'D', rank: 'constructor', }, line: 8, @@ -2467,12 +2597,30 @@ class Foo { { messageId: 'incorrectGroupOrder', data: { - name: 'F', + name: 'E', rank: 'constructor', }, line: 9, column: 3, }, + { + messageId: 'incorrectGroupOrder', + data: { + name: 'F', + rank: 'constructor', + }, + line: 10, + column: 3, + }, + { + messageId: 'incorrectGroupOrder', + data: { + name: 'F', + rank: 'constructor', + }, + line: 11, + column: 3, + }, ], }, { @@ -4175,6 +4323,93 @@ class Foo { }, ], }, + { + name: 'with private identifier', + code: ` +// no accessibility === public +class Foo { + #imPrivate() {} + imPublic() {} +} + `, + options: [ + { + default: { + memberTypes: ['public-method', '#private-method'], + order: 'alphabetically-case-insensitive', + }, + }, + ], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'imPublic', + rank: '#private method', + }, + line: 5, + column: 3, + }, + ], + }, + { + name: 'private and #private member order', + code: ` +// no accessibility === public +class Foo { + #imPrivate() {} + private imPrivate() {} +} + `, + options: [ + { + default: { + memberTypes: ['private-method', '#private-method'], + order: 'alphabetically-case-insensitive', + }, + }, + ], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'imPrivate', + rank: '#private method', + }, + line: 5, + column: 3, + }, + ], + }, + { + name: '#private and private member order', + code: ` +// no accessibility === public +class Foo { + private imPrivate() {} + #imPrivate() {} +} + `, + options: [ + { + default: { + memberTypes: ['#private-method', 'private-method'], + order: 'alphabetically-case-insensitive', + }, + }, + ], + errors: [ + { + messageId: 'incorrectGroupOrder', + data: { + name: 'imPrivate', + rank: 'private method', + }, + line: 5, + column: 3, + }, + ], + }, ], };