From 64a49589e74447da668bd5acab8375206a10253f Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Sat, 27 Jun 2020 20:34:21 +1000 Subject: 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). --- crates/ra_ssr/src/lib.rs | 3 ++- crates/ra_ssr/src/matching.rs | 2 +- crates/ra_ssr/src/replacing.rs | 33 +++++++++++++++++---------------- crates/ra_ssr/src/tests.rs | 17 +++++++++++++++++ 4 files changed, 37 insertions(+), 18 deletions(-) (limited to 'crates/ra_ssr') 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> { if matches.matches.is_empty() { None } else { - Some(replacing::matches_to_edit(&matches)) + use ra_db::SourceDatabaseExt; + Some(replacing::matches_to_edit(&matches, &self.sema.db.file_text(file_id))) } } 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 { "1+2" ); - let edit = crate::replacing::matches_to_edit(&matches); + let edit = crate::replacing::matches_to_edit(&matches, input); let mut after = input.to_string(); edit.apply(&mut after); 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; /// Returns a text edit that will replace each match in `matches` with its corresponding replacement /// template. Placeholders in the template will have been substituted with whatever they matched to /// in the original code. -pub(crate) fn matches_to_edit(matches: &SsrMatches) -> TextEdit { - matches_to_edit_at_offset(matches, 0.into()) +pub(crate) fn matches_to_edit(matches: &SsrMatches, file_src: &str) -> TextEdit { + matches_to_edit_at_offset(matches, file_src, 0.into()) } -fn matches_to_edit_at_offset(matches: &SsrMatches, relative_start: TextSize) -> TextEdit { +fn matches_to_edit_at_offset( + matches: &SsrMatches, + file_src: &str, + relative_start: TextSize, +) -> TextEdit { let mut edit_builder = ra_text_edit::TextEditBuilder::default(); for m in &matches.matches { - edit_builder.replace(m.range.checked_sub(relative_start).unwrap(), render_replace(m)); + edit_builder + .replace(m.range.checked_sub(relative_start).unwrap(), render_replace(m, file_src)); } edit_builder.finish() } -fn render_replace(match_info: &Match) -> String { +fn render_replace(match_info: &Match, file_src: &str) -> String { let mut out = String::new(); - let match_start = match_info.matched_node.text_range().start(); for r in &match_info.template.tokens { match r { PatternElement::Token(t) => out.push_str(t.text.as_str()), @@ -33,16 +37,13 @@ fn render_replace(match_info: &Match) -> String { match_info.placeholder_values.get(&Var(p.ident.to_string())) { let range = &placeholder_value.range.range; - let mut matched_text = if let Some(node) = &placeholder_value.node { - node.text().to_string() - } else { - let relative_range = range.checked_sub(match_start).unwrap(); - match_info.matched_node.text().to_string() - [usize::from(relative_range.start())..usize::from(relative_range.end())] - .to_string() - }; - let edit = - matches_to_edit_at_offset(&placeholder_value.inner_matches, range.start()); + let mut matched_text = + file_src[usize::from(range.start())..usize::from(range.end())].to_owned(); + let edit = matches_to_edit_at_offset( + &placeholder_value.inner_matches, + file_src, + range.start(), + ); edit.apply(&mut matched_text); out.push_str(&matched_text); } 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() { fn f() {macro1!(bar(5.x()).o2())}"#, ) } + +#[test] +fn preserves_whitespace_within_macro_expansion() { + assert_ssr_transform( + "$a + $b ==>> $b - $a", + r#" + macro_rules! macro1 { + ($a:expr) => {$a} + } + fn f() {macro1!(1 * 2 + 3 + 4}"#, + r#" + macro_rules! macro1 { + ($a:expr) => {$a} + } + fn f() {macro1!(4 - 3 - 1 * 2}"#, + ) +} -- cgit v1.2.3