5096: Fix handling of whitespace when applying SSR within macro expansions. r=matklad a=davidlattimore

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).

5097: Fix SSR prompt following #4919 r=matklad a=davidlattimore



Co-authored-by: David Lattimore <dml@google.com>
This commit is contained in:
bors[bot] 2020-06-29 16:03:10 +00:00 committed by GitHub
commit e1a5bd866e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 38 additions and 19 deletions

View file

@ -69,7 +69,8 @@ impl<'db> MatchFinder<'db> {
if matches.matches.is_empty() { if matches.matches.is_empty() {
None None
} else { } else {
Some(replacing::matches_to_edit(&matches)) use ra_db::SourceDatabaseExt;
Some(replacing::matches_to_edit(&matches, &self.sema.db.file_text(file_id)))
} }
} }

View file

@ -585,7 +585,7 @@ mod tests {
"1+2" "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(); let mut after = input.to_string();
edit.apply(&mut after); edit.apply(&mut after);
assert_eq!(after, "fn main() { bar(1+2); }"); assert_eq!(after, "fn main() { bar(1+2); }");

View file

@ -10,21 +10,25 @@ use ra_text_edit::TextEdit;
/// Returns a text edit that will replace each match in `matches` with its corresponding replacement /// 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 /// template. Placeholders in the template will have been substituted with whatever they matched to
/// in the original code. /// in the original code.
pub(crate) fn matches_to_edit(matches: &SsrMatches) -> TextEdit { pub(crate) fn matches_to_edit(matches: &SsrMatches, file_src: &str) -> TextEdit {
matches_to_edit_at_offset(matches, 0.into()) 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(); let mut edit_builder = ra_text_edit::TextEditBuilder::default();
for m in &matches.matches { 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() 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 mut out = String::new();
let match_start = match_info.matched_node.text_range().start();
for r in &match_info.template.tokens { for r in &match_info.template.tokens {
match r { match r {
PatternElement::Token(t) => out.push_str(t.text.as_str()), 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())) match_info.placeholder_values.get(&Var(p.ident.to_string()))
{ {
let range = &placeholder_value.range.range; let range = &placeholder_value.range.range;
let mut matched_text = if let Some(node) = &placeholder_value.node { let mut matched_text =
node.text().to_string() file_src[usize::from(range.start())..usize::from(range.end())].to_owned();
} else { let edit = matches_to_edit_at_offset(
let relative_range = range.checked_sub(match_start).unwrap(); &placeholder_value.inner_matches,
match_info.matched_node.text().to_string() file_src,
[usize::from(relative_range.start())..usize::from(relative_range.end())] range.start(),
.to_string() );
};
let edit =
matches_to_edit_at_offset(&placeholder_value.inner_matches, range.start());
edit.apply(&mut matched_text); edit.apply(&mut matched_text);
out.push_str(&matched_text); out.push_str(&matched_text);
} else { } else {

View file

@ -606,3 +606,20 @@ fn replace_within_macro_expansion() {
fn f() {macro1!(bar(5.x()).o2())}"#, 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}"#,
)
}

View file

@ -158,7 +158,7 @@ export function ssr(ctx: Ctx): Cmd {
const options: vscode.InputBoxOptions = { const options: vscode.InputBoxOptions = {
value: "() ==>> ()", value: "() ==>> ()",
prompt: "Enter request, for example 'Foo($a:expr) ==> Foo::new($a)' ", prompt: "Enter request, for example 'Foo($a) ==> Foo::new($a)' ",
validateInput: async (x: string) => { validateInput: async (x: string) => {
try { try {
await client.sendRequest(ra.ssr, { query: x, parseOnly: true }); await client.sendRequest(ra.ssr, { query: x, parseOnly: true });