Skip to content

Commit 576baf4

Browse files
ConaclosConaclos
authored
fix(noUnusedPrivateClassMembers): ignore all TS private members if a this[computed] is found (#7419)
1 parent 53ff5ae commit 576baf4

6 files changed

Lines changed: 184 additions & 27 deletions

File tree

‎.changeset/little-heads-divide.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
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`.
6+
7+
In the following example, `member` as previously reported as unused.
8+
It is no longer reported.
9+
10+
```ts
11+
class TsBioo {
12+
private member: number;
13+
14+
set_with_name(name: string, value: number) {
15+
this[name] = value;
16+
}
17+
}
18+
```

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

Lines changed: 72 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ use biome_console::markup;
1010
use biome_diagnostics::Severity;
1111
use biome_js_semantic::ReferencesExtensions;
1212
use biome_js_syntax::{
13-
AnyJsClassMember, AnyJsClassMemberName, AnyJsFormalParameter, AnyJsName,
14-
JsAssignmentExpression, JsClassDeclaration, JsSyntaxKind, JsSyntaxNode,
15-
TsAccessibilityModifier, TsPropertyParameter,
13+
AnyJsClassMember, AnyJsClassMemberName, AnyJsComputedMember, AnyJsExpression,
14+
AnyJsFormalParameter, AnyJsName, JsAssignmentExpression, JsClassDeclaration, JsSyntaxKind,
15+
JsSyntaxNode, TsAccessibilityModifier, TsPropertyParameter,
1616
};
1717
use biome_rowan::{
1818
AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, SyntaxNodeOptionExt, TextRange,
@@ -65,6 +65,21 @@ declare_lint_rule! {
6565
/// }
6666
/// ```
6767
///
68+
/// ## Caveats
69+
///
70+
/// The rule currently considers that all TypeScript private members are used if it encounters a computed access.
71+
/// In the following example `member` is not reported. It is considered as used.
72+
///
73+
/// ```ts
74+
/// class TsBioo {
75+
/// private member: number;
76+
///
77+
/// set_with_name(name: string, value: number) {
78+
/// this[name] = value;
79+
/// }
80+
/// }
81+
/// ```
82+
///
6883
pub NoUnusedPrivateClassMembers {
6984
version: "1.3.3",
7085
name: "noUnusedPrivateClassMembers",
@@ -234,37 +249,55 @@ fn traverse_members_usage(
234249
syntax: &JsSyntaxNode,
235250
mut private_members: FxHashSet<AnyMember>,
236251
) -> Vec<AnyMember> {
237-
let iter = syntax.preorder();
238-
239-
for event in iter {
240-
match event {
241-
biome_rowan::WalkEvent::Enter(node) => {
242-
if let Some(js_name) = AnyJsName::cast(node) {
243-
private_members.retain(|private_member| {
244-
let member_being_used =
245-
private_member.match_js_name(&js_name) == Some(true);
246-
247-
if !member_being_used {
248-
return true;
249-
}
252+
// `true` is at least one member is a TypeScript private member like `private member`.
253+
// The other private members are sharp members `#member`.
254+
let mut ts_private_count = private_members
255+
.iter()
256+
.filter(|member| !member.is_private_sharp())
257+
.count();
250258

251-
let is_write_only =
252-
is_write_only(&js_name) == Some(true) && !private_member.is_accessor();
253-
let is_in_update_expression = is_in_update_expression(&js_name);
259+
for node in syntax.descendants() {
260+
match AnyJsName::try_cast(node) {
261+
Ok(js_name) => {
262+
private_members.retain(|private_member| {
263+
let member_being_used = private_member.match_js_name(&js_name) == Some(true);
254264

255-
if is_in_update_expression || is_write_only {
256-
return true;
257-
}
265+
if !member_being_used {
266+
return true;
267+
}
258268

259-
false
260-
});
269+
let is_write_only =
270+
is_write_only(&js_name) == Some(true) && !private_member.is_accessor();
271+
let is_in_update_expression = is_in_update_expression(&js_name);
261272

262-
if private_members.is_empty() {
263-
break;
273+
if is_in_update_expression || is_write_only {
274+
return true;
264275
}
276+
277+
if !private_member.is_private_sharp() {
278+
ts_private_count -= 1;
279+
}
280+
281+
false
282+
});
283+
284+
if private_members.is_empty() {
285+
break;
286+
}
287+
}
288+
Err(node) => {
289+
if ts_private_count != 0
290+
&& let Some(computed_member) = AnyJsComputedMember::cast(node)
291+
&& matches!(
292+
computed_member.object(),
293+
Ok(AnyJsExpression::JsThisExpression(_))
294+
)
295+
{
296+
// We consider that all TypeScript private members are used in expressions like `this[something]`.
297+
private_members.retain(|private_member| private_member.is_private_sharp());
298+
ts_private_count = 0;
265299
}
266300
}
267-
biome_rowan::WalkEvent::Leave(_) => {}
268301
}
269302
}
270303

@@ -409,6 +442,18 @@ impl AnyMember {
409442
)
410443
}
411444

445+
/// Returns `true` if it is a private property starting with `#`.
446+
fn is_private_sharp(&self) -> bool {
447+
if let Self::AnyJsClassMember(member) = self {
448+
matches!(
449+
member.name(),
450+
Ok(Some(AnyJsClassMemberName::JsPrivateClassMemberName(_)))
451+
)
452+
} else {
453+
false
454+
}
455+
}
456+
412457
fn is_private(&self) -> Option<bool> {
413458
match self {
414459
Self::AnyJsClassMember(member) => {

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export class Sample {
2+
private member;
3+
#prop;
4+
5+
constructor() {
6+
this.#prop = 0;
7+
this.member = 0;
8+
}
9+
10+
method(name) {
11+
return this[name];
12+
}
13+
}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: invalid_dynamic_access.ts
4+
---
5+
# Input
6+
```ts
7+
export class Sample {
8+
private member;
9+
#prop;
10+
11+
constructor() {
12+
this.#prop = 0;
13+
this.member = 0;
14+
}
15+
16+
method(name) {
17+
return this[name];
18+
}
19+
}
20+
21+
```
22+
23+
# Diagnostics
24+
```
25+
invalid_dynamic_access.ts:3:3 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━
26+
27+
! This private class member is defined but never used.
28+
29+
1 │ export class Sample {
30+
2 │ private member;
31+
> 3 │ #prop;
32+
│ ^^^^^
33+
4 │
34+
5 │ constructor() {
35+
36+
i Unsafe fix: Remove unused declaration.
37+
38+
1 1 │ export class Sample {
39+
2 2 │ private member;
40+
3 │ - ··#prop;
41+
4 3 │
42+
5 4 │ constructor() {
43+
44+
45+
```

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/* should not generate diagnostics */
2+
3+
export class Sample {
4+
private add: () => void;
5+
6+
constructor(private remove: () => void) {
7+
this.add = () => {};
8+
this.remove = () => {};
9+
}
10+
11+
on(action: "add" | "remove"): void {
12+
this[action]();
13+
}
14+
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: valid_dynamic_access.ts
4+
---
5+
# Input
6+
```ts
7+
/* should not generate diagnostics */
8+
9+
export class Sample {
10+
private add: () => void;
11+
12+
constructor(private remove: () => void) {
13+
this.add = () => {};
14+
this.remove = () => {};
15+
}
16+
17+
on(action: "add" | "remove"): void {
18+
this[action]();
19+
}
20+
}
21+
22+
```

0 commit comments

Comments
 (0)