From 3600c43f49f9901ffc94a139a8a3655944e91e4e Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 29 Jul 2020 16:01:00 +1000 Subject: SSR: Don't mix non-path-based rules with path-based If any rules contain paths, then we reject any rules that don't contain paths. Allowing a mix leads to strange semantics, since the path-based rules only match things where the path refers to semantically the same thing, whereas the non-path-based rules could match anything. Specifically, if we have a rule like `foo ==>> bar` we only want to match the `foo` that is in the current scope, not any `foo`. However "foo" can be parsed as a pattern (BIND_PAT -> NAME -> IDENT). Allowing such a rule through would result in renaming everything called `foo` to `bar`. It'd also be slow, since without a path, we'd have to use the slow-scan search mechanism. --- crates/ra_ssr/src/tests.rs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'crates/ra_ssr/src/tests.rs') diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index 18ef2506a..851e573ae 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs @@ -886,6 +886,45 @@ fn ufcs_matches_method_call() { ); } +#[test] +fn pattern_is_a_single_segment_path() { + mark::check!(pattern_is_a_single_segment_path); + // The first function should not be altered because the `foo` in scope at the cursor position is + // a different `foo`. This case is special because "foo" can be parsed as a pattern (BIND_PAT -> + // NAME -> IDENT), which contains no path. If we're not careful we'll end up matching the `foo` + // in `let foo` from the first function. Whether we should match the `let foo` in the second + // function is less clear. At the moment, we don't. Doing so sounds like a rename operation, + // which isn't really what SSR is for, especially since the replacement `bar` must be able to be + // resolved, which means if we rename `foo` we'll get a name collision. + assert_ssr_transform( + "foo ==>> bar", + r#" + fn f1() -> i32 { + let foo = 1; + let bar = 2; + foo + } + fn f1() -> i32 { + let foo = 1; + let bar = 2; + foo<|> + } + "#, + expect![[r#" + fn f1() -> i32 { + let foo = 1; + let bar = 2; + foo + } + fn f1() -> i32 { + let foo = 1; + let bar = 2; + bar + } + "#]], + ); +} + #[test] fn replace_local_variable_reference() { // The pattern references a local variable `foo` in the block containing the cursor. We should -- cgit v1.2.3