Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_analyze): no unused variables accepting ts public/private…
Browse files Browse the repository at this point in the history
… constructor parameters (#3316)
  • Loading branch information
xunilrj authored Oct 4, 2022
1 parent 2f552e7 commit 66ab560
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use rome_js_semantic::{AllReferencesExtensions, SemanticScopeExtensions};
use rome_js_syntax::{
JsClassExpression, JsConstructorParameterList, JsConstructorParameters, JsFunctionDeclaration,
JsFunctionExpression, JsIdentifierBinding, JsParameterList, JsParameters, JsSyntaxKind,
JsVariableDeclarator,
JsVariableDeclarator, TsPropertyParameter,
};
use rome_rowan::AstNode;
use rome_rowan::{AstNode, SyntaxNodeCast};

declare_rule! {
/// Disallow unused variables.
Expand Down Expand Up @@ -110,6 +110,7 @@ fn is_typescript_unused_ok(binding: &JsIdentifierBinding) -> Option<()> {
let parameter = binding.syntax().parent()?;
let parent = parameter.parent()?;
match parent.kind() {
// example: abstract f(a: number);
JsSyntaxKind::JS_PARAMETER_LIST => {
let parameters = JsParameterList::cast(parent)?.parent::<JsParameters>()?;
match parameters.syntax().parent()?.kind() {
Expand All @@ -121,6 +122,7 @@ fn is_typescript_unused_ok(binding: &JsIdentifierBinding) -> Option<()> {
_ => None,
}
}
// example: constructor(a: number);
JsSyntaxKind::JS_CONSTRUCTOR_PARAMETER_LIST => {
let parameters = JsConstructorParameterList::cast(parent)?
.parent::<JsConstructorParameters>()?;
Expand All @@ -130,12 +132,33 @@ fn is_typescript_unused_ok(binding: &JsIdentifierBinding) -> Option<()> {
_ => None,
}
}
// example: abstract set a(a: number);
// We don't need get because getter do not have parameters
JsSyntaxKind::TS_SETTER_SIGNATURE_TYPE_MEMBER
| JsSyntaxKind::TS_SETTER_SIGNATURE_CLASS_MEMBER => Some(()),
// example: constructor(a, private b, public c) {}
JsSyntaxKind::TS_PROPERTY_PARAMETER => {
if let Some(parent) = parent.cast::<TsPropertyParameter>() {
for modifier in parent.modifiers().into_iter() {
if let Some(modifier) = modifier.as_ts_accessibility_modifier() {
match modifier.modifier_token().ok()?.kind() {
JsSyntaxKind::PRIVATE_KW | JsSyntaxKind::PUBLIC_KW => {
return Some(())
}
_ => {}
}
}
}
}

None
}
_ => None,
}
}
// example: [key: string]: string;
JsSyntaxKind::TS_INDEX_SIGNATURE_PARAMETER => Some(()),
// example: declare function notUsedParameters(a);
JsSyntaxKind::TS_DECLARE_FUNCTION_DECLARATION => Some(()),
_ => None,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ f();

export type Command = (...args: any[]) => unknown;

declare function notUsedParameters(a);

function used_overloaded(): number;
function used_overloaded(s: string): string;
function used_overloaded(s?: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ f();

export type Command = (...args: any[]) => unknown;

declare function notUsedParameters(a);

function used_overloaded(): number;
function used_overloaded(s: string): string;
function used_overloaded(s?: string) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class A {
constructor(a, private b, public c) {
}
}

console.log(new A(1, 2, 3));
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: privateOrPublicConstructorParameters.ts
---
# Input
```js
class A {
constructor(a, private b, public c) {
}
}

console.log(new A(1, 2, 3));

```

# Diagnostics
```
privateOrPublicConstructorParameters.ts:2:17 lint/nursery/noUnusedVariables ━━━━━━━━━━━━━━━━━━━━━━━━
! This parameter is unused.
1 │ class A {
> 2 │ constructor(a, private b, public c) {
│ ^
3 │ }
4 │ }
i Unused variables usually are result of incomplete refactoring, typos and other source of bugs.
```


0 comments on commit 66ab560

Please sign in to comment.