Skip to content

Commit 4a50b81

Browse files
vladimir-ivanovvladimir-ivanov
committed
feat(biome-js-analyze): adjusts members update assignments to be marked as valid updates and no errors triggered
1 parent b2da976 commit 4a50b81

File tree

8 files changed

+447
-212
lines changed

8 files changed

+447
-212
lines changed

‎.changeset/no_unused_private_class_members_bug.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 [#7192](https://github.com/biomejs/biome/issues/7192):
6+
`noUnusedPrivateClassMembers` now treats private members in compound assignments (+=, -=, ??=, etc.) as used,
7+
while plain assignments (=) do not count as usage.
8+
9+
Example that is now correctly flagged:
10+
11+
```typescript
12+
class App {
13+
#persistenceRequest: Promise<boolean> | undefined;
14+
saveData() {
15+
this.#persistenceRequest += 2;
16+
}
17+
}
18+
```

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

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ use biome_diagnostics::Severity;
1111
use biome_js_semantic::ReferencesExtensions;
1212
use biome_js_syntax::{
1313
AnyJsClassMember, AnyJsClassMemberName, AnyJsComputedMember, AnyJsExpression,
14-
AnyJsFormalParameter, AnyJsName, JsAssignmentExpression, JsClassDeclaration, JsSyntaxKind,
15-
JsSyntaxNode, TsAccessibilityModifier, TsPropertyParameter,
14+
AnyJsFormalParameter, AnyJsName, JsAssignmentExpression, JsAssignmentOperator,
15+
JsClassDeclaration, JsSyntaxKind, JsSyntaxNode, TsAccessibilityModifier, TsPropertyParameter,
1616
};
1717
use biome_rowan::{
1818
AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, SyntaxNodeOptionExt, TextRange,
@@ -268,7 +268,11 @@ fn traverse_members_usage(
268268
is_write_only(&js_name) == Some(true) && !private_member.is_accessor();
269269
let is_in_update_expression = is_in_update_expression(&js_name);
270270

271-
if is_in_update_expression || is_write_only {
271+
if is_in_update_expression {
272+
return false;
273+
}
274+
275+
if is_write_only {
272276
return true;
273277
}
274278

@@ -423,10 +427,19 @@ fn is_in_update_expression(js_name: &AnyJsName) -> bool {
423427
return false;
424428
}
425429

426-
matches!(
427-
grand_parent.kind(),
428-
JsSyntaxKind::JS_POST_UPDATE_EXPRESSION | JsSyntaxKind::JS_PRE_UPDATE_EXPRESSION
429-
)
430+
// grand_parent can also be js expression statement
431+
match grand_parent.kind() {
432+
JsSyntaxKind::JS_POST_UPDATE_EXPRESSION | JsSyntaxKind::JS_PRE_UPDATE_EXPRESSION => true,
433+
JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION => {
434+
if let Some(assign_expr) = JsAssignmentExpression::cast(grand_parent.clone())
435+
&& let Ok(op_kind) = assign_expr.operator()
436+
{
437+
return op_kind != JsAssignmentOperator::Assign;
438+
}
439+
false
440+
}
441+
_ => false,
442+
}
430443
}
431444

432445
impl AnyMember {

‎crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid.js‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,6 @@ class OnlyWrite {
1414
}
1515
}
1616

17-
class SelfUpdate {
18-
#usedOnlyToUpdateItself = 5;
19-
20-
method() {
21-
this.#usedOnlyToUpdateItself++;
22-
}
23-
}
24-
2517
class Accessor {
2618
get #unusedAccessor() {}
2719
set #unusedAccessor(value) {}

‎crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid.js.snap‎

Lines changed: 42 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
---
22
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
assertion_line: 152
34
expression: invalid.js
45
---
56
# Input
@@ -20,14 +21,6 @@ class OnlyWrite {
2021
}
2122
}
2223
23-
class SelfUpdate {
24-
#usedOnlyToUpdateItself = 5;
25-
26-
method() {
27-
this.#usedOnlyToUpdateItself++;
28-
}
29-
}
30-
3124
class Accessor {
3225
get #unusedAccessor() {}
3326
set #unusedAccessor(value) {}
@@ -133,156 +126,90 @@ invalid.js:10:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━
133126
```
134127
135128
```
136-
invalid.js:18:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
129+
invalid.js:18:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
137130
138131
! This private class member is defined but never used.
139132
140-
17 │ class SelfUpdate {
141-
> 18 │ #usedOnlyToUpdateItself = 5;
142-
│ ^^^^^^^^^^^^^^^^^^^^^^^
143-
19 │
144-
20 │ method() {
145-
146-
i Unsafe fix: Remove unused declaration.
147-
148-
16 16 │
149-
17 17 │ class SelfUpdate {
150-
18 │ - → #usedOnlyToUpdateItself·=·5;
151-
19 18 │
152-
20 19 │ method() {
153-
154-
155-
```
156-
157-
```
158-
invalid.js:26:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
159-
160-
! This private class member is defined but never used.
161-
162-
25 │ class Accessor {
163-
> 26 │ get #unusedAccessor() {}
133+
17 │ class Accessor {
134+
> 18 │ get #unusedAccessor() {}
164135
│ ^^^^^^^^^^^^^^^
165-
27 │ set #unusedAccessor(value) {}
166-
28 │ }
136+
19 │ set #unusedAccessor(value) {}
137+
20 │ }
167138
168139
i Unsafe fix: Remove unused declaration.
169140
170-
24 24 │
171-
25 25 │ class Accessor {
172-
26 │ - → get·#unusedAccessor()·{}
173-
27 26 │ set #unusedAccessor(value) {}
174-
28 27 │ }
141+
16 16 │
142+
17 17 │ class Accessor {
143+
18 │ - → get·#unusedAccessor()·{}
144+
19 18 │ set #unusedAccessor(value) {}
145+
20 19 │ }
175146
176147
177148
```
178149
179150
```
180-
invalid.js:27:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
151+
invalid.js:19:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
181152
182153
! This private class member is defined but never used.
183154
184-
25 │ class Accessor {
185-
26 │ get #unusedAccessor() {}
186-
> 27 │ set #unusedAccessor(value) {}
155+
17 │ class Accessor {
156+
18 │ get #unusedAccessor() {}
157+
> 19 │ set #unusedAccessor(value) {}
187158
│ ^^^^^^^^^^^^^^^
188-
28 │ }
189-
29 │
159+
20 │ }
160+
21 │
190161
191162
i Unsafe fix: Remove unused declaration.
192163
193-
25 25 │ class Accessor {
194-
26 26 │ get #unusedAccessor() {}
195-
27 │ - → set·#unusedAccessor(value)·{}
196-
28 27 │ }
197-
29 28 │
164+
17 17 │ class Accessor {
165+
18 18 │ get #unusedAccessor() {}
166+
19 │ - → set·#unusedAccessor(value)·{}
167+
20 19 │ }
168+
21 20 │
198169
199170
200171
```
201172
202173
```
203-
invalid.js:31:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
174+
invalid.js:23:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
204175
205176
! This private class member is defined but never used.
206177
207-
30 │ class First {
208-
> 31 │ #unusedMemberInFirstClass = 5;
178+
22 │ class First {
179+
> 23 │ #unusedMemberInFirstClass = 5;
209180
│ ^^^^^^^^^^^^^^^^^^^^^^^^^
210-
32 │ }
211-
33 │
181+
24 │ }
182+
25 │
212183
213184
i Unsafe fix: Remove unused declaration.
214185
215-
29 29 │
216-
30 30 │ class First {
217-
31 │ - → #unusedMemberInFirstClass·=·5;
218-
32 31 │ }
219-
33 32 │
186+
21 21 │
187+
22 22 │ class First {
188+
23 │ - → #unusedMemberInFirstClass·=·5;
189+
24 23 │ }
190+
25 24 │
220191
221192
222193
```
223194
224195
```
225-
invalid.js:35:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
196+
invalid.js:27:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
226197
227198
! This private class member is defined but never used.
228199
229-
34 │ class Foo {
230-
> 35 │ #usedOnlyInWrite = 5;
200+
26 │ class Foo {
201+
> 27 │ #usedOnlyInWrite = 5;
231202
│ ^^^^^^^^^^^^^^^^
232-
36 │ method() {
233-
37 │ this.#usedOnlyInWrite = 42;
234-
235-
i Unsafe fix: Remove unused declaration.
236-
237-
33 33 │
238-
34 34 │ class Foo {
239-
35 │ - → #usedOnlyInWrite·=·5;
240-
36 35 │ method() {
241-
37 36 │ this.#usedOnlyInWrite = 42;
242-
243-
244-
```
245-
246-
```
247-
invalid.js:42:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
248-
249-
! This private class member is defined but never used.
250-
251-
41 │ class Foo {
252-
> 42 │ #usedOnlyInWriteStatement = 5;
253-
│ ^^^^^^^^^^^^^^^^^^^^^^^^^
254-
43 │ method() {
255-
44 │ this.#usedOnlyInWriteStatement += 42;
256-
257-
i Unsafe fix: Remove unused declaration.
258-
259-
40 40 │
260-
41 41 │ class Foo {
261-
42 │ - → #usedOnlyInWriteStatement·=·5;
262-
43 42 │ method() {
263-
44 43 │ this.#usedOnlyInWriteStatement += 42;
264-
265-
266-
```
267-
268-
```
269-
invalid.js:49:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
270-
271-
! This private class member is defined but never used.
272-
273-
48 │ class C {
274-
> 49 │ #usedOnlyInIncrement;
275-
│ ^^^^^^^^^^^^^^^^^^^^
276-
50 │
277-
51 │ foo() {
203+
28 │ method() {
204+
29 │ this.#usedOnlyInWrite = 42;
278205
279206
i Unsafe fix: Remove unused declaration.
280207
281-
47 47 │
282-
48 48 │ class C {
283-
49 │ - → #usedOnlyInIncrement;
284-
50 49 │
285-
51 50 │ foo() {
208+
25 25 │
209+
26 26 │ class Foo {
210+
27 │ - → #usedOnlyInWrite·=·5;
211+
28 27 │ method() {
212+
29 28 │ this.#usedOnlyInWrite = 42;
286213
287214
288215
```

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,6 @@ class TsOnlyWrite {
2121
}
2222
}
2323

24-
class TsSelfUpdate {
25-
private usedOnlyToUpdateItself = 5;
26-
27-
method() {
28-
this.usedOnlyToUpdateItself++;
29-
}
30-
}
31-
3224
class TsAccessor {
3325
private get unusedAccessor() { }
3426
private set unusedAccessor(value) { }

0 commit comments

Comments
 (0)