From 34b77b871b759d2ee9344ac245b39dc5f4a40535 Mon Sep 17 00:00:00 2001 From: bradzacher Date: Mon, 3 Mar 2025 12:28:13 +1030 Subject: [PATCH 01/12] feat(eslint-plugin): [no-unused-private-class-members] new extension rule --- .../rules/no-unused-private-class-members.mdx | 82 +++ packages/eslint-plugin/src/configs/all.ts | 2 + packages/eslint-plugin/src/rules/index.ts | 2 + .../rules/no-unused-private-class-members.ts | 266 +++++++++ .../no-unused-private-class-members.shot | 24 + .../no-unused-private-class-members.test.ts | 520 ++++++++++++++++++ packages/typescript-eslint/src/configs/all.ts | 2 + 7 files changed, 898 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx create mode 100644 packages/eslint-plugin/src/rules/no-unused-private-class-members.ts create mode 100644 packages/eslint-plugin/tests/docs-eslint-output-snapshots/no-unused-private-class-members.shot create mode 100644 packages/eslint-plugin/tests/rules/no-unused-private-class-members.test.ts 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..b055ecbf3a15 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx @@ -0,0 +1,82 @@ +--- +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) Nested classes using private members of the outer class: + +```ts +class Foo { + private prop = 123; + + method(a: Foo) { + return class { + // ✅ Detected as a usage + prop = this.prop; + + // ❌ NOT detected as a usage + innerProp = 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/all.ts b/packages/eslint-plugin/src/configs/all.ts index 6febc52e3977..e7a349994486 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -111,6 +111,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/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index f1587b5d2b1f..9f05b7ea6469 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -87,6 +87,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'; @@ -220,6 +221,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..34f909ddb435 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -0,0 +1,266 @@ +// 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 == null) { + 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 == null) { + return; + } + + // In case any other usage was already detected, we can short circuit the + // logic here. + const memberDefinition = classBody.get(node.name); + if (memberDefinition == null) { + 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 all usages of the private identifier and remove a member from + // `declaredAndUnusedPrivateMembers` if we deem it used. + PrivateIdentifier(node): void { + processPrivateIdentifier(node); + }, + + // Process all usages of the identifier and remove a member from + // `declaredAndUnusedPrivateMembers` if we deem it used. + MemberExpression(node): void { + if ( + !node.computed && + node.object.type === AST_NODE_TYPES.ThisExpression && + node.property.type === AST_NODE_TYPES.Identifier + ) { + processPrivateIdentifier(node.property); + } + }, + + // 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 == null) { + 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: + declaredNode.key.type === AST_NODE_TYPES.PrivateIdentifier + ? `#${classMemberName}` + : classMemberName, + }, + }); + } + }, + }; + }, +}); 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..5cd87e322619 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unused-private-class-members.test.ts @@ -0,0 +1,520 @@ +// 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 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 { + #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, + }, + ], + }, + + // 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', + ), + }, + })), + }, + ]), + ], +}); diff --git a/packages/typescript-eslint/src/configs/all.ts b/packages/typescript-eslint/src/configs/all.ts index 4bf1a3b4c5d4..557f725da3f8 100644 --- a/packages/typescript-eslint/src/configs/all.ts +++ b/packages/typescript-eslint/src/configs/all.ts @@ -125,6 +125,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', From 08540073bdffc3651d4898bff2773cb6a8688f3d Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 9 Mar 2025 12:02:13 +1030 Subject: [PATCH 02/12] support accessor properties --- .../rules/no-unused-private-class-members.mdx | 12 ++--- .../rules/no-unused-private-class-members.ts | 20 ++++---- .../no-unused-private-class-members.test.ts | 47 +++++++++++++++++++ 3 files changed, 63 insertions(+), 16 deletions(-) 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 index b055ecbf3a15..aad7b5bac464 100644 --- a/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx +++ b/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx @@ -47,20 +47,18 @@ class A { This rule does not detect the following cases: -(1) Nested classes using private members of the outer class: +(1) Private members only used using private only without `this`: ```ts class Foo { private prop = 123; method(a: Foo) { - return class { - // ✅ Detected as a usage - prop = this.prop; + // ✅ Detected as a usage + const prop = this.prop; - // ❌ NOT detected as a usage - innerProp = a.prop; - }; + // ❌ NOT detected as a usage + const otherProp = a.prop; } } ``` 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 index 34f909ddb435..daed826ebddf 100644 --- a/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -11,15 +11,6 @@ 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: { @@ -39,6 +30,15 @@ export default createRule({ }, defaultOptions: [], create(context) { + interface PrivateMember { + declaredNode: + | TSESTree.AccessorProperty + | TSESTree.MethodDefinitionComputedName + | TSESTree.MethodDefinitionNonComputedName + | TSESTree.PropertyDefinitionComputedName + | TSESTree.PropertyDefinitionNonComputedName; + isAccessor: boolean; + } const trackedClassMembers: Map[] = []; /** @@ -48,6 +48,7 @@ export default createRule({ * track whether a method is being used. */ const trackedClassMembersUsed = new Set< + | TSESTree.AccessorProperty | TSESTree.MethodDefinitionComputedName | TSESTree.MethodDefinitionNonComputedName | TSESTree.PropertyDefinitionComputedName @@ -199,6 +200,7 @@ export default createRule({ for (const bodyMember of classBodyNode.body) { if ( (bodyMember.type === AST_NODE_TYPES.PropertyDefinition || + bodyMember.type === AST_NODE_TYPES.AccessorProperty || bodyMember.type === AST_NODE_TYPES.MethodDefinition) && (bodyMember.key.type === AST_NODE_TYPES.PrivateIdentifier || (bodyMember.key.type === AST_NODE_TYPES.Identifier && 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 index 5cd87e322619..b25b947cc7c1 100644 --- 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 @@ -54,6 +54,37 @@ class C { } } `, + ` +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; + } +} + `, + // TODO make this work + skip: true, + }, ...[ ` @@ -290,6 +321,22 @@ class C { }, ], }, + { + 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 { From df85b86e7d8273d6bcac27a8d6d0d5b20da09a33 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 6 Jul 2025 12:22:44 +0930 Subject: [PATCH 03/12] WIP class scope analyser --- .../classScopeAnalyzer.ts | 325 ++++++++++++++++++ .../extractComputedName.ts | 56 +++ .../src/util/class-scope-analyzer/types.ts | 23 ++ 3 files changed, 404 insertions(+) create mode 100644 packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts create mode 100644 packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts create mode 100644 packages/eslint-plugin/src/util/class-scope-analyzer/types.ts 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..d538563a83c2 --- /dev/null +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -0,0 +1,325 @@ +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, FunctionNode, Key, MemberNode } from './types'; + +import { + extractNameForMember, + extractNameForMemberExpression, +} from './extractComputedName'; +import { privateKey } from './types'; + +export class Member { + /** + * The node that declares this member + */ + public readonly node: MemberNode; + + /** + * The member name, as given in the source code. + */ + public readonly name: Key; + + /** + * The number of references to this member. + */ + public referenceCount = 0; + + private constructor(node: MemberNode, name: Key) { + this.node = node; + this.name = name; + } + public static create(node: MemberNode): Member | null { + const name = extractNameForMember(node); + if (name == null) { + return null; + } + return new Member(node, name); + } + + /** + * True if the variable is considered an accessor + */ + public isAccessor(): boolean { + return ( + this.node.type === AST_NODE_TYPES.AccessorProperty || + ('kind' in this.node && + (this.node.kind === 'get' || this.node.kind === 'set')) + ); + } + + /** + * True if the variable has the `private` accessibility modifier. + */ + public isPrivate(): boolean { + return this.node.accessibility === 'private'; + } + /** + * True if the variable has the `protected` accessibility modifier. + */ + public isProtected(): boolean { + return this.node.accessibility === 'protected'; + } + + /** + * True if the variable has the `public` accessibility modifier. + */ + public isPublic(): boolean { + return this.node.accessibility === 'public'; + } +} + +abstract class ThisScope extends Visitor { + /** + * 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, + ) { + super({}); + this.scopeManager = scopeManager; + this.upper = upper; + + if (thisContext === 'self') { + this.thisContext = this as unknown as ClassScope; + } else if (thisContext === 'none') { + this.thisContext = null; + } else { + this.thisContext = thisContext; + } + } + + private getObjectClass( + node: TSESTree.MemberExpression, + ): { thisContext: ThisScope; type: 'instance' | 'static' } | null { + switch (node.object.type) { + case AST_NODE_TYPES.ThisExpression: { + if (this.thisContext == null) { + return null; + } + return { thisContext: this.thisContext, type: 'instance' }; + } + + case AST_NODE_TYPES.Identifier: { + const thisContext = this.findClassScopeWithName(node.object.name); + if (thisContext != null) { + return { thisContext, type: 'static' }; + } + return null; + } + case AST_NODE_TYPES.MemberExpression: + // TODO - we could probably recurse here to do some more complex analysis and support like + return null; + + default: + return null; + } + } + + private visitClass(node: ClassNode): void { + const classScope = new ClassScope(node, this, this.scopeManager); + this.childScopes.push(classScope); + classScope.visit(node); + } + + private visitFunction(node: FunctionNode): void { + const functionScope = new FunctionScope(this.scopeManager, this, node); + this.childScopes.push(functionScope); + functionScope.visit(node); + } + + /** + * Gets the nearest class scope with the given name. + */ + public findClassScopeWithName(name: string): ClassScope | null { + // eslint-disable-next-line @typescript-eslint/no-this-alias + 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.visitFunction(node); + } + + protected FunctionExpression(node: TSESTree.FunctionExpression): void { + if ( + (node.parent.type === AST_NODE_TYPES.MethodDefinition || + node.parent.type === AST_NODE_TYPES.PropertyDefinition) && + node.parent.value === node + ) { + // if the function is a method's implementation then the `this` is guaranteed to be the class + this.visit(node); + } else { + // TODO(bradzacher): + // - we could handle manual bound functions like `(function () { ... }).bind(this)` + // - we could handle auto-bound methods like `[].map(function () {}, this)` + this.visitFunction(node); + } + } + + protected MemberExpression(node: TSESTree.MemberExpression): void { + const propertyName = extractNameForMemberExpression(node); + if (propertyName == null) { + return; + } + + const objectClassName = this.getObjectClass(node); + } + + protected PrivateIdentifier(node: TSESTree.PrivateIdentifier): void { + const member = this.thisContext?.members.get(privateKey(node)); + if (member == null) { + return; + } + + member.referenceCount += 1; + } +} + +/** + * 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. + */ +export class FunctionScope extends ThisScope { + constructor( + scopeManager: ScopeManager, + upper: ThisScope, + node: FunctionNode, + ) { + if ( + node.params[0].type !== AST_NODE_TYPES.Identifier || + node.params[0].name !== 'this' + ) { + super(scopeManager, upper, 'none'); + return; + } + + const thisType = node.params[0].typeAnnotation?.typeAnnotation; + if ( + thisType == null || + thisType.type !== AST_NODE_TYPES.TSTypeReference || + thisType.typeName.type !== AST_NODE_TYPES.Identifier + ) { + super(scopeManager, upper, 'none'); + return; + } + + const thisContext = upper.findClassScopeWithName(thisType.typeName.name); + if (thisContext == null) { + super(scopeManager, upper, 'none'); + return; + } + + super(scopeManager, upper, thisContext); + } +} + +export class ClassScope extends ThisScope { + /** + * The classes name as given in the source code. + * If this is `null` then the class is an anonymous class. + */ + public readonly className: string | null; + + /** + * The class's members, keyed by their name + */ + public readonly members = new Map(); + + /** + * The node that declares this class. + */ + public readonly theClass: ClassNode; + + public constructor( + theClass: ClassNode, + upper: ClassScope | FunctionScope | null, + scopeManager: ScopeManager, + ) { + super(scopeManager, upper, 'self'); + + 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; + } + this.members.set(member.name, 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( + theClass: ClassNode, + scopeManager: ScopeManager, +): ClassScope { + const analyzer = new ClassScope(theClass, null, scopeManager); + analyzer.visit(theClass); + return analyzer; +} 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..020d3bf1fa6e --- /dev/null +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts @@ -0,0 +1,56 @@ +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 | null { + if (computedName.type === AST_NODE_TYPES.Literal) { + return publicKey(computedName.value?.toString() ?? 'null'); + } + if ( + computedName.type === AST_NODE_TYPES.TemplateLiteral && + computedName.expressions.length === 0 + ) { + return publicKey(computedName.quasis[0].value.raw); + } + return null; +} +function extractNonComputedName( + nonComputedName: TSESTree.Identifier | TSESTree.PrivateIdentifier, +): Key { + if (nonComputedName.type === AST_NODE_TYPES.PrivateIdentifier) { + return privateKey(nonComputedName); + } + return publicKey(nonComputedName.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 | 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 | 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..69db0fa37cfb --- /dev/null +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts @@ -0,0 +1,23 @@ +import type { TSESTree } from '@typescript-eslint/utils'; + +export type ClassNode = TSESTree.ClassDeclaration | TSESTree.ClassExpression; +export type FunctionNode = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression; +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; +} From 8710810d01f6e14a6172c78666f0d9e616c93827 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 6 Jul 2025 16:34:01 +0930 Subject: [PATCH 04/12] WIP --- .../classScopeAnalyzer.ts | 269 ++++++++++++------ .../src/util/class-scope-analyzer/types.ts | 3 - 2 files changed, 188 insertions(+), 84 deletions(-) diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts index d538563a83c2..41d101d90935 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -1,10 +1,11 @@ +/* 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, FunctionNode, Key, MemberNode } from './types'; +import type { ClassNode, Key, MemberNode } from './types'; import { extractNameForMember, @@ -40,39 +41,28 @@ export class Member { return new Member(node, name); } - /** - * True if the variable is considered an accessor - */ - public isAccessor(): boolean { - return ( - this.node.type === AST_NODE_TYPES.AccessorProperty || - ('kind' in this.node && - (this.node.kind === 'get' || this.node.kind === 'set')) - ); - } - - /** - * True if the variable has the `private` accessibility modifier. - */ public isPrivate(): boolean { return this.node.accessibility === 'private'; } - /** - * True if the variable has the `protected` accessibility modifier. - */ - public isProtected(): boolean { - return this.node.accessibility === 'protected'; - } - /** - * True if the variable has the `public` accessibility modifier. - */ - public isPublic(): boolean { - return this.node.accessibility === 'public'; + public isStatic(): boolean { + return this.node.static; } } +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. @@ -98,13 +88,18 @@ abstract class ThisScope extends Visitor { scopeManager: ScopeManager, upper: ThisScope | null, thisContext: 'none' | 'self' | ClassScope, + isStaticThisContext: boolean, ) { super({}); this.scopeManager = scopeManager; this.upper = upper; + this.isStaticThisContext = isStaticThisContext; if (thisContext === 'self') { - this.thisContext = this as unknown as ClassScope; + 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 { @@ -112,9 +107,10 @@ abstract class ThisScope extends Visitor { } } - private getObjectClass( - node: TSESTree.MemberExpression, - ): { thisContext: ThisScope; type: 'instance' | 'static' } | null { + 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) { @@ -128,10 +124,21 @@ abstract class ThisScope extends Visitor { 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 + // TODO - we could probably recurse here to do some more complex analysis and support like `foo.bar.baz` nested references return null; default: @@ -145,17 +152,20 @@ abstract class ThisScope extends Visitor { classScope.visit(node); } - private visitFunction(node: FunctionNode): void { - const functionScope = new FunctionScope(this.scopeManager, this, node); - this.childScopes.push(functionScope); - functionScope.visit(node); + private visitIntermediate(node: IntermediateNode): void { + const intermediateScope = new IntermediateScope( + this.scopeManager, + this, + node, + ); + this.childScopes.push(intermediateScope); + intermediateScope.visit(node); } /** * Gets the nearest class scope with the given name. */ public findClassScopeWithName(name: string): ClassScope | null { - // eslint-disable-next-line @typescript-eslint/no-this-alias let currentScope: ThisScope | null = this; while (currentScope != null) { if ( @@ -182,95 +192,173 @@ abstract class ThisScope extends Visitor { } protected FunctionDeclaration(node: TSESTree.FunctionDeclaration): void { - this.visitFunction(node); + this.visitIntermediate(node); } protected FunctionExpression(node: TSESTree.FunctionExpression): void { - if ( - (node.parent.type === AST_NODE_TYPES.MethodDefinition || - node.parent.type === AST_NODE_TYPES.PropertyDefinition) && - node.parent.value === node - ) { - // if the function is a method's implementation then the `this` is guaranteed to be the class - this.visit(node); - } else { - // TODO(bradzacher): - // - we could handle manual bound functions like `(function () { ... }).bind(this)` - // - we could handle auto-bound methods like `[].map(function () {}, this)` - this.visitFunction(node); - } + this.visitIntermediate(node); } protected MemberExpression(node: TSESTree.MemberExpression): void { + 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; + } - protected PrivateIdentifier(node: TSESTree.PrivateIdentifier): void { - const member = this.thisContext?.members.get(privateKey(node)); + if (objectClassName.thisContext == null) { + return; + } + + const members = + objectClassName.type === 'instance' + ? objectClassName.thisContext.members.instance + : objectClassName.thisContext.members.static; + const member = members.get(propertyName); if (member == null) { return; } member.referenceCount += 1; } + + protected PrivateIdentifier(node: TSESTree.PrivateIdentifier): void { + // 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) { + member.referenceCount += 1; + 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. */ -export class FunctionScope extends ThisScope { +class IntermediateScope extends ThisScope { constructor( scopeManager: ScopeManager, - upper: ThisScope, - node: FunctionNode, + upper: ThisScope | null, + node: IntermediateNode, ) { - if ( - node.params[0].type !== AST_NODE_TYPES.Identifier || - node.params[0].name !== 'this' - ) { - super(scopeManager, upper, 'none'); + 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; } - const thisType = node.params[0].typeAnnotation?.typeAnnotation; + // method definition if ( - thisType == null || - thisType.type !== AST_NODE_TYPES.TSTypeReference || - thisType.typeName.type !== AST_NODE_TYPES.Identifier + (node.parent.type === AST_NODE_TYPES.MethodDefinition || + node.parent.type === AST_NODE_TYPES.PropertyDefinition) && + node.parent.value === node ) { - super(scopeManager, upper, 'none'); + 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; } - const thisContext = upper.findClassScopeWithName(thisType.typeName.name); - if (thisContext == null) { - super(scopeManager, upper, 'none'); - 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, thisContext); + super(scopeManager, upper, 'none', false); } } -export class ClassScope extends ThisScope { +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 = new Map(); + public readonly members: ClassScopeResult['members'] = { + instance: new Map(), + static: new Map(), + }; /** * The node that declares this class. @@ -279,10 +367,10 @@ export class ClassScope extends ThisScope { public constructor( theClass: ClassNode, - upper: ClassScope | FunctionScope | null, + upper: ClassScope | IntermediateScope | null, scopeManager: ScopeManager, ) { - super(scopeManager, upper, 'self'); + super(scopeManager, upper, 'self', false); this.theClass = theClass; this.className = theClass.id?.name ?? null; @@ -299,7 +387,11 @@ export class ClassScope extends ThisScope { if (member == null) { continue; } - this.members.set(member.name, member); + if (member.isStatic()) { + this.members.static.set(member.name, member); + } else { + this.members.instance.set(member.name, member); + } break; } @@ -316,10 +408,25 @@ export class ClassScope extends ThisScope { } export function analyzeClassMemberUsage( - theClass: ClassNode, + program: TSESTree.Program, scopeManager: ScopeManager, -): ClassScope { - const analyzer = new ClassScope(theClass, null, scopeManager); - analyzer.visit(theClass); - return analyzer; +): 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/types.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts index 69db0fa37cfb..481aff1a4a20 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts @@ -1,9 +1,6 @@ import type { TSESTree } from '@typescript-eslint/utils'; export type ClassNode = TSESTree.ClassDeclaration | TSESTree.ClassExpression; -export type FunctionNode = - | TSESTree.FunctionDeclaration - | TSESTree.FunctionExpression; export type MemberNode = | TSESTree.AccessorProperty | TSESTree.MethodDefinition From fe8c3006db55ce40a80d6fd009d398e329dd8175 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 6 Jul 2025 20:09:19 +0930 Subject: [PATCH 05/12] WIP --- .../rules/no-unused-private-class-members.ts | 263 ++---------------- .../classScopeAnalyzer.ts | 44 ++- .../extractComputedName.ts | 21 +- .../no-unused-private-class-members.test.ts | 2 +- 4 files changed, 76 insertions(+), 254 deletions(-) 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 index daed826ebddf..ba3e94cb34b2 100644 --- a/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -1,12 +1,7 @@ -// 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 { ESLintUtils } from '@typescript-eslint/utils'; import { createRule } from '../util'; +import { analyzeClassMemberUsage } from '../util/class-scope-analyzer/classScopeAnalyzer'; type Options = []; export type MessageIds = 'unusedPrivateClassMember'; @@ -23,244 +18,40 @@ export default createRule({ messages: { unusedPrivateClassMember: - "'{{classMemberName}}' is defined but never used.", + "Private class member '{{classMemberName}}' is defined but never used.", }, schema: [], }, defaultOptions: [], create(context) { - interface PrivateMember { - declaredNode: - | TSESTree.AccessorProperty - | TSESTree.MethodDefinitionComputedName - | TSESTree.MethodDefinitionNonComputedName - | TSESTree.PropertyDefinitionComputedName - | TSESTree.PropertyDefinitionNonComputedName; - isAccessor: boolean; - } - 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.AccessorProperty - | 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 == null) { - 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 == null) { - return; - } - - // In case any other usage was already detected, we can short circuit the - // logic here. - const memberDefinition = classBody.get(node.name); - if (memberDefinition == null) { - 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.AccessorProperty || - 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 all usages of the private identifier and remove a member from - // `declaredAndUnusedPrivateMembers` if we deem it used. - PrivateIdentifier(node): void { - processPrivateIdentifier(node); - }, - - // Process all usages of the identifier and remove a member from - // `declaredAndUnusedPrivateMembers` if we deem it used. - MemberExpression(node): void { - if ( - !node.computed && - node.object.type === AST_NODE_TYPES.ThisExpression && - node.property.type === AST_NODE_TYPES.Identifier - ) { - processPrivateIdentifier(node.property); - } - }, - - // 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 == null) { - return; - } + 'Program:exit'(node) { + const result = analyzeClassMemberUsage( + node, + ESLintUtils.nullThrows( + context.sourceCode.scopeManager, + 'Missing required scope manager', + ), + ); - for (const [ - classMemberName, - { declaredNode }, - ] of unusedPrivateMembers.entries()) { - if (trackedClassMembersUsed.has(declaredNode)) { - continue; + for (const classScope of result.values()) { + for (const member of [ + ...classScope.members.instance.values(), + ...classScope.members.static.values(), + ]) { + if (!member.isPrivate() && !member.isHashPrivate()) { + continue; + } + + if (member.referenceCount === 0) { + context.report({ + node: member.node.key, + messageId: 'unusedPrivateClassMember', + data: { classMemberName: member.name }, + }); + } } - context.report({ - loc: declaredNode.key.loc, - node: declaredNode, - messageId: 'unusedPrivateClassMember', - data: { - classMemberName: - declaredNode.key.type === AST_NODE_TYPES.PrivateIdentifier - ? `#${classMemberName}` - : classMemberName, - }, - }); } }, }; diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts index 41d101d90935..1204855f3900 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -19,18 +19,24 @@ export class 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: Key; + public readonly name: string; /** * The number of references to this member. */ public referenceCount = 0; - private constructor(node: MemberNode, name: Key) { + private constructor(node: MemberNode, key: Key, name: string) { this.node = node; + this.key = key; this.name = name; } public static create(node: MemberNode): Member | null { @@ -38,7 +44,11 @@ export class Member { if (name == null) { return null; } - return new Member(node, name); + return new Member(node, ...name); + } + + public isHashPrivate(): boolean { + return this.node.key.type === AST_NODE_TYPES.PrivateIdentifier; } public isPrivate(): boolean { @@ -116,7 +126,10 @@ abstract class ThisScope extends Visitor { if (this.thisContext == null) { return null; } - return { thisContext: this.thisContext, type: 'instance' }; + return { + thisContext: this.thisContext, + type: this.isStaticThisContext ? 'static' : 'instance', + }; } case AST_NODE_TYPES.Identifier: { @@ -149,7 +162,7 @@ abstract class ThisScope extends Visitor { private visitClass(node: ClassNode): void { const classScope = new ClassScope(node, this, this.scopeManager); this.childScopes.push(classScope); - classScope.visit(node); + classScope.visitChildren(node); } private visitIntermediate(node: IntermediateNode): void { @@ -159,7 +172,7 @@ abstract class ThisScope extends Visitor { node, ); this.childScopes.push(intermediateScope); - intermediateScope.visit(node); + intermediateScope.visitChildren(node); } /** @@ -200,6 +213,8 @@ abstract class ThisScope extends Visitor { } 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; @@ -223,7 +238,7 @@ abstract class ThisScope extends Visitor { objectClassName.type === 'instance' ? objectClassName.thisContext.members.instance : objectClassName.thisContext.members.static; - const member = members.get(propertyName); + const member = members.get(propertyName[0]); if (member == null) { return; } @@ -232,6 +247,17 @@ abstract class ThisScope extends Visitor { } 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. // @@ -388,9 +414,9 @@ class ClassScope extends ThisScope implements ClassScopeResult { continue; } if (member.isStatic()) { - this.members.static.set(member.name, member); + this.members.static.set(member.key, member); } else { - this.members.instance.set(member.name, member); + this.members.instance.set(member.key, member); } break; } diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts index 020d3bf1fa6e..3b3aca0acdaa 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts @@ -6,31 +6,36 @@ import type { Key, MemberNode } from './types'; import { privateKey, publicKey } from './types'; -function extractComputedName(computedName: TSESTree.Expression): Key | null { +function extractComputedName( + computedName: TSESTree.Expression, +): [Key, string] | null { if (computedName.type === AST_NODE_TYPES.Literal) { - return publicKey(computedName.value?.toString() ?? 'null'); + const name = computedName.value?.toString() ?? 'null'; + return [publicKey(name), name]; } if ( computedName.type === AST_NODE_TYPES.TemplateLiteral && computedName.expressions.length === 0 ) { - return publicKey(computedName.quasis[0].value.raw); + const name = computedName.quasis[0].value.raw; + return [publicKey(name), name]; } return null; } function extractNonComputedName( nonComputedName: TSESTree.Identifier | TSESTree.PrivateIdentifier, -): Key { +): [Key, string] | null { + const name = nonComputedName.name; if (nonComputedName.type === AST_NODE_TYPES.PrivateIdentifier) { - return privateKey(nonComputedName); + return [privateKey(nonComputedName), `#${name}`]; } - return publicKey(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 | null { +export function extractNameForMember(node: MemberNode): [Key, string] | null { if (node.computed) { return extractComputedName(node.key); } @@ -47,7 +52,7 @@ export function extractNameForMember(node: MemberNode): Key | null { */ export function extractNameForMemberExpression( node: TSESTree.MemberExpression, -): Key | null { +): [Key, string] | null { if (node.computed) { return extractComputedName(node.property); } 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 index b25b947cc7c1..2fa6d9ee9111 100644 --- 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 @@ -254,7 +254,7 @@ class C { #unusedInOuterClass; foo() { - return class { + return class D { #unusedInOuterClass; bar() { From e72b89a34357ff79e502daa12b3eb54d2fae7a3a Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 10 Aug 2025 14:56:49 +0930 Subject: [PATCH 06/12] got it all working --- .../rules/no-unused-private-class-members.ts | 17 +-- .../classScopeAnalyzer.ts | 115 +++++++++++++++++- .../no-unused-private-class-members.test.ts | 2 - 3 files changed, 120 insertions(+), 14 deletions(-) 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 index ba3e94cb34b2..d5bb6e9f2379 100644 --- a/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -40,17 +40,18 @@ export default createRule({ ...classScope.members.instance.values(), ...classScope.members.static.values(), ]) { - if (!member.isPrivate() && !member.isHashPrivate()) { + if ( + (!member.isPrivate() && !member.isHashPrivate()) || + member.isUsed() + ) { continue; } - if (member.referenceCount === 0) { - context.report({ - node: member.node.key, - messageId: 'unusedPrivateClassMember', - data: { classMemberName: member.name }, - }); - } + 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 index 1204855f3900..752b64709396 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -6,6 +6,7 @@ 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, @@ -30,9 +31,14 @@ export class Member { public readonly name: string; /** - * The number of references to this member. + * The number of writes to this member. */ - public referenceCount = 0; + public writeCount = 0; + + /** + * The number of reads from this member. + */ + public readCount = 0; private constructor(node: MemberNode, key: Key, name: string) { this.node = node; @@ -58,6 +64,107 @@ export class Member { 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 = @@ -243,7 +350,7 @@ abstract class ThisScope extends Visitor { return; } - member.referenceCount += 1; + countReference(node, member); } protected PrivateIdentifier(node: TSESTree.PrivateIdentifier): void { @@ -277,7 +384,7 @@ abstract class ThisScope extends Visitor { currentScope.thisContext.members.instance.get(key) ?? currentScope.thisContext.members.static.get(key); if (member != null) { - member.referenceCount += 1; + countReference(node.parent, member); return; } } 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 index 2fa6d9ee9111..73eef14eb20f 100644 --- 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 @@ -82,8 +82,6 @@ class C { } } `, - // TODO make this work - skip: true, }, ...[ From db7e2a819ecae03602ac88f92898c1c3a3483327 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 20 Oct 2025 21:01:15 +1030 Subject: [PATCH 07/12] support parameter properties --- .../rules/no-unused-private-class-members.ts | 2 +- .../classScopeAnalyzer.ts | 43 ++++++++++++-- .../extractComputedName.ts | 57 ++++++++++++++++--- .../src/util/class-scope-analyzer/types.ts | 3 +- .../no-unused-private-class-members.test.ts | 48 ++++++++++++++++ .../no-unused-private-class-members.shot | 10 ++++ 6 files changed, 148 insertions(+), 15 deletions(-) create mode 100644 packages/eslint-plugin/tests/schema-snapshots/no-unused-private-class-members.shot 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 index d5bb6e9f2379..d120184d6e5a 100644 --- a/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts +++ b/packages/eslint-plugin/src/rules/no-unused-private-class-members.ts @@ -48,7 +48,7 @@ export default createRule({ } context.report({ - node: member.node.key, + node: member.nameNode, 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 index 752b64709396..6b5f2dcdad0e 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -30,6 +30,12 @@ export class Member { */ public readonly name: string; + /** + * The node that represents the member name in the source code. + * Used for reporting errors. + */ + public readonly nameNode: TSESTree.Node; + /** * The number of writes to this member. */ @@ -40,21 +46,30 @@ export class Member { */ public readCount = 0; - private constructor(node: MemberNode, key: Key, name: string) { + private constructor( + node: MemberNode, + key: Key, + name: string, + nameNode: TSESTree.Node, + ) { this.node = node; this.key = key; this.name = name; + this.nameNode = nameNode; } public static create(node: MemberNode): Member | null { const name = extractNameForMember(node); if (name == null) { return null; } - return new Member(node, ...name); + return new Member(node, name.key, name.codeName, name.nameNode); } public isHashPrivate(): boolean { - return this.node.key.type === AST_NODE_TYPES.PrivateIdentifier; + return ( + 'key' in this.node && + this.node.key.type === AST_NODE_TYPES.PrivateIdentifier + ); } public isPrivate(): boolean { @@ -345,7 +360,7 @@ abstract class ThisScope extends Visitor { objectClassName.type === 'instance' ? objectClassName.thisContext.members.instance : objectClassName.thisContext.members.static; - const member = members.get(propertyName[0]); + const member = members.get(propertyName.key); if (member == null) { return; } @@ -510,8 +525,26 @@ class ClassScope extends ThisScope implements ClassScopeResult { for (const memberNode of theClass.body.body) { switch (memberNode.type) { - case AST_NODE_TYPES.AccessorProperty: case AST_NODE_TYPES.MethodDefinition: + if (memberNode.kind === 'constructor') { + for (let parameter of memberNode.value.params) { + if (parameter.type !== AST_NODE_TYPES.TSParameterProperty) { + continue; + } + + const member = Member.create(parameter); + if (member == null) { + continue; + } + + this.members.instance.set(member.key, member); + } + + // break instead of falling through because the constructor is not a "member" we track + break; + } + // intentional fall through + case AST_NODE_TYPES.AccessorProperty: case AST_NODE_TYPES.PropertyDefinition: case AST_NODE_TYPES.TSAbstractAccessorProperty: case AST_NODE_TYPES.TSAbstractMethodDefinition: diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts index 3b3aca0acdaa..dcb2329299bc 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts @@ -6,36 +6,77 @@ import type { Key, MemberNode } from './types'; import { privateKey, publicKey } from './types'; +export type ExtractedName = { + key: Key; + codeName: string; + nameNode: TSESTree.Node; +}; + function extractComputedName( computedName: TSESTree.Expression, -): [Key, string] | null { +): ExtractedName | null { if (computedName.type === AST_NODE_TYPES.Literal) { const name = computedName.value?.toString() ?? 'null'; - return [publicKey(name), name]; + return { + key: publicKey(name), + codeName: name, + nameNode: computedName, + }; } if ( computedName.type === AST_NODE_TYPES.TemplateLiteral && computedName.expressions.length === 0 ) { const name = computedName.quasis[0].value.raw; - return [publicKey(name), name]; + return { + key: publicKey(name), + codeName: name, + nameNode: computedName, + }; } return null; } function extractNonComputedName( nonComputedName: TSESTree.Identifier | TSESTree.PrivateIdentifier, -): [Key, string] | null { +): ExtractedName | null { const name = nonComputedName.name; if (nonComputedName.type === AST_NODE_TYPES.PrivateIdentifier) { - return [privateKey(nonComputedName), `#${name}`]; + return { + key: privateKey(nonComputedName), + codeName: `#${name}`, + nameNode: nonComputedName, + }; } - return [publicKey(name), name]; + return { + key: publicKey(name), + codeName: name, + nameNode: nonComputedName, + }; } /** * 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 { +export function extractNameForMember(node: MemberNode): ExtractedName | null { + if (node.type === AST_NODE_TYPES.TSParameterProperty) { + switch (node.parameter.type) { + case AST_NODE_TYPES.ArrayPattern: + case AST_NODE_TYPES.ObjectPattern: + case AST_NODE_TYPES.RestElement: + // Nonsensical properties -- see https://github.com/typescript-eslint/typescript-eslint/issues/11708 + return null; + + case AST_NODE_TYPES.AssignmentPattern: + if (node.parameter.left.type !== AST_NODE_TYPES.Identifier) { + return null; + } + return extractNonComputedName(node.parameter.left); + + case AST_NODE_TYPES.Identifier: + return extractNonComputedName(node.parameter); + } + } + if (node.computed) { return extractComputedName(node.key); } @@ -52,7 +93,7 @@ export function extractNameForMember(node: MemberNode): [Key, string] | null { */ export function extractNameForMemberExpression( node: TSESTree.MemberExpression, -): [Key, string] | null { +): ExtractedName | null { if (node.computed) { return extractComputedName(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 index 481aff1a4a20..30a7da7fcbb6 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts @@ -7,7 +7,8 @@ export type MemberNode = | TSESTree.PropertyDefinition | TSESTree.TSAbstractAccessorProperty | TSESTree.TSAbstractMethodDefinition - | TSESTree.TSAbstractPropertyDefinition; + | TSESTree.TSAbstractPropertyDefinition + | TSESTree.TSParameterProperty; export type PrivateKey = string & { __brand: 'private-key' }; export type PublicKey = string & { __brand: 'public-key' }; export type Key = PrivateKey | PublicKey; 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 index 73eef14eb20f..63e6aa8d886d 100644 --- 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 @@ -83,6 +83,30 @@ class C { } `, }, + ` +class Test1 { + constructor(private parameterProperty: number) {} + method() { + return this.parameterProperty; + } +} + `, + ` +class Test1 { + constructor(private readonly parameterProperty: number) {} + method() { + return this.parameterProperty; + } +} + `, + ` +class Test1 { + constructor(private readonly parameterProperty: number = 1) {} + method() { + return this.parameterProperty; + } +} + `, ...[ ` @@ -335,6 +359,30 @@ class C { `, errors: [definedError('staticMember', false)], }, + { + code: ` +class Test1 { + constructor(private parameterProperty: number) {} +} + `, + errors: [definedError('parameterProperty', false)], + }, + { + code: ` +class Test1 { + constructor(private readonly parameterProperty: number) {} +} + `, + errors: [definedError('parameterProperty', false)], + }, + { + code: ` +class Test1 { + constructor(private readonly parameterProperty: number = 1) {} +} + `, + errors: [definedError('parameterProperty', false)], + }, // intentionally not handled cases { diff --git a/packages/eslint-plugin/tests/schema-snapshots/no-unused-private-class-members.shot b/packages/eslint-plugin/tests/schema-snapshots/no-unused-private-class-members.shot new file mode 100644 index 000000000000..cdd9f8375858 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/no-unused-private-class-members.shot @@ -0,0 +1,10 @@ + +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = []; From 73a1c8e2fff5a6e0f0af75048c4a6a32da13d68a Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 20 Oct 2025 22:00:54 +1030 Subject: [PATCH 08/12] handle some simple scope analysis cases --- .../rules/no-unused-private-class-members.mdx | 43 +++++- .../classScopeAnalyzer.ts | 143 ++++++++++++++++-- .../no-unused-private-class-members.test.ts | 61 ++++++-- 3 files changed, 223 insertions(+), 24 deletions(-) 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 index aad7b5bac464..1b96484955fa 100644 --- a/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx +++ b/packages/eslint-plugin/docs/rules/no-unused-private-class-members.mdx @@ -47,19 +47,25 @@ class A { This rule does not detect the following cases: -(1) Private members only used using private only without `this`: +(1) Private members only used via a variable that would require type analysis to resolve the type. ```ts +type T = Foo; class Foo { private prop = 123; - method(a: Foo) { + method1(a: Foo) { // ✅ Detected as a usage const prop = this.prop; - // ❌ NOT detected as a usage + // ✅ Detected as a usage (variables with explicit and simple type annotations are handled) const otherProp = a.prop; } + + method2(a: T) { + // ❌ NOT detected as a usage (complex type annotation that requires type information to handle) + const prop = a.prop; + } } ``` @@ -75,6 +81,37 @@ const instance = new Foo(); console.log(foo['prop']); ``` +(3) Reassignments of `this` multiple times: + +```ts +class Foo { + private prop = 123; + + foo() { + const self1 = this; + const self2 = self1; + return self2.prop; + } +} +``` + +(4) Mutable reassignments of `this`: + +```ts +class Foo { + private prop = 123; + private parent: Foo | null; + + foo() { + let self = this; + while (self.parent != null) { + self = self.parent; + } + return self.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/util/class-scope-analyzer/classScopeAnalyzer.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts index 6b5f2dcdad0e..881d32f8f513 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -1,12 +1,16 @@ /* 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 { + Scope, + ScopeManager, + Variable, +} 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 { findVariable, nullThrows, NullThrowsReasons } from '..'; import { extractNameForMember, @@ -239,6 +243,38 @@ abstract class ThisScope extends Visitor { } } + private findNearestScope(node: TSESTree.Node): Scope | null { + let currentScope = null; + let currentNode = node; + let i = 0; + do { + currentScope = this.scopeManager.acquire(currentNode); + if (currentNode?.parent == null) { + break; + } + currentNode = currentNode?.parent; + } while (currentScope == null); + return currentScope; + } + private findVariableInScope( + node: TSESTree.Node, + name: string, + ): Variable | null { + let currentScope = this.findNearestScope(node); + let variable = null; + + while (currentScope != null) { + variable = currentScope.set.get(name) ?? null; + if (variable != null) { + break; + } + + currentScope = currentScope.upper; + } + + return variable; + } + private getObjectClass(node: TSESTree.MemberExpression): { thisContext: ThisScope['thisContext']; type: 'instance' | 'static'; @@ -260,16 +296,100 @@ abstract class ThisScope extends Visitor { 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; - // } - // } + // the following code does some very rudimentary scope analysis to handle some trivial cases + const variable = this.findVariableInScope(node, node.object.name); + if (variable == null || variable.defs.length === 0) { + return null; + } + + const firstDef = variable.defs[0]; + switch (firstDef.node.type) { + // detect simple reassignment of `this` + // ``` + // class Foo { + // private prop: number; + // method(thing: Foo) { + // const self = this; + // return self.prop; + // } + // } + // ``` + case AST_NODE_TYPES.VariableDeclarator: { + const value = firstDef.node.init; + if (value == null || value.type !== AST_NODE_TYPES.ThisExpression) { + return null; + } + + if ( + variable.references.filter( + ref => ref.isWrite() && ref.init !== true, + ).length > 0 + ) { + // variable is assigned to multiple times so we can't be sure that it's still the same class + return null; + } + + // we have a case like `const self = this` or `let self = this` that is not reassigned + // so we can safely assume that it's still the same class! + return { + thisContext: this.thisContext, + type: this.isStaticThisContext ? 'static' : 'instance', + }; + } + + // Look for variables typed as the current class: + // ``` + // 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; + // } + // } + // ``` + default: { + const typeAnnotation = (() => { + if ( + 'typeAnnotation' in firstDef.name && + firstDef.name.typeAnnotation != null && + firstDef.name.typeAnnotation.type === + AST_NODE_TYPES.TSTypeAnnotation + ) { + return firstDef.name.typeAnnotation.typeAnnotation; + } + + return null; + })(); + + if (typeAnnotation == null) { + return null; + } + + // Cases like `method(thing: Foo) { ... }` + if ( + typeAnnotation.type === AST_NODE_TYPES.TSTypeReference && + typeAnnotation.typeName.type === AST_NODE_TYPES.Identifier + ) { + const typeName = typeAnnotation.typeName.name; + const typeScope = this.findClassScopeWithName(typeName); + if (typeScope != null) { + return { thisContext: typeScope, type: 'instance' }; + } + } + // Cases like `method(thing: typeof Foo) { ... }` + if ( + typeAnnotation.type === AST_NODE_TYPES.TSTypeQuery && + typeAnnotation.exprName.type === AST_NODE_TYPES.Identifier + ) { + const exprName = typeAnnotation.exprName.name; + const exprScope = this.findClassScopeWithName(exprName); + if (exprScope != null) { + return { thisContext: exprScope, type: 'static' }; + } + } + } + } return null; } case AST_NODE_TYPES.MemberExpression: @@ -460,6 +580,7 @@ class IntermediateScope extends ThisScope { // function with a `this` parameter if ( upper != null && + node.params.length > 0 && node.params[0].type === AST_NODE_TYPES.Identifier && node.params[0].name === 'this' ) { 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 index 63e6aa8d886d..6ccf6adcc984 100644 --- 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 @@ -107,6 +107,34 @@ class Test1 { } } `, + ` +class Foo { + private prop: number; + + method(thing: Foo) { + return thing.prop; + } +} + `, + ` +class Foo { + private static staticProp: number; + + method(thing: typeof Foo) { + return thing.staticProp; + } +} + `, + ` +class Foo { + private prop: number; + + method() { + const self = this; + return self.prop; + } +} + `, ...[ ` @@ -384,32 +412,45 @@ class Test1 { errors: [definedError('parameterProperty', false)], }, - // intentionally not handled cases { + // usage of a property outside the class code: ` class C { - private usedInInnerClass; - - method(a: C) { - return class { - foo = a.usedInInnerClass; - }; - } + private usedOutsideClass; } + +const instance = new C(); +console.log(instance.usedOutsideClass); `, - errors: [definedError('usedInInnerClass', false)], + errors: [definedError('usedOutsideClass', false)], }, { + // usage of a property outside the class code: ` class C { private usedOutsideClass; } const instance = new C(); -console.log(instance.usedOutsideClass); +console.log(instance['usedOutsideClass']); `, errors: [definedError('usedOutsideClass', false)], }, + { + // too much indirection so we just bail + code: ` + class Foo { + private prop: number; + + method() { + const self1 = this; + const self2 = self1; + return self2.prop; + } + } + `, + errors: [definedError('prop', false)], + }, ...[ { From 73a1f530a5686eec9039e3e6e96ffbfcc5d8b182 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 20 Oct 2025 22:03:07 +1030 Subject: [PATCH 09/12] comment --- .../src/util/class-scope-analyzer/extractComputedName.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts index dcb2329299bc..497ed27f35da 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts @@ -55,7 +55,7 @@ function extractNonComputedName( } /** * Extracts the string name for a member. - * @returns `null` if the name cannot be extracted due to it being a computed. + * @returns `null` if the name cannot be extracted due to it being computed. */ export function extractNameForMember(node: MemberNode): ExtractedName | null { if (node.type === AST_NODE_TYPES.TSParameterProperty) { From d3077cba27fd2a2bf5bd15cc3ba4aded6775d45b Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 11 Nov 2025 11:17:28 -0500 Subject: [PATCH 10/12] lint --- .../classScopeAnalyzer.ts | 55 +- .../extractComputedName.ts | 12 +- .../no-unused-private-class-members.test.ts | 1046 +++++++++++++---- 3 files changed, 844 insertions(+), 269 deletions(-) diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts index 881d32f8f513..b72ece7f9a25 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/classScopeAnalyzer.ts @@ -10,8 +10,8 @@ import { Visitor } from '@typescript-eslint/scope-manager'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import type { ClassNode, Key, MemberNode } from './types'; -import { findVariable, nullThrows, NullThrowsReasons } from '..'; +import { nullThrows, NullThrowsReasons } from '..'; import { extractNameForMember, extractNameForMemberExpression, @@ -69,6 +69,20 @@ export class Member { return new Member(node, name.key, name.codeName, name.nameNode); } + 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 isHashPrivate(): boolean { return ( 'key' in this.node && @@ -84,20 +98,6 @@ export class Member { 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 || @@ -172,13 +172,13 @@ function countReference(identifierParent: TSESTree.Node, member: Member) { } // [...this.#unusedInRestPattern] = bar; - if (identifierGrandparent.type === 'RestElement') { + if (identifierGrandparent.type === AST_NODE_TYPES.RestElement) { member.writeCount += 1; return; } // [this.#unusedInAssignmentPattern] = bar; - if (identifierGrandparent.type === 'ArrayPattern') { + if (identifierGrandparent.type === AST_NODE_TYPES.ArrayPattern) { member.writeCount += 1; return; } @@ -244,15 +244,14 @@ abstract class ThisScope extends Visitor { } private findNearestScope(node: TSESTree.Node): Scope | null { - let currentScope = null; - let currentNode = node; - let i = 0; + let currentScope: Scope | null | undefined; + let currentNode: TSESTree.Node | undefined = node; do { currentScope = this.scopeManager.acquire(currentNode); - if (currentNode?.parent == null) { + if (currentNode.parent == null) { break; } - currentNode = currentNode?.parent; + currentNode = currentNode.parent; } while (currentScope == null); return currentScope; } @@ -321,9 +320,9 @@ abstract class ThisScope extends Visitor { } if ( - variable.references.filter( + variable.references.some( ref => ref.isWrite() && ref.init !== true, - ).length > 0 + ) ) { // variable is assigned to multiple times so we can't be sure that it's still the same class return null; @@ -351,9 +350,7 @@ abstract class ThisScope extends Visitor { const typeAnnotation = (() => { if ( 'typeAnnotation' in firstDef.name && - firstDef.name.typeAnnotation != null && - firstDef.name.typeAnnotation.type === - AST_NODE_TYPES.TSTypeAnnotation + firstDef.name.typeAnnotation != null ) { return firstDef.name.typeAnnotation.typeAnnotation; } @@ -648,7 +645,7 @@ class ClassScope extends ThisScope implements ClassScopeResult { switch (memberNode.type) { case AST_NODE_TYPES.MethodDefinition: if (memberNode.kind === 'constructor') { - for (let parameter of memberNode.value.params) { + for (const parameter of memberNode.value.params) { if (parameter.type !== AST_NODE_TYPES.TSParameterProperty) { continue; } @@ -664,7 +661,7 @@ class ClassScope extends ThisScope implements ClassScopeResult { // break instead of falling through because the constructor is not a "member" we track break; } - // intentional fall through + // intentional fallthrough case AST_NODE_TYPES.AccessorProperty: case AST_NODE_TYPES.PropertyDefinition: case AST_NODE_TYPES.TSAbstractAccessorProperty: diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts index 497ed27f35da..e100cce75900 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/extractComputedName.ts @@ -6,11 +6,11 @@ import type { Key, MemberNode } from './types'; import { privateKey, publicKey } from './types'; -export type ExtractedName = { +export interface ExtractedName { key: Key; codeName: string; nameNode: TSESTree.Node; -}; +} function extractComputedName( computedName: TSESTree.Expression, @@ -18,8 +18,8 @@ function extractComputedName( if (computedName.type === AST_NODE_TYPES.Literal) { const name = computedName.value?.toString() ?? 'null'; return { - key: publicKey(name), codeName: name, + key: publicKey(name), nameNode: computedName, }; } @@ -29,8 +29,8 @@ function extractComputedName( ) { const name = computedName.quasis[0].value.raw; return { - key: publicKey(name), codeName: name, + key: publicKey(name), nameNode: computedName, }; } @@ -42,14 +42,14 @@ function extractNonComputedName( const name = nonComputedName.name; if (nonComputedName.type === AST_NODE_TYPES.PrivateIdentifier) { return { - key: privateKey(nonComputedName), codeName: `#${name}`, + key: privateKey(nonComputedName), nameNode: nonComputedName, }; } return { - key: publicKey(name), codeName: name, + key: publicKey(name), nameNode: nonComputedName, }; } 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 index 6ccf6adcc984..342d472e4499 100644 --- 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 @@ -2,29 +2,12 @@ // 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 {}', @@ -136,166 +119,312 @@ class Foo { } `, - ...[ - ` + ` class Foo { - @# = 42; + #privateMember = 42; method() { - return this.#; + return this.#privateMember; } } - `, - ` + `, + ` class Foo { - @# = 42; - anotherMember = this.#; + private privateMember = 42; + method() { + return this.privateMember; + } } - `, - ` + `, + ` +class Foo { + #privateMember = 42; + anotherMember = this.#privateMember; +} + `, + ` +class Foo { + private privateMember = 42; + anotherMember = this.privateMember; +} + `, + ` class Foo { - @# = 42; + #privateMember = 42; foo() { - anotherMember = this.#; + anotherMember = this.#privateMember; } } - `, - ` + `, + ` +class Foo { + private privateMember = 42; + foo() { + anotherMember = this.privateMember; + } +} + `, + ` class C { - @#; + #privateMember; foo() { - bar((this.# += 1)); + bar((this.#privateMember += 1)); } } - `, - ` + `, + ` +class C { + private privateMember; + + foo() { + bar((this.privateMember += 1)); + } +} + `, + ` class Foo { - @# = 42; + #privateMember = 42; method() { - return someGlobalMethod(this.#); + return someGlobalMethod(this.#privateMember); } } - `, - ` + `, + ` +class Foo { + private privateMember = 42; + method() { + return someGlobalMethod(this.privateMember); + } +} + `, + ` class C { - @#; + #privateMember; foo() { return class {}; } bar() { - return this.#; + return this.#privateMember; } } - `, - ` + `, + ` +class C { + private privateMember; + + foo() { + return class {}; + } + + bar() { + return this.privateMember; + } +} + `, + ` class Foo { - @#; + #privateMember; method() { - for (const bar in this.#) { + for (const bar in this.#privateMember) { } } } - `, - ` + `, + ` class Foo { - @#; + private privateMember; method() { - for (const bar of this.#) { + for (const bar in this.privateMember) { } } } - `, - ` + `, + ` class Foo { - @#; + #privateMember; method() { - [bar = 1] = this.#; + for (const bar of this.#privateMember) { + } } } - `, - ` + `, + ` class Foo { - @#; + private privateMember; method() { - [bar] = this.#; + for (const bar of this.privateMember) { + } } } - `, - ` + `, + ` +class Foo { + #privateMember; + method() { + [bar = 1] = this.#privateMember; + } +} + `, + ` +class Foo { + private privateMember; + method() { + [bar = 1] = this.privateMember; + } +} + `, + ` +class Foo { + #privateMember; + method() { + [bar] = this.#privateMember; + } +} + `, + ` +class Foo { + private privateMember; + method() { + [bar] = this.privateMember; + } +} + `, + ` class C { - @#; + #privateMember; method() { - ({ [this.#]: a } = foo); + ({ [this.#privateMember]: a } = foo); } } - `, - ` + `, + ` +class C { + private privateMember; + + method() { + ({ [this.privateMember]: a } = foo); + } +} + `, + ` class C { - @set #(value) { + set #privateMember(value) { doSomething(value); } - @get #() { + get #privateMember() { return something(); } method() { - this.# += 1; + this.#privateMember += 1; } } - `, - ` + `, + ` +class C { + private set privateMember(value) { + doSomething(value); + } + private get privateMember() { + return something(); + } + method() { + this.privateMember += 1; + } +} + `, + ` class Foo { - @set #(value) {} + set #privateMember(value) {} method(a) { - [this.#] = a; + [this.#privateMember] = a; } } - `, - ` + `, + ` +class Foo { + private set privateMember(value) {} + + method(a) { + [this.privateMember] = a; + } +} + `, + ` class C { - @get #() { + get #privateMember() { return something(); } - @set #(value) { + set #privateMember(value) { doSomething(value); } method() { - this.# += 1; + this.#privateMember += 1; } } - `, + `, + ` +class C { + private get privateMember() { + return something(); + } + private set privateMember(value) { + doSomething(value); + } + method() { + this.privateMember += 1; + } +} + `, - //-------------------------------------------------------------------------- - // Method definitions - //-------------------------------------------------------------------------- - ` + //-------------------------------------------------------------------------- + // Method definitions + //-------------------------------------------------------------------------- + ` class Foo { - @#() { + #privateMember() { return 42; } anotherMethod() { - return this.#(); + return this.#privateMember(); } } - `, - ` + `, + ` +class Foo { + private privateMember() { + return 42; + } + anotherMethod() { + return this.privateMember(); + } +} + `, + ` class C { - @set #(value) { + set #privateMember(value) { doSomething(value); } foo() { - this.# = 1; + this.#privateMember = 1; } } - `, - ].flatMap(code => [ - code.replaceAll('#', '#privateMember').replaceAll('@', ''), - code.replaceAll('#', 'privateMember').replaceAll('@', 'private '), - ]), + `, + ` +class C { + private set privateMember(value) { + doSomething(value); + } + + foo() { + this.privateMember = 1; + } +} + `, ], invalid: [ { @@ -314,7 +443,14 @@ class C { } } `, - errors: [definedError('unusedInOuterClass')], + errors: [ + { + data: { + classMemberName: '#unusedInOuterClass', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { code: ` @@ -342,7 +478,14 @@ class C { } } `, - errors: [definedError('unusedOnlyInSecondNestedClass')], + errors: [ + { + data: { + classMemberName: '#unusedOnlyInSecondNestedClass', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { code: ` @@ -366,8 +509,11 @@ class C { `, errors: [ { - ...definedError('usedOnlyInTheSecondInnerClass'), + data: { + classMemberName: '#usedOnlyInTheSecondInnerClass', + }, line: 3, + messageId: 'unusedPrivateClassMember', }, ], }, @@ -377,7 +523,14 @@ class C { private accessor accessorMember = 42; } `, - errors: [definedError('accessorMember', false)], + errors: [ + { + data: { + classMemberName: 'accessorMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { code: ` @@ -385,7 +538,14 @@ class C { private static staticMember = 42; } `, - errors: [definedError('staticMember', false)], + errors: [ + { + data: { + classMemberName: 'staticMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { code: ` @@ -393,7 +553,14 @@ class Test1 { constructor(private parameterProperty: number) {} } `, - errors: [definedError('parameterProperty', false)], + errors: [ + { + data: { + classMemberName: 'parameterProperty', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { code: ` @@ -401,7 +568,14 @@ class Test1 { constructor(private readonly parameterProperty: number) {} } `, - errors: [definedError('parameterProperty', false)], + errors: [ + { + data: { + classMemberName: 'parameterProperty', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { code: ` @@ -409,7 +583,14 @@ class Test1 { constructor(private readonly parameterProperty: number = 1) {} } `, - errors: [definedError('parameterProperty', false)], + errors: [ + { + data: { + classMemberName: 'parameterProperty', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { @@ -422,7 +603,14 @@ class C { const instance = new C(); console.log(instance.usedOutsideClass); `, - errors: [definedError('usedOutsideClass', false)], + errors: [ + { + data: { + classMemberName: 'usedOutsideClass', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { // usage of a property outside the class @@ -434,221 +622,611 @@ class C { const instance = new C(); console.log(instance['usedOutsideClass']); `, - errors: [definedError('usedOutsideClass', false)], + errors: [ + { + data: { + classMemberName: 'usedOutsideClass', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, { // too much indirection so we just bail code: ` - class Foo { - private prop: number; +class Foo { + private prop: number; - method() { - const self1 = this; - const self2 = self1; - return self2.prop; - } + method() { + const self1 = this; + const self2 = self1; + return self2.prop; } +} `, - errors: [definedError('prop', false)], + errors: [ + { + data: { + classMemberName: 'prop', + }, + messageId: 'unusedPrivateClassMember', + }, + ], }, - ...[ - { - code: ` + { + code: ` class Foo { - @# = 5; -} - `, - errors: [definedError('#', false)], - }, - { - code: ` -class First {} -class Second { - @# = 5; + #privateMember = 5; } `, - errors: [definedError('#', false)], - }, - { - code: ` -class First { - @# = 5; + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember = 5; } -class Second {} `, - errors: [definedError('#', false)], - }, - { - code: ` -class First { - @# = 5; - @#2 = 5; + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class First {} +class Second { + #privateMember = 5; +} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class First {} +class Second { + private privateMember = 5; } `, - errors: [definedError('#', false), definedError('#2', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class First { + #privateMember = 5; +} +class Second {} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class First { + private privateMember = 5; +} +class Second {} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class First { + #privateMember = 5; + #privateMember2 = 5; +} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + { + data: { + classMemberName: '#privateMember2', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class First { + private privateMember = 5; + private privateMember2 = 5; +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + { + data: { + classMemberName: 'privateMember2', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @# = 5; + #privateMember = 5; method() { - this.# = 42; + this.#privateMember = 42; } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember = 5; + method() { + this.privateMember = 42; + } +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @# = 5; + #privateMember = 5; method() { - this.# += 42; + this.#privateMember += 42; + } +} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember = 5; + method() { + this.privateMember += 42; + } +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class C { + #privateMember; + + foo() { + this.#privateMember++; } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class C { - @#; + private privateMember; foo() { - this.#++; + this.privateMember++; } } `, - errors: [definedError('#', false)], - }, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, - //-------------------------------------------------------------------------- - // Unused method definitions - //-------------------------------------------------------------------------- - { - code: ` + //-------------------------------------------------------------------------- + // Unused method definitions + //-------------------------------------------------------------------------- + { + code: ` +class Foo { + #privateMember() {} +} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember() {} +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#() {} + #privateMember() {} + #privateMemberUsed() { + return 42; + } + publicMethod() { + return this.#privateMemberUsed(); + } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#() {} - @#Used() { + private privateMember() {} + private privateMemberUsed() { return 42; } publicMethod() { - return this.#Used(); + return this.privateMemberUsed(); } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @set #(value) {} + set #privateMember(value) {} } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#; + private set privateMember(value) {} +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + #privateMember; method() { - for (this.# in bar) { + for (this.#privateMember in bar) { } } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#; + private privateMember; method() { - for (this.# of bar) { + for (this.privateMember in bar) { } } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#; + #privateMember; method() { - ({ x: this.# } = bar); + for (this.#privateMember of bar) { + } } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#; + private privateMember; method() { - [...this.#] = bar; + for (this.privateMember of bar) { + } } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#; + #privateMember; method() { - [this.# = 1] = bar; + ({ x: this.#privateMember } = bar); } } `, - errors: [definedError('#', false)], - }, - { - code: ` + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember; + method() { + ({ x: this.privateMember } = bar); + } +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + #privateMember; + method() { + [...this.#privateMember] = bar; + } +} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember; + method() { + [...this.privateMember] = bar; + } +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` class Foo { - @#; + #privateMember; method() { - [this.#] = bar; + [this.#privateMember = 1] = bar; } } `, - errors: [definedError('#', false)], - }, - ].flatMap(({ code, errors }) => [ - { - code: code.replaceAll('#', '#privateMember').replaceAll('@', ''), - errors: errors.map(error => ({ - ...error, + errors: [ + { data: { - classMemberName: (error.data?.classMemberName as string).replaceAll( - '#', - '#privateMember', - ), + classMemberName: '#privateMember', }, - })), - }, - { - code: code.replaceAll('#', 'privateMember').replaceAll('@', 'private '), - errors: errors.map(error => ({ - ...error, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember; + method() { + [this.privateMember = 1] = bar; + } +} + `, + errors: [ + { data: { - classMemberName: (error.data?.classMemberName as string).replaceAll( - '#', - 'privateMember', - ), + classMemberName: 'privateMember', }, - })), - }, - ]), + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + #privateMember; + method() { + [this.#privateMember] = bar; + } +} + `, + errors: [ + { + data: { + classMemberName: '#privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, + { + code: ` +class Foo { + private privateMember; + method() { + [this.privateMember] = bar; + } +} + `, + errors: [ + { + data: { + classMemberName: 'privateMember', + }, + messageId: 'unusedPrivateClassMember', + }, + ], + }, ], }); From 2ccb6387b4f5f08fa7942ab09e44a952102959a3 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 11 Nov 2025 11:19:30 -0500 Subject: [PATCH 11/12] test --- .../no-unused-private-class-members.shot | 14 +++----------- .../rules/no-unused-private-class-members.test.ts | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) 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 index 1bea7a556d71..5e4a1df2eefd 100644 --- 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 @@ -1,17 +1,11 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Validating rule docs no-unused-private-class-members.mdx code examples ESLint output 1`] = ` -"Incorrect +Incorrect class A { private foo = 123; - ~~~ 'foo' is defined but never used. + ~~~ Private class member 'foo' is defined but never used. } -" -`; -exports[`Validating rule docs no-unused-private-class-members.mdx code examples ESLint output 2`] = ` -"Correct +Correct class A { private foo = 123; @@ -20,5 +14,3 @@ class A { 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 index 342d472e4499..c6541fc7464c 100644 --- 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 @@ -678,7 +678,7 @@ class Foo { errors: [ { data: { - classMemberName: '#privateMember', + classMemberName: 'privateMember', }, messageId: 'unusedPrivateClassMember', }, From f7beddc4ddfa7ce64b970189addc550c1a100556 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 11 Nov 2025 11:21:50 -0500 Subject: [PATCH 12/12] review --- packages/eslint-plugin/src/util/class-scope-analyzer/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts index 30a7da7fcbb6..72c308a6166f 100644 --- a/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts +++ b/packages/eslint-plugin/src/util/class-scope-analyzer/types.ts @@ -13,8 +13,8 @@ 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 publicKey(name: string): PublicKey { + return name as PublicKey; } export function privateKey(node: TSESTree.PrivateIdentifier): PrivateKey { return `#private@@${node.name}` as PrivateKey;