diff --git a/packages/eslint-plugin/docs/rules/class-methods-use-this.md b/packages/eslint-plugin/docs/rules/class-methods-use-this.md index d694edd254d..098d4c692b1 100644 --- a/packages/eslint-plugin/docs/rules/class-methods-use-this.md +++ b/packages/eslint-plugin/docs/rules/class-methods-use-this.md @@ -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 = { @@ -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 @@ -53,3 +59,33 @@ class X implements Y { property = () => {}; } ``` + +#### `'public-fields'` + +Example of a incorrect code when `ignoreClassesThatImplementAnInterface` is set to `'public-fields'`: + + + +##### ❌ Incorrect + +```ts +class X implements Y { + method() {} + property = () => {}; + + private privateMethod() {} + private privateProperty = () => {}; + + protected privateMethod() {} + protected privateProperty = () => {}; +} +``` + +##### ✅ Correct + +```ts +class X implements Y { + method() {} + property = () => {}; +} +``` diff --git a/packages/eslint-plugin/src/rules/class-methods-use-this.ts b/packages/eslint-plugin/src/rules/class-methods-use-this.ts index d67f054e918..6940f11a7ae 100644 --- a/packages/eslint-plugin/src/rules/class-methods-use-this.ts +++ b/packages/eslint-plugin/src/rules/class-methods-use-this.ts @@ -14,7 +14,7 @@ type Options = [ exceptMethods?: string[]; enforceForClassFields?: boolean; ignoreOverrideMethods?: boolean; - ignoreClassesThatImplementAnInterface?: boolean; + ignoreClassesThatImplementAnInterface?: boolean | 'public-fields'; }, ]; type MessageIds = 'missingThis'; @@ -53,7 +53,18 @@ export default createRule({ 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', }, @@ -146,6 +157,16 @@ export default createRule({ 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 */ @@ -189,8 +210,11 @@ export default createRule({ 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; } diff --git a/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts b/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts index 5f2532b7b3d..31303569bea 100644 --- a/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts +++ b/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts @@ -18,6 +18,22 @@ class Foo implements Bar { }, { code: ` +class Foo implements Bar { + get getter() {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: true }], + }, + { + code: ` +class Foo implements Bar { + set setter() {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: true }], + }, + { + code: ` class Foo { override method() {} } @@ -26,6 +42,70 @@ class Foo { }, { code: ` +class Foo { + private override method() {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + protected override method() {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + override get getter(): number {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + private override get getter(): number {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + protected override get getter(): number {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + override set setter(v: number) {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + private override set setter(v: number) {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + protected override set setter(v: number) {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` class Foo implements Bar { override method() {} } @@ -39,6 +119,128 @@ class Foo implements Bar { }, { code: ` +class Foo implements Bar { + private override method() {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected override method() {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + override get getter(): number {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: true, + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + private override get getter(): number {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected override get getter(): number {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + override set setter(v: number) {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: true, + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + private override set setter(v: number) {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected override set setter(v: number) {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` class Foo implements Bar { property = () => {}; } @@ -55,6 +257,22 @@ class Foo { }, { code: ` +class Foo { + private override property = () => {}; +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo { + protected override property = () => {}; +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` class Foo implements Bar { override property = () => {}; } @@ -92,15 +310,47 @@ class Foo { }, ], }, + { + code: ` +class Foo implements Bar { + private override property = () => {}; +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should check only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected override property = () => {}; +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should check only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + // But overridden properties should be ignored. + ignoreOverrideMethods: true, + }, + ], + }, ], invalid: [ { code: ` -class Foo implements Bar { +class Foo { method() {} } `, - options: [{ ignoreClassesThatImplementAnInterface: false }], + options: [{}], errors: [ { messageId: 'missingThis', @@ -110,10 +360,10 @@ class Foo implements Bar { { code: ` class Foo { - override method() {} + private method() {} } `, - options: [{ ignoreOverrideMethods: false }], + options: [{}], errors: [ { messageId: 'missingThis', @@ -122,16 +372,24 @@ class Foo { }, { code: ` -class Foo implements Bar { - override method() {} +class Foo { + protected method() {} } `, - options: [ + options: [{}], + errors: [ { - ignoreClassesThatImplementAnInterface: false, - ignoreOverrideMethods: false, + messageId: 'missingThis', }, ], + }, + { + code: ` +class Foo { + #method() {} +} + `, + options: [{}], errors: [ { messageId: 'missingThis', @@ -140,11 +398,11 @@ class Foo implements Bar { }, { code: ` -class Foo implements Bar { - property = () => {}; +class Foo { + get getter(): number {} } `, - options: [{ ignoreClassesThatImplementAnInterface: false }], + options: [{}], errors: [ { messageId: 'missingThis', @@ -154,10 +412,10 @@ class Foo implements Bar { { code: ` class Foo { - override property = () => {}; + private get getter(): number {} } `, - options: [{ ignoreOverrideMethods: false }], + options: [{}], errors: [ { messageId: 'missingThis', @@ -166,8 +424,34 @@ class Foo { }, { code: ` -class Foo implements Bar { - override property = () => {}; +class Foo { + protected get getter(): number {} +} + `, + options: [{}], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + get #getter(): number {} +} + `, + options: [{}], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + set setter(b: number) {} } `, options: [ @@ -182,5 +466,426 @@ class Foo implements Bar { }, ], }, + { + code: ` +class Foo { + private set setter(b: number) {} +} + `, + options: [{}], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + protected set setter(b: number) {} +} + `, + options: [{}], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + set #setter(b: number) {} +} + `, + options: [{}], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + method() {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + #method() {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + private method() {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected method() {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + get getter(): number {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + get #getter(): number {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + private get getter(): number {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected get getter(): number {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + set setter(v: number) {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + set #setter(v: number) {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + private set setter(v: number) {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected set setter(v: number) {} +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + override method() {} +} + `, + options: [{ ignoreOverrideMethods: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + override get getter(): number {} +} + `, + options: [{ ignoreOverrideMethods: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + override set setter(v: number) {} +} + `, + options: [{ ignoreOverrideMethods: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + override method() {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + override get getter(): number {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + override set setter(v: number) {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + property = () => {}; +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + #property = () => {}; +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + override property = () => {}; +} + `, + options: [{ ignoreOverrideMethods: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + override property = () => {}; +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + private property = () => {}; +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + protected property = () => {}; +} + `, + options: [ + { + // _interface_ cannot have `private`/`protected` modifier on members. + // We should ignore only public members. + ignoreClassesThatImplementAnInterface: 'public-fields', + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot b/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot index 14858cd2617..97e773bac40 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot @@ -22,7 +22,17 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos }, "ignoreClassesThatImplementAnInterface": { "description": "Ignore classes that specifically implement some interface", - "type": "boolean" + "oneOf": [ + { + "description": "Ignore all classes that implement an interface", + "type": "boolean" + }, + { + "description": "Ignore only the public fields of classes that implement an interface", + "enum": ["public-fields"], + "type": "string" + } + ] }, "ignoreOverrideMethods": { "description": "Ingore members marked with the \`override\` modifier", @@ -43,7 +53,13 @@ type Options = [ /** Allows specified method names to be ignored with this rule */ exceptMethods?: string[]; /** Ignore classes that specifically implement some interface */ - ignoreClassesThatImplementAnInterface?: boolean; + ignoreClassesThatImplementAnInterface?: /** + * Ignore classes that specifically implement some interface + * Ignore all classes that implement an interface + */ + | boolean + /** Ignore only the public fields of classes that implement an interface */ + | 'public-fields'; /** Ingore members marked with the \`override\` modifier */ ignoreOverrideMethods?: boolean; },