Move string escaping to syntax::semantic and fix

crates/ide/src/ide/assists/rewrite_string.rs

@@ -3,13 +3,14 @@
 //! - Attribute names <-> Double quoted strings
 //! - Double quoted strings <-> Indented strings
 use std::convert::Infallible;
+use std::fmt::Write;
 
 use super::{AssistKind, AssistsCtx};
 use crate::TextEdit;
 use syntax::ast::{self, AstNode};
 use syntax::semantic::{
     is_valid_ident, strip_indent, unescape_string, unescape_string_escape, unescape_string_literal,
-    StrippedStringPart, UnescapedStringPart,
+    EscapeStringFragment, StrippedStringPart, UnescapedStringPart,
 };
 use syntax::SyntaxKind;
 
@@ -220,22 +221,32 @@ pub(super) fn rewrite_string_to_indented(ctx: &mut AssistsCtx<'_>) -> Option<()>
 /// ```
 pub(super) fn rewrite_indented_to_string(ctx: &mut AssistsCtx<'_>) -> Option<()> {
     let node = ctx.covering_node::<ast::IndentString>()?;
-    let mut text = String::from('"');
-    let _ = strip_indent::<Infallible>(&node, |part| {
+    let mut ret = String::from('"');
+
+    // Concatenate all contiguous fragments and escape them in a single run.
+    // This correctly handles `${` on two individual fragments/escapes.
+    // Eg. `''${` => [Escape("$"), Fragment("{")] => "${" => "\${"
+    // Note that either part alone doesn't require escaping.
+    let mut last_frag = String::new();
+    strip_indent::<Infallible>(&node, |part| {
         match part {
             StrippedStringPart::Fragment(frag) => {
-                escape_dquote_string(&mut text, frag);
+                last_frag += frag;
             }
             StrippedStringPart::Escape(esc) => {
-                escape_dquote_string(&mut text, unescape_string_escape(esc.text()));
+                last_frag += unescape_string_escape(esc.text());
             }
             StrippedStringPart::Dynamic(dyna) => {
-                text += &dyna.syntax().to_string();
+                write!(ret, "{}", EscapeStringFragment(&last_frag)).unwrap();
+                last_frag.clear();
+                ret += &dyna.syntax().to_string();
             }
         }
         Ok(())
-    });
-    text.push('"');
+    })
+    .unwrap();
+    write!(ret, "{}", EscapeStringFragment(&last_frag)).unwrap();
+    ret.push('"');
 
     ctx.add(
         "rewrite_indented_to_string",
@@ -243,41 +254,13 @@ pub(super) fn rewrite_indented_to_string(ctx: &mut AssistsCtx<'_>) -> Option<()>
         AssistKind::RefactorRewrite,
         vec![TextEdit {
             delete: node.syntax().text_range(),
-            insert: text.into(),
+            insert: ret.into(),
         }],
     );
 
     Some(())
 }
 
-/// Escape `text` and write to `out`,
-/// `text` may be only a fragment of the original Nix string.
-fn escape_dquote_string(out: &mut String, text: &str) {
-    let mut xs = text.chars();
-    while let Some(x) = xs.next() {
-        match x {
-            '"' | '\\' => {
-                out.push('\\');
-                out.push(x);
-            }
-            '$' => match xs.next() {
-                Some('{') => out.push_str("\\${"),
-                Some(y) => {
-                    out.push('$');
-                    out.push(y);
-                }
-                // It's impossible to know from this context whether the next
-                // character is '{' or not, so we assume it is just to be safe
-                None => out.push_str("\\$"),
-            },
-            '\n' => out.push_str("\\n"),
-            '\r' => out.push_str("\\r"),
-            '\t' => out.push_str("\\t"),
-            _ => out.push(x),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use expect_test::expect;
@@ -364,5 +347,9 @@ mod tests {
         check(r"$0''\n\r\t''", expect![r#""\\n\\r\\t""#]);
         check(r"'''''''$0", expect![r#""''""#]);
         check(r"$0''''${foo}''", expect![r#""\${foo}""#]);
+
+        // See comments in `rewrite_indented_to_string`.
+        check(r"$0'' ''${ ''", expect![[r#""\${ ""#]]);
+        check(r"$0'' ''$ ''", expect![[r#""$ ""#]]);
     }
 }

crates/syntax/src/semantic.rs

@@ -4,7 +4,7 @@ use crate::ast::{self, AstChildren, AstNode, Attr, Expr, HasStringParts, StringP
 use crate::lexer::KEYWORDS;
 use crate::{SyntaxNode, SyntaxToken};
 use std::borrow::Cow;
-use std::str;
+use std::{fmt, str};
 
 /// Check if a name is a valid identifier.
 pub fn is_valid_ident(name: &str) -> bool {
@@ -20,15 +20,39 @@ pub fn is_valid_ident(name: &str) -> bool {
 /// Escape a literal Attr. Quote it if it's not a valid identifier.
 pub fn escape_literal_attr(name: &str) -> Cow<'_, str> {
     if is_valid_ident(name) {
-        return Cow::Borrowed(name);
+        Cow::Borrowed(name)
+    } else {
+        Cow::Owned(escape_string(name))
+    }
+}
+
+/// Escape the text in a string literal with double-quotes.
+pub fn escape_string(text: &str) -> String {
+    format!("\"{}\"", EscapeStringFragment(text))
+}
+
+#[derive(Debug, Clone)]
+pub struct EscapeStringFragment<'a>(pub &'a str);
+
+impl fmt::Display for EscapeStringFragment<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for (i, ch) in self.0.char_indices() {
+            match ch {
+                '"' => "\\\"",
+                '\\' => "\\\\",
+                '\n' => "\\n",
+                '\r' => "\\r",
+                '\t' => "\\r",
+                '$' if self.0[i..].starts_with("${") => "\\$",
+                _ => {
+                    ch.fmt(f)?;
+                    continue;
+                }
+            }
+            .fmt(f)?;
+        }
+        Ok(())
     }
-    Cow::Owned(
-        std::iter::empty()
-            .chain(Some('"'))
-            .chain(name.chars().flat_map(|ch| ch.escape_default()))
-            .chain(Some('"'))
-            .collect(),
-    )
 }
 
 /// Unescape a single string escape sequence.
@@ -348,6 +372,13 @@ mod tests {
         assert_eq!(escape_literal_attr("in"), r#""in""#);
         assert_eq!(escape_literal_attr(" "), r#"" ""#);
         assert_eq!(escape_literal_attr("\n"), r#""\n""#);
+        assert_eq!(escape_literal_attr("$ ${"), r#""$ \${""#);
+    }
+
+    #[test]
+    fn escape_string_() {
+        assert_eq!(escape_string(""), r#""""#);
+        assert_eq!(escape_string("n\"$a \n${b"), r#""n\"$a \n\${b""#);
     }
 
     #[test]