diff --git a/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx b/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx new file mode 100644 index 000000000000..15f7c271f3e7 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx @@ -0,0 +1,48 @@ +--- +description: 'Disallow unused private class members.' +--- + +import Tabs from '@theme/Tabs'; +import TabItem from '@theme/TabItem'; + +> πŸ›‘ This file is source code, not the primary documentation location! πŸ›‘ +> +> See **https://typescript-eslint.io/rules/no-unused-private-class-members** for documentation. + +This rule extends the base [`eslint/no-unused-private-class-members`](https://eslint.org/docs/rules/no-unused-private-class-members) rule. +It adds support for members declared with TypeScript's `private` keyword. + +## Options + +This rule has no options. + +## Examples + + + + +```ts +class A { + private foo = 123; +} +``` + + + + +```tsx +class A { + private foo = 123; + + constructor() { + console.log(this.foo); + } +} +``` + + + + +## When Not To Use It + +If you don’t want to be notified about unused private class members, you can safely turn this rule off. diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 72c62f3e0122..a200c8cc7b9f 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -85,6 +85,7 @@ import noUnsafeMemberAccess from './no-unsafe-member-access'; import noUnsafeReturn from './no-unsafe-return'; import noUnsafeUnaryMinus from './no-unsafe-unary-minus'; import noUnusedExpressions from './no-unused-expressions'; +import noUnusedPrivateClassMembers from './no-unused-private-class-members'; import noUnusedVars from './no-unused-vars'; import noUseBeforeDefine from './no-use-before-define'; import noUselessConstructor from './no-useless-constructor'; @@ -215,6 +216,7 @@ const rules = { 'no-unsafe-return': noUnsafeReturn, 'no-unsafe-unary-minus': noUnsafeUnaryMinus, 'no-unused-expressions': noUnusedExpressions, + 'no-unused-private-class-members': noUnusedPrivateClassMembers, 'no-unused-vars': noUnusedVars, 'no-use-before-define': noUseBeforeDefine, 'no-useless-constructor': noUselessConstructor, diff --git a/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts new file mode 100644 index 000000000000..b2fe3bf0b153 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -0,0 +1,270 @@ +// The following code is adapted from the code in eslint/eslint. +// Source: https://github.com/eslint/eslint/blob/522d8a32f326c52886c531f43cf6a1ff15af8286/lib/rules/no-unused-private-class-members.js +// License: https://github.com/eslint/eslint/blob/522d8a32f326c52886c531f43cf6a1ff15af8286/LICENSE + +import type { TSESTree } from '@typescript-eslint/utils'; + +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import { createRule } from '../util'; + +type Options = []; +export type MessageIds = 'unusedPrivateClassMember'; + +interface PrivateMember { + declaredNode: + | TSESTree.MethodDefinitionComputedName + | TSESTree.MethodDefinitionNonComputedName + | TSESTree.PropertyDefinitionComputedName + | TSESTree.PropertyDefinitionNonComputedName; + isAccessor: boolean; +} + +export default createRule({ + name: 'no-unused-private-class-members', + meta: { + type: 'problem', + docs: { + description: 'Disallow unused private class members', + extendsBaseRule: true, + requiresTypeChecking: false, + }, + + messages: { + unusedPrivateClassMember: + "'{{classMemberName}}' is defined but never used.", + }, + + schema: [], + }, + defaultOptions: [], + create(context) { + const trackedClassMembers: Map[] = []; + + /** + * The core ESLint rule tracks methods by adding an extra property of + * "isUsed" to the method, which is a bug according to Joshua Goldberg. + * Instead, in our extended rule, we create a separate data structure to + * track whether a method is being used. + */ + const trackedClassMembersUsed = new Set< + | TSESTree.MethodDefinitionComputedName + | TSESTree.MethodDefinitionNonComputedName + | TSESTree.PropertyDefinitionComputedName + | TSESTree.PropertyDefinitionNonComputedName + >(); + + /** + * Check whether the current node is in a write only assignment. + * @param node Node referring to a private identifier + * @returns Whether the node is in a write only assignment + * @private + */ + function isWriteOnlyAssignment( + node: TSESTree.Identifier | TSESTree.PrivateIdentifier, + ): boolean { + const parentStatement = node.parent.parent; + if (parentStatement === undefined) { + return false; + } + + const isAssignmentExpression = + parentStatement.type === AST_NODE_TYPES.AssignmentExpression; + + if ( + !isAssignmentExpression && + parentStatement.type !== AST_NODE_TYPES.ForInStatement && + parentStatement.type !== AST_NODE_TYPES.ForOfStatement && + parentStatement.type !== AST_NODE_TYPES.AssignmentPattern + ) { + return false; + } + + // It is a write-only usage, since we still allow usages on the right for + // reads. + if (parentStatement.left !== node.parent) { + return false; + } + + // For any other operator (such as '+=') we still consider it a read + // operation. + if (isAssignmentExpression && parentStatement.operator !== '=') { + // However, if the read operation is "discarded" in an empty statement, + // then we consider it write only. + return ( + parentStatement.parent.type === AST_NODE_TYPES.ExpressionStatement + ); + } + + return true; + } + + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + function processPrivateIdentifier( + node: TSESTree.Identifier | TSESTree.PrivateIdentifier, + ): void { + const classBody = trackedClassMembers.find(classProperties => + classProperties.has(node.name), + ); + + // Can't happen, as it is a parser error to have a missing class body, but + // let's code defensively here. + if (classBody === undefined) { + return; + } + + // In case any other usage was already detected, we can short circuit the + // logic here. + const memberDefinition = classBody.get(node.name); + if (memberDefinition === undefined) { + return; + } + if (trackedClassMembersUsed.has(memberDefinition.declaredNode)) { + return; + } + + // The definition of the class member itself + if ( + node.parent.type === AST_NODE_TYPES.PropertyDefinition || + node.parent.type === AST_NODE_TYPES.MethodDefinition + ) { + return; + } + + // Any usage of an accessor is considered a read, as the getter/setter can + // have side-effects in its definition. + if (memberDefinition.isAccessor) { + trackedClassMembersUsed.add(memberDefinition.declaredNode); + return; + } + + // Any assignments to this member, except for assignments that also read + if (isWriteOnlyAssignment(node)) { + return; + } + + const wrappingExpressionType = node.parent.parent?.type; + const parentOfWrappingExpressionType = node.parent.parent?.parent?.type; + + // A statement which only increments (`this.#x++;`) + if ( + wrappingExpressionType === AST_NODE_TYPES.UpdateExpression && + parentOfWrappingExpressionType === AST_NODE_TYPES.ExpressionStatement + ) { + return; + } + + /* + * ({ x: this.#usedInDestructuring } = bar); + * + * But should treat the following as a read: + * ({ [this.#x]: a } = foo); + */ + if ( + wrappingExpressionType === AST_NODE_TYPES.Property && + parentOfWrappingExpressionType === AST_NODE_TYPES.ObjectPattern && + node.parent.parent?.value === node.parent + ) { + return; + } + + // [...this.#unusedInRestPattern] = bar; + if (wrappingExpressionType === AST_NODE_TYPES.RestElement) { + return; + } + + // [this.#unusedInAssignmentPattern] = bar; + if (wrappingExpressionType === AST_NODE_TYPES.ArrayPattern) { + return; + } + + // We can't delete the memberDefinition, as we need to keep track of which + // member we are marking as used. In the case of nested classes, we only + // mark the first member we encounter as used. If you were to delete the + // member, then any subsequent usage could incorrectly mark the member of + // an encapsulating parent class as used, which is incorrect. + trackedClassMembersUsed.add(memberDefinition.declaredNode); + } + + return { + // Collect all declared members/methods up front and assume they are all + // unused. + ClassBody(classBodyNode): void { + const privateMembers = new Map(); + + trackedClassMembers.unshift(privateMembers); + for (const bodyMember of classBodyNode.body) { + if ( + (bodyMember.type === AST_NODE_TYPES.PropertyDefinition || + bodyMember.type === AST_NODE_TYPES.MethodDefinition) && + (bodyMember.key.type === AST_NODE_TYPES.PrivateIdentifier || + (bodyMember.key.type === AST_NODE_TYPES.Identifier && + bodyMember.accessibility === 'private')) + ) { + privateMembers.set(bodyMember.key.name, { + declaredNode: bodyMember, + isAccessor: + bodyMember.type === AST_NODE_TYPES.MethodDefinition && + (bodyMember.kind === 'set' || bodyMember.kind === 'get'), + }); + } + } + }, + + // Process nodes like: + // ```ts + // class A { + // #myPrivateMember = 123; + // } + // ``` + PrivateIdentifier(node): void { + processPrivateIdentifier(node); + }, + + // Process nodes like: + // ```ts + // class A { + // private myPrivateMember = 123; + // } + // ``` + PropertyDefinition(node): void { + if ( + node.accessibility === 'private' && + node.key.type === AST_NODE_TYPES.Identifier + ) { + processPrivateIdentifier(node.key); + } + }, + + // Post-process the class members and report any remaining members. Since + // private members can only be accessed in the current class context, we + // can safely assume that all usages are within the current class body. + 'ClassBody:exit'(): void { + const unusedPrivateMembers = trackedClassMembers.shift(); + if (unusedPrivateMembers === undefined) { + return; + } + + for (const [ + classMemberName, + { declaredNode }, + ] of unusedPrivateMembers.entries()) { + if (trackedClassMembersUsed.has(declaredNode)) { + continue; + } + context.report({ + loc: declaredNode.key.loc, + node: declaredNode, + messageId: 'unusedPrivateClassMember', + data: { + classMemberName: `#${classMemberName}`, + }, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-unused-private-class-members.test.ts b/packages/eslint-plugin/tests/rules/no-unused-private-class-members.test.ts new file mode 100644 index 000000000000..9f3c61b6b32d --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unused-private-class-members.test.ts @@ -0,0 +1,487 @@ +// The following tests are adapted from the tests in eslint. +// Original Code: https://github.com/eslint/eslint/blob/522d8a32f326c52886c531f43cf6a1ff15af8286/tests/lib/rules/no-unused-private-class-members.js +// License : https://github.com/eslint/eslint/blob/522d8a32f326c52886c531f43cf6a1ff15af8286/LICENSE + +import type { TestCaseError } from '@typescript-eslint/rule-tester'; + +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import type { MessageIds } from '../../src/rules/no-unused-private-class-members'; + +import rule from '../../src/rules/no-unused-private-class-members'; + +const ruleTester = new RuleTester(); + +/** Returns an expected error for defined-but-not-used private class member. */ +function definedError(classMemberName: string): TestCaseError { + return { + data: { + classMemberName: `#${classMemberName}`, + }, + messageId: 'unusedPrivateClassMember', + }; +} + +ruleTester.run('no-unused-private-class-members', rule, { + valid: [ + 'class Foo {}', + ` +class Foo { + publicMember = 42; +} + `, + ` +class Foo { + #usedMember = 42; + method() { + return this.#usedMember; + } +} + `, + ` +class Foo { + #usedMember = 42; + anotherMember = this.#usedMember; +} + `, + ` +class Foo { + #usedMember = 42; + foo() { + anotherMember = this.#usedMember; + } +} + `, + ` +class C { + #usedMember; + + foo() { + bar((this.#usedMember += 1)); + } +} + `, + ` +class Foo { + #usedMember = 42; + method() { + return someGlobalMethod(this.#usedMember); + } +} + `, + ` +class C { + #usedInOuterClass; + + foo() { + return class {}; + } + + bar() { + return this.#usedInOuterClass; + } +} + `, + ` +class Foo { + #usedInForInLoop; + method() { + for (const bar in this.#usedInForInLoop) { + } + } +} + `, + ` +class Foo { + #usedInForOfLoop; + method() { + for (const bar of this.#usedInForOfLoop) { + } + } +} + `, + ` +class Foo { + #usedInAssignmentPattern; + method() { + [bar = 1] = this.#usedInAssignmentPattern; + } +} + `, + ` +class Foo { + #usedInArrayPattern; + method() { + [bar] = this.#usedInArrayPattern; + } +} + `, + ` +class Foo { + #usedInAssignmentPattern; + method() { + [bar] = this.#usedInAssignmentPattern; + } +} + `, + ` +class C { + #usedInObjectAssignment; + + method() { + ({ [this.#usedInObjectAssignment]: a } = foo); + } +} + `, + ` +class C { + set #accessorWithSetterFirst(value) { + doSomething(value); + } + get #accessorWithSetterFirst() { + return something(); + } + method() { + this.#accessorWithSetterFirst += 1; + } +} + `, + ` +class Foo { + set #accessorUsedInMemberAccess(value) {} + + method(a) { + [this.#accessorUsedInMemberAccess] = a; + } +} + `, + ` +class C { + get #accessorWithGetterFirst() { + return something(); + } + set #accessorWithGetterFirst(value) { + doSomething(value); + } + method() { + this.#accessorWithGetterFirst += 1; + } +} + `, + ` +class C { + #usedInInnerClass; + + method(a) { + return class { + foo = a.#usedInInnerClass; + }; + } +} + `, + + //-------------------------------------------------------------------------- + // Method definitions + //-------------------------------------------------------------------------- + ` +class Foo { + #usedMethod() { + return 42; + } + anotherMethod() { + return this.#usedMethod(); + } +} + `, + ` +class C { + set #x(value) { + doSomething(value); + } + + foo() { + this.#x = 1; + } +} + `, + ], + invalid: [ + { + code: ` +class Foo { + #unusedMember = 5; +} + `, + errors: [definedError('unusedMember')], + }, + { + code: ` +class First {} +class Second { + #unusedMemberInSecondClass = 5; +} + `, + errors: [definedError('unusedMemberInSecondClass')], + }, + { + code: ` +class First { + #unusedMemberInFirstClass = 5; +} +class Second {} + `, + errors: [definedError('unusedMemberInFirstClass')], + }, + { + code: ` +class First { + #firstUnusedMemberInSameClass = 5; + #secondUnusedMemberInSameClass = 5; +} + `, + errors: [ + definedError('firstUnusedMemberInSameClass'), + definedError('secondUnusedMemberInSameClass'), + ], + }, + { + code: ` +class Foo { + #usedOnlyInWrite = 5; + method() { + this.#usedOnlyInWrite = 42; + } +} + `, + errors: [definedError('usedOnlyInWrite')], + }, + { + code: ` +class Foo { + #usedOnlyInWriteStatement = 5; + method() { + this.#usedOnlyInWriteStatement += 42; + } +} + `, + errors: [definedError('usedOnlyInWriteStatement')], + }, + { + code: ` +class C { + #usedOnlyInIncrement; + + foo() { + this.#usedOnlyInIncrement++; + } +} + `, + errors: [definedError('usedOnlyInIncrement')], + }, + { + code: ` +class C { + #unusedInOuterClass; + + foo() { + return class { + #unusedInOuterClass; + + bar() { + return this.#unusedInOuterClass; + } + }; + } +} + `, + errors: [definedError('unusedInOuterClass')], + }, + { + code: ` +class C { + #unusedOnlyInSecondNestedClass; + + foo() { + return class { + #unusedOnlyInSecondNestedClass; + + bar() { + return this.#unusedOnlyInSecondNestedClass; + } + }; + } + + baz() { + return this.#unusedOnlyInSecondNestedClass; + } + + bar() { + return class { + #unusedOnlyInSecondNestedClass; + }; + } +} + `, + errors: [definedError('unusedOnlyInSecondNestedClass')], + }, + + //-------------------------------------------------------------------------- + // Unused method definitions + //-------------------------------------------------------------------------- + { + code: ` +class Foo { + #unusedMethod() {} +} + `, + errors: [definedError('unusedMethod')], + }, + { + code: ` +class Foo { + #unusedMethod() {} + #usedMethod() { + return 42; + } + publicMethod() { + return this.#usedMethod(); + } +} + `, + errors: [definedError('unusedMethod')], + }, + { + code: ` +class Foo { + set #unusedSetter(value) {} +} + `, + errors: [definedError('unusedSetter')], + }, + { + code: ` +class Foo { + #unusedForInLoop; + method() { + for (this.#unusedForInLoop in bar) { + } + } +} + `, + errors: [definedError('unusedForInLoop')], + }, + { + code: ` +class Foo { + #unusedForOfLoop; + method() { + for (this.#unusedForOfLoop of bar) { + } + } +} + `, + errors: [definedError('unusedForOfLoop')], + }, + { + code: ` +class Foo { + #unusedInDestructuring; + method() { + ({ x: this.#unusedInDestructuring } = bar); + } +} + `, + errors: [definedError('unusedInDestructuring')], + }, + { + code: ` +class Foo { + #unusedInRestPattern; + method() { + [...this.#unusedInRestPattern] = bar; + } +} + `, + errors: [definedError('unusedInRestPattern')], + }, + { + code: ` +class Foo { + #unusedInAssignmentPattern; + method() { + [this.#unusedInAssignmentPattern = 1] = bar; + } +} + `, + errors: [definedError('unusedInAssignmentPattern')], + }, + { + code: ` +class Foo { + #unusedInAssignmentPattern; + method() { + [this.#unusedInAssignmentPattern] = bar; + } +} + `, + errors: [definedError('unusedInAssignmentPattern')], + }, + { + code: ` +class C { + #usedOnlyInTheSecondInnerClass; + + method(a) { + return class { + #usedOnlyInTheSecondInnerClass; + + method2(b) { + foo = b.#usedOnlyInTheSecondInnerClass; + } + + method3(b) { + foo = b.#usedOnlyInTheSecondInnerClass; + } + }; + } +} + `, + errors: [ + { + ...definedError('usedOnlyInTheSecondInnerClass'), + line: 3, + }, + ], + }, + + //-------------------------------------------------------------------------- + // TypeScript checks + //-------------------------------------------------------------------------- + { + code: ` +class A { + private unusedMember; +} + `, + errors: [ + { + ...definedError('unusedMember'), + line: 3, + }, + ], + }, + { + code: ` +class A { + private unusedMethod() {} +} + `, + errors: [ + { + ...definedError('unusedMethod'), + line: 3, + }, + ], + }, + ], +});