diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_const.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_const.rs index 2fb9460a2da..3c3f30c0d20 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_const.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/use_const.rs @@ -42,11 +42,6 @@ declare_rule! { /// } /// ``` /// - /// ```js,expect_diagnostic - /// let a = 1, b = 2; - /// b = 3; - /// ``` - /// /// ## Valid /// /// ```js @@ -54,6 +49,11 @@ declare_rule! { /// a = 3; /// console.log(a); /// ``` + /// + /// ```js + /// let a = 1, b = 2; + /// b = 3; + /// ``` pub(crate) UseConst { version: "11.0.0", name: "useConst", @@ -150,7 +150,10 @@ impl ConstBindings { declaration, VariableDeclaration::JsForVariableDeclaration(..) ); + let mut bindings = 0; declaration.for_each_binding(|binding, declarator| { + bindings += 1; + let has_initializer = declarator.initializer().is_some(); let fix = check_binding_can_be_const(&binding, in_for_in_or_of_loop, has_initializer, model); @@ -163,7 +166,9 @@ impl ConstBindings { None => state.can_fix = false, } }); - if state.can_be_const.is_empty() { + + // Only flag if all bindings can be const + if state.can_be_const.len() != bindings { None } else { Some(state) diff --git a/crates/rome_js_analyze/src/utils/rename.rs b/crates/rome_js_analyze/src/utils/rename.rs index 087837540f7..a15345e25c1 100644 --- a/crates/rome_js_analyze/src/utils/rename.rs +++ b/crates/rome_js_analyze/src/utils/rename.rs @@ -170,10 +170,7 @@ impl RenameSymbolExtensions for BatchMutation { node: impl RenamableNode, new_name: &str, ) -> bool { - let prev_binding = match node - .binding(model) - .and_then(|node| node.cast::()) - { + let prev_binding = match node.binding(model).and_then(AnyJsIdentifierBinding::cast) { Some(prev_binding) => prev_binding, None => return false, }; @@ -234,13 +231,14 @@ impl RenameSymbolExtensions for BatchMutation { // Now it is safe to push changes to the batch mutation // Rename binding + let Ok(prev_name_token) = prev_binding.name_token() else { + return false; + }; let next_name_token = token_with_new_text(&name_token, new_name); - let next_binding = prev_binding.clone().with_name_token(next_name_token); - self.replace_node(prev_binding, next_binding); + self.replace_token(prev_name_token, next_name_token); // Rename all references - for (prev_token, next_token) in changes { self.replace_token(prev_token, next_token); } @@ -257,6 +255,9 @@ mod tests { ok_rename_declaration, "let a;", "let b;", + ok_rename_declaration_with_multiple_declarators, + "let a1, a2;", + "let b1, b2;", ok_rename_declaration_inner_scope, "let b; if (true) { let a; }", "let b; if (true) { let b; }", diff --git a/crates/rome_js_analyze/src/utils/tests.rs b/crates/rome_js_analyze/src/utils/tests.rs index 264465cf73a..132b4851cb2 100644 --- a/crates/rome_js_analyze/src/utils/tests.rs +++ b/crates/rome_js_analyze/src/utils/tests.rs @@ -11,29 +11,40 @@ use rome_js_syntax::{ }; use rome_rowan::{AstNode, BatchMutationExt, SyntaxNodeCast}; -/// Search and renames a binding named "a" to "b". +/// Search and renames alls bindings where the name contains "a" replacing it to "b". /// Asserts the renaming worked. pub fn assert_rename_binding_a_to_b_ok(before: &str, expected: &str) { let r = rome_js_parser::parse(before, FileId::zero(), SourceType::js_module()); let model = semantic_model(&r.tree(), SemanticModelOptions::default()); - let binding_a = r + let bindings: Vec = r .syntax() .descendants() - .filter_map(|x| x.cast::()) - .find(|x| x.text() == "a") - .unwrap(); + .filter_map(JsIdentifierBinding::cast) + .filter(|x| x.text().contains('a')) + .collect(); let mut batch = r.tree().begin(); - assert!(batch.rename_node_declaration(&model, binding_a, "b")); - let root = batch.commit(); + for binding in bindings { + let new_name = binding + .name_token() + .unwrap() + .text_trimmed() + .replace('a', "b"); + assert!(batch.rename_node_declaration(&model, binding, &new_name)); + } + let root = batch.commit(); let after = root.to_string(); assert_eq!(expected, after.as_str()); + + assert!(!rome_js_parser::test_utils::has_bogus_nodes_or_empty_slots( + &root + )); } -/// Search and renames a binding named "a" to "b". -/// Asserts the renaming to fail. +/// Search and renames one binding named "a" to "b". +/// Asserts the renaming fails. pub fn assert_rename_binding_a_to_b_nok(before: &str) { let r = rome_js_parser::parse(before, FileId::zero(), SourceType::js_module()); let model = semantic_model(&r.tree(), SemanticModelOptions::default()); diff --git a/crates/rome_js_analyze/tests/spec_tests.rs b/crates/rome_js_analyze/tests/spec_tests.rs index c8f5a71ac9c..6767184f83a 100644 --- a/crates/rome_js_analyze/tests/spec_tests.rs +++ b/crates/rome_js_analyze/tests/spec_tests.rs @@ -261,7 +261,10 @@ fn check_code_action( assert_eq!(new_tree.to_string(), output); if has_bogus_nodes_or_empty_slots(&new_tree) { - panic!("modified tree has bogus nodes or empty slots:\n{new_tree:#?}") + panic!( + "modified tree has bogus nodes or empty slots:\n{new_tree:#?} \n\n {}", + new_tree + ) } // Checks the returned tree contains no missing children node diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts b/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts index a6f067eb8d4..7f7f7436025 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts @@ -12,3 +12,5 @@ const [i] = [1]; var [{ j }] = [{ j: 1 }] var { k: [l] } = { k: [1] } + +let m, n; diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts.snap b/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts.snap index e60ff6c49e5..3ff4a6df89a 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/invalidVariables.ts.snap @@ -19,6 +19,8 @@ const [i] = [1]; var [{ j }] = [{ j: 1 }] var { k: [l] } = { k: [1] } +let m, n; + ``` # Diagnostics @@ -247,6 +249,7 @@ invalidVariables.ts:14:11 lint/correctness/noUnusedVariables FIXABLE ━━━ > 14 │ var { k: [l] } = { k: [1] }· │ ^ 15 │ + 16 │ let m, n; i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. @@ -257,6 +260,55 @@ invalidVariables.ts:14:11 lint/correctness/noUnusedVariables FIXABLE ━━━ 14 │ - var·{·k:·[l]·}·=·{·k:·[1]·}· 14 │ + var·{·k:·[_l]·}·=·{·k:·[1]·}· 15 15 │ + 16 16 │ let m, n; + + +``` + +``` +invalidVariables.ts:16:5 lint/correctness/noUnusedVariables FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This variable is unused. + + 14 │ var { k: [l] } = { k: [1] }· + 15 │ + > 16 │ let m, n; + │ ^ + 17 │ + + i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + i Suggested fix: If this is intentional, prepend m with an underscore. + + 14 14 │ var { k: [l] } = { k: [1] }· + 15 15 │ + 16 │ - let·m,·n; + 16 │ + let·_m,·n; + 17 17 │ + + +``` + +``` +invalidVariables.ts:16:8 lint/correctness/noUnusedVariables FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This variable is unused. + + 14 │ var { k: [l] } = { k: [1] }· + 15 │ + > 16 │ let m, n; + │ ^ + 17 │ + + i Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + i Suggested fix: If this is intentional, prepend n with an underscore. + + 14 14 │ var { k: [l] } = { k: [1] }· + 15 15 │ + 16 │ - let·m,·n; + 16 │ + let·m,·_n; + 17 17 │ ``` diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/validVariables.ts b/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/validVariables.tsx similarity index 74% rename from crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/validVariables.ts rename to crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/validVariables.tsx index f3f891b97fe..29282bc47d4 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/validVariables.ts +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnusedVariables/validVariables.tsx @@ -9,3 +9,7 @@ console.log(a, b, c); let value; function Button() {} console.log(