fix(noUnusedPrivateClassMembers): ignore all TS private members if a this[computed] is found

.changeset/little-heads-divide.md

@@ -0,0 +1,18 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed [#7323](https://github.com/biomejs/biome/issues/7323). [`noUnusedPrivateClassMembers`](https://biomejs.dev/linter/rules/no-unused-private-class-members/) no longer reports as unused TypeScript `private` members if the rule encounters a computed access on `this`.
+
+In the following example, `member` as previously reported as unused.
+It is no longer reported.
+
+```ts
+class TsBioo {
+  private member: number;
+
+  set_with_name(name: string, value: number) {
+    this[name] = value;
+  }
+}
+```

crates/biome_js_analyze/src/lint/correctness/no_unused_private_class_members.rs

@@ -10,9 +10,9 @@ use biome_console::markup;
 use biome_diagnostics::Severity;
 use biome_js_semantic::ReferencesExtensions;
 use biome_js_syntax::{
-    AnyJsClassMember, AnyJsClassMemberName, AnyJsFormalParameter, AnyJsName,
-    JsAssignmentExpression, JsClassDeclaration, JsSyntaxKind, JsSyntaxNode,
-    TsAccessibilityModifier, TsPropertyParameter,
+    AnyJsClassMember, AnyJsClassMemberName, AnyJsComputedMember, AnyJsExpression,
+    AnyJsFormalParameter, AnyJsName, JsAssignmentExpression, JsClassDeclaration, JsSyntaxKind,
+    JsSyntaxNode, TsAccessibilityModifier, TsPropertyParameter,
 };
 use biome_rowan::{
     AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, SyntaxNodeOptionExt, TextRange,
@@ -65,6 +65,21 @@ declare_lint_rule! {
     /// }
     /// ```
     ///
+    /// ## Caveats
+    ///
+    /// The rule currently considers that all TypeScript private members are used if it encounters a computed access.
+    /// In the following example `member` is not reported. It is considered as used.
+    ///
+    /// ```ts
+    ///  class TsBioo {
+    ///    private member: number;
+    ///
+    ///    set_with_name(name: string, value: number) {
+    ///      this[name] = value;
+    ///    }
+    ///  }
+    /// ```
+    ///
     pub NoUnusedPrivateClassMembers {
         version: "1.3.3",
         name: "noUnusedPrivateClassMembers",
@@ -234,37 +249,55 @@ fn traverse_members_usage(
     syntax: &JsSyntaxNode,
     mut private_members: FxHashSet<AnyMember>,
 ) -> Vec<AnyMember> {
-    let iter = syntax.preorder();
-
-    for event in iter {
-        match event {
-            biome_rowan::WalkEvent::Enter(node) => {
-                if let Some(js_name) = AnyJsName::cast(node) {
-                    private_members.retain(|private_member| {
-                        let member_being_used =
-                            private_member.match_js_name(&js_name) == Some(true);
-
-                        if !member_being_used {
-                            return true;
-                        }
+    // `true` is at least one member is a TypeScript private member like `private member`.
+    // The other private members are sharp members `#member`.
+    let mut ts_private_count = private_members
+        .iter()
+        .filter(|member| !member.is_private_sharp())
+        .count();
 
-                        let is_write_only =
-                            is_write_only(&js_name) == Some(true) && !private_member.is_accessor();
-                        let is_in_update_expression = is_in_update_expression(&js_name);
+    for node in syntax.descendants() {
+        match AnyJsName::try_cast(node) {
+            Ok(js_name) => {
+                private_members.retain(|private_member| {
+                    let member_being_used = private_member.match_js_name(&js_name) == Some(true);
 
-                        if is_in_update_expression || is_write_only {
-                            return true;
-                        }
+                    if !member_being_used {
+                        return true;
+                    }
 
-                        false
-                    });
+                    let is_write_only =
+                        is_write_only(&js_name) == Some(true) && !private_member.is_accessor();
+                    let is_in_update_expression = is_in_update_expression(&js_name);
 
-                    if private_members.is_empty() {
-                        break;
+                    if is_in_update_expression || is_write_only {
+                        return true;
                     }
+
+                    if !private_member.is_private_sharp() {
+                        ts_private_count -= 1;
+                    }
+
+                    false
+                });
+
+                if private_members.is_empty() {
+                    break;
+                }
+            }
+            Err(node) => {
+                if ts_private_count != 0
+                    && let Some(computed_member) = AnyJsComputedMember::cast(node)
+                    && matches!(
+                        computed_member.object(),
+                        Ok(AnyJsExpression::JsThisExpression(_))
+                    )
+                {
+                    // We consider that all TypeScript private members are used in expressions like `this[something]`.
+                    private_members.retain(|private_member| private_member.is_private_sharp());
+                    ts_private_count = 0;
                 }
             }
-            biome_rowan::WalkEvent::Leave(_) => {}
         }
     }
 
@@ -409,6 +442,18 @@ impl AnyMember {
         )
     }
 
+    /// Returns `true` if it is a private property starting with `#`.
+    fn is_private_sharp(&self) -> bool {
+        if let Self::AnyJsClassMember(member) = self {
+            matches!(
+                member.name(),
+                Ok(Some(AnyJsClassMemberName::JsPrivateClassMemberName(_)))
+            )
+        } else {
+            false
+        }
+    }
+
     fn is_private(&self) -> Option<bool> {
         match self {
             Self::AnyJsClassMember(member) => {

crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid_dynamic_access.ts

@@ -0,0 +1,13 @@
+export class Sample {
+  private member;
+  #prop;
+
+  constructor() {
+    this.#prop = 0;
+    this.member = 0;
+  }
+
+  method(name) {
+    return this[name];
+  }
+}

crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid_dynamic_access.ts.snap

@@ -0,0 +1,45 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: invalid_dynamic_access.ts
+---
+# Input
+```ts
+export class Sample {
+  private member;
+  #prop;
+
+  constructor() {
+    this.#prop = 0;
+    this.member = 0;
+  }
+
+  method(name) {
+    return this[name];
+  }
+}
+
+```
+
+# Diagnostics
+```
+invalid_dynamic_access.ts:3:3 lint/correctness/noUnusedPrivateClassMembers  FIXABLE  ━━━━━━━━━━━━━━━
+
+  ! This private class member is defined but never used.
+
+    1 │ export class Sample {
+    2 │   private member;
+  > 3 │   #prop;
+      │   ^^^^^
+    4 │ 
+    5 │   constructor() {
+
+  i Unsafe fix: Remove unused declaration.
+
+     1  1 │   export class Sample {
+     2  2 │     private member;
+     3    │ - ··#prop;
+     4  3 │   
+     5  4 │     constructor() {
+
+
+```

crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/valid_dynamic_access.ts

@@ -0,0 +1,14 @@
+/* should not generate diagnostics */
+
+export class Sample {
+  private add: () => void;
+
+  constructor(private remove: () => void) {
+    this.add = () => {};
+    this.remove = () => {};
+  }
+
+  on(action: "add" | "remove"): void {
+    this[action]();
+  }
+}

crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/valid_dynamic_access.ts.snap

@@ -0,0 +1,22 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: valid_dynamic_access.ts
+---
+# Input
+```ts
+/* should not generate diagnostics */
+
+export class Sample {
+  private add: () => void;
+
+  constructor(private remove: () => void) {
+    this.add = () => {};
+    this.remove = () => {};
+  }
+
+  on(action: "add" | "remove"): void {
+    this[action]();
+  }
+}
+
+```