Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Add no-unused-private-class-members rule (fixes #14859) #14895

Merged
merged 12 commits into from Oct 22, 2021
1 change: 1 addition & 0 deletions conf/eslint-recommended.js
Expand Up @@ -64,6 +64,7 @@ module.exports = {
"no-unsafe-negation": "error",
"no-unsafe-optional-chaining": "error",
"no-unused-labels": "error",
"no-unused-private-class-members": "error",
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
"no-unused-vars": "error",
"no-useless-backreference": "error",
"no-useless-catch": "error",
Expand Down
57 changes: 57 additions & 0 deletions docs/rules/no-unused-private-class-members.md
@@ -0,0 +1,57 @@
# Disallow Unused Private Class Members (no-unused-private-class-members)

Private class members that are declared and not used anywhere in the code are most likely an error due to incomplete refactoring. Such class members take up space in the code and can lead to confusion by readers.

## Rule Details

A private class member `#foo` is considered to be used if any of the following are true:

* It is called (`this.#foo()`)
* It is read (`this.#foo`)
* It is passed into a function as an argument (`doSomething(this.#foo)`)
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved

Examples of **incorrect** code for this rule:

```js
/*eslint no-unused-private-class-members: "error"*/
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
class Foo {
#unusedMember = 5;
}

class Foo {
#usedOnlyInWrite = 5;
method() {
this.#usedOnlyInWrite = 42;
}
}

TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
class Foo {
#unusedMethod() {}
}
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
```

Examples of **correct** code for this rule:

```js
/*eslint no-unused-private-class-members: "error"*/

class Foo {
#usedMember = 42;
method() {
return this.#usedMember;
}
}

class Foo {
#usedMethod() {
return 42;
}
anotherMethod() {
return this.#usedMethod();
}
}
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
```

## 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.
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -221,6 +221,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
"no-unsafe-optional-chaining": () => require("./no-unsafe-optional-chaining"),
"no-unused-expressions": () => require("./no-unused-expressions"),
"no-unused-labels": () => require("./no-unused-labels"),
"no-unused-private-class-members": () => require("./no-unused-private-class-members"),
"no-unused-vars": () => require("./no-unused-vars"),
"no-use-before-define": () => require("./no-use-before-define"),
"no-useless-backreference": () => require("./no-useless-backreference"),
Expand Down
88 changes: 88 additions & 0 deletions lib/rules/no-unused-private-class-members.js
@@ -0,0 +1,88 @@
/**
* @fileoverview Rule to flag declared but unused private class members
* @author Tim van der Lippe
*/

"use strict";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
type: "problem",

docs: {
description: "disallow unused private class members",
category: "Variables",
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
recommended: true,
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
url: "https://eslint.org/docs/rules/no-unused-private-class-members"
},

schema: [],

messages: {
unusedPrivateClassMember: "'{{classMemberName}}' is defined but never used."
}
},

create(context) {

const declaredAndUnusedPrivateMembers = new Map();

//--------------------------------------------------------------------------
// Public
//--------------------------------------------------------------------------

return {

// Collect all declared members up front and assume they are all unused
ClassBody(classBodyNode) {
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
for (const bodyMember of classBodyNode.body) {
if (bodyMember.type === "PropertyDefinition" || bodyMember.type === "MethodDefinition") {
if (bodyMember.key.type === "PrivateIdentifier") {
declaredAndUnusedPrivateMembers.set(bodyMember.key.name, bodyMember);
}
}
}
},

/*
* Process all usages of the private identifier and remove a member from
* `declaredAndUnusedPrivateMembers` if we deem it used.
*/
PrivateIdentifier(privateIdentifierNode) {
if (

// The definition of the class member itself
privateIdentifierNode.parent.type !== "PropertyDefinition" &&
privateIdentifierNode.parent.type !== "MethodDefinition" &&

// Any usage of this member, except for writes (if it is a property)
privateIdentifierNode.parent.parent.type !== "AssignmentExpression") {
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved

declaredAndUnusedPrivateMembers.delete(privateIdentifierNode.name);
}
},

/*
* 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"() {
for (const [classMemberName, remainingDeclaredMember] of declaredAndUnusedPrivateMembers.entries()) {
context.report({
node: remainingDeclaredMember,
messageId: "unusedPrivateClassMember",
data: {
classMemberName
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
}
});
}
declaredAndUnusedPrivateMembers.clear();
}
TimvdLippe marked this conversation as resolved.
Show resolved Hide resolved
};
}
};
135 changes: 135 additions & 0 deletions tests/lib/rules/no-unused-private-class-members.js
@@ -0,0 +1,135 @@
/**
* @fileoverview Tests for no-unused-private-class-members rule.
* @author Tim van der Lippe
*/

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require("../../../lib/rules/no-unused-private-class-members"),
{ RuleTester } = require("../../../lib/rule-tester");

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2022 } });

/**
* Returns an expected error for defined-but-not-used private class member.
* @param {string} classMemberName The name of the class member
* @returns {Object} An expected error object
*/
function definedError(classMemberName) {
return {
messageId: "unusedPrivateClassMember",
data: {
classMemberName
}
};
}

ruleTester.run("no-unused-private-class-members", rule, {
valid: [
"class Foo {}",
`
class Foo {
publicMember = 42;
}`,
`
class Foo {
#usedMember = 42;
method() {
return this.#usedMember;
}
}`,
`
class Foo {
#usedMember = 42;
anotherMember = this.#usedMember;
}`,
`
class Foo {
#usedMember = 42;
method() {
return someGlobalMethod(this.#usedMember);
}
}`,

//--------------------------------------------------------------------------
// Method definitions
//--------------------------------------------------------------------------
`
class Foo {
#usedMethod() {
return 42;
}
anotherMethod() {
return this.#usedMethod();
}
}`
],
invalid: [
{
code: `class Foo {
#unusedMember = 5;
}`,
errors: [definedError("unusedMember")]
},
{
code: `class First {}
class Second {
#unusedMemberInSecondClass = 5;
}`,
errors: [definedError("unusedMemberInSecondClass")]
},
{
code: `class First {
#unusedMemberInFirstClass = 5;
}
class Second {}`,
errors: [definedError("unusedMemberInFirstClass")]
},
{
code: `class First {
#firstUnusedMemberInSameClass = 5;
#secondUnusedMemberInSameClass = 5;
}`,
errors: [definedError("firstUnusedMemberInSameClass"), definedError("secondUnusedMemberInSameClass")]
},
{
code: `class Foo {
#usedOnlyInWrite = 5;
method() {
this.#usedOnlyInWrite = 42;
}
}`,
errors: [definedError("usedOnlyInWrite")]
},

//--------------------------------------------------------------------------
// Unused method definitions
//--------------------------------------------------------------------------
{
code: `class Foo {
#unusedMethod() {}
}`,
errors: [definedError("unusedMethod")]
},
{
code: `class Foo {
#unusedMethod() {}
#usedMethod() {
return 42;
}
publicMethod() {
return this.#usedMethod();
}
}`,
errors: [definedError("unusedMethod")]
}
]
});
1 change: 1 addition & 0 deletions tools/rule-types.json
Expand Up @@ -208,6 +208,7 @@
"no-unsafe-optional-chaining": "problem",
"no-unused-expressions": "suggestion",
"no-unused-labels": "suggestion",
"no-unused-private-class-members": "problem",
"no-unused-vars": "problem",
"no-use-before-define": "problem",
"no-useless-backreference": "problem",
Expand Down