diff options
author | David Lattimore <[email protected]> | 2020-06-27 11:34:21 +0100 |
---|---|---|
committer | David Lattimore <[email protected]> | 2020-06-27 11:38:31 +0100 |
commit | 64a49589e74447da668bd5acab8375206a10253f (patch) | |
tree | d8520baa999ebc1a3dc72030d64799634a4a63c7 /crates | |
parent | b081018363a210ffa0b95a4d3350a0a3e9b0cd83 (diff) |
Fix handling of whitespace when applying SSR within macro expansions.
I originally did replacement by passing in the full file text. Then as some point I thought I could do without it. Turns out calling .text() on a node coming from a macro expansion isn't a great idea, especially when you then try and use ranges from the original source to cut that text. The test I added here actually panics without the rest of this change (sorry I didn't notice sooner).
Diffstat (limited to 'crates')
-rw-r--r-- | crates/ra_ssr/src/lib.rs | 3 | ||||
-rw-r--r-- | crates/ra_ssr/src/matching.rs | 2 | ||||
-rw-r--r-- | crates/ra_ssr/src/replacing.rs | 33 | ||||
-rw-r--r-- | crates/ra_ssr/src/tests.rs | 17 |
4 files changed, 37 insertions, 18 deletions
diff --git a/crates/ra_ssr/src/lib.rs b/crates/ra_ssr/src/lib.rs index 8f149e3db..e148f4564 100644 --- a/crates/ra_ssr/src/lib.rs +++ b/crates/ra_ssr/src/lib.rs | |||
@@ -69,7 +69,8 @@ impl<'db> MatchFinder<'db> { | |||
69 | if matches.matches.is_empty() { | 69 | if matches.matches.is_empty() { |
70 | None | 70 | None |
71 | } else { | 71 | } else { |
72 | Some(replacing::matches_to_edit(&matches)) | 72 | use ra_db::SourceDatabaseExt; |
73 | Some(replacing::matches_to_edit(&matches, &self.sema.db.file_text(file_id))) | ||
73 | } | 74 | } |
74 | } | 75 | } |
75 | 76 | ||
diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 85420ed3c..bb87bda43 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs | |||
@@ -585,7 +585,7 @@ mod tests { | |||
585 | "1+2" | 585 | "1+2" |
586 | ); | 586 | ); |
587 | 587 | ||
588 | let edit = crate::replacing::matches_to_edit(&matches); | 588 | let edit = crate::replacing::matches_to_edit(&matches, input); |
589 | let mut after = input.to_string(); | 589 | let mut after = input.to_string(); |
590 | edit.apply(&mut after); | 590 | edit.apply(&mut after); |
591 | assert_eq!(after, "fn main() { bar(1+2); }"); | 591 | assert_eq!(after, "fn main() { bar(1+2); }"); |
diff --git a/crates/ra_ssr/src/replacing.rs b/crates/ra_ssr/src/replacing.rs index 5dcde82a2..70ce1c185 100644 --- a/crates/ra_ssr/src/replacing.rs +++ b/crates/ra_ssr/src/replacing.rs | |||
@@ -10,21 +10,25 @@ use ra_text_edit::TextEdit; | |||
10 | /// Returns a text edit that will replace each match in `matches` with its corresponding replacement | 10 | /// Returns a text edit that will replace each match in `matches` with its corresponding replacement |
11 | /// template. Placeholders in the template will have been substituted with whatever they matched to | 11 | /// template. Placeholders in the template will have been substituted with whatever they matched to |
12 | /// in the original code. | 12 | /// in the original code. |
13 | pub(crate) fn matches_to_edit(matches: &SsrMatches) -> TextEdit { | 13 | pub(crate) fn matches_to_edit(matches: &SsrMatches, file_src: &str) -> TextEdit { |
14 | matches_to_edit_at_offset(matches, 0.into()) | 14 | matches_to_edit_at_offset(matches, file_src, 0.into()) |
15 | } | 15 | } |
16 | 16 | ||
17 | fn matches_to_edit_at_offset(matches: &SsrMatches, relative_start: TextSize) -> TextEdit { | 17 | fn matches_to_edit_at_offset( |
18 | matches: &SsrMatches, | ||
19 | file_src: &str, | ||
20 | relative_start: TextSize, | ||
21 | ) -> TextEdit { | ||
18 | let mut edit_builder = ra_text_edit::TextEditBuilder::default(); | 22 | let mut edit_builder = ra_text_edit::TextEditBuilder::default(); |
19 | for m in &matches.matches { | 23 | for m in &matches.matches { |
20 | edit_builder.replace(m.range.checked_sub(relative_start).unwrap(), render_replace(m)); | 24 | edit_builder |
25 | .replace(m.range.checked_sub(relative_start).unwrap(), render_replace(m, file_src)); | ||
21 | } | 26 | } |
22 | edit_builder.finish() | 27 | edit_builder.finish() |
23 | } | 28 | } |
24 | 29 | ||
25 | fn render_replace(match_info: &Match) -> String { | 30 | fn render_replace(match_info: &Match, file_src: &str) -> String { |
26 | let mut out = String::new(); | 31 | let mut out = String::new(); |
27 | let match_start = match_info.matched_node.text_range().start(); | ||
28 | for r in &match_info.template.tokens { | 32 | for r in &match_info.template.tokens { |
29 | match r { | 33 | match r { |
30 | PatternElement::Token(t) => out.push_str(t.text.as_str()), | 34 | PatternElement::Token(t) => out.push_str(t.text.as_str()), |
@@ -33,16 +37,13 @@ fn render_replace(match_info: &Match) -> String { | |||
33 | match_info.placeholder_values.get(&Var(p.ident.to_string())) | 37 | match_info.placeholder_values.get(&Var(p.ident.to_string())) |
34 | { | 38 | { |
35 | let range = &placeholder_value.range.range; | 39 | let range = &placeholder_value.range.range; |
36 | let mut matched_text = if let Some(node) = &placeholder_value.node { | 40 | let mut matched_text = |
37 | node.text().to_string() | 41 | file_src[usize::from(range.start())..usize::from(range.end())].to_owned(); |
38 | } else { | 42 | let edit = matches_to_edit_at_offset( |
39 | let relative_range = range.checked_sub(match_start).unwrap(); | 43 | &placeholder_value.inner_matches, |
40 | match_info.matched_node.text().to_string() | 44 | file_src, |
41 | [usize::from(relative_range.start())..usize::from(relative_range.end())] | 45 | range.start(), |
42 | .to_string() | 46 | ); |
43 | }; | ||
44 | let edit = | ||
45 | matches_to_edit_at_offset(&placeholder_value.inner_matches, range.start()); | ||
46 | edit.apply(&mut matched_text); | 47 | edit.apply(&mut matched_text); |
47 | out.push_str(&matched_text); | 48 | out.push_str(&matched_text); |
48 | } else { | 49 | } else { |
diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index 7a3141be8..8be60c293 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs | |||
@@ -606,3 +606,20 @@ fn replace_within_macro_expansion() { | |||
606 | fn f() {macro1!(bar(5.x()).o2())}"#, | 606 | fn f() {macro1!(bar(5.x()).o2())}"#, |
607 | ) | 607 | ) |
608 | } | 608 | } |
609 | |||
610 | #[test] | ||
611 | fn preserves_whitespace_within_macro_expansion() { | ||
612 | assert_ssr_transform( | ||
613 | "$a + $b ==>> $b - $a", | ||
614 | r#" | ||
615 | macro_rules! macro1 { | ||
616 | ($a:expr) => {$a} | ||
617 | } | ||
618 | fn f() {macro1!(1 * 2 + 3 + 4}"#, | ||
619 | r#" | ||
620 | macro_rules! macro1 { | ||
621 | ($a:expr) => {$a} | ||
622 | } | ||
623 | fn f() {macro1!(4 - 3 - 1 * 2}"#, | ||
624 | ) | ||
625 | } | ||