From 80081a805903141345ef56f16d3ea164804299e6 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Mon, 3 Oct 2022 11:50:44 -0300 Subject: [PATCH 1/5] no unused variables accepting ts public/private constructor parameters --- .../nursery/no_unused_variables.rs | 27 ++++++++++++++-- .../noUnusedVariables/noUnusedVariables.ts | 2 ++ .../noUnusedVariables.ts.snap | 2 ++ .../privateOrPublicConstructorParameters.ts | 6 ++++ ...ivateOrPublicConstructorParameters.ts.snap | 31 +++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs index 45049fee33a..460754d3761 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs @@ -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. @@ -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::()?; match parameters.syntax().parent()?.kind() { @@ -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::()?; @@ -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::() { + 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(()) + } + _ => {} + } + } + } + } + + return None; + } _ => None, } } + // example: [key: string]: string; JsSyntaxKind::TS_INDEX_SIGNATURE_PARAMETER => Some(()), + // example: declare function notUsedParameters(a); JsSyntaxKind::TS_DECLARE_FUNCTION_DECLARATION => Some(()), _ => None, } diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts index 122a4acb4f1..b77be7947a1 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts @@ -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) { diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts.snap index 7f4e9404665..2284a4367bc 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/noUnusedVariables.ts.snap @@ -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) { diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts new file mode 100644 index 00000000000..54613baa71d --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts @@ -0,0 +1,6 @@ +class A { + constructor(a, private b, public c) { + + } +} +console.log(new A(1, 2, 3)); \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap new file mode 100644 index 00000000000..45b25c50857 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap @@ -0,0 +1,31 @@ +--- +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. + + ┌─ privateOrPublicConstructorParameters.ts:2:17 + │ + 2 │ constructor(a, private b, public c) { + │ ^ + + i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + + From 8ae153a0c0f419e889d5da0aab88123e4bfd28c8 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Mon, 3 Oct 2022 14:31:24 -0300 Subject: [PATCH 2/5] fmt and clippy issues --- .../src/semantic_analyzers/nursery/no_unused_variables.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs index 460754d3761..8779244135c 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_unused_variables.rs @@ -151,7 +151,7 @@ fn is_typescript_unused_ok(binding: &JsIdentifierBinding) -> Option<()> { } } - return None; + None } _ => None, } From 3c7c8ad4272aa900ed8190a77c5890172c096edf Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Mon, 3 Oct 2022 15:02:30 -0300 Subject: [PATCH 3/5] fixing snap --- .../noUnusedVariables/privateOrPublicConstructorParameters.ts | 3 +-- .../privateOrPublicConstructorParameters.ts.snap | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts index 54613baa71d..f2bdeaee89b 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts @@ -1,6 +1,5 @@ class A { constructor(a, private b, public c) { - } } -console.log(new A(1, 2, 3)); \ No newline at end of file +console.log(new A(1, 2, 3)); diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap index 45b25c50857..5c1b078fa05 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap @@ -6,10 +6,10 @@ expression: privateOrPublicConstructorParameters.ts ```js class A { constructor(a, private b, public c) { - } } console.log(new A(1, 2, 3)); + ``` # Diagnostics From 7bbd25a772c345f07b3ef6a8a3542b290f0cdefe Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Mon, 3 Oct 2022 15:37:08 -0300 Subject: [PATCH 4/5] fixing snap --- .../noUnusedVariables/privateOrPublicConstructorParameters.ts | 1 + .../privateOrPublicConstructorParameters.ts.snap | 1 + 2 files changed, 2 insertions(+) diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts index f2bdeaee89b..2c9a50bd36f 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts @@ -2,4 +2,5 @@ class A { constructor(a, private b, public c) { } } + console.log(new A(1, 2, 3)); diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap index 5c1b078fa05..e40e52e15c7 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap @@ -8,6 +8,7 @@ class A { constructor(a, private b, public c) { } } + console.log(new A(1, 2, 3)); ``` From b9a8f1ac878678e115415baf4301673c9f3faae3 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Mon, 3 Oct 2022 16:01:28 -0300 Subject: [PATCH 5/5] fixing snap --- .../privateOrPublicConstructorParameters.ts.snap | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap index e40e52e15c7..df8ae6e57dd 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap @@ -19,10 +19,11 @@ privateOrPublicConstructorParameters.ts:2:17 lint/nursery/noUnusedVariables ━ ! This parameter is unused. - ┌─ privateOrPublicConstructorParameters.ts:2:17 - │ - 2 │ constructor(a, private b, public c) { - │ ^ + 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.