Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
40 changes: 38 additions & 2 deletions packages/eslint-plugin/docs/rules/class-methods-use-this.md
Original file line number Diff line number Diff line change
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.
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
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