Skip to content

Commit e1a5bd8

Browse files
Merge #5096 #5097
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 <[email protected]>
3 parents 86f1556 + 64a4958 + 2a18ef0 commit e1a5bd8

File tree

5 files changed

+38
-19
lines changed

5 files changed

+38
-19
lines changed

crates/ra_ssr/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ impl<'db> MatchFinder<'db> {
6969
if matches.matches.is_empty() {
7070
None
7171
} 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)))
7374
}
7475
}
7576

crates/ra_ssr/src/matching.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ mod tests {
585585
"1+2"
586586
);
587587

588-
let edit = crate::replacing::matches_to_edit(&matches);
588+
let edit = crate::replacing::matches_to_edit(&matches, input);
589589
let mut after = input.to_string();
590590
edit.apply(&mut after);
591591
assert_eq!(after, "fn main() { bar(1+2); }");

crates/ra_ssr/src/replacing.rs

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,25 @@ use ra_text_edit::TextEdit;
1010
/// Returns a text edit that will replace each match in `matches` with its corresponding replacement
1111
/// template. Placeholders in the template will have been substituted with whatever they matched to
1212
/// in the original code.
13-
pub(crate) fn matches_to_edit(matches: &SsrMatches) -> TextEdit {
14-
matches_to_edit_at_offset(matches, 0.into())
13+
pub(crate) fn matches_to_edit(matches: &SsrMatches, file_src: &str) -> TextEdit {
14+
matches_to_edit_at_offset(matches, file_src, 0.into())
1515
}
1616

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 {
1822
let mut edit_builder = ra_text_edit::TextEditBuilder::default();
1923
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));
2126
}
2227
edit_builder.finish()
2328
}
2429

25-
fn render_replace(match_info: &Match) -> String {
30+
fn render_replace(match_info: &Match, file_src: &str) -> String {
2631
let mut out = String::new();
27-
let match_start = match_info.matched_node.text_range().start();
2832
for r in &match_info.template.tokens {
2933
match r {
3034
PatternElement::Token(t) => out.push_str(t.text.as_str()),
@@ -33,16 +37,13 @@ fn render_replace(match_info: &Match) -> String {
3337
match_info.placeholder_values.get(&Var(p.ident.to_string()))
3438
{
3539
let range = &placeholder_value.range.range;
36-
let mut matched_text = if let Some(node) = &placeholder_value.node {
37-
node.text().to_string()
38-
} else {
39-
let relative_range = range.checked_sub(match_start).unwrap();
40-
match_info.matched_node.text().to_string()
41-
[usize::from(relative_range.start())..usize::from(relative_range.end())]
42-
.to_string()
43-
};
44-
let edit =
45-
matches_to_edit_at_offset(&placeholder_value.inner_matches, range.start());
40+
let mut matched_text =
41+
file_src[usize::from(range.start())..usize::from(range.end())].to_owned();
42+
let edit = matches_to_edit_at_offset(
43+
&placeholder_value.inner_matches,
44+
file_src,
45+
range.start(),
46+
);
4647
edit.apply(&mut matched_text);
4748
out.push_str(&matched_text);
4849
} else {

crates/ra_ssr/src/tests.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,3 +606,20 @@ fn replace_within_macro_expansion() {
606606
fn f() {macro1!(bar(5.x()).o2())}"#,
607607
)
608608
}
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+
}

editors/code/src/commands.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ export function ssr(ctx: Ctx): Cmd {
158158

159159
const options: vscode.InputBoxOptions = {
160160
value: "() ==>> ()",
161-
prompt: "Enter request, for example 'Foo($a:expr) ==> Foo::new($a)' ",
161+
prompt: "Enter request, for example 'Foo($a) ==> Foo::new($a)' ",
162162
validateInput: async (x: string) => {
163163
try {
164164
await client.sendRequest(ra.ssr, { query: x, parseOnly: true });

0 commit comments

Comments
 (0)