about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-07-20 10:51:31 +0000
committerbors <bors@rust-lang.org>2022-07-20 10:51:31 +0000
commitf3e9b38e26646a75fdb3c25a2909bce68cde596c (patch)
treea38761d98c53fd2dbffd30b0956796f41ed603b5
parent84544134f6e2e1f53a9ce3f821dbe7a70f924145 (diff)
parentcfc52adc65327190b5027de19a1a6d96dcf504ab (diff)
downloadrust-f3e9b38e26646a75fdb3c25a2909bce68cde596c.tar.gz
rust-f3e9b38e26646a75fdb3c25a2909bce68cde596c.zip
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?).
-rw-r--r--crates/ide-completion/src/completions/postfix.rs81
-rw-r--r--crates/ide-completion/src/completions/postfix/format_like.rs16
-rw-r--r--crates/ide-completion/src/snippet.rs2
3 files changed, 84 insertions, 15 deletions
diff --git a/crates/ide-completion/src/completions/postfix.rs b/crates/ide-completion/src/completions/postfix.rs
index 5af44aa4b68..b09f4634c65 100644
--- a/crates/ide-completion/src/completions/postfix.rs
+++ b/crates/ide-completion/src/completions/postfix.rs
@@ -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 {
@@ -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
@@ -518,6 +528,49 @@ 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 => "\\\$",
+    })
+}
+"#,
+        );
     }
 
     #[test]
diff --git a/crates/ide-completion/src/completions/postfix/format_like.rs b/crates/ide-completion/src/completions/postfix/format_like.rs
index b5ef87b8812..16f902489b5 100644
--- a/crates/ide-completion/src/completions/postfix/format_like.rs
+++ b/crates/ide-completion/src/completions/postfix/format_like.rs
@@ -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) {
@@ -127,6 +128,9 @@ impl FormatStrParser {
                     self.state = State::MaybeIncorrect;
                 }
                 (State::NotExpr, _) => {
+                    if matches!(chr, '\\' | '$') {
+                        self.output.push('\\');
+                    }
                     self.output.push(chr);
                 }
                 (State::MaybeIncorrect, '}') => {
@@ -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;
                 }
@@ -187,6 +194,9 @@ impl FormatStrParser {
                     inexpr_open_count += 1;
                 }
                 (State::Expr, _) => {
+                    if matches!(chr, '\\' | '$') {
+                        current_expr.push('\\');
+                    }
                     current_expr.push(chr);
                 }
                 (State::FormatOpts, '}') => {
@@ -194,6 +204,9 @@ impl FormatStrParser {
                     self.state = State::NotExpr;
                 }
                 (State::FormatOpts, _) => {
+                    if matches!(chr, '\\' | '$') {
+                        self.output.push('\\');
+                    }
                     self.output.push(chr);
                 }
             }
@@ -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"]]),
diff --git a/crates/ide-completion/src/snippet.rs b/crates/ide-completion/src/snippet.rs
index d2680c77583..40c72b53511 100644
--- a/crates/ide-completion/src/snippet.rs
+++ b/crates/ide-completion/src/snippet.rs
@@ -17,7 +17,7 @@
 //       "body": [
 //         "thread::spawn(move || {",
 //         "\t$0",
-//         ")};",
+//         "});",
 //       ],
 //       "description": "Insert a thread::spawn call",
 //       "requires": "std::thread",