Merge pull request #8110 from aakropotkin/quote-reserved

Quote reserved keywords when printing expressions
NixOS · May 9, 2023 · aacde38 · aacde38
2 parents 4539ab5 + 82d1d74
commit aacde38
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 5 deletions.
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
@@ -94,7 +94,6 @@ RootValue allocRootValue(Value * v)
 #endif
 }
 
-
 void Value::print(const SymbolTable & symbols, std::ostream & str,
     std::set<const void *> * seen) const
 {

diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc
@@ -1,4 +1,5 @@
 #include "print.hh"
+#include <unordered_set>
 
 namespace nix {
 
@@ -25,11 +26,26 @@ printLiteralBool(std::ostream & str, bool boolean)
     return str;
 }
 
+// Returns `true' is a string is a reserved keyword which requires quotation
+// when printing attribute set field names.
+//
+// This list should generally be kept in sync with `./lexer.l'.
+// You can test if a keyword needs to be added by running:
+//   $ nix eval --expr '{ <KEYWORD> = 1; }'
+// For example `or' doesn't need to be quoted.
+bool isReservedKeyword(const std::string_view str)
+{
+    static const std::unordered_set<std::string_view> reservedKeywords = {
+        "if", "then", "else", "assert", "with", "let", "in", "rec", "inherit"
+    };
+    return reservedKeywords.contains(str);
+}
+
 std::ostream &
 printIdentifier(std::ostream & str, std::string_view s) {
     if (s.empty())
         str << "\"\"";
-    else if (s == "if") // FIXME: handle other keywords
+    else if (isReservedKeyword(s))
         str << '"' << s << '"';
     else {
         char c = s[0];
@@ -50,10 +66,10 @@ printIdentifier(std::ostream & str, std::string_view s) {
     return str;
 }
 
-// FIXME: keywords
 static bool isVarName(std::string_view s)
 {
     if (s.size() == 0) return false;
+    if (isReservedKeyword(s)) return false;
     char c = s[0];
     if ((c >= '0' && c <= '9') || c == '-' || c == '\'') return false;
     for (auto & i : s)

diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh
@@ -35,6 +35,12 @@ namespace nix {
      */
     std::ostream & printAttributeName(std::ostream & o, std::string_view s);
 
+    /**
+     * Returns `true' is a string is a reserved keyword which requires quotation
+     * when printing attribute set field names.
+     */
+    bool isReservedKeyword(const std::string_view str);
+
     /**
      * Print a string as an identifier in the Nix expression language syntax.
      *

diff --git a/tests/eval.sh b/tests/eval.sh
@@ -16,19 +16,21 @@ nix eval --expr 'assert 1 + 2 == 3; true'
 [[ $(nix eval int -f "./eval.nix") == 123 ]]
 [[ $(nix eval str -f "./eval.nix") == '"foo"' ]]
 [[ $(nix eval str --raw -f "./eval.nix") == 'foo' ]]
-[[ $(nix eval attr -f "./eval.nix") == '{ foo = "bar"; }' ]]
+[[ "$(nix eval attr -f "./eval.nix")" == '{ foo = "bar"; }' ]]
 [[ $(nix eval attr --json -f "./eval.nix") == '{"foo":"bar"}' ]]
 [[ $(nix eval int -f - < "./eval.nix") == 123 ]]
+[[ "$(nix eval --expr '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]]
 
 # Check if toFile can be utilized during restricted eval
 [[ $(nix eval --restrict-eval --expr 'import (builtins.toFile "source" "42")') == 42 ]]
 
 nix-instantiate --eval -E 'assert 1 + 2 == 3; true'
 [[ $(nix-instantiate -A int --eval "./eval.nix") == 123 ]]
 [[ $(nix-instantiate -A str --eval "./eval.nix") == '"foo"' ]]
-[[ $(nix-instantiate -A attr --eval "./eval.nix") == '{ foo = "bar"; }' ]]
+[[ "$(nix-instantiate -A attr --eval "./eval.nix")" == '{ foo = "bar"; }' ]]
 [[ $(nix-instantiate -A attr --eval --json "./eval.nix") == '{"foo":"bar"}' ]]
 [[ $(nix-instantiate -A int --eval - < "./eval.nix") == 123 ]]
+[[ "$(nix-instantiate --eval -E '{"assert"=1;bar=2;}')" == '{ "assert" = 1; bar = 2; }' ]]
 
 # Check that symlink cycles don't cause a hang.
 ln -sfn cycle.nix $TEST_ROOT/cycle.nix