Skip to content

Commit

Permalink
fixup! fix(eslint-plugin): [class-methods-use-this] detect a problema…
Browse files Browse the repository at this point in the history 
…tic case for private/protected members if `ignoreClassesThatImplementAnInterface` is `true`
  • Loading branch information
@tetsuharuohzeki
tetsuharuohzeki committed Oct 23, 2023
1 parent f00b6ec commit 80b3efe
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 25 deletions.
36 changes: 34 additions & 2 deletions packages/eslint-plugin/docs/rules/class-methods-use-this.md
View file
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,12 @@ 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.

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
Expand All @@ -53,3 +55,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 = () => {};
}
```
20 changes: 15 additions & 5 deletions packages/eslint-plugin/src/rules/class-methods-use-this.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type Options = [
exceptMethods?: string[];
enforceForClassFields?: boolean;
ignoreOverrideMethods?: boolean;
ignoreClassesThatImplementAnInterface?: boolean;
ignoreClassesThatImplementAnInterface?: boolean | 'public-fields';
},
];
type MessageIds = 'missingThis';
Expand Down Expand Up @@ -52,7 +52,15 @@ export default createRule<Options, MessageIds>({
description: 'Ingore members marked with the `override` modifier',
},
ignoreClassesThatImplementAnInterface: {
type: 'boolean',
oneOf: [
{
type: 'boolean',
},
{
type: 'string',
enum: ['public-fields'],
},
],
description:
'Ignore classes that specifically implement some interface',
},
Expand Down Expand Up @@ -145,7 +153,7 @@ export default createRule<Options, MessageIds>({
return oldStack;
}

function isPublicMethod(
function isPublicField(
accessibility: TSESTree.Accessibility | undefined,
): boolean {
if (!accessibility || accessibility === 'public') {
Expand Down Expand Up @@ -199,9 +207,11 @@ export default createRule<Options, MessageIds>({
stackContext.class == null ||
stackContext.usesThis ||
(ignoreOverrideMethods && stackContext.member.override) ||
(ignoreClassesThatImplementAnInterface &&
(ignoreClassesThatImplementAnInterface === true &&
stackContext.class.implements != null) ||
(ignoreClassesThatImplementAnInterface === 'public-fields' &&
stackContext.class.implements != null &&
isPublicMethod(stackContext.member.accessibility))
isPublicField(stackContext.member.accessibility))
) {
return;
}
Expand Down
32 changes: 16 additions & 16 deletions packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand All @@ -143,7 +143,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand Down Expand Up @@ -172,7 +172,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand All @@ -188,7 +188,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand Down Expand Up @@ -217,7 +217,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand All @@ -233,7 +233,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand Down Expand Up @@ -320,7 +320,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should check only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand All @@ -336,7 +336,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should check only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
// But overridden properties should be ignored.
ignoreOverrideMethods: true,
},
Expand Down Expand Up @@ -541,7 +541,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
},
],
errors: [
Expand All @@ -560,7 +560,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
},
],
errors: [
Expand Down Expand Up @@ -605,7 +605,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
},
],
errors: [
Expand All @@ -624,7 +624,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
},
],
errors: [
Expand Down Expand Up @@ -669,7 +669,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
ignoreOverrideMethods: false,
},
],
Expand All @@ -689,7 +689,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
ignoreOverrideMethods: false,
},
],
Expand Down Expand Up @@ -859,7 +859,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
},
],
errors: [
Expand All @@ -878,7 +878,7 @@ class Foo implements Bar {
{
// _interface_ does not `private`/`protected` modifier on members.
// We should ignore only public members.
ignoreClassesThatImplementAnInterface: true,
ignoreClassesThatImplementAnInterface: 'public-fields',
},
],
errors: [
Expand Down
15 changes: 13 additions & 2 deletions packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 80b3efe

Please sign in to comment.