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..aad7b5bac464 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx @@ -0,0 +1,80 @@ +--- +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); + } +} +``` + + + + +## Limitations + +This rule does not detect the following cases: + +(1) Private members only used using private only without `this`: + +```ts +class Foo { + private prop = 123; + + method(a: Foo) { + // ✅ Detected as a usage + const prop = this.prop; + + // ❌ NOT detected as a usage + const otherProp = a.prop; + } +} +``` + +(2) Usages of the private member outside of the class: + +```ts +class Foo { + private prop = 123; +} + +const instance = new Foo(); +// ❌ NOT detected as a usage +console.log(foo['prop']); +``` + +## 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/configs/eslintrc/all.ts b/packages/eslint-plugin/src/configs/eslintrc/all.ts index 34f7fd540450..78c8420e2643 100644 --- a/packages/eslint-plugin/src/configs/eslintrc/all.ts +++ b/packages/eslint-plugin/src/configs/eslintrc/all.ts @@ -112,6 +112,8 @@ export = { '@typescript-eslint/no-unsafe-unary-minus': 'error', 'no-unused-expressions': 'off', '@typescript-eslint/no-unused-expressions': 'error', + 'no-unused-private-class-members': 'off', + '@typescript-eslint/no-unused-private-class-members': 'error', 'no-unused-vars': 'off', '@typescript-eslint/no-unused-vars': 'error', 'no-use-before-define': 'off', diff --git a/packages/eslint-plugin/src/configs/flat/all.ts b/packages/eslint-plugin/src/configs/flat/all.ts index 777028d8580c..43792419ab3b 100644 --- a/packages/eslint-plugin/src/configs/flat/all.ts +++ b/packages/eslint-plugin/src/configs/flat/all.ts @@ -126,6 +126,8 @@ export default ( '@typescript-eslint/no-unsafe-unary-minus': 'error', 'no-unused-expressions': 'off', '@typescript-eslint/no-unused-expressions': 'error', + 'no-unused-private-class-members': 'off', + '@typescript-eslint/no-unused-private-class-members': 'error', 'no-unused-vars': 'off', '@typescript-eslint/no-unused-vars': 'error', 'no-use-before-define': 'off', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 2a82c7e18c6b..9ee5903aa5f7 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -88,6 +88,7 @@ import noUnsafeReturn from './no-unsafe-return'; import noUnsafeTypeAssertion from './no-unsafe-type-assertion'; 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'; @@ -222,6 +223,7 @@ const rules = { 'no-unsafe-type-assertion': noUnsafeTypeAssertion, '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..d5bb6e9f2379 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -0,0 +1,60 @@ +import { ESLintUtils } from '@typescript-eslint/utils'; + +import { createRule } from '../util'; +import { analyzeClassMemberUsage } from '../util/class-scope-analyzer/classScopeAnalyzer'; + +type Options = []; +export type MessageIds = 'unusedPrivateClassMember'; + +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: + "Private class member '{{classMemberName}}' is defined but never used.", + }, + + schema: [], + }, + defaultOptions: [], + create(context) { + return { + 'Program:exit'(node) { + const result = analyzeClassMemberUsage( + node, + ESLintUtils.nullThrows( + context.sourceCode.scopeManager, + 'Missing required scope manager', + ), + ); + + for (const classScope of result.values()) { + for (const member of [ + ...classScope.members.instance.values(), + ...classScope.members.static.values(), + ]) { + if ( + (!member.isPrivate() && !member.isHashPrivate()) || + member.isUsed() + ) { + continue; + } + + context.report({ + node: member.node.key, + messageId: 'unusedPrivateClassMember', + data: { classMemberName: member.name }, + }); + } + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts new file mode 100644 index 000000000000..752b64709396 --- /dev/null +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -0,0 +1,565 @@ +/* eslint-disable @typescript-eslint/no-this-alias -- we do a bunch of "start iterating from here" code which isn't actually aliasing `this` */ +import type { ScopeManager } from '@typescript-eslint/scope-manager'; +import type { TSESTree } from '@typescript-eslint/utils'; + +import { Visitor } from '@typescript-eslint/scope-manager'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import type { ClassNode, Key, MemberNode } from './types'; +import { nullThrows, NullThrowsReasons } from '..'; + +import { + extractNameForMember, + extractNameForMemberExpression, +} from './extractComputedName'; +import { privateKey } from './types'; + +export class Member { + /** + * The node that declares this member + */ + public readonly node: MemberNode; + + /** + * The resolved, unique key for this member. + */ + public readonly key: Key; + + /** + * The member name, as given in the source code. + */ + public readonly name: string; + + /** + * The number of writes to this member. + */ + public writeCount = 0; + + /** + * The number of reads from this member. + */ + public readCount = 0; + + private constructor(node: MemberNode, key: Key, name: string) { + this.node = node; + this.key = key; + this.name = name; + } + public static create(node: MemberNode): Member | null { + const name = extractNameForMember(node); + if (name == null) { + return null; + } + return new Member(node, ...name); + } + + public isHashPrivate(): boolean { + return this.node.key.type === AST_NODE_TYPES.PrivateIdentifier; + } + + public isPrivate(): boolean { + return this.node.accessibility === 'private'; + } + + public isStatic(): boolean { + return this.node.static; + } + + public isAccessor(): boolean { + if ( + this.node.type === AST_NODE_TYPES.MethodDefinition || + this.node.type === AST_NODE_TYPES.TSAbstractMethodDefinition + ) { + return this.node.kind === 'set' || this.node.kind === 'get'; + } + + return ( + this.node.type === AST_NODE_TYPES.AccessorProperty || + this.node.type === AST_NODE_TYPES.TSAbstractAccessorProperty + ); + } + + public isUsed(): boolean { + return ( + this.readCount > 0 || + // any usage of an accessor is considered a usage as accessor can have side effects + (this.writeCount > 0 && this.isAccessor()) + ); + } +} + +function isWriteOnlyUsage(node: TSESTree.Node, parent: TSESTree.Node): boolean { + if ( + parent.type !== AST_NODE_TYPES.AssignmentExpression && + parent.type !== AST_NODE_TYPES.ForInStatement && + parent.type !== AST_NODE_TYPES.ForOfStatement && + parent.type !== AST_NODE_TYPES.AssignmentPattern + ) { + return false; + } + + // If it's on the right then it's a read not a write + if (parent.left !== node) { + return false; + } + + if ( + parent.type === AST_NODE_TYPES.AssignmentExpression && + // For any other operator (such as '+=') we still consider it a read operation + parent.operator !== '=' + ) { + // if the read operation is "discarded" in an empty statement, then it is write only. + return parent.parent.type === AST_NODE_TYPES.ExpressionStatement; + } + + return true; +} + +function countReference(identifierParent: TSESTree.Node, member: Member) { + const identifierGrandparent = nullThrows( + identifierParent.parent, + NullThrowsReasons.MissingParent, + ); + + if (isWriteOnlyUsage(identifierParent, identifierGrandparent)) { + member.writeCount += 1; + return; + } + + const identifierGreatGrandparent = identifierGrandparent.parent; + + // A statement which only increments (`this.#x++;`) + if ( + identifierGrandparent.type === AST_NODE_TYPES.UpdateExpression && + identifierGreatGrandparent?.type === AST_NODE_TYPES.ExpressionStatement + ) { + member.writeCount += 1; + return; + } + + /* + * ({ x: this.#usedInDestructuring } = bar); + * + * But should treat the following as a read: + * ({ [this.#x]: a } = foo); + */ + if ( + identifierGrandparent.type === AST_NODE_TYPES.Property && + identifierGreatGrandparent?.type === AST_NODE_TYPES.ObjectPattern && + identifierGrandparent.value === identifierParent + ) { + member.writeCount += 1; + return; + } + + // [...this.#unusedInRestPattern] = bar; + if (identifierGrandparent.type === 'RestElement') { + member.writeCount += 1; + return; + } + + // [this.#unusedInAssignmentPattern] = bar; + if (identifierGrandparent.type === 'ArrayPattern') { + member.writeCount += 1; + return; + } + + member.readCount += 1; +} + +type IntermediateNode = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.Program + | TSESTree.StaticBlock; + +abstract class ThisScope extends Visitor { + /** + * True if the context is considered a static context and so `this` refers to + * the class and not an instance (eg a static method or a static block). + */ + protected readonly isStaticThisContext: boolean; + + /** + * The classes directly declared within this class -- for example a class declared within a method. + * This does not include grandchild classes. + */ + public readonly childScopes: ThisScope[] = []; + + /** + * The scope manager instance used to resolve variables to improve discovery of usages. + */ + public readonly scopeManager: ScopeManager; + + /** + * The parent class scope if one exists. + */ + public readonly upper: ThisScope | null; + + /** + * The context of the `this` reference in the current scope. + */ + protected readonly thisContext: ClassScope | null; + + constructor( + scopeManager: ScopeManager, + upper: ThisScope | null, + thisContext: 'none' | 'self' | ClassScope, + isStaticThisContext: boolean, + ) { + super({}); + this.scopeManager = scopeManager; + this.upper = upper; + this.isStaticThisContext = isStaticThisContext; + + if (thisContext === 'self') { + if (!(this instanceof ClassScope)) { + throw new Error('Cannot use `self` unless it is in a ClassScope'); + } + this.thisContext = this; + } else if (thisContext === 'none') { + this.thisContext = null; + } else { + this.thisContext = thisContext; + } + } + + private getObjectClass(node: TSESTree.MemberExpression): { + thisContext: ThisScope['thisContext']; + type: 'instance' | 'static'; + } | null { + switch (node.object.type) { + case AST_NODE_TYPES.ThisExpression: { + if (this.thisContext == null) { + return null; + } + return { + thisContext: this.thisContext, + type: this.isStaticThisContext ? 'static' : 'instance', + }; + } + + case AST_NODE_TYPES.Identifier: { + const thisContext = this.findClassScopeWithName(node.object.name); + if (thisContext != null) { + return { thisContext, type: 'static' }; + } + + // TODO -- use scope analysis to do some basic type resolution to handle cases like: + // ``` + // class Foo { + // private prop: number; + // method(thing: Foo) { + // // this references the private instance member but not via `this` so we can't see it + // thing.prop = 1; + // } + // } + + return null; + } + case AST_NODE_TYPES.MemberExpression: + // TODO - we could probably recurse here to do some more complex analysis and support like `foo.bar.baz` nested references + return null; + + default: + return null; + } + } + + private visitClass(node: ClassNode): void { + const classScope = new ClassScope(node, this, this.scopeManager); + this.childScopes.push(classScope); + classScope.visitChildren(node); + } + + private visitIntermediate(node: IntermediateNode): void { + const intermediateScope = new IntermediateScope( + this.scopeManager, + this, + node, + ); + this.childScopes.push(intermediateScope); + intermediateScope.visitChildren(node); + } + + /** + * Gets the nearest class scope with the given name. + */ + public findClassScopeWithName(name: string): ClassScope | null { + let currentScope: ThisScope | null = this; + while (currentScope != null) { + if ( + currentScope instanceof ClassScope && + currentScope.className === name + ) { + return currentScope; + } + currentScope = currentScope.upper; + } + return null; + } + + ///////////////////// + // Visit selectors // + ///////////////////// + + protected ClassDeclaration(node: TSESTree.ClassDeclaration): void { + this.visitClass(node); + } + + protected ClassExpression(node: TSESTree.ClassExpression): void { + this.visitClass(node); + } + + protected FunctionDeclaration(node: TSESTree.FunctionDeclaration): void { + this.visitIntermediate(node); + } + + protected FunctionExpression(node: TSESTree.FunctionExpression): void { + this.visitIntermediate(node); + } + + protected MemberExpression(node: TSESTree.MemberExpression): void { + this.visitChildren(node); + + if (node.property.type === AST_NODE_TYPES.PrivateIdentifier) { + // will be handled by the PrivateIdentifier visitor + return; + } + + const propertyName = extractNameForMemberExpression(node); + if (propertyName == null) { + return; + } + + const objectClassName = this.getObjectClass(node); + if (objectClassName == null) { + return; + } + + if (objectClassName.thisContext == null) { + return; + } + + const members = + objectClassName.type === 'instance' + ? objectClassName.thisContext.members.instance + : objectClassName.thisContext.members.static; + const member = members.get(propertyName[0]); + if (member == null) { + return; + } + + countReference(node, member); + } + + protected PrivateIdentifier(node: TSESTree.PrivateIdentifier): void { + this.visitChildren(node); + + if ( + (node.parent.type === AST_NODE_TYPES.MethodDefinition || + node.parent.type === AST_NODE_TYPES.PropertyDefinition) && + node.parent.key === node + ) { + // ignore the member definition + return; + } + + // We can actually be pretty loose with our code here thanks to how private + // members are designed. + // + // 1) classes CANNOT have a static and instance private member with the + // same name, so we don't need to match up static access. + // 2) nested classes CANNOT access a private member of their parent class if + // the member has the same name as a private member of the nested class. + // + // together this means that we can just look for the member upwards until we + // find a match and we know that will be the correct match! + + let currentScope: ThisScope | null = this; + const key = privateKey(node); + while (currentScope != null) { + if (currentScope.thisContext != null) { + const member = + currentScope.thisContext.members.instance.get(key) ?? + currentScope.thisContext.members.static.get(key); + if (member != null) { + countReference(node.parent, member); + return; + } + } + + currentScope = currentScope.upper; + } + } + + protected StaticBlock(node: TSESTree.StaticBlock): void { + this.visitIntermediate(node); + } +} + +/** + * Any other scope that is not a class scope + * + * When we visit a function declaration/expression the `this` reference is + * rebound so it no longer refers to the class. + * + * This also supports a function's `this` parameter. + */ +class IntermediateScope extends ThisScope { + constructor( + scopeManager: ScopeManager, + upper: ThisScope | null, + node: IntermediateNode, + ) { + if (node.type === AST_NODE_TYPES.Program) { + super(scopeManager, upper, 'none', false); + return; + } + + if (node.type === AST_NODE_TYPES.StaticBlock) { + if (upper == null || !(upper instanceof ClassScope)) { + throw new Error( + 'Cannot have a static block without an upper ClassScope', + ); + } + super(scopeManager, upper, upper, true); + return; + } + + // method definition + if ( + (node.parent.type === AST_NODE_TYPES.MethodDefinition || + node.parent.type === AST_NODE_TYPES.PropertyDefinition) && + node.parent.value === node + ) { + if (upper == null || !(upper instanceof ClassScope)) { + throw new Error( + 'Cannot have a class method/property without an upper ClassScope', + ); + } + super(scopeManager, upper, upper, node.parent.static); + return; + } + + // function with a `this` parameter + if ( + upper != null && + node.params[0].type === AST_NODE_TYPES.Identifier && + node.params[0].name === 'this' + ) { + const thisType = node.params[0].typeAnnotation?.typeAnnotation; + if ( + thisType?.type === AST_NODE_TYPES.TSTypeReference && + thisType.typeName.type === AST_NODE_TYPES.Identifier + ) { + const thisContext = upper.findClassScopeWithName( + thisType.typeName.name, + ); + if (thisContext != null) { + super(scopeManager, upper, thisContext, false); + return; + } + } + } + + super(scopeManager, upper, 'none', false); + } +} + +export interface ClassScopeResult { + /** + * The classes name as given in the source code. + * If this is `null` then the class is an anonymous class. + */ + readonly className: string | null; + /** + * The class's members, keyed by their name + */ + readonly members: { + readonly instance: Map; + readonly static: Map; + }; +} + +class ClassScope extends ThisScope implements ClassScopeResult { + public readonly className: string | null; + + /** + * The class's members, keyed by their name + */ + public readonly members: ClassScopeResult['members'] = { + instance: new Map(), + static: new Map(), + }; + + /** + * The node that declares this class. + */ + public readonly theClass: ClassNode; + + public constructor( + theClass: ClassNode, + upper: ClassScope | IntermediateScope | null, + scopeManager: ScopeManager, + ) { + super(scopeManager, upper, 'self', false); + + this.theClass = theClass; + this.className = theClass.id?.name ?? null; + + for (const memberNode of theClass.body.body) { + switch (memberNode.type) { + case AST_NODE_TYPES.AccessorProperty: + case AST_NODE_TYPES.MethodDefinition: + case AST_NODE_TYPES.PropertyDefinition: + case AST_NODE_TYPES.TSAbstractAccessorProperty: + case AST_NODE_TYPES.TSAbstractMethodDefinition: + case AST_NODE_TYPES.TSAbstractPropertyDefinition: { + const member = Member.create(memberNode); + if (member == null) { + continue; + } + if (member.isStatic()) { + this.members.static.set(member.key, member); + } else { + this.members.instance.set(member.key, member); + } + break; + } + + case AST_NODE_TYPES.StaticBlock: + // static blocks declare no members + continue; + + case AST_NODE_TYPES.TSIndexSignature: + // index signatures are type signatures only and are fully computed + continue; + } + } + } +} + +export function analyzeClassMemberUsage( + program: TSESTree.Program, + scopeManager: ScopeManager, +): ReadonlyMap { + const rootScope = new IntermediateScope(scopeManager, null, program); + rootScope.visit(program); + return traverseScopes(rootScope); +} + +function traverseScopes( + currentScope: ThisScope, + analysisResults = new Map(), +) { + if (currentScope instanceof ClassScope) { + analysisResults.set(currentScope.theClass, currentScope); + } + + for (const childScope of currentScope.childScopes) { + traverseScopes(childScope, analysisResults); + } + + return analysisResults; +} diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts new file mode 100644 index 000000000000..3b3aca0acdaa --- /dev/null +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts @@ -0,0 +1,61 @@ +import type { TSESTree } from '@typescript-eslint/utils'; + +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import type { Key, MemberNode } from './types'; + +import { privateKey, publicKey } from './types'; + +function extractComputedName( + computedName: TSESTree.Expression, +): [Key, string] | null { + if (computedName.type === AST_NODE_TYPES.Literal) { + const name = computedName.value?.toString() ?? 'null'; + return [publicKey(name), name]; + } + if ( + computedName.type === AST_NODE_TYPES.TemplateLiteral && + computedName.expressions.length === 0 + ) { + const name = computedName.quasis[0].value.raw; + return [publicKey(name), name]; + } + return null; +} +function extractNonComputedName( + nonComputedName: TSESTree.Identifier | TSESTree.PrivateIdentifier, +): [Key, string] | null { + const name = nonComputedName.name; + if (nonComputedName.type === AST_NODE_TYPES.PrivateIdentifier) { + return [privateKey(nonComputedName), `#${name}`]; + } + return [publicKey(name), name]; +} +/** + * Extracts the string name for a member. + * @returns `null` if the name cannot be extracted due to it being a computed. + */ +export function extractNameForMember(node: MemberNode): [Key, string] | null { + if (node.computed) { + return extractComputedName(node.key); + } + + if (node.key.type === AST_NODE_TYPES.Literal) { + return extractComputedName(node.key); + } + + return extractNonComputedName(node.key); +} +/** + * Extracts the string property name for a member. + * @returns `null` if the name cannot be extracted due to it being a computed. + */ +export function extractNameForMemberExpression( + node: TSESTree.MemberExpression, +): [Key, string] | null { + if (node.computed) { + return extractComputedName(node.property); + } + + return extractNonComputedName(node.property); +} diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts new file mode 100644 index 000000000000..481aff1a4a20 --- /dev/null +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts @@ -0,0 +1,20 @@ +import type { TSESTree } from '@typescript-eslint/utils'; + +export type ClassNode = TSESTree.ClassDeclaration | TSESTree.ClassExpression; +export type MemberNode = + | TSESTree.AccessorProperty + | TSESTree.MethodDefinition + | TSESTree.PropertyDefinition + | TSESTree.TSAbstractAccessorProperty + | TSESTree.TSAbstractMethodDefinition + | TSESTree.TSAbstractPropertyDefinition; +export type PrivateKey = string & { __brand: 'private-key' }; +export type PublicKey = string & { __brand: 'public-key' }; +export type Key = PrivateKey | PublicKey; + +export function publicKey(node: string): PublicKey { + return node as PublicKey; +} +export function privateKey(node: TSESTree.PrivateIdentifier): PrivateKey { + return `#private@@${node.name}` as PrivateKey; +} diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unused-private-class-members.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unused-private-class-members.shot new file mode 100644 index 000000000000..1bea7a556d71 --- /dev/null +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unused-private-class-members.shot @@ -0,0 +1,24 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Validating rule docs no-unused-private-class-members.mdx code examples ESLint output 1`] = ` +"Incorrect + +class A { + private foo = 123; + ~~~ 'foo' is defined but never used. +} +" +`; + +exports[`Validating rule docs no-unused-private-class-members.mdx code examples ESLint output 2`] = ` +"Correct + +class A { + private foo = 123; + + constructor() { + console.log(this.foo); + } +} +" +`; 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..73eef14eb20f --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unused-private-class-members.test.ts @@ -0,0 +1,565 @@ +// 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, + addHashTag = true, +): TestCaseError { + return { + data: { + classMemberName: addHashTag ? `#${classMemberName}` : classMemberName, + }, + messageId: 'unusedPrivateClassMember', + }; +} + +ruleTester.run('no-unused-private-class-members', rule, { + valid: [ + 'class Foo {}', + ` +class Foo { + publicMember = 42; +} + `, + ` +class Foo { + public publicMember = 42; +} + `, + ` +class Foo { + protected publicMember = 42; +} + `, + ` +class C { + #usedInInnerClass; + + method(a) { + return class { + foo = a.#usedInInnerClass; + }; + } +} + `, + ` +class C { + private accessor accessorMember = 42; + + method() { + return this.accessorMember; + } +} + `, + ` +class C { + private static staticMember = 42; + + static method() { + return this.staticMember; + } +} + `, + { + code: ` +class C { + private static staticMember = 42; + + method() { + return C.staticMember; + } +} + `, + }, + + ...[ + ` +class Foo { + @# = 42; + method() { + return this.#; + } +} + `, + ` +class Foo { + @# = 42; + anotherMember = this.#; +} + `, + ` +class Foo { + @# = 42; + foo() { + anotherMember = this.#; + } +} + `, + ` +class C { + @#; + + foo() { + bar((this.# += 1)); + } +} + `, + ` +class Foo { + @# = 42; + method() { + return someGlobalMethod(this.#); + } +} + `, + ` +class C { + @#; + + foo() { + return class {}; + } + + bar() { + return this.#; + } +} + `, + ` +class Foo { + @#; + method() { + for (const bar in this.#) { + } + } +} + `, + ` +class Foo { + @#; + method() { + for (const bar of this.#) { + } + } +} + `, + ` +class Foo { + @#; + method() { + [bar = 1] = this.#; + } +} + `, + ` +class Foo { + @#; + method() { + [bar] = this.#; + } +} + `, + ` +class C { + @#; + + method() { + ({ [this.#]: a } = foo); + } +} + `, + ` +class C { + @set #(value) { + doSomething(value); + } + @get #() { + return something(); + } + method() { + this.# += 1; + } +} + `, + ` +class Foo { + @set #(value) {} + + method(a) { + [this.#] = a; + } +} + `, + ` +class C { + @get #() { + return something(); + } + @set #(value) { + doSomething(value); + } + method() { + this.# += 1; + } +} + `, + + //-------------------------------------------------------------------------- + // Method definitions + //-------------------------------------------------------------------------- + ` +class Foo { + @#() { + return 42; + } + anotherMethod() { + return this.#(); + } +} + `, + ` +class C { + @set #(value) { + doSomething(value); + } + + foo() { + this.# = 1; + } +} + `, + ].flatMap(code => [ + code.replaceAll('#', '#privateMember').replaceAll('@', ''), + code.replaceAll('#', 'privateMember').replaceAll('@', 'private '), + ]), + ], + invalid: [ + { + code: ` +class C { + #unusedInOuterClass; + + foo() { + return class D { + #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')], + }, + { + code: ` +class C { + #usedOnlyInTheSecondInnerClass; + + method(a) { + return class { + #usedOnlyInTheSecondInnerClass; + + method2(b) { + foo = b.#usedOnlyInTheSecondInnerClass; + } + + method3(b) { + foo = b.#usedOnlyInTheSecondInnerClass; + } + }; + } +} + `, + errors: [ + { + ...definedError('usedOnlyInTheSecondInnerClass'), + line: 3, + }, + ], + }, + { + code: ` +class C { + private accessor accessorMember = 42; +} + `, + errors: [definedError('accessorMember', false)], + }, + { + code: ` +class C { + private static staticMember = 42; +} + `, + errors: [definedError('staticMember', false)], + }, + + // intentionally not handled cases + { + code: ` +class C { + private usedInInnerClass; + + method(a: C) { + return class { + foo = a.usedInInnerClass; + }; + } +} + `, + errors: [definedError('usedInInnerClass', false)], + }, + { + code: ` +class C { + private usedOutsideClass; +} + +const instance = new C(); +console.log(instance.usedOutsideClass); + `, + errors: [definedError('usedOutsideClass', false)], + }, + + ...[ + { + code: ` +class Foo { + @# = 5; +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class First {} +class Second { + @# = 5; +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class First { + @# = 5; +} +class Second {} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class First { + @# = 5; + @#2 = 5; +} + `, + errors: [definedError('#', false), definedError('#2', false)], + }, + { + code: ` +class Foo { + @# = 5; + method() { + this.# = 42; + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @# = 5; + method() { + this.# += 42; + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class C { + @#; + + foo() { + this.#++; + } +} + `, + errors: [definedError('#', false)], + }, + + //-------------------------------------------------------------------------- + // Unused method definitions + //-------------------------------------------------------------------------- + { + code: ` +class Foo { + @#() {} +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#() {} + @#Used() { + return 42; + } + publicMethod() { + return this.#Used(); + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @set #(value) {} +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#; + method() { + for (this.# in bar) { + } + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#; + method() { + for (this.# of bar) { + } + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#; + method() { + ({ x: this.# } = bar); + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#; + method() { + [...this.#] = bar; + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#; + method() { + [this.# = 1] = bar; + } +} + `, + errors: [definedError('#', false)], + }, + { + code: ` +class Foo { + @#; + method() { + [this.#] = bar; + } +} + `, + errors: [definedError('#', false)], + }, + ].flatMap(({ code, errors }) => [ + { + code: code.replaceAll('#', '#privateMember').replaceAll('@', ''), + errors: errors.map(error => ({ + ...error, + data: { + classMemberName: (error.data?.classMemberName as string).replaceAll( + '#', + '#privateMember', + ), + }, + })), + }, + { + code: code.replaceAll('#', 'privateMember').replaceAll('@', 'private '), + errors: errors.map(error => ({ + ...error, + data: { + classMemberName: (error.data?.classMemberName as string).replaceAll( + '#', + 'privateMember', + ), + }, + })), + }, + ]), + ], +});