feat(linter): support eslint/no-unused-private-class-members rule (#1820)

crates/oxc_ast/src/ast/js.rs

@@ -1894,6 +1894,9 @@ impl MethodDefinitionKind {
     pub fn is_constructor(&self) -> bool {
         matches!(self, Self::Constructor)
     }
+    pub fn is_method(&self) -> bool {
+        matches!(self, Self::Method)
+    }
 }
 
 #[derive(Debug, Clone, Hash)]

crates/oxc_linter/src/rules.rs

@@ -83,6 +83,7 @@ mod eslint {
     pub mod no_unsafe_negation;
     pub mod no_unsafe_optional_chaining;
     pub mod no_unused_labels;
+    pub mod no_unused_private_class_members;
     pub mod no_useless_catch;
     pub mod no_useless_escape;
     pub mod require_yield;
@@ -319,6 +320,7 @@ oxc_macros::declare_all_lint_rules! {
     eslint::no_unsafe_negation,
     eslint::no_unsafe_optional_chaining,
     eslint::no_unused_labels,
+    eslint::no_unused_private_class_members,
     eslint::no_useless_catch,
     eslint::no_useless_escape,
     eslint::require_yield,

crates/oxc_linter/src/rules/eslint/no_unused_private_class_members.rs

@@ -0,0 +1,460 @@
+use itertools::Itertools;
+use oxc_ast::AstKind;
+use oxc_diagnostics::{
+    miette::{self, Diagnostic},
+    thiserror::{self, Error},
+};
+use oxc_macros::declare_oxc_lint;
+use oxc_semantic::{AstNode, AstNodeId, AstNodes};
+use oxc_span::{Atom, Span};
+
+use crate::{context::LintContext, rule::Rule};
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("eslint(no-unused-private-class-members): '{0}' is defined but never used.")]
+#[diagnostic(severity(warning))]
+struct NoUnusedPrivateClassMembersDiagnostic(Atom, #[label] pub Span);
+
+#[derive(Debug, Default, Clone)]
+pub struct NoUnusedPrivateClassMembers;
+
+declare_oxc_lint!(
+    /// ### What it does
+    ///
+    /// Disallow unused private class members
+    ///
+    /// ### Why is this bad?
+    ///
+    /// 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.
+    ///
+    /// ### Example
+    /// ```javascript
+    ///
+    /// /// bad
+    /// class A {
+    ///		#unusedMember = 5;
+    ///	}
+    ///
+    ///	class B {
+    ///			#usedOnlyInWrite = 5;
+    ///			method() {
+    ///					this.#usedOnlyInWrite = 42;
+    ///			}
+    ///	}
+    ///
+    ///	class C {
+    ///			#usedOnlyToUpdateItself = 5;
+    ///			method() {
+    ///					this.#usedOnlyToUpdateItself++;
+    ///			}
+    ///	}
+    ///
+    ///	class D {
+    ///			#unusedMethod() {}
+    ///	}
+    ///
+    ///	class E {
+    ///			get #unusedAccessor() {}
+    ///			set #unusedAccessor(value) {}
+    ///	}
+    ///
+    /// /// Good
+    /// class A {
+    ///		#usedMember = 42;
+    ///		method() {
+    ///				return this.#usedMember;
+    ///		}
+    ///	}
+
+    ///	class B {
+    ///			#usedMethod() {
+    ///					return 42;
+    ///			}
+    ///			anotherMethod() {
+    ///					return this.#usedMethod();
+    ///			}
+    ///	}
+
+    ///	class C {
+    ///			get #usedAccessor() {}
+    ///			set #usedAccessor(value) {}
+    ///
+    ///			method() {
+    ///					this.#usedAccessor = 42;
+    ///			}
+    ///	}
+    ///
+    /// ```
+    NoUnusedPrivateClassMembers,
+    correctness
+);
+
+impl Rule for NoUnusedPrivateClassMembers {
+    fn run_once(&self, ctx: &LintContext) {
+        ctx.semantic().classes().iter_enumerated().for_each(|(class_id, _)| {
+            for (property_id, property) in
+                ctx.semantic().classes().properties[class_id].iter_enumerated()
+            {
+                if property.is_private
+                    && !ctx.semantic().classes().iter_private_identifiers(class_id).any(|ident| {
+                        is_read(ident.id, ctx.semantic().nodes())
+                            && ident.property_id.is_some_and(|id| id == property_id)
+                    })
+                {
+                    ctx.diagnostic(NoUnusedPrivateClassMembersDiagnostic(
+                        property.name.clone(),
+                        property.span,
+                    ));
+                }
+            }
+
+            for (method_id, method) in ctx.semantic().classes().methods[class_id].iter_enumerated()
+            {
+                if method.is_private
+                    && !ctx
+                        .semantic()
+                        .classes()
+                        .iter_private_identifiers(class_id)
+                        .any(|ident| ident.method_ids.contains(&method_id))
+                {
+                    ctx.diagnostic(NoUnusedPrivateClassMembersDiagnostic(
+                        method.name.clone(),
+                        method.span,
+                    ));
+                }
+            }
+        });
+    }
+}
+
+fn is_read(current_node_id: AstNodeId, nodes: &AstNodes) -> bool {
+    for (curr, parent) in nodes
+        .iter_parents(nodes.parent_id(current_node_id).unwrap_or(current_node_id))
+        .tuple_windows::<(&AstNode<'_>, &AstNode<'_>)>()
+    {
+        match (curr.kind(), parent.kind()) {
+            (
+                AstKind::SimpleAssignmentTarget(_) | AstKind::MemberExpression(_),
+                AstKind::AssignmentTarget(_) | AstKind::SimpleAssignmentTarget(_),
+            ) => {
+                continue;
+            }
+            (
+                AstKind::AssignmentTarget(_),
+                AstKind::ForInStatement(_)
+                | AstKind::ForOfStatement(_)
+                | AstKind::AssignmentTargetWithDefault(_)
+                | AstKind::AssignmentTarget(_),
+            )
+            | (AstKind::SimpleAssignmentTarget(_), AstKind::AssignmentExpression(_)) => {
+                return false
+            }
+            (AstKind::AssignmentTarget(_), AstKind::AssignmentExpression(_))
+            | (_, AstKind::UpdateExpression(_)) => {
+                return !matches!(
+                    nodes.parent_kind(parent.id()),
+                    Some(AstKind::ExpressionStatement(_))
+                );
+            }
+            _ => return true,
+        }
+    }
+
+    true
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        r"class Foo {}",
+        r"class Foo {
+			    publicMember = 42;
+			}",
+        r"class Foo {
+			    #usedMember = 42;
+			    method() {
+			        return this.#usedMember;
+			    }
+			}",
+        r"class Foo {
+			    #usedMember = 42;
+			    anotherMember = this.#usedMember;
+			}",
+        r"class Foo {
+			    #usedMember = 42;
+			    foo() {
+			        anotherMember = this.#usedMember;
+			    }
+			}",
+        r"class C {
+			    #usedMember;
+			
+			    foo() {
+			        bar(this.#usedMember += 1);
+			    }
+			}",
+        r"class Foo {
+			    #usedMember = 42;
+			    method() {
+			        return someGlobalMethod(this.#usedMember);
+			    }
+			}",
+        r"class C {
+			    #usedInOuterClass;
+			
+			    foo() {
+			        return class {};
+			    }
+			
+			    bar() {
+			        return this.#usedInOuterClass;
+			    }
+			}",
+        r"class Foo {
+			    #usedInForInLoop;
+			    method() {
+			        for (const bar in this.#usedInForInLoop) {
+			
+			        }
+			    }
+			}",
+        r"class Foo {
+			    #usedInForOfLoop;
+			    method() {
+			        for (const bar of this.#usedInForOfLoop) {
+			
+			        }
+			    }
+			}",
+        r"class Foo {
+			    #usedInAssignmentPattern;
+			    method() {
+			        [bar = 1] = this.#usedInAssignmentPattern;
+			    }
+			}",
+        r"class Foo {
+			    #usedInArrayPattern;
+			    method() {
+			        [bar] = this.#usedInArrayPattern;
+			    }
+			}",
+        r"class Foo {
+			    #usedInAssignmentPattern;
+			    method() {
+			        [bar] = this.#usedInAssignmentPattern;
+			    }
+			}",
+        r"class C {
+			    #usedInObjectAssignment;
+			
+			    method() {
+			        ({ [this.#usedInObjectAssignment]: a } = foo);
+			    }
+			}",
+        r"class C {
+            set #accessorWithSetterFirst(value) {
+                doSomething(value);
+            }
+            get #accessorWithSetterFirst() {
+                return something();
+            }
+            method() {
+                this.#accessorWithSetterFirst += 1;
+            }
+        }",
+        r"class Foo {
+            set #accessorUsedInMemberAccess(value) {}
+
+            method(a) {
+                [this.#accessorUsedInMemberAccess] = a;
+            }
+        }",
+        r"class C {
+            get #accessorWithGetterFirst() {
+                return something();
+            }
+            set #accessorWithGetterFirst(value) {
+                doSomething(value);
+            }
+            method() {
+                this.#accessorWithGetterFirst += 1;
+            }
+        }",
+        // This is complicated case. Support this case maybe effect performance, so we don't support it now.
+        // r"class C {
+        //     #usedInInnerClass;
+
+        //     method(a) {
+        //         return class {
+        //             foo = a.#usedInInnerClass;
+        //         }
+        //     }
+        // }",
+        r"class Foo {
+			    #usedMethod() {
+			        return 42;
+			    }
+			    anotherMethod() {
+			        return this.#usedMethod();
+			    }
+			}",
+        r"class C {
+            set #x(value) {
+                doSomething(value);
+            }
+
+            foo() {
+                this.#x = 1;
+            }
+        }",
+    ];
+
+    let fail = vec![
+        r"class Foo {
+			    #unusedMember = 5;
+			}",
+        r"class First {}
+			class Second {
+			    #unusedMemberInSecondClass = 5;
+			}",
+        r"class First {
+			    #unusedMemberInFirstClass = 5;
+			}
+			class Second {}",
+        r"class First {
+			    #firstUnusedMemberInSameClass = 5;
+			    #secondUnusedMemberInSameClass = 5;
+			}",
+        r"class Foo {
+			    #usedOnlyInWrite = 5;
+			    method() {
+			        this.#usedOnlyInWrite = 42;
+			    }
+			}",
+        r"class Foo {
+			    #usedOnlyInWriteStatement = 5;
+			    method() {
+			        this.#usedOnlyInWriteStatement += 42;
+			    }
+			}",
+        r"class C {
+			    #usedOnlyInIncrement;
+			
+			    foo() {
+			        this.#usedOnlyInIncrement++;
+			    }
+			}",
+        r"class C {
+			    #unusedInOuterClass;
+			
+			    foo() {
+			        return class {
+			            #unusedInOuterClass;
+			
+			            bar() {
+			                return this.#unusedInOuterClass;
+			            }
+			        };
+			    }
+			}",
+        r"class C {
+			    #unusedOnlyInSecondNestedClass;
+			
+			    foo() {
+			        return class {
+			            #unusedOnlyInSecondNestedClass;
+			
+			            bar() {
+			                return this.#unusedOnlyInSecondNestedClass;
+			            }
+			        };
+			    }
+			
+			    baz() {
+			        return this.#unusedOnlyInSecondNestedClass;
+			    }
+			
+			    bar() {
+			        return class {
+			            #unusedOnlyInSecondNestedClass;
+			        }
+			    }
+			}",
+        r"class Foo {
+			    #unusedMethod() {}
+			}",
+        r"class Foo {
+			    #unusedMethod() {}
+			    #usedMethod() {
+			        return 42;
+			    }
+			    publicMethod() {
+			        return this.#usedMethod();
+			    }
+			}",
+        r"class Foo {
+			    set #unusedSetter(value) {}
+			}",
+        r"class Foo {
+			    #unusedForInLoop;
+			    method() {
+			        for (this.#unusedForInLoop in bar) {
+			
+			        }
+			    }
+			}",
+        r"class Foo {
+			    #unusedForOfLoop;
+			    method() {
+			        for (this.#unusedForOfLoop of bar) {
+			
+			        }
+			    }
+			}",
+        r"class Foo {
+			    #unusedInDestructuring;
+			    method() {
+			        ({ x: this.#unusedInDestructuring } = bar);
+			    }
+			}",
+        r"class Foo {
+			    #unusedInRestPattern;
+			    method() {
+			        [...this.#unusedInRestPattern] = bar;
+			    }
+			}",
+        r"class Foo {
+			    #unusedInAssignmentPattern;
+			    method() {
+			        [this.#unusedInAssignmentPattern = 1] = bar;
+			    }
+			}",
+        r"class Foo {
+			    #unusedInAssignmentPattern;
+			    method() {
+			        [this.#unusedInAssignmentPattern] = bar;
+			    }
+			}",
+        r"class C {
+			    #usedOnlyInTheSecondInnerClass;
+			
+			    method(a) {
+			        return class {
+			            #usedOnlyInTheSecondInnerClass;
+			
+			            method2(b) {
+			                foo = b.#usedOnlyInTheSecondInnerClass;
+			            }
+			
+			            method3(b) {
+			                foo = b.#usedOnlyInTheSecondInnerClass;
+			            }
+			        }
+			    }
+			}",
+    ];
+
+    Tester::new_without_config(NoUnusedPrivateClassMembers::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/no_unused_private_class_members.snap

@@ -0,0 +1,165 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: no_unused_private_class_members
+---
+  ⚠ eslint(no-unused-private-class-members): 'unusedMember' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedMember = 5;
+   ·                 ─────────────
+ 3 │             }
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedMemberInSecondClass' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:2:1]
+ 2 │             class Second {
+ 3 │                 #unusedMemberInSecondClass = 5;
+   ·                 ──────────────────────────
+ 4 │             }
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedMemberInFirstClass' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class First {
+ 2 │                 #unusedMemberInFirstClass = 5;
+   ·                 ─────────────────────────
+ 3 │             }
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'firstUnusedMemberInSameClass' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class First {
+ 2 │                 #firstUnusedMemberInSameClass = 5;
+   ·                 ─────────────────────────────
+ 3 │                 #secondUnusedMemberInSameClass = 5;
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'secondUnusedMemberInSameClass' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:2:1]
+ 2 │                 #firstUnusedMemberInSameClass = 5;
+ 3 │                 #secondUnusedMemberInSameClass = 5;
+   ·                 ──────────────────────────────
+ 4 │             }
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'usedOnlyInWrite' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #usedOnlyInWrite = 5;
+   ·                 ────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'usedOnlyInWriteStatement' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #usedOnlyInWriteStatement = 5;
+   ·                 ─────────────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'usedOnlyInIncrement' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class C {
+ 2 │                 #usedOnlyInIncrement;
+   ·                 ────────────────────
+ 3 │             
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedInOuterClass' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class C {
+ 2 │                 #unusedInOuterClass;
+   ·                 ───────────────────
+ 3 │             
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedOnlyInSecondNestedClass' is defined but never used.
+    ╭─[no_unused_private_class_members.tsx:19:1]
+ 19 │                     return class {
+ 20 │                         #unusedOnlyInSecondNestedClass;
+    ·                         ──────────────────────────────
+ 21 │                     }
+    ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedMethod' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedMethod() {}
+   ·                 ─────────────
+ 3 │             }
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedMethod' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedMethod() {}
+   ·                 ─────────────
+ 3 │                 #usedMethod() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedSetter' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 set #unusedSetter(value) {}
+   ·                     ─────────────
+ 3 │             }
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedForInLoop' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedForInLoop;
+   ·                 ────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedForOfLoop' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedForOfLoop;
+   ·                 ────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedInDestructuring' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedInDestructuring;
+   ·                 ──────────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedInRestPattern' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedInRestPattern;
+   ·                 ────────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedInAssignmentPattern' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedInAssignmentPattern;
+   ·                 ──────────────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'unusedInAssignmentPattern' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class Foo {
+ 2 │                 #unusedInAssignmentPattern;
+   ·                 ──────────────────────────
+ 3 │                 method() {
+   ╰────
+
+  ⚠ eslint(no-unused-private-class-members): 'usedOnlyInTheSecondInnerClass' is defined but never used.
+   ╭─[no_unused_private_class_members.tsx:1:1]
+ 1 │ class C {
+ 2 │                 #usedOnlyInTheSecondInnerClass;
+   ·                 ──────────────────────────────
+ 3 │             
+   ╰────
+
+

crates/oxc_semantic/src/checker/javascript.rs

@@ -286,7 +286,7 @@ fn check_private_identifier(ctx: &SemanticBuilder<'_>) {
     if let Some(class_id) = ctx.class_table_builder.current_class_id {
         ctx.class_table_builder.classes.iter_private_identifiers(class_id).for_each(|reference| {
             if reference.property_id.is_none()
-                && reference.method_id.is_none()
+                && reference.method_ids.is_empty()
                 && !ctx.class_table_builder.classes.ancestors(class_id).skip(1).any(|class_id| {
                     ctx.class_table_builder
                         .classes

crates/oxc_semantic/src/class/builder.rs

@@ -74,17 +74,18 @@ impl ClassTableBuilder {
             {
                 if let Some(class_id) = self.current_class_id {
                     let property_id = self.classes.get_property_id(class_id, &ident.name);
-                    let method_id = if property_id.is_some() {
+                    let method_ids = if property_id.is_some() {
-                        None
+                        Vec::default()
                     } else {
-                        self.classes.get_method_id(class_id, &ident.name)
+                        self.classes.get_method_ids(class_id, &ident.name)
                     };
+
                     let reference = PrivateIdentifierReference::new(
                         current_node_id,
                         ident.name.clone(),
                         ident.span,
                         property_id,
-                        method_id,
+                        method_ids,
                     );
                     self.classes.add_private_identifier_reference(class_id, reference);
                 }
@@ -108,7 +109,6 @@ impl ClassTableBuilder {
             }
         }
     }
-
     pub fn pop_class(&mut self) {
         self.current_class_id = self
             .current_class_id

crates/oxc_semantic/src/class/table.rs

@@ -39,7 +39,8 @@ pub struct PrivateIdentifierReference {
     pub name: Atom,
     pub span: Span,
     pub property_id: Option<PropertyId>,
-    pub method_id: Option<MethodId>,
+    /// If the private identifier is used in a get/set method, this will be has 2 method ids
+    pub method_ids: Vec<MethodId>,
 }
 
 impl PrivateIdentifierReference {
@@ -48,9 +49,9 @@ impl PrivateIdentifierReference {
         name: Atom,
         span: Span,
         property_id: Option<PropertyId>,
-        method_id: Option<MethodId>,
+        method_ids: Vec<MethodId>,
     ) -> Self {
-        Self { id, name, span, property_id, method_id }
+        Self { id, name, span, property_id, method_ids }
     }
 }
 
@@ -95,14 +96,23 @@ impl ClassTable {
         })
     }
 
-    pub fn get_method_id(&self, class_id: ClassId, name: &Atom) -> Option<MethodId> {
+    pub fn get_method_ids(&self, class_id: ClassId, name: &Atom) -> Vec<MethodId> {
-        self.methods[class_id].iter_enumerated().find_map(|(method_id, method)| {
+        let mut method_ids = vec![];
+        for (method_id, method) in self.methods[class_id].iter_enumerated() {
             if method.name == *name {
-                Some(method_id)
+                method_ids.push(method_id);
-            } else {
+                // Only have 1 method id for MethodDefinition::Method
-                None
+                if method.kind.is_method() {
+                    break;
+                }
+                // Maximum 2 method ids, for get/set
+                if method_ids.len() == 2 {
+                    break;
+                }
             }
-        })
+        }
+
+        method_ids
     }
 
     pub fn has_private_definition(&self, class_id: ClassId, name: &Atom) -> bool {