Deduplicate string literal rendering, fix 4909

7 files changed, 86 insertions, 39 deletions

diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc
index 57848a5d3..1366622c7 100644
--- a/src/libcmd/repl.cc
+++ b/src/libcmd/repl.cc
@@ -40,6 +40,7 @@ extern "C" {
 #include "markdown.hh"
 #include "local-fs-store.hh"
 #include "progress-bar.hh"
+#include "value/print.hh"
 
 #if HAVE_BOEHMGC
 #define GC_INCLUDE_NEW
@@ -894,17 +895,6 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
 }
 
 
-std::ostream & printStringValue(std::ostream & str, const char * string) {
-    str << "\"";
-    for (const char * i = string; *i; i++)
-        if (*i == '\"' || *i == '\\') str << "\\" << *i;
-        else if (*i == '\n') str << "\\n";
-        else if (*i == '\r') str << "\\r";
-        else if (*i == '\t') str << "\\t";
-        else str << *i;
-    str << "\"";
-    return str;
-}
 
 
 // FIXME: lot of cut&paste from Nix's eval.cc.
@@ -922,12 +912,14 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
         break;
 
     case nBool:
-        str << ANSI_CYAN << (v.boolean ? "true" : "false") << ANSI_NORMAL;
+        str << ANSI_CYAN;
+        printLiteral(str, v.boolean);
+        str << ANSI_NORMAL;
         break;
 
     case nString:
         str << ANSI_WARNING;
-        printStringValue(str, v.string.s);
+        printLiteral(str, v.string.s);
         str << ANSI_NORMAL;
         break;
 
@@ -967,7 +959,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
                 if (isVarName(i.first))
                     str << i.first;
                 else
-                    printStringValue(str, i.first.c_str());
+                    printLiteral(str, i.first);
                 str << " = ";
                 if (seen.count(i.second))
                     str << "«repeated»";
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 18cfd9531..06208897f 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -9,6 +9,7 @@
 #include "filetransfer.hh"
 #include "function-trace.hh"
 #include "profiles.hh"
+#include "value/print.hh"
 
 #include <algorithm>
 #include <chrono>
@@ -104,18 +105,10 @@ void Value::print(const SymbolTable & symbols, std::ostream & str,
         str << integer;
         break;
     case tBool:
-        str << (boolean ? "true" : "false");
+        printLiteral(str, boolean);
         break;
     case tString:
-        str << "\"";
-        for (const char * i = string.s; *i; i++)
-            if (*i == '\"' || *i == '\\') str << "\\" << *i;
-            else if (*i == '\n') str << "\\n";
-            else if (*i == '\r') str << "\\r";
-            else if (*i == '\t') str << "\\t";
-            else if (*i == '$' && *(i+1) == '{') str << "\\" << *i;
-            else str << *i;
-        str << "\"";
+        printLiteral(str, string.s);
         break;
     case tPath:
         str << path; // !!! escaping?
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index eb6f062b4..ca6df0af3 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -3,6 +3,7 @@
 #include "eval.hh"
 #include "symbol-table.hh"
 #include "util.hh"
+#include "value/print.hh"
 
 #include <cstdlib>
 
@@ -62,18 +63,6 @@ Pos::operator std::shared_ptr<AbstractPos>() const
 
 /* Displaying abstract syntax trees. */
 
-static void showString(std::ostream & str, std::string_view s)
-{
-    str << '"';
-    for (auto c : s)
-        if (c == '"' || c == '\\' || c == '$') str << "\\" << c;
-        else if (c == '\n') str << "\\n";
-        else if (c == '\r') str << "\\r";
-        else if (c == '\t') str << "\\t";
-        else str << c;
-    str << '"';
-}
-
 std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
 {
     std::string_view s = symbol;
@@ -85,7 +74,7 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
     else {
         char c = s[0];
         if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_')) {
-            showString(str, s);
+            printLiteral(str, s);
             return str;
         }
         for (auto c : s)
@@ -93,7 +82,7 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol)
                   (c >= 'A' && c <= 'Z') ||
                   (c >= '0' && c <= '9') ||
                   c == '_' || c == '\'' || c == '-')) {
-                showString(str, s);
+                printLiteral(str, s);
                 return str;
             }
         str << s;
@@ -118,7 +107,7 @@ void ExprFloat::show(const SymbolTable & symbols, std::ostream & str) const
 
 void ExprString::show(const SymbolTable & symbols, std::ostream & str) const
 {
-    showString(str, s);
+    printLiteral(str, s);
 }
 
 void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const
diff --git a/src/libexpr/value/print.cc b/src/libexpr/value/print.cc
new file mode 100644
index 000000000..cf97def46
--- /dev/null
+++ b/src/libexpr/value/print.cc
@@ -0,0 +1,26 @@
+#include "value/print.hh"
+
+namespace nix {
+
+std::ostream &
+printLiteral(std::ostream & str, const std::string_view string) {
+    str << "\"";
+    for (auto i = string.begin(); i != string.end(); ++i) {
+        if (*i == '\"' || *i == '\\') str << "\\" << *i;
+        else if (*i == '\n') str << "\\n";
+        else if (*i == '\r') str << "\\r";
+        else if (*i == '\t') str << "\\t";
+        else if (*i == '$' && *(i+1) == '{') str << "\\" << *i;
+        else str << *i;
+    }
+    str << "\"";
+    return str;
+}
+
+std::ostream &
+printLiteral(std::ostream & str, bool boolean) {
+    str << (boolean ? "true" : "false");
+    return str;
+}
+
+}
diff --git a/src/libexpr/value/print.hh b/src/libexpr/value/print.hh
new file mode 100644
index 000000000..31c94eb85
--- /dev/null
+++ b/src/libexpr/value/print.hh
@@ -0,0 +1,30 @@
+#pragma once
+/**
+ * @file
+ * @brief Common printing functions for the Nix language
+ *
+ * While most types come with their own methods for printing, they share some
+ * functions that are placed here.
+ */
+
+#include <iostream>
+
+namespace nix {
+    /**
+     * Print a string as a Nix string literal.
+     *
+     * Quotes and fairly minimal escaping are added.
+     *
+     * @param s The logical string
+     */
+    std::ostream & printLiteral(std::ostream & o, std::string_view s);
+    inline std::ostream & printLiteral(std::ostream & o, const char * s) {
+        return printLiteral(o, std::string_view(s));
+    }
+    inline std::ostream & printLiteral(std::ostream & o, const std::string & s) {
+        return printLiteral(o, std::string_view(s));
+    }
+
+    /** Print `true` or `false`. */
+    std::ostream & printLiteral(std::ostream & o, bool b);
+}
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index abdfb1978..9948862e5 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -313,6 +313,15 @@ Derivation parseDerivation(const Store & store, std::string && s, std::string_vi
 }
 
 
+/**
+ * Print a derivation string literal to an std::string.
+ *
+ * This syntax does not generalize to the expression language, which needs to
+ * escape `$`.
+ *
+ * @param res Where to print to
+ * @param s Which logical string to print
+ */
 static void printString(std::string & res, std::string_view s)
 {
     boost::container::small_vector<char, 64 * 1024> buffer;
diff --git a/tests/repl.sh b/tests/repl.sh
index be8adb742..2b3789521 100644
--- a/tests/repl.sh
+++ b/tests/repl.sh
@@ -79,6 +79,14 @@ testReplResponse '
 "result: ${a}"
 ' "result: 2"
 
+# check dollar escaping https://github.com/NixOS/nix/issues/4909
+# note the escaped \,
+#    \\
+# because the second argument is a regex
+testReplResponse '
+"$" + "{hi}"
+' '"\\${hi}"'
+
 testReplResponse '
 drvPath
 ' '".*-simple.drv"' \