libcmd: error if first argument for --arg/--argstr isn't an a valid identifier

Step two for #496.

The idea is to allow `nix-build --arg config.allowUnfree true` do the
right thing in the future. However, that's a breaking change since
people might be relying on the ability to set `"config.allowUnfree"` as
attribute-name when auto-calling a Nix-expression.

As a first step, a warning got introduced in 2.92, the next step is now
to reject this usage in 2.94 and await feedback if any so that we can do
the change in a future Lix release.

Change-Id: I6e38fafe26e234204f5bba2a3a4c1da10f80e5f2

doc/manual/rl-next/arg-identifier.md

@@ -0,0 +1,16 @@
+---
+synopsis: First argument to `--arg`/`--argstr` must be a valid Nix identifier
+issues: [fj#496]
+category: "Breaking Changes"
+credits: [ma27]
+---
+
+The first argument to `--arg`/`--argstr` must be a valid Nix identifier, i.e.
+`nix-build --arg config.allowUnfree true` is now rejected.
+
+This is because that invocation is a false friend since it doesn't set
+`{ config = { allowUnfree = true; }; }`, but `{ "config.allowUnfree" = true; }`.
+
+The idea is to change the behavior to the latter in the long-term. For that,
+non-identifiers started giving a warning since 2.92 and are now rejected to give people
+who depend on that a chance to notice and potentially weigh in on the discussion.

lix/libcmd/common-eval-args.cc

@@ -9,6 +9,7 @@
 #include "lix/libstore/store-api.hh"
 #include "lix/libcmd/command.hh"
 #include "lix/libutil/async.hh"
+#include "lix/libutil/error.hh"
 #include "lix/libutil/regex.hh"
 
 #include <regex>
@@ -16,31 +17,36 @@
 namespace nix {
 
 static std::regex const identifierRegex = regex::parse("^[A-Za-z_][A-Za-z0-9_'-]*$");
-static void warnInvalidNixIdentifier(const std::string & name)
+static void checkValidNixIdentifier(const std::string & name)
 {
     std::smatch match;
     if (!std::regex_match(name, match, identifierRegex)) {
-        warn("This Nix invocation specifies a value for argument '%s' which isn't a valid \
-Nix identifier. The project is considering to drop support for this \
-or to require quotes around args that aren't valid Nix identifiers. \
-If you depend on this behvior, please reach out in \
-https://git.lix.systems/lix-project/lix/issues/496 so we can discuss \
-your use-case.", name);
+        throw UsageError(
+            "This invocation specifies a value for argument '%s' "
+            "which isn't a valid Nix identifier. "
+            "The project is dropping support for this so that it's possible to make e.g. "
+            "'%s' evaluating to '%s' in the future. "
+            "If you depend on this behavior, please reach out in "
+            "<https://git.lix.systems/lix-project/lix/issues/496> so we can discuss your use-case.",
+            name,
+            "--arg config.allowUnfree true",
+            "{ config.allowUnfree = true; }"
+        );
     }
 }
 
 MixEvalArgs::MixEvalArgs()
 {
-    addFlag({
-        .longName = "arg",
-        .description = "Pass the value *expr* as the argument *name* to Nix functions.",
-        .category = category,
-        .labels = {"name", "expr"},
-        .handler = {[&](std::string name, std::string expr) {
-            warnInvalidNixIdentifier(name);
-            autoArgs[name] = 'E' + expr;
-        }}
-    });
+    addFlag(
+        {.longName = "arg",
+         .description = "Pass the value *expr* as the argument *name* to Nix functions.",
+         .category = category,
+         .labels = {"name", "expr"},
+         .handler = {[&](std::string name, std::string expr) {
+             checkValidNixIdentifier(name);
+             autoArgs[name] = 'E' + expr;
+         }}}
+    );
 
     addFlag({
         .longName = "argstr",
@@ -48,7 +54,7 @@ MixEvalArgs::MixEvalArgs()
         .category = category,
         .labels = {"name", "string"},
         .handler = {[&](std::string name, std::string s) {
-            warnInvalidNixIdentifier(name);
+            checkValidNixIdentifier(name);
             autoArgs[name] = 'S' + s;
         }},
     });