Skip to content

Commit

Permalink
fix(eslint-plugin): [class-methods-use-this] detect a problematic cas…
Browse files Browse the repository at this point in the history
…e for private/protected members if `ignoreClassesThatImplementAnInterface` is set (#7705)

* fix(eslint-plugin): [no-shadow] fix lint error in tests

* fix(eslint-plugin): [class-methods-use-this] Add the test case for getter/setter

* fix(eslint-plugin): [class-methods-use-this] Add the test case for class private field

* fix(eslint-plugin): [class-methods-use-this] Allow to detect a problematic case for private/protected members if `ignoreClassesThatImplementAnInterface` is set

* Revert no-shadow.test.ts changes

---------

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
  • Loading branch information
tetsuharuohzeki and JoshuaKGoldberg committed Nov 17, 2023
1 parent 5ab15ce commit 155aa1f
Show file tree
Hide file tree
Showing 4 changed files with 805 additions and 24 deletions.
40 changes: 38 additions & 2 deletions packages/eslint-plugin/docs/rules/class-methods-use-this.md
Expand Up @@ -16,7 +16,7 @@ This rule adds the following options:
```ts
interface Options extends BaseClassMethodsUseThisOptions {
ignoreOverrideMethods?: boolean;
ignoreClassesThatImplementAnInterface?: boolean;
ignoreClassesThatImplementAnInterface?: boolean | 'public-fields';
}

const defaultOptions: Options = {
Expand All @@ -41,10 +41,16 @@ class X {

### `ignoreClassesThatImplementAnInterface`

Makes the rule ignore all class members that are defined within a class that `implements` a type.
Makes the rule ignore class members that are defined within a class that `implements` a type.
If specified, it can be either:

- `true`: Ignore all classes that implement an interface
- `'public-fields'`: Ignore only the public fields of classes that implement an interface

It's important to note that this option does not only apply to members defined in the interface as that would require type information.

#### `true`

Example of a correct code when `ignoreClassesThatImplementAnInterface` is set to `true`:

```ts option='{ "ignoreClassesThatImplementAnInterface": true }' showPlaygroundButton
Expand All @@ -53,3 +59,33 @@ class X implements Y {
property = () => {};
}
```

#### `'public-fields'`

Example of a incorrect code when `ignoreClassesThatImplementAnInterface` is set to `'public-fields'`:

<!--tabs-->

##### ❌ Incorrect

```ts
class X implements Y {
method() {}
property = () => {};

private privateMethod() {}
private privateProperty = () => {};

protected privateMethod() {}
protected privateProperty = () => {};
}
```

##### ✅ Correct

```ts
class X implements Y {
method() {}
property = () => {};
}
```
32 changes: 28 additions & 4 deletions packages/eslint-plugin/src/rules/class-methods-use-this.ts
Expand Up @@ -14,7 +14,7 @@ type Options = [
exceptMethods?: string[];
enforceForClassFields?: boolean;
ignoreOverrideMethods?: boolean;
ignoreClassesThatImplementAnInterface?: boolean;
ignoreClassesThatImplementAnInterface?: boolean | 'public-fields';
},
];
type MessageIds = 'missingThis';
Expand Down Expand Up @@ -53,7 +53,18 @@ export default createRule<Options, MessageIds>({
description: 'Ingore members marked with the `override` modifier',
},
ignoreClassesThatImplementAnInterface: {
type: 'boolean',
oneOf: [
{
type: 'boolean',
description: 'Ignore all classes that implement an interface',
},
{
type: 'string',
enum: ['public-fields'],
description:
'Ignore only the public fields of classes that implement an interface',
},
],
description:
'Ignore classes that specifically implement some interface',
},
Expand Down Expand Up @@ -146,6 +157,16 @@ export default createRule<Options, MessageIds>({
return oldStack;
}

function isPublicField(
accessibility: TSESTree.Accessibility | undefined,
): boolean {
if (!accessibility || accessibility === 'public') {
return true;
}

return false;
}

/**
* Check if the node is an instance method not excluded by config
*/
Expand Down Expand Up @@ -189,8 +210,11 @@ export default createRule<Options, MessageIds>({
stackContext?.member == null ||
stackContext.usesThis ||
(ignoreOverrideMethods && stackContext.member.override) ||
(ignoreClassesThatImplementAnInterface &&
stackContext.class.implements.length)
(ignoreClassesThatImplementAnInterface === true &&
stackContext.class.implements.length > 0) ||
(ignoreClassesThatImplementAnInterface === 'public-fields' &&
stackContext.class.implements.length > 0 &&
isPublicField(stackContext.member.accessibility))
) {
return;
}
Expand Down

0 comments on commit 155aa1f

Please sign in to comment.