Skip to content

Commit

Permalink
feat(eslint-plugin): Support abstract members in member-ordering rule (…
Browse files Browse the repository at this point in the history 
…#395) (#1004)
  • Loading branch information
@hsmitty93 @JamesHenry
hsmitty93 authored and JamesHenry committed Oct 16, 2019
1 parent 1918024 commit 5f093ac
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 13 deletions.
28 changes: 24 additions & 4 deletions packages/eslint-plugin/docs/rules/member-ordering.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -50,14 +53,17 @@ 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',
]
```

Note: If you only specify some of the possible types, the non-specified ones can have any particular order. This means that they can be placed before, within or after the specified types and the linter won't complain about it.

### 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
[
Expand Down Expand Up @@ -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'])
]
```

Expand All @@ -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'])
]
```

Expand All @@ -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",

Expand All @@ -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"
]
Expand Down
31 changes: 26 additions & 5 deletions packages/eslint-plugin/src/rules/member-ordering.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ const allMemberTypes = ['field', 'method', 'constructor'].reduce<string[]>(
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}`);
}
Expand Down Expand Up @@ -138,12 +138,17 @@ export default util.createRule<Options, MessageIds>({
'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',

Expand All @@ -157,12 +162,17 @@ export default util.createRule<Options, MessageIds>({
'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',
],
Expand All @@ -185,15 +195,15 @@ export default util.createRule<Options, MessageIds>({
): 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'
Expand All @@ -215,8 +225,10 @@ export default util.createRule<Options, MessageIds>({
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'
Expand Down Expand Up @@ -268,7 +280,16 @@ export default util.createRule<Options, MessageIds>({
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
Expand Down
5 changes: 4 additions & 1 deletion packages/eslint-plugin/src/util/misc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ export function findFirstResult<T, U>(
* 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)) {
Expand Down
144 changes: 141 additions & 3 deletions packages/eslint-plugin/tests/rules/member-ordering.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1211,12 +1211,45 @@ type Foo = {
`,
options: [{ default: ['method', 'constructor', 'field'] }],
},
`
{
code: `
abstract class Foo {
B: string;
abstract A: () => {}
}
`,
},
{
code: `
interface Foo {
public B: string;
[A:string]: number;
}
`,
},
{
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;
abstract verify(): void;
}
`,
options: [{ classes: ['field', 'constructor', 'method'] }],
},
],
invalid: [
{
Expand Down Expand Up @@ -3319,7 +3352,7 @@ type Foo = {
{
code: `
abstract class Foo {
abstract A: () => {}
abstract A = () => {};
B: string;
}
`,
Expand All @@ -3328,12 +3361,117 @@ 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,
},
],
},
{
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;
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,
},
],
},
],
});

0 comments on commit 5f093ac

Please sign in to comment.