From 66ab5605bd0f90825f54d339930d172dd306da8b Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Tue, 4 Oct 2022 08:44:25 +0100 Subject: [PATCH] fix(rome_js_analyze): no unused variables accepting ts public/private constructor parameters (#3316) --- .../nursery/no_unused_variables.rs | 27 +++++++++++++-- .../noUnusedVariables/noUnusedVariables.ts | 2 ++ .../noUnusedVariables.ts.snap | 2 ++ .../privateOrPublicConstructorParameters.ts | 6 ++++ ...ivateOrPublicConstructorParameters.ts.snap | 33 +++++++++++++++++++ 5 files changed, 68 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..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 @@ -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(()) + } + _ => {} + } + } + } + } + + 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..2c9a50bd36f --- /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)); 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..df8ae6e57dd --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnusedVariables/privateOrPublicConstructorParameters.ts.snap @@ -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. + + +``` + +