Skip to content

Commit

Permalink
Auto merge of rust-lang#12646 - lowr:fix/11897, r=lowr
Browse files Browse the repository at this point in the history
fix: escape receiver texts in completion

This PR fixes rust-lang#11897 by escaping '\\' and '$' in the text of the receiver position expression. See [here](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#snippet_syntax) for the specification of the snippet syntax (especially [this section](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#grammar) discusses escaping).

Although not all occurrences of '\\' and '$' have to be replaced, I chose to replace all as that's simpler and easier to understand. There *are* more clever ways to implement it, but I thought they were premature optimization for the time being (maybe I should put FIXME notes?).
  • Loading branch information
bors committed Jul 20, 2022
2 parents 8454413 + cfc52ad commit f3e9b38
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 15 deletions.
81 changes: 67 additions & 14 deletions crates/ide-completion/src/completions/postfix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,21 @@ pub(crate) fn complete_postfix(
}

fn get_receiver_text(receiver: &ast::Expr, receiver_is_ambiguous_float_literal: bool) -> String {
if receiver_is_ambiguous_float_literal {
let text = if receiver_is_ambiguous_float_literal {
let text = receiver.syntax().text();
let without_dot = ..text.len() - TextSize::of('.');
text.slice(without_dot).to_string()
} else {
receiver.to_string()
}
};

// The receiver texts should be interpreted as-is, as they are expected to be
// normal Rust expressions. We escape '\' and '$' so they don't get treated as
// snippet-specific constructs.
//
// Note that we don't need to escape the other characters that can be escaped,
// because they wouldn't be treated as snippet-specific constructs without '$'.
text.replace('\\', "\\\\").replace('$', "\\$")
}

fn include_references(initial_element: &ast::Expr) -> ast::Expr {
Expand Down Expand Up @@ -494,19 +502,21 @@ fn main() {

#[test]
fn custom_postfix_completion() {
let config = CompletionConfig {
snippets: vec![Snippet::new(
&[],
&["break".into()],
&["ControlFlow::Break(${receiver})".into()],
"",
&["core::ops::ControlFlow".into()],
crate::SnippetScope::Expr,
)
.unwrap()],
..TEST_CONFIG
};

check_edit_with_config(
CompletionConfig {
snippets: vec![Snippet::new(
&[],
&["break".into()],
&["ControlFlow::Break(${receiver})".into()],
"",
&["core::ops::ControlFlow".into()],
crate::SnippetScope::Expr,
)
.unwrap()],
..TEST_CONFIG
},
config.clone(),
"break",
r#"
//- minicore: try
Expand All @@ -516,6 +526,49 @@ fn main() { 42.$0 }
use core::ops::ControlFlow;
fn main() { ControlFlow::Break(42) }
"#,
);

// The receiver texts should be escaped, see comments in `get_receiver_text()`
// for detail.
//
// Note that the last argument is what *lsp clients would see* rather than
// what users would see. Unescaping happens thereafter.
check_edit_with_config(
config.clone(),
"break",
r#"
//- minicore: try
fn main() { '\\'.$0 }
"#,
r#"
use core::ops::ControlFlow;
fn main() { ControlFlow::Break('\\\\') }
"#,
);

check_edit_with_config(
config.clone(),
"break",
r#"
//- minicore: try
fn main() {
match true {
true => "${1:placeholder}",
false => "\$",
}.$0
}
"#,
r#"
use core::ops::ControlFlow;
fn main() {
ControlFlow::Break(match true {
true => "\${1:placeholder}",
false => "\\\$",
})
}
"#,
);
}
Expand Down
16 changes: 16 additions & 0 deletions crates/ide-completion/src/completions/postfix/format_like.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ impl FormatStrParser {
// "{MyStruct { val_a: 0, val_b: 1 }}".
let mut inexpr_open_count = 0;

// We need to escape '\' and '$'. See the comments on `get_receiver_text()` for detail.
let mut chars = self.input.chars().peekable();
while let Some(chr) = chars.next() {
match (self.state, chr) {
Expand All @@ -127,6 +128,9 @@ impl FormatStrParser {
self.state = State::MaybeIncorrect;
}
(State::NotExpr, _) => {
if matches!(chr, '\\' | '$') {
self.output.push('\\');
}
self.output.push(chr);
}
(State::MaybeIncorrect, '}') => {
Expand All @@ -150,6 +154,9 @@ impl FormatStrParser {
self.state = State::NotExpr;
}
(State::MaybeExpr, _) => {
if matches!(chr, '\\' | '$') {
current_expr.push('\\');
}
current_expr.push(chr);
self.state = State::Expr;
}
Expand Down Expand Up @@ -187,13 +194,19 @@ impl FormatStrParser {
inexpr_open_count += 1;
}
(State::Expr, _) => {
if matches!(chr, '\\' | '$') {
current_expr.push('\\');
}
current_expr.push(chr);
}
(State::FormatOpts, '}') => {
self.output.push(chr);
self.state = State::NotExpr;
}
(State::FormatOpts, _) => {
if matches!(chr, '\\' | '$') {
self.output.push('\\');
}
self.output.push(chr);
}
}
Expand Down Expand Up @@ -241,8 +254,11 @@ mod tests {
fn format_str_parser() {
let test_vector = &[
("no expressions", expect![["no expressions"]]),
(r"no expressions with \$0$1", expect![r"no expressions with \\\$0\$1"]),
("{expr} is {2 + 2}", expect![["{} is {}; expr, 2 + 2"]]),
("{expr:?}", expect![["{:?}; expr"]]),
("{expr:1$}", expect![[r"{:1\$}; expr"]]),
("{$0}", expect![[r"{}; \$0"]]),
("{malformed", expect![["-"]]),
("malformed}", expect![["-"]]),
("{{correct", expect![["{{correct"]]),
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-completion/src/snippet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// "body": [
// "thread::spawn(move || {",
// "\t$0",
// ")};",
// "});",
// ],
// "description": "Insert a thread::spawn call",
// "requires": "std::thread",
Expand Down

0 comments on commit f3e9b38

Please sign in to comment.