From 6a57933592a64528d694f8197b74999d8f0f3cd7 Mon Sep 17 00:00:00 2001 From: Hailey Smith Date: Mon, 23 Sep 2019 14:43:52 -0700 Subject: [PATCH 1/6] feat(eslint-plugin): support abstract members (#395) --- .../src/rules/member-ordering.ts | 31 ++++++++++++++++--- packages/eslint-plugin/src/util/misc.ts | 2 +- .../tests/rules/member-ordering.test.ts | 15 +++++++-- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 72020afc600..7231b1b7dd2 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -24,8 +24,8 @@ const allMemberTypes = ['field', 'method', 'constructor'].reduce( all.push(`${accessibility}-${type}`); // e.g. `public-field` if (type !== 'constructor') { - // There is no `static-constructor` or `instance-constructor - ['static', 'instance'].forEach(scope => { + // There is no `static-constructor` or `instance-constructor or `abstract-constructor` + ['static', 'instance', 'abstract'].forEach(scope => { if (!all.includes(`${scope}-${type}`)) { all.push(`${scope}-${type}`); } @@ -138,12 +138,17 @@ export default util.createRule({ 'protected-instance-field', 'private-instance-field', + 'public-abstract-field', + 'protected-abstract-field', + 'private-abstract-field', + 'public-field', 'protected-field', 'private-field', 'static-field', 'instance-field', + 'abstract-field', 'field', @@ -157,12 +162,17 @@ export default util.createRule({ 'protected-instance-method', 'private-instance-method', + 'public-abstract-method', + 'protected-abstract-method', + 'private-abstract-method', + 'public-method', 'protected-method', 'private-method', 'static-method', 'instance-method', + 'abstract-method', 'method', ], @@ -185,15 +195,15 @@ export default util.createRule({ ): string | null { // TODO: add missing TSCallSignatureDeclaration // TODO: add missing TSIndexSignature - // TODO: add missing TSAbstractClassProperty - // TODO: add missing TSAbstractMethodDefinition switch (node.type) { + case AST_NODE_TYPES.TSAbstractMethodDefinition: case AST_NODE_TYPES.MethodDefinition: return node.kind; case AST_NODE_TYPES.TSMethodSignature: return 'method'; case AST_NODE_TYPES.TSConstructSignatureDeclaration: return 'constructor'; + case AST_NODE_TYPES.TSAbstractClassProperty: case AST_NODE_TYPES.ClassProperty: return node.value && functionExpressions.includes(node.value.type) ? 'method' @@ -215,8 +225,10 @@ export default util.createRule({ switch (node.type) { case AST_NODE_TYPES.TSPropertySignature: case AST_NODE_TYPES.TSMethodSignature: + case AST_NODE_TYPES.TSAbstractClassProperty: case AST_NODE_TYPES.ClassProperty: return util.getNameFromPropertyName(node.key); + case AST_NODE_TYPES.TSAbstractMethodDefinition: case AST_NODE_TYPES.MethodDefinition: return node.kind === 'constructor' ? 'constructor' @@ -268,7 +280,16 @@ export default util.createRule({ return order.length - 1; } - const scope = 'static' in node && node.static ? 'static' : 'instance'; + const abstract = + node.type === 'TSAbstractClassProperty' || + node.type === 'TSAbstractMethodDefinition'; + + const scope = + 'static' in node && node.static + ? 'static' + : abstract + ? 'abstract' + : 'instance'; const accessibility = 'accessibility' in node && node.accessibility ? node.accessibility diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index cae887c229f..934d2c6425f 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -96,7 +96,7 @@ export function findFirstResult( * or ClassProperty node, with handling for computed property names. */ export function getNameFromClassMember( - methodDefinition: TSESTree.MethodDefinition | TSESTree.ClassProperty, + methodDefinition: TSESTree.MethodDefinition | TSESTree.ClassProperty| TSESTree.TSAbstractMethodDefinition, sourceCode: TSESLint.SourceCode, ): string { if (keyCanBeReadAsPropertyName(methodDefinition.key)) { diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index 81e67999b8d..7dcba378343 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -1211,12 +1211,23 @@ type Foo = { `, options: [{ default: ['method', 'constructor', 'field'] }], }, - ` + { + code: ` abstract class Foo { B: string; abstract A: () => {} } - `, + `}, + { + code: ` +abstract class Foo { + protected typeChecker: (data: any) => boolean; + public abstract required: boolean; + abstract verify(): void; +} + `, + options: [{ classes: ['field', 'constructor', 'method'] }], + }, ], invalid: [ { From 043887279ee1e06a0c14d65a17bddf4c79fcfe25 Mon Sep 17 00:00:00 2001 From: Hailey Smith Date: Mon, 23 Sep 2019 16:24:09 -0700 Subject: [PATCH 2/6] feat(eslint-plugin): add more Unit tests (#395) --- .../tests/rules/member-ordering.test.ts | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index 7dcba378343..dbb0fc36e6e 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -1217,7 +1217,8 @@ abstract class Foo { B: string; abstract A: () => {} } - `}, + `, + }, { code: ` abstract class Foo { @@ -3330,7 +3331,7 @@ type Foo = { { code: ` abstract class Foo { - abstract A: () => {} + abstract A = () => {}; B: string; } `, @@ -3339,7 +3340,50 @@ abstract class Foo { messageId: 'incorrectOrder', data: { name: 'B', - rank: 'method', + rank: 'public abstract method', + }, + line: 4, + column: 5, + }, + ], + }, + { + code: ` +abstract class Foo { + abstract A: () => {}; + B: string; + public C() {}; + private D() {}; + abstract E() {}; +} + `, + errors: [ + { + messageId: 'incorrectOrder', + data: { + name: 'B', + rank: 'public abstract field', + }, + line: 4, + column: 5, + }, + ], + }, + { + code: ` +abstract class Foo { + B: string; + abstract C = () => {}; + abstract A: () => {}; +} + `, + options: [{ default: ['method', 'constructor', 'field'] }], + errors: [ + { + messageId: 'incorrectOrder', + data: { + name: 'C', + rank: 'field', }, line: 4, column: 5, From d8dfd6fdb0971cbb40479470430a05f6f5aa90da Mon Sep 17 00:00:00 2001 From: Hailey Smith Date: Mon, 23 Sep 2019 17:05:43 -0700 Subject: [PATCH 3/6] feat(eslint-plugin): update member-ordering doc --- .../docs/rules/member-ordering.md | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/member-ordering.md b/packages/eslint-plugin/docs/rules/member-ordering.md index 50c4a7eacd3..ed8bc228456 100644 --- a/packages/eslint-plugin/docs/rules/member-ordering.md +++ b/packages/eslint-plugin/docs/rules/member-ordering.md @@ -37,6 +37,9 @@ There are multiple ways to specify the member types. The most explicit and granu 'public-instance-field', 'protected-instance-field', 'private-instance-field', + 'public-abstract-field', + 'protected-abstract-field', + 'private-abstract-field', // Constructors 'public-constructor', @@ -50,6 +53,9 @@ There are multiple ways to specify the member types. The most explicit and granu 'public-instance-method', 'protected-instance-method', 'private-instance-method', + 'public-abstract-method', + 'protected-abstract-method', + 'private-abstract-method', ] ``` @@ -57,7 +63,7 @@ Note: If you only specify some of the possible types, the non-specified ones can ### Member group types (with accessibility, ignoring scope) -It is also possible to group member types by their accessibility (`static`, `instance`), ignoring their scope. +It is also possible to group member types by their accessibility (`static`, `instance`, `abstract`), ignoring their scope. ```json5 [ @@ -85,13 +91,15 @@ Another option is to group the member types by their scope (`public`, `protected // 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']) // Constructors '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'] + 'instance-method', // = ['public-instance-method', 'protected-instance-method', 'private-instance-method']) + 'abstract-method', // = ['public-abstract-method', 'protected-abstract-method', 'private-abstract-method'] ] ``` @@ -102,13 +110,15 @@ The third grouping option is to ignore both scope and accessibility. ```json5 [ // Fields - 'field', // = ['public-static-field', 'protected-static-field', 'private-static-field', 'public-instance-field', 'protected-instance-field', 'private-instance-field']) + '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']) // 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']) + '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']) ] ``` @@ -127,12 +137,17 @@ The default configuration looks as follows: "protected-instance-field", "private-instance-field", + "public-abstract-field", + "protected-abstract-field", + "private-abstract-field", + "public-field", "protected-field", "private-field", "static-field", "instance-field", + "abstract-field", "field", @@ -146,12 +161,17 @@ The default configuration looks as follows: "protected-instance-method", "private-instance-method", + "public-abstract-method", + "protected-abstract-method", + "private-abstract-method", + "public-method", "protected-method", "private-method", "static-method", "instance-method", + "abstract-method", "method" ] From 8aec50c86c03389815e30c1b913a31b2991dc2af Mon Sep 17 00:00:00 2001 From: Hailey Smith Date: Mon, 23 Sep 2019 17:29:48 -0700 Subject: [PATCH 4/6] feat(eslint-plugin): fix format --- .../docs/rules/member-ordering.md | 2 +- .../src/rules/member-ordering.ts | 20 +++++++++---------- packages/eslint-plugin/src/util/misc.ts | 5 ++++- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/member-ordering.md b/packages/eslint-plugin/docs/rules/member-ordering.md index ed8bc228456..517673f09d6 100644 --- a/packages/eslint-plugin/docs/rules/member-ordering.md +++ b/packages/eslint-plugin/docs/rules/member-ordering.md @@ -99,7 +99,7 @@ Another option is to group the member types by their scope (`public`, `protected // 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'] + 'abstract-method', // = ['public-abstract-method', 'protected-abstract-method', 'private-abstract-method']) ] ``` diff --git a/packages/eslint-plugin/src/rules/member-ordering.ts b/packages/eslint-plugin/src/rules/member-ordering.ts index 7231b1b7dd2..b1707685694 100644 --- a/packages/eslint-plugin/src/rules/member-ordering.ts +++ b/packages/eslint-plugin/src/rules/member-ordering.ts @@ -280,16 +280,16 @@ export default util.createRule({ return order.length - 1; } - const abstract = - node.type === 'TSAbstractClassProperty' || - node.type === 'TSAbstractMethodDefinition'; - - const scope = - 'static' in node && node.static - ? 'static' - : abstract - ? 'abstract' - : 'instance'; + const abstract = + node.type === 'TSAbstractClassProperty' || + node.type === 'TSAbstractMethodDefinition'; + + const scope = + 'static' in node && node.static + ? 'static' + : abstract + ? 'abstract' + : 'instance'; const accessibility = 'accessibility' in node && node.accessibility ? node.accessibility diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index 934d2c6425f..58c41249d47 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -96,7 +96,10 @@ export function findFirstResult( * or ClassProperty node, with handling for computed property names. */ export function getNameFromClassMember( - methodDefinition: TSESTree.MethodDefinition | TSESTree.ClassProperty| TSESTree.TSAbstractMethodDefinition, + methodDefinition: + | TSESTree.MethodDefinition + | TSESTree.ClassProperty + | TSESTree.TSAbstractMethodDefinition, sourceCode: TSESLint.SourceCode, ): string { if (keyCanBeReadAsPropertyName(methodDefinition.key)) { From 8ab095406ef0ef83127499215b13f27c5641c0a8 Mon Sep 17 00:00:00 2001 From: Hailey Smith Date: Tue, 24 Sep 2019 10:18:41 -0700 Subject: [PATCH 5/6] feat(eslint-plugin): add more unit tests for increased coverage --- .../tests/rules/member-ordering.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index dbb0fc36e6e..346e30d0b03 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -1221,6 +1221,19 @@ abstract class Foo { }, { code: ` +abstract class Foo { + private static C: string; + B: string; + private D: string; + protected static F(): {}; + public E(): {}; + public abstract A = () => {}; + protected abstract G(): void; +} + `, + }, + { + code: ` abstract class Foo { protected typeChecker: (data: any) => boolean; public abstract required: boolean; @@ -3390,5 +3403,36 @@ abstract class Foo { }, ], }, + { + code: ` +abstract class Foo { + abstract B: string; + abstract A(): void; + public C(): {}; + +} + `, + options: [{ default: ['method', 'constructor', 'field'] }], + errors: [ + { + messageId: 'incorrectOrder', + data: { + name: 'A', + rank: 'field', + }, + line: 4, + column: 5, + }, + { + messageId: 'incorrectOrder', + data: { + name: 'C', + rank: 'field', + }, + line: 5, + column: 5, + }, + ], + }, ], }); From eb8853f02bc3729319febd897c8ff83dffd850a3 Mon Sep 17 00:00:00 2001 From: Hailey Smith Date: Tue, 24 Sep 2019 14:56:15 -0700 Subject: [PATCH 6/6] feat(eslint-plugin): update unit tests --- .../tests/rules/member-ordering.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/member-ordering.test.ts b/packages/eslint-plugin/tests/rules/member-ordering.test.ts index 346e30d0b03..936fe3a8900 100644 --- a/packages/eslint-plugin/tests/rules/member-ordering.test.ts +++ b/packages/eslint-plugin/tests/rules/member-ordering.test.ts @@ -1221,6 +1221,14 @@ abstract class Foo { }, { code: ` +interface Foo { + public B: string; + [A:string]: number; +} + `, + }, + { + code: ` abstract class Foo { private static C: string; B: string; @@ -3405,6 +3413,37 @@ abstract class Foo { }, { code: ` +class Foo { + C: number; + [A:string]: number; + public static D(): {}; + private static [B:string]: number; +} + `, + options: [ + { + default: [ + 'field', + 'method', + 'public-static-method', + 'private-static-method', + ], + }, + ], + errors: [ + { + messageId: 'incorrectOrder', + data: { + name: 'D', + rank: 'private static method', + }, + line: 5, + column: 5, + }, + ], + }, + { + code: ` abstract class Foo { abstract B: string; abstract A(): void;