Skip to content

feat(biome-js-analyze): adjusts members update assignments to be marked as valid in noUnusedPrivateClassMembers #7500

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

feat(biome-js-analyze): adjusts members update assignments to be marked as valid in noUnusedPrivateClassMembers #7500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/no_unused_private_class_members_bug.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
"@biomejs/biome": patch
---

Fixed [#7192](https://github.com/biomejs/biome/issues/7192):
`noUnusedPrivateClassMembers` now treats private members in compound assignments (+=, -=, ??=, etc.) as used,
while plain assignments (=) do not count as usage.

Example that is now correctly flagged:

```typescript
class App {
#persistenceRequest: Promise<boolean> | undefined;
saveData() {
this.#persistenceRequest += 2;
}
}
```
27 changes: 20 additions & 7 deletions crates/biome_js_analyze/src/lint/correctness/no_unused_private_class_members.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use biome_diagnostics::Severity;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{
AnyJsClassMember, AnyJsClassMemberName, AnyJsComputedMember, AnyJsExpression,
AnyJsFormalParameter, AnyJsName, JsAssignmentExpression, JsClassDeclaration, JsSyntaxKind,
JsSyntaxNode, TsAccessibilityModifier, TsPropertyParameter,
AnyJsFormalParameter, AnyJsName, JsAssignmentExpression, JsAssignmentOperator,
JsClassDeclaration, JsSyntaxKind, JsSyntaxNode, TsAccessibilityModifier, TsPropertyParameter,
};
use biome_rowan::{
AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, SyntaxNodeOptionExt, TextRange,
Expand Down Expand Up @@ -268,7 +268,11 @@ fn traverse_members_usage(
is_write_only(&js_name) == Some(true) && !private_member.is_accessor();
let is_in_update_expression = is_in_update_expression(&js_name);

if is_in_update_expression || is_write_only {
if is_in_update_expression {
return false;
}

if is_write_only {
return true;
}

Expand Down Expand Up @@ -423,10 +427,19 @@ fn is_in_update_expression(js_name: &AnyJsName) -> bool {
return false;
}

matches!(
grand_parent.kind(),
JsSyntaxKind::JS_POST_UPDATE_EXPRESSION | JsSyntaxKind::JS_PRE_UPDATE_EXPRESSION
)
// grand_parent can also be js expression statement
match grand_parent.kind() {
JsSyntaxKind::JS_POST_UPDATE_EXPRESSION | JsSyntaxKind::JS_PRE_UPDATE_EXPRESSION => true,
JsSyntaxKind::JS_ASSIGNMENT_EXPRESSION => {
if let Some(assign_expr) = JsAssignmentExpression::cast(grand_parent.clone())
&& let Ok(op_kind) = assign_expr.operator()
{
return op_kind != JsAssignmentOperator::Assign;
}
false
}
_ => false,
}
}

impl AnyMember {
Expand Down
8 changes: 0 additions & 8 deletions crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,6 @@ class OnlyWrite {
}
}

class SelfUpdate {
#usedOnlyToUpdateItself = 5;

method() {
this.#usedOnlyToUpdateItself++;
}
}

class Accessor {
get #unusedAccessor() {}
set #unusedAccessor(value) {}
Expand Down
157 changes: 42 additions & 115 deletions crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 152
expression: invalid.js
---
# Input
Expand All @@ -20,14 +21,6 @@ class OnlyWrite {
}
}

class SelfUpdate {
#usedOnlyToUpdateItself = 5;

method() {
this.#usedOnlyToUpdateItself++;
}
}

class Accessor {
get #unusedAccessor() {}
set #unusedAccessor(value) {}
Expand Down Expand Up @@ -133,156 +126,90 @@ invalid.js:10:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━
```

```
invalid.js:18:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:18:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

17 │ class SelfUpdate {
> 18 │ #usedOnlyToUpdateItself = 5;
│ ^^^^^^^^^^^^^^^^^^^^^^^
19 │
20 │ method() {

i Unsafe fix: Remove unused declaration.

16 16 │
17 17 │ class SelfUpdate {
18 │ - → #usedOnlyToUpdateItself·=·5;
19 18 │
20 19 │ method() {


```

```
invalid.js:26:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

25 │ class Accessor {
> 26 │ get #unusedAccessor() {}
17 │ class Accessor {
> 18 │ get #unusedAccessor() {}
│ ^^^^^^^^^^^^^^^
27 │ set #unusedAccessor(value) {}
28 │ }
19 │ set #unusedAccessor(value) {}
20 │ }

i Unsafe fix: Remove unused declaration.

24 24 │
25 25 │ class Accessor {
26 │ - → get·#unusedAccessor()·{}
27 26 │ set #unusedAccessor(value) {}
28 27 │ }
16 16 │
17 17 │ class Accessor {
18 │ - → get·#unusedAccessor()·{}
19 18 │ set #unusedAccessor(value) {}
20 19 │ }


```

```
invalid.js:27:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:19:6 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

25 │ class Accessor {
26 │ get #unusedAccessor() {}
> 27 │ set #unusedAccessor(value) {}
17 │ class Accessor {
18 │ get #unusedAccessor() {}
> 19 │ set #unusedAccessor(value) {}
│ ^^^^^^^^^^^^^^^
28 │ }
29 │
20 │ }
21 │

i Unsafe fix: Remove unused declaration.

25 25 │ class Accessor {
26 26 │ get #unusedAccessor() {}
27 │ - → set·#unusedAccessor(value)·{}
28 27 │ }
29 28 │
17 17 │ class Accessor {
18 18 │ get #unusedAccessor() {}
19 │ - → set·#unusedAccessor(value)·{}
20 19 │ }
21 20 │


```

```
invalid.js:31:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:23:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

30 │ class First {
> 31 │ #unusedMemberInFirstClass = 5;
22 │ class First {
> 23 │ #unusedMemberInFirstClass = 5;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^
32 │ }
33 │
24 │ }
25 │

i Unsafe fix: Remove unused declaration.

29 29 │
30 30 │ class First {
31 │ - → #unusedMemberInFirstClass·=·5;
32 31 │ }
33 32 │
21 21 │
22 22 │ class First {
23 │ - → #unusedMemberInFirstClass·=·5;
24 23 │ }
25 24 │


```

```
invalid.js:35:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.js:27:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

34 │ class Foo {
> 35 │ #usedOnlyInWrite = 5;
26 │ class Foo {
> 27 │ #usedOnlyInWrite = 5;
│ ^^^^^^^^^^^^^^^^
36 │ method() {
37 │ this.#usedOnlyInWrite = 42;

i Unsafe fix: Remove unused declaration.

33 33 │
34 34 │ class Foo {
35 │ - → #usedOnlyInWrite·=·5;
36 35 │ method() {
37 36 │ this.#usedOnlyInWrite = 42;


```

```
invalid.js:42:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

41 │ class Foo {
> 42 │ #usedOnlyInWriteStatement = 5;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^
43 │ method() {
44 │ this.#usedOnlyInWriteStatement += 42;

i Unsafe fix: Remove unused declaration.

40 40 │
41 41 │ class Foo {
42 │ - → #usedOnlyInWriteStatement·=·5;
43 42 │ method() {
44 43 │ this.#usedOnlyInWriteStatement += 42;


```

```
invalid.js:49:2 lint/correctness/noUnusedPrivateClassMembers FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This private class member is defined but never used.

48 │ class C {
> 49 │ #usedOnlyInIncrement;
│ ^^^^^^^^^^^^^^^^^^^^
50 │
51 │ foo() {
28 │ method() {
29 │ this.#usedOnlyInWrite = 42;

i Unsafe fix: Remove unused declaration.

47 47 │
48 48 │ class C {
49 │ - → #usedOnlyInIncrement;
50 49 │
51 50 │ foo() {
25 25 │
26 26 │ class Foo {
27 │ - → #usedOnlyInWrite·=·5;
28 27 │ method() {
29 28 │ this.#usedOnlyInWrite = 42;


```
8 changes: 0 additions & 8 deletions crates/biome_js_analyze/tests/specs/correctness/noUnusedPrivateClassMembers/invalid.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,6 @@ class TsOnlyWrite {
}
}

class TsSelfUpdate {
private usedOnlyToUpdateItself = 5;

method() {
this.usedOnlyToUpdateItself++;
}
}

class TsAccessor {
private get unusedAccessor() { }
private set unusedAccessor(value) { }
Expand Down
Loading