From b2f6b5a6cd533ab3fb9ed5955041f4d113fa2ec5 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Thu, 11 Aug 2022 17:47:27 +0100 Subject: [PATCH] feat(rome_js_analyze): improvements to noUnusedVariables to consider catch, typescript and export (#3004) * improvements to noUnusedVariables to consider catch, typescript and export. --- crates/rome_cli/tests/main.rs | 3 +- .../tests/snapshots/apply_suggested.snap | 2 +- .../snapshots/apply_suggested_error.snap | 2 +- .../js/no_unused_variables.rs | 92 +++++-- crates/rome_js_analyze/src/utils/batch.rs | 168 +++++++++--- crates/rome_js_analyze/src/utils/tests.rs | 11 +- crates/rome_js_analyze/tests/spec_tests.rs | 3 +- .../noUnusedVariables.js | 11 +- .../noUnusedVariables.js.snap | 243 ++++++++++-------- .../js/noUnusedVariables/noUnusedVariables.ts | 30 +++ .../noUnusedVariables.ts.snap | 99 +++++++ crates/rome_service/src/configuration/mod.rs | 2 +- crates/tests_macros/src/lib.rs | 18 +- 13 files changed, 508 insertions(+), 176 deletions(-) rename crates/rome_js_analyze/tests/specs/js/{ => noUnusedVariables}/noUnusedVariables.js (66%) rename crates/rome_js_analyze/tests/specs/js/{ => noUnusedVariables}/noUnusedVariables.js.snap (51%) create mode 100644 crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts create mode 100644 crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts.snap diff --git a/crates/rome_cli/tests/main.rs b/crates/rome_cli/tests/main.rs index 92879060e28..fea935fbb53 100644 --- a/crates/rome_cli/tests/main.rs +++ b/crates/rome_cli/tests/main.rs @@ -41,9 +41,10 @@ if(a != 0) {} const APPLY_SUGGESTED_BEFORE: &str = "let a = 4; debugger; +console.log(a); "; -const APPLY_SUGGESTED_AFTER: &str = "let a = 4;\n"; +const APPLY_SUGGESTED_AFTER: &str = "let a = 4;\nconsole.log(a);\n"; const CUSTOM_FORMAT_BEFORE: &str = r#" function f() { diff --git a/crates/rome_cli/tests/snapshots/apply_suggested.snap b/crates/rome_cli/tests/snapshots/apply_suggested.snap index 0cf3081537c..8473008c2dd 100644 --- a/crates/rome_cli/tests/snapshots/apply_suggested.snap +++ b/crates/rome_cli/tests/snapshots/apply_suggested.snap @@ -1,12 +1,12 @@ --- source: crates/rome_cli/tests/snap_test.rs -assertion_line: 137 expression: content --- ## `fix.js` ```js let a = 4; +console.log(a); ``` diff --git a/crates/rome_cli/tests/snapshots/apply_suggested_error.snap b/crates/rome_cli/tests/snapshots/apply_suggested_error.snap index 94be9946614..4e76512b8f5 100644 --- a/crates/rome_cli/tests/snapshots/apply_suggested_error.snap +++ b/crates/rome_cli/tests/snapshots/apply_suggested_error.snap @@ -1,6 +1,5 @@ --- source: crates/rome_cli/tests/snap_test.rs -assertion_line: 107 expression: content --- ## `fix.js` @@ -8,6 +7,7 @@ expression: content ```js let a = 4; debugger; +console.log(a); ``` diff --git a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs index 45317a3fc7a..92c089a49d7 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs @@ -6,7 +6,8 @@ use rome_console::markup; use rome_diagnostics::Applicability; use rome_js_semantic::{AllReferencesExtensions, SemanticScopeExtensions}; use rome_js_syntax::{ - JsFormalParameter, JsFunctionDeclaration, JsIdentifierBinding, JsSyntaxKind, + JsCatchDeclaration, JsConstructorParameterList, JsConstructorParameters, JsFormalParameter, + JsFunctionDeclaration, JsIdentifierBinding, JsParameterList, JsParameters, JsSyntaxKind, JsVariableDeclarator, }; use rome_rowan::{AstNode, BatchMutationExt}; @@ -71,6 +72,43 @@ declare_rule! { } } +// It is ok in some Typescripts constructs for a parameter to be unused. +fn is_typescript_unused_ok(binding: &JsIdentifierBinding) -> Option<()> { + match binding.syntax().parent()?.kind() { + JsSyntaxKind::JS_FORMAL_PARAMETER => { + let parameter = binding.parent::()?; + match parameter.syntax().parent()?.kind() { + JsSyntaxKind::JS_PARAMETER_LIST => { + let parameters = parameter + .parent::()? + .parent::()?; + match parameters.syntax().parent()?.kind() { + JsSyntaxKind::TS_METHOD_SIGNATURE_CLASS_MEMBER + | JsSyntaxKind::TS_CALL_SIGNATURE_TYPE_MEMBER + | JsSyntaxKind::TS_METHOD_SIGNATURE_TYPE_MEMBER => Some(()), + _ => None, + } + } + JsSyntaxKind::JS_CONSTRUCTOR_PARAMETER_LIST => { + let parameters = parameter + .parent::()? + .parent::()?; + match parameters.syntax().parent()?.kind() { + JsSyntaxKind::TS_CONSTRUCT_SIGNATURE_TYPE_MEMBER + | JsSyntaxKind::TS_CONSTRUCTOR_SIGNATURE_CLASS_MEMBER => Some(()), + _ => None, + } + } + JsSyntaxKind::TS_SETTER_SIGNATURE_TYPE_MEMBER + | JsSyntaxKind::TS_SETTER_SIGNATURE_CLASS_MEMBER => Some(()), + _ => None, + } + } + JsSyntaxKind::TS_INDEX_SIGNATURE_PARAMETER => Some(()), + _ => None, + } +} + impl Rule for NoUnusedVariables { const CATEGORY: RuleCategory = RuleCategory::Lint; @@ -82,6 +120,14 @@ impl Rule for NoUnusedVariables { let binding = ctx.query(); let model = ctx.model(); + if is_typescript_unused_ok(binding).is_some() { + return None; + } + + if model.is_exported(binding) { + return None; + } + let all_references = binding.all_references(model); if all_references.count() == 0 { @@ -158,26 +204,36 @@ impl Rule for NoUnusedVariables { let mut batch = root.begin(); - // If this is a function, remove the whole declaration - if let Some(declaration) = binding.parent::() { - batch.remove_node(declaration) + // Try to remove node + let removed = if let Some(declaration) = binding.parent::() { + batch.remove_node(declaration); + true } else if let Some(variable_declarator) = binding.parent::() { - batch.remove_js_variable_declarator(&variable_declarator); + batch.remove_js_variable_declarator(&variable_declarator) } else if let Some(formal_parameter) = binding.parent::() { - batch.remove_js_formal_parameter(&formal_parameter); - } - - let symbol_type = match binding.syntax().parent().unwrap().kind() { - JsSyntaxKind::JS_FORMAL_PARAMETER => "parameter", - JsSyntaxKind::JS_FUNCTION_DECLARATION => "function", - _ => "variable", + batch.remove_js_formal_parameter(&formal_parameter) + } else if let Some(catch_declaration) = binding.parent::() { + batch.remove_node(catch_declaration); + true + } else { + false }; - Some(JsRuleAction { - category: ActionCategory::QuickFix, - applicability: Applicability::Unspecified, - message: markup! { "Remove this " {symbol_type} "." }.to_owned(), - mutation: batch, - }) + if removed { + let symbol_type = match binding.syntax().parent().unwrap().kind() { + JsSyntaxKind::JS_FORMAL_PARAMETER => "parameter", + JsSyntaxKind::JS_FUNCTION_DECLARATION => "function", + _ => "variable", + }; + + Some(JsRuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + message: markup! { "Remove this " {symbol_type} "." }.to_owned(), + mutation: batch, + }) + } else { + None + } } } diff --git a/crates/rome_js_analyze/src/utils/batch.rs b/crates/rome_js_analyze/src/utils/batch.rs index 43e3f707f2e..1476d507bdc 100644 --- a/crates/rome_js_analyze/src/utils/batch.rs +++ b/crates/rome_js_analyze/src/utils/batch.rs @@ -1,5 +1,6 @@ use rome_js_syntax::{ - JsAnyFormalParameter, JsAnyParameter, JsFormalParameter, JsLanguage, JsParameterList, + JsAnyConstructorParameter, JsAnyFormalParameter, JsAnyParameter, JsConstructorParameterList, + JsFormalParameter, JsLanguage, JsParameterList, JsSyntaxKind, JsSyntaxNode, JsVariableDeclaration, JsVariableDeclarator, JsVariableDeclaratorList, JsVariableStatement, }; use rome_rowan::{AstNode, AstSeparatedList, BatchMutation}; @@ -8,15 +9,99 @@ pub trait JsBatchMutation { /// Removes the declarator, and: /// 1 - removes the statement if the declaration only has one declarator; /// 2 - removes commas around the declarator to keep the list valid. - fn remove_js_variable_declarator(&mut self, declarator: &JsVariableDeclarator); + fn remove_js_variable_declarator(&mut self, declarator: &JsVariableDeclarator) -> bool; /// Removes the parameter, and: /// 1 - removes commas around the parameter to keep the list valid. - fn remove_js_formal_parameter(&mut self, parameter: &JsFormalParameter); + fn remove_js_formal_parameter(&mut self, parameter: &JsFormalParameter) -> bool; +} + +fn remove_js_formal_parameter_from_js_parameter_list( + batch: &mut BatchMutation, + parameter: &JsFormalParameter, + list: JsSyntaxNode, +) -> Option { + let list = JsParameterList::cast(list)?; + let mut elements = list.elements(); + + // Find the parameter we want to remove + // remove its trailing comma, if there is one + let mut previous_element = None; + for element in elements.by_ref() { + if let Ok(node) = element.node() { + match node { + JsAnyParameter::JsAnyFormalParameter(JsAnyFormalParameter::JsFormalParameter( + node, + )) if node == parameter => { + batch.remove_node(node.clone()); + if let Some(comma) = element.trailing_separator().ok().flatten() { + batch.remove_token(comma.clone()); + } + break; + } + _ => {} + } + } + previous_element = Some(element); + } + + // if it is the last parameter of the list + // removes the comma before this element + if elements.next().is_none() { + if let Some(element) = previous_element { + if let Some(comma) = element.trailing_separator().ok().flatten() { + batch.remove_token(comma.clone()); + } + } + } + + Some(true) +} + +fn remove_js_formal_parameter_from_js_constructor_parameter_list( + batch: &mut BatchMutation, + parameter: &JsFormalParameter, + list: JsSyntaxNode, +) -> Option { + let list = JsConstructorParameterList::cast(list)?; + let mut elements = list.elements(); + + // Find the parameter we want to remove + // remove its trailing comma, if there is one + let mut previous_element = None; + for element in elements.by_ref() { + if let Ok(node) = element.node() { + match node { + JsAnyConstructorParameter::JsAnyFormalParameter( + JsAnyFormalParameter::JsFormalParameter(node), + ) if node == parameter => { + batch.remove_node(node.clone()); + if let Some(comma) = element.trailing_separator().ok().flatten() { + batch.remove_token(comma.clone()); + } + break; + } + _ => {} + } + } + previous_element = Some(element); + } + + // if it is the last parameter of the list + // removes the comma before this element + if elements.next().is_none() { + if let Some(element) = previous_element { + if let Some(comma) = element.trailing_separator().ok().flatten() { + batch.remove_token(comma.clone()); + } + } + } + + Some(true) } impl JsBatchMutation for BatchMutation { - fn remove_js_variable_declarator(&mut self, declarator: &JsVariableDeclarator) { + fn remove_js_variable_declarator(&mut self, declarator: &JsVariableDeclarator) -> bool { declarator .parent::() .and_then(|list| { @@ -61,47 +146,27 @@ impl JsBatchMutation for BatchMutation { } } - Some(()) - }); + Some(true) + }) + .unwrap_or(false) } - fn remove_js_formal_parameter(&mut self, parameter: &JsFormalParameter) { - parameter.parent::().map(|list| { - let mut elements = list.elements(); - - // Find the parameter we want to remove - // remove its trailing comma, if there is one - let mut previous_element = None; - for element in elements.by_ref() { - if let Ok(node) = element.node() { - match node { - JsAnyParameter::JsAnyFormalParameter( - JsAnyFormalParameter::JsFormalParameter(node), - ) if node == parameter => { - self.remove_node(node.clone()); - if let Some(comma) = element.trailing_separator().ok().flatten() { - self.remove_token(comma.clone()); - } - break; - } - _ => {} - } + fn remove_js_formal_parameter(&mut self, parameter: &JsFormalParameter) -> bool { + parameter + .syntax() + .parent() + .and_then(|parent| match parent.kind() { + JsSyntaxKind::JS_PARAMETER_LIST => { + remove_js_formal_parameter_from_js_parameter_list(self, parameter, parent) } - previous_element = Some(element); - } - - // if it is the last parameter of the list - // removes the comma before this element - if elements.next().is_none() { - if let Some(element) = previous_element { - if let Some(comma) = element.trailing_separator().ok().flatten() { - self.remove_token(comma.clone()); - } + JsSyntaxKind::JS_CONSTRUCTOR_PARAMETER_LIST => { + remove_js_formal_parameter_from_js_constructor_parameter_list( + self, parameter, parent, + ) } - } - - Some(()) - }); + _ => None, + }) + .unwrap_or(false) } } @@ -128,7 +193,7 @@ mod tests { "let b, c;", } - // Remove JsFormalParameter + // Remove JsFormalParameter from functions assert_remove_ok! { ok_remove_formal_parameter_single, "function f(a) {}", @@ -146,4 +211,23 @@ mod tests { "function f(b, a, c) {}", "function f(b, c) {}", } + + // Remove JsFormalParameter from class constructors + assert_remove_ok! { + ok_remove_formal_parameter_from_class_constructor_single, + "class A { constructor(a) {} }", + "class A { constructor() {} }", + ok_remove_formal_parameter_from_class_constructor_first, + "class A { constructor(a, b) {} }", + "class A { constructor(b) {} }", + ok_remove_formal_parameter_from_class_constructor_second, + "class A { constructor(b, a) {} }", + "class A { constructor(b) {} }", + ok_remove_formal_parameter_from_class_constructor_second_trailing_comma, + "class A { constructor(b, a,) {} }", + "class A { constructor(b) {} }", + ok_remove_formal_parameter_from_class_constructor_middle, + "class A { constructor(b, a, c) {} }", + "class A { constructor(b, c) {} }", + } } diff --git a/crates/rome_js_analyze/src/utils/tests.rs b/crates/rome_js_analyze/src/utils/tests.rs index 67f3d49c3ee..86431e58914 100644 --- a/crates/rome_js_analyze/src/utils/tests.rs +++ b/crates/rome_js_analyze/src/utils/tests.rs @@ -57,11 +57,14 @@ pub fn assert_remove_ok(before: &str, expected: &str) { let mut batch = r.tree().begin(); - if let Some(parameter) = binding_a.parent::() { - batch.remove_js_formal_parameter(¶meter); + let r = if let Some(parameter) = binding_a.parent::() { + batch.remove_js_formal_parameter(¶meter) } else if let Some(declarator) = binding_a.parent::() { - batch.remove_js_variable_declarator(&declarator); - } + batch.remove_js_variable_declarator(&declarator) + } else { + panic!("Don't know how to remove this node: {:?}", binding_a); + }; + assert!(r); let root = batch.commit(); let after = root.to_string(); diff --git a/crates/rome_js_analyze/tests/spec_tests.rs b/crates/rome_js_analyze/tests/spec_tests.rs index 6a524bcadd4..85d940ee50e 100644 --- a/crates/rome_js_analyze/tests/spec_tests.rs +++ b/crates/rome_js_analyze/tests/spec_tests.rs @@ -31,7 +31,8 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { let parsed = parse(&input_code, 0, source_type); let root = parsed.tree(); - let (group, rule) = parse_test_path(input_file); + let (group, rule) = dbg!(parse_test_path(input_file)); + let rule_filter = RuleFilter::Rule(group, rule); let filter = AnalysisFilter { enabled_rules: Some(slice::from_ref(&rule_filter)), diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js similarity index 66% rename from crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js rename to crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js index bb82f531ff4..1f9ccb23c09 100644 --- a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js +++ b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js @@ -1,5 +1,6 @@ const a = 1; -const b = 2, c = 3; +const b = 2, + c = 3; console.log(c); function f1() {} @@ -33,3 +34,11 @@ const f5 = () => {}; const f6 = () => { f6(); }; + +try { +} catch (e) {} + +export function exported_function() {} + +function exported_function_2() {} +export { exported_function_2 }; diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js.snap b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js.snap similarity index 51% rename from crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js.snap rename to crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js.snap index f608fa18c77..6dac33f4909 100644 --- a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js.snap +++ b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.js.snap @@ -5,7 +5,8 @@ expression: noUnusedVariables.js # Input ```js const a = 1; -const b = 2, c = 3; +const b = 2, + c = 3; console.log(c); function f1() {} @@ -40,6 +41,14 @@ const f6 = () => { f6(); }; +try { +} catch (e) {} + +export function exported_function() {} + +function exported_function_2() {} +export { exported_function_2 }; + ``` # Diagnostics @@ -54,9 +63,9 @@ Suggested fix: Remove this variable. | @@ -1,4 +1,4 @@ 0 | - const a = 1; 0 | + -1 1 | const b = 2, c = 3; -2 2 | console.log(c); -3 3 | +1 1 | const b = 2, +2 2 | c = 3; +3 3 | console.log(c); = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -67,17 +76,17 @@ Suggested fix: Remove this variable. warning[js/noUnusedVariables]: This variable is unused. ┌─ noUnusedVariables.js:2:7 │ -2 │ const b = 2, c = 3; +2 │ const b = 2, │ - Suggested fix: Remove this variable. | @@ -1,5 +1,5 @@ 0 0 | const a = 1; -1 | - const b = 2, c = 3; - 1 | + const c = 3; -2 2 | console.log(c); -3 3 | -4 4 | function f1() {} +1 | - const b = 2, + 1 | + const +2 2 | c = 3; +3 3 | console.log(c); +4 4 | = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -86,21 +95,21 @@ Suggested fix: Remove this variable. ``` warning[js/noUnusedVariables]: This function is unused. - ┌─ noUnusedVariables.js:5:10 + ┌─ noUnusedVariables.js:6:10 │ -5 │ function f1() {} +6 │ function f1() {} │ -- Suggested fix: Remove this function. - | @@ -2,8 +2,6 @@ -1 1 | const b = 2, c = 3; -2 2 | console.log(c); -3 3 | -4 | - function f1() {} -5 | - -6 4 | function f2() { -7 5 | f2(); -8 6 | } + | @@ -3,8 +3,6 @@ +2 2 | c = 3; +3 3 | console.log(c); +4 4 | +5 | - function f1() {} +6 | - +7 5 | function f2() { +8 6 | f2(); +9 7 | } = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -109,23 +118,23 @@ Suggested fix: Remove this function. ``` warning[js/noUnusedVariables]: This function is unused. - ┌─ noUnusedVariables.js:7:10 + ┌─ noUnusedVariables.js:8:10 │ -7 │ function f2() { +8 │ function f2() { │ -- Suggested fix: Remove this function. - | @@ -4,10 +4,6 @@ -3 3 | -4 4 | function f1() {} -5 5 | -6 | - function f2() { -7 | - f2(); -8 | - } -9 | - -10 6 | function f3() { -11 7 | function g() { -12 8 | f3(); + | @@ -5,10 +5,6 @@ +4 4 | +5 5 | function f1() {} +6 6 | +7 | - function f2() { +8 | - f2(); +9 | - } +10 | - +11 7 | function f3() { +12 8 | function g() { +13 9 | f3(); = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -134,26 +143,26 @@ Suggested fix: Remove this function. ``` warning[js/noUnusedVariables]: This function is unused. - ┌─ noUnusedVariables.js:11:10 + ┌─ noUnusedVariables.js:12:10 │ -11 │ function f3() { +12 │ function f3() { │ -- Suggested fix: Remove this function. - | @@ -8,13 +8,6 @@ - 7 7 | f2(); - 8 8 | } - 9 9 | -10 | - function f3() { -11 | - function g() { -12 | - f3(); -13 | - } -14 | - g(); -15 | - } -16 | - -17 10 | function f41(a) {} -18 11 | f41(); -19 12 | + | @@ -9,13 +9,6 @@ + 8 8 | f2(); + 9 9 | } +10 10 | +11 | - function f3() { +12 | - function g() { +13 | - f3(); +14 | - } +15 | - g(); +16 | - } +17 | - +18 11 | function f41(a) {} +19 12 | f41(); +20 13 | = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -162,21 +171,21 @@ Suggested fix: Remove this function. ``` warning[js/noUnusedVariables]: This parameter is unused. - ┌─ noUnusedVariables.js:18:14 + ┌─ noUnusedVariables.js:19:14 │ -18 │ function f41(a) {} +19 │ function f41(a) {} │ - Suggested fix: Remove this parameter. - | @@ -15,7 +15,7 @@ -14 14 | g(); -15 15 | } -16 16 | -17 | - function f41(a) {} - 17 | + function f41() {} -18 18 | f41(); -19 19 | -20 20 | function f42(a, b) { + | @@ -16,7 +16,7 @@ +15 15 | g(); +16 16 | } +17 17 | +18 | - function f41(a) {} + 18 | + function f41() {} +19 19 | f41(); +20 20 | +21 21 | function f42(a, b) { = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -185,21 +194,21 @@ Suggested fix: Remove this parameter. ``` warning[js/noUnusedVariables]: This parameter is unused. - ┌─ noUnusedVariables.js:21:17 + ┌─ noUnusedVariables.js:22:17 │ -21 │ function f42(a, b) { +22 │ function f42(a, b) { │ - Suggested fix: Remove this parameter. - | @@ -18,7 +18,7 @@ -17 17 | function f41(a) {} -18 18 | f41(); -19 19 | -20 | - function f42(a, b) { - 20 | + function f42(a) { -21 21 | console.log(a); -22 22 | } -23 23 | f42(); + | @@ -19,7 +19,7 @@ +18 18 | function f41(a) {} +19 19 | f41(); +20 20 | +21 | - function f42(a, b) { + 21 | + function f42(a) { +22 22 | console.log(a); +23 23 | } +24 24 | f42(); = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -208,21 +217,21 @@ Suggested fix: Remove this parameter. ``` warning[js/noUnusedVariables]: This parameter is unused. - ┌─ noUnusedVariables.js:26:17 + ┌─ noUnusedVariables.js:27:17 │ -26 │ function f43(a, b) { +27 │ function f43(a, b) { │ - Suggested fix: Remove this parameter. - | @@ -23,7 +23,7 @@ -22 22 | } -23 23 | f42(); -24 24 | -25 | - function f43(a, b) { - 25 | + function f43(a) { -26 26 | console.log(a); -27 27 | } -28 28 | f43(); + | @@ -24,7 +24,7 @@ +23 23 | } +24 24 | f42(); +25 25 | +26 | - function f43(a, b) { + 26 | + function f43(a) { +27 27 | console.log(a); +28 28 | } +29 29 | f43(); = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -231,21 +240,21 @@ Suggested fix: Remove this parameter. ``` warning[js/noUnusedVariables]: This variable is unused. - ┌─ noUnusedVariables.js:31:7 + ┌─ noUnusedVariables.js:32:7 │ -31 │ const f5 = () => {}; +32 │ const f5 = () => {}; │ -- Suggested fix: Remove this variable. - | @@ -28,8 +28,6 @@ -27 27 | } -28 28 | f43(); -29 29 | -30 | - const f5 = () => {}; -31 | - -32 30 | const f6 = () => { -33 31 | f6(); -34 32 | }; + | @@ -29,8 +29,6 @@ +28 28 | } +29 29 | f43(); +30 30 | +31 | - const f5 = () => {}; +32 | - +33 31 | const f6 = () => { +34 32 | f6(); +35 33 | }; = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -254,20 +263,46 @@ Suggested fix: Remove this variable. ``` warning[js/noUnusedVariables]: This variable is unused. - ┌─ noUnusedVariables.js:33:7 + ┌─ noUnusedVariables.js:34:7 │ -33 │ const f6 = () => { +34 │ const f6 = () => { │ -- Suggested fix: Remove this variable. - | @@ -29,7 +29,3 @@ -28 28 | f43(); -29 29 | -30 30 | const f5 = () => {}; -31 | - -32 | - const f6 = () => { -33 | - f6(); -34 | - }; + | @@ -31,10 +31,6 @@ +30 30 | +31 31 | const f5 = () => {}; +32 32 | +33 | - const f6 = () => { +34 | - f6(); +35 | - }; +36 | - +37 33 | try { +38 34 | } catch (e) {} +39 35 | + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +warning[js/noUnusedVariables]: This variable is unused. + ┌─ noUnusedVariables.js:39:10 + │ +39 │ } catch (e) {} + │ - + +Suggested fix: Remove this variable. + | @@ -36,7 +36,7 @@ +35 35 | }; +36 36 | +37 37 | try { +38 | - } catch (e) {} + 38 | + } catch {} +39 39 | +40 40 | export function exported_function() {} +41 41 | = note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts new file mode 100644 index 00000000000..dddee34ec33 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts @@ -0,0 +1,30 @@ +// Invalid + +class D { + constructor(a: number) {} + f(a: number) {} + set a(a: number) {} +} +console.log(new D()); + +// Valid + +interface A { + f(a: number); + set a(a: number); + [key: string]: string; +} + +abstract class B { + constructor(a: number); + abstract f(a: number); + g(a: number); + abstract set a(a: number); +} +console.log(new B()); + +class C { + constructor(a: number); + f(a: number); +} +console.log(new C()); diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts.snap b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts.snap new file mode 100644 index 00000000000..e7f265f5aed --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts.snap @@ -0,0 +1,99 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: noUnusedVariables.ts +--- +# Input +```js +// Invalid + +class D { + constructor(a: number) {} + f(a: number) {} + set a(a: number) {} +} +console.log(new D()); + +// Valid + +interface A { + f(a: number); + set a(a: number); + [key: string]: string; +} + +abstract class B { + constructor(a: number); + abstract f(a: number); + g(a: number); + abstract set a(a: number); +} +console.log(new B()); + +class C { + constructor(a: number); + f(a: number); +} +console.log(new C()); + +``` + +# Diagnostics +``` +warning[js/noUnusedVariables]: This parameter is unused. + ┌─ noUnusedVariables.ts:4:14 + │ +4 │ constructor(a: number) {} + │ - + +Suggested fix: Remove this parameter. + | @@ -1,7 +1,7 @@ +0 0 | // Invalid +1 1 | +2 2 | class D { +3 | - constructor(a: number) {} + 3 | + constructor() {} +4 4 | f(a: number) {} +5 5 | set a(a: number) {} +6 6 | } + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +warning[js/noUnusedVariables]: This parameter is unused. + ┌─ noUnusedVariables.ts:5:4 + │ +5 │ f(a: number) {} + │ - + +Suggested fix: Remove this parameter. + | @@ -2,7 +2,7 @@ +1 1 | +2 2 | class D { +3 3 | constructor(a: number) {} +4 | - f(a: number) {} + 4 | + f() {} +5 5 | set a(a: number) {} +6 6 | } +7 7 | console.log(new D()); + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +warning[js/noUnusedVariables]: This parameter is unused. + ┌─ noUnusedVariables.ts:6:8 + │ +6 │ set a(a: number) {} + │ - + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + + diff --git a/crates/rome_service/src/configuration/mod.rs b/crates/rome_service/src/configuration/mod.rs index 990d9d8ac12..33213e55648 100644 --- a/crates/rome_service/src/configuration/mod.rs +++ b/crates/rome_service/src/configuration/mod.rs @@ -118,10 +118,10 @@ pub fn load_config( file_system: &DynRef, ) -> Result, RomeError> { let config_name = file_system.config_name(); + println!("Config: {:?}", config_name); let configuration_path = PathBuf::from(config_name); let options = OpenOptions::default().read(true).write(true); let file = file_system.open_with_options(&configuration_path, options); - match file { Ok(mut file) => { let mut buffer = String::new(); diff --git a/crates/tests_macros/src/lib.rs b/crates/tests_macros/src/lib.rs index dcbebac6d84..d239e69ebca 100644 --- a/crates/tests_macros/src/lib.rs +++ b/crates/tests_macros/src/lib.rs @@ -96,7 +96,13 @@ fn transform_file_name(input: &str) -> String { | "typeof" ); - let is_number = result.as_str().parse::().is_ok(); + let is_number = result + .chars() + .next() + .unwrap() + .to_string() + .parse::() + .is_ok(); if is_keyword { result.push('_'); @@ -164,7 +170,15 @@ impl Arguments { let path = path.as_ref(); let file_stem = path.file_stem()?; let file_stem = file_stem.to_str()?; - let test_name = file_stem.to_snake(); + let test_name = format!( + "{}{}", + file_stem.to_snake(), + if let Some(extension) = path.extension().and_then(|ext| ext.to_str()) { + format!("_{}", extension) + } else { + "".to_string() + } + ); let test_directory = path.parent().unwrap().display().to_string(); let test_full_path = path.display().to_string();