Auto merge of #12646 - lowr:fix/11897, r=lowr

fix: escape receiver texts in completion

This PR fixes #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?).
This commit is contained in:
bors 2022-07-20 10:51:31 +00:00
commit f3e9b38e26
3 changed files with 84 additions and 15 deletions

View file

@ -193,13 +193,21 @@ pub(crate) fn complete_postfix(
} }
fn get_receiver_text(receiver: &ast::Expr, receiver_is_ambiguous_float_literal: bool) -> String { 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 text = receiver.syntax().text();
let without_dot = ..text.len() - TextSize::of('.'); let without_dot = ..text.len() - TextSize::of('.');
text.slice(without_dot).to_string() text.slice(without_dot).to_string()
} else { } else {
receiver.to_string() 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 { fn include_references(initial_element: &ast::Expr) -> ast::Expr {
@ -494,19 +502,21 @@ fn main() {
#[test] #[test]
fn custom_postfix_completion() { 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( check_edit_with_config(
CompletionConfig { config.clone(),
snippets: vec![Snippet::new(
&[],
&["break".into()],
&["ControlFlow::Break(${receiver})".into()],
"",
&["core::ops::ControlFlow".into()],
crate::SnippetScope::Expr,
)
.unwrap()],
..TEST_CONFIG
},
"break", "break",
r#" r#"
//- minicore: try //- minicore: try
@ -516,6 +526,49 @@ fn main() { 42.$0 }
use core::ops::ControlFlow; use core::ops::ControlFlow;
fn main() { ControlFlow::Break(42) } 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 => "\\\$",
})
}
"#, "#,
); );
} }

View file

@ -115,6 +115,7 @@ impl FormatStrParser {
// "{MyStruct { val_a: 0, val_b: 1 }}". // "{MyStruct { val_a: 0, val_b: 1 }}".
let mut inexpr_open_count = 0; 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(); let mut chars = self.input.chars().peekable();
while let Some(chr) = chars.next() { while let Some(chr) = chars.next() {
match (self.state, chr) { match (self.state, chr) {
@ -127,6 +128,9 @@ impl FormatStrParser {
self.state = State::MaybeIncorrect; self.state = State::MaybeIncorrect;
} }
(State::NotExpr, _) => { (State::NotExpr, _) => {
if matches!(chr, '\\' | '$') {
self.output.push('\\');
}
self.output.push(chr); self.output.push(chr);
} }
(State::MaybeIncorrect, '}') => { (State::MaybeIncorrect, '}') => {
@ -150,6 +154,9 @@ impl FormatStrParser {
self.state = State::NotExpr; self.state = State::NotExpr;
} }
(State::MaybeExpr, _) => { (State::MaybeExpr, _) => {
if matches!(chr, '\\' | '$') {
current_expr.push('\\');
}
current_expr.push(chr); current_expr.push(chr);
self.state = State::Expr; self.state = State::Expr;
} }
@ -187,6 +194,9 @@ impl FormatStrParser {
inexpr_open_count += 1; inexpr_open_count += 1;
} }
(State::Expr, _) => { (State::Expr, _) => {
if matches!(chr, '\\' | '$') {
current_expr.push('\\');
}
current_expr.push(chr); current_expr.push(chr);
} }
(State::FormatOpts, '}') => { (State::FormatOpts, '}') => {
@ -194,6 +204,9 @@ impl FormatStrParser {
self.state = State::NotExpr; self.state = State::NotExpr;
} }
(State::FormatOpts, _) => { (State::FormatOpts, _) => {
if matches!(chr, '\\' | '$') {
self.output.push('\\');
}
self.output.push(chr); self.output.push(chr);
} }
} }
@ -241,8 +254,11 @@ mod tests {
fn format_str_parser() { fn format_str_parser() {
let test_vector = &[ let test_vector = &[
("no expressions", expect![["no expressions"]]), ("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} is {2 + 2}", expect![["{} is {}; expr, 2 + 2"]]),
("{expr:?}", expect![["{:?}; expr"]]), ("{expr:?}", expect![["{:?}; expr"]]),
("{expr:1$}", expect![[r"{:1\$}; expr"]]),
("{$0}", expect![[r"{}; \$0"]]),
("{malformed", expect![["-"]]), ("{malformed", expect![["-"]]),
("malformed}", expect![["-"]]), ("malformed}", expect![["-"]]),
("{{correct", expect![["{{correct"]]), ("{{correct", expect![["{{correct"]]),

View file

@ -17,7 +17,7 @@
// "body": [ // "body": [
// "thread::spawn(move || {", // "thread::spawn(move || {",
// "\t$0", // "\t$0",
// ")};", // "});",
// ], // ],
// "description": "Insert a thread::spawn call", // "description": "Insert a thread::spawn call",
// "requires": "std::thread", // "requires": "std::thread",