New diagnostics infra

doc/manual/rl-next/lint-url-literals.md

@@ -0,0 +1,42 @@
+---
+synopsis: "New diagnostics infrastructure, with `lint-url-literals` and `lint-short-path-literals` settings"
+prs: [15326]
+issues: [10048, 10281]
+---
+
+A new diagnostics infrastructure has been added for controlling language features that we are considering deprecating.
+
+## [`lint-url-literals`](@docroot@/command-ref/conf-file.md#conf-lint-url-literals)
+
+The `no-url-literals` experimental feature has been stabilized and replaced with a new [`lint-url-literals`](@docroot@/command-ref/conf-file.md#conf-lint-url-literals) setting.
+
+To migrate from the experimental feature, replace:
+```
+experimental-features = no-url-literals
+```
+with:
+```
+lint-url-literals = fatal
+```
+
+## [`lint-short-path-literals`](@docroot@/command-ref/conf-file.md#conf-lint-short-path-literals)
+
+The [`warn-short-path-literals`](@docroot@/command-ref/conf-file.md#conf-warn-short-path-literals) boolean setting has been deprecated and replaced with [`lint-short-path-literals`](@docroot@/command-ref/conf-file.md#conf-lint-short-path-literals).
+
+To migrate, replace:
+```
+warn-short-path-literals = true
+```
+with:
+```
+lint-short-path-literals = warn
+```
+
+## Setting values
+
+Both settings accept three values:
+- `ignore`: Allow the feature without any diagnostic (default)
+- `warn`: Emit a warning when the feature is used
+- `fatal`: Treat the feature as a parse error
+
+In the future, we may change what the defaults are.

maintainers/flake-module.nix

@@ -95,6 +95,11 @@
               ''^tests/functional/lang/eval-fail-bad-string-interpolation-3\.nix$''
               ''^tests/functional/lang/eval-fail-bad-string-interpolation-4\.nix$''
               ''^tests/functional/lang/eval-okay-regex-match2\.nix$''
+
+              # URL literal tests - nixfmt converts unquoted URLs to strings
+              ''^tests/functional/lang/eval-fail-url-literal\.nix$''
+              ''^tests/functional/lang/eval-okay-url-literal-warn\.nix$''
+              ''^tests/functional/lang/eval-okay-url-literal-default\.nix$''
             ];
           };
           clang-format = {

src/libexpr/diagnose.cc

@@ -0,0 +1,55 @@
+#include "nix/expr/diagnose.hh"
+#include "nix/util/configuration.hh"
+#include "nix/util/config-impl.hh"
+#include "nix/util/abstract-setting-to-json.hh"
+
+#include <nlohmann/json.hpp>
+
+namespace nix {
+
+template<>
+Diagnose BaseSetting<Diagnose>::parse(const std::string & str) const
+{
+    if (str == "ignore")
+        return Diagnose::Ignore;
+    else if (str == "warn")
+        return Diagnose::Warn;
+    else if (str == "fatal")
+        return Diagnose::Fatal;
+    else
+        throw UsageError("option '%s' has invalid value '%s' (expected 'ignore', 'warn', or 'fatal')", name, str);
+}
+
+template<>
+struct BaseSetting<Diagnose>::trait
+{
+    static constexpr bool appendable = false;
+};
+
+template<>
+std::string BaseSetting<Diagnose>::to_string() const
+{
+    switch (value) {
+    case Diagnose::Ignore:
+        return "ignore";
+    case Diagnose::Warn:
+        return "warn";
+    case Diagnose::Fatal:
+        return "fatal";
+    default:
+        unreachable();
+    }
+}
+
+NLOHMANN_JSON_SERIALIZE_ENUM(
+    Diagnose,
+    {
+        {Diagnose::Ignore, "ignore"},
+        {Diagnose::Warn, "warn"},
+        {Diagnose::Fatal, "fatal"},
+    });
+
+/* Explicit instantiation of templates */
+template class BaseSetting<Diagnose>;
+
+} // namespace nix

src/libexpr/eval-settings.cc

@@ -1,11 +1,32 @@
 #include "nix/util/users.hh"
+#include "nix/util/logging.hh"
 #include "nix/store/globals.hh"
 #include "nix/store/profiles.hh"
 #include "nix/expr/eval.hh"
 #include "nix/expr/eval-settings.hh"
 
 namespace nix {
 
+void DeprecatedWarnSetting::assign(const bool & v)
+{
+    value = v;
+    warn("'%s' is deprecated, use '%s = %s' instead", name, targetName, v ? "warn" : "ignore");
+    if (!target.overridden)
+        target = v ? Diagnose::Warn : Diagnose::Ignore;
+}
+
+void DeprecatedWarnSetting::appendOrSet(bool newValue, bool append)
+{
+    assert(!append);
+    assign(newValue);
+}
+
+void DeprecatedWarnSetting::override(const bool & v)
+{
+    overridden = true;
+    assign(v);
+}
+
 /* Very hacky way to parse $NIX_PATH, which is colon-separated, but
    can contain URLs (e.g. "nixpkgs=https://bla...:foo=https://"). */
 Strings EvalSettings::parseNixPath(const std::string & s)

src/libexpr/include/nix/expr/diagnose.hh

@@ -0,0 +1,76 @@
+#pragma once
+///@file
+
+#include <optional>
+
+#include "nix/util/ansicolor.hh"
+#include "nix/util/configuration.hh"
+#include "nix/util/error.hh"
+#include "nix/util/logging.hh"
+
+namespace nix {
+
+/**
+ * Diagnostic level for deprecated or non-portable language features.
+ */
+enum struct Diagnose {
+    /**
+     * Ignore the feature without any diagnostic.
+     */
+    Ignore,
+    /**
+     * Warn when the feature is used, but allow it.
+     */
+    Warn,
+    /**
+     * Treat the feature as a fatal error.
+     */
+    Fatal,
+};
+
+template<>
+Diagnose BaseSetting<Diagnose>::parse(const std::string & str) const;
+
+template<>
+std::string BaseSetting<Diagnose>::to_string() const;
+
+/**
+ * Check a diagnostic setting and either do nothing, log a warning, or throw an error.
+ *
+ * The setting name is automatically appended to the error message.
+ *
+ * @param setting The diagnostic setting to check
+ * @param mkError A function that takes a bool (true if fatal, false if warning) and
+ *                returns an optional error to throw (or warn with).
+ *                Only called if level is not `Ignore`.
+ *                If the function returns `std::nullopt`, no diagnostic is emitted.
+ *
+ * @throws The error returned by mkError if level is `Fatal` and mkError returns a value
+ */
+template<typename F>
+void diagnose(const Setting<Diagnose> & setting, F && mkError)
+{
+    auto withError = [&](bool fatal, auto && handler) {
+        auto maybeError = mkError(fatal);
+        if (!maybeError)
+            return;
+        auto & info = maybeError->unsafeInfo();
+        // Append the setting name to help users find the right setting
+        info.msg = HintFmt("%s (" ANSI_BOLD "%s" ANSI_NORMAL ")", Uncolored(info.msg.str()), setting.name);
+        maybeError->recalcWhat();
+        handler(std::move(*maybeError));
+    };
+
+    switch (setting.get()) {
+    case Diagnose::Ignore:
+        return;
+    case Diagnose::Warn:
+        withError(false, [](auto && error) { logWarning(error.info()); });
+        return;
+    case Diagnose::Fatal:
+        withError(true, [](auto && error) { throw std::move(error); });
+        return;
+    }
+}
+
+} // namespace nix

src/libexpr/include/nix/expr/eval-settings.hh

@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include "nix/expr/diagnose.hh"
 #include "nix/expr/eval-profiler-settings.hh"
 #include "nix/util/configuration.hh"
 #include "nix/util/source-path.hh"
@@ -10,6 +11,36 @@ namespace nix {
 class EvalState;
 struct PrimOp;
 
+/**
+ * A deprecated bool setting that migrates to a `Setting<Diagnose>`.
+ * When set to true, it emits a deprecation warning and sets the target
+ * `Setting<Diagnose>` setting to `warn`.
+ */
+class DeprecatedWarnSetting : public BaseSetting<bool>
+{
+    Setting<Diagnose> & target;
+    const char * targetName;
+
+public:
+    DeprecatedWarnSetting(
+        Config * options,
+        Setting<Diagnose> & target,
+        const char * targetName,
+        const std::string & name,
+        const std::string & description,
+        const StringSet & aliases = {})
+        : BaseSetting<bool>(false, true, name, description, aliases, std::nullopt)
+        , target(target)
+        , targetName(targetName)
+    {
+        options->addSetting(this);
+    }
+
+    void assign(const bool & v) override;
+    void appendOrSet(bool newValue, bool append) override;
+    void override(const bool & v) override;
+};
+
 struct EvalSettings : Config
 {
     /**
@@ -328,20 +359,70 @@ struct EvalSettings : Config
           This option can be enabled by setting `NIX_ABORT_ON_WARN=1` in the environment.
         )"};
 
-    Setting<bool> warnShortPathLiterals{
+    Setting<Diagnose> lintShortPathLiterals{
         this,
-        false,
-        "warn-short-path-literals",
+        Diagnose::Ignore,
+        "lint-short-path-literals",
         R"(
-          If set to true, the Nix evaluator will warn when encountering relative path literals
-          that don't start with `./` or `../`.
+          Controls handling of relative path literals that don't start with `./` or `../`.
+
+          - `ignore`: Ignore without warning (default)
+          - `warn`: Emit a warning suggesting to use `./` prefix
+          - `fatal`: Treat as a parse error
 
-          For example, with this setting enabled, `foo/bar` would emit a warning
-          suggesting to use `./foo/bar` instead.
+          For example, with this setting set to `warn` or `fatal`, `foo/bar` would
+          suggest using `./foo/bar` instead.
 
           This is useful for improving code readability and making path literals
           more explicit.
-    )"};
+        )",
+    };
+
+    DeprecatedWarnSetting warnShortPathLiterals{
+        this,
+        lintShortPathLiterals,
+        "lint-short-path-literals",
+        "warn-short-path-literals",
+        R"(
+          Deprecated. Use [`lint-short-path-literals`](#conf-lint-short-path-literals)` = warn` instead.
+        )",
+    };
+
+    Setting<Diagnose> lintUrlLiterals{
+        this,
+        Diagnose::Ignore,
+        "lint-url-literals",
+        R"(
+          Controls handling of unquoted URLs as part of the Nix language syntax.
+          The Nix language allows for URL literals, like so:
+
+          ```
+          $ nix repl
+          nix-repl> http://foo
+          "http://foo"
+          ```
+
+          Setting this to `warn` or `fatal` will cause the Nix parser to
+          warn or throw an error when encountering a URL literal:
+
+          ```
+          $ nix repl --lint-url-literals fatal
+          nix-repl> http://foo
+          error: URL literal 'http://foo' is deprecated
+                 at «string»:1:1:
+
+                      1| http://foo
+                       | ^
+          ```
+
+          Unquoted URLs are being deprecated and their usage is discouraged.
+
+          The reason is that, as opposed to path literals, URLs have no
+          special properties that distinguish them from regular strings, URLs
+          containing query parameters have to be quoted anyway, and unquoted URLs
+          may confuse external tooling.
+        )",
+    };
 
     Setting<unsigned> bindingsUpdateLayerRhsSizeThreshold{
         this,

src/libexpr/include/nix/expr/meson.build

@@ -11,6 +11,7 @@ headers = [ config_pub_h ] + files(
   'attr-path.hh',
   'attr-set.hh',
   'counter.hh',
+  'diagnose.hh',
   'eval-cache.hh',
   'eval-error.hh',
   'eval-gc.hh',

src/libexpr/meson.build

@@ -153,6 +153,7 @@ endforeach
 sources = files(
   'attr-path.cc',
   'attr-set.cc',
+  'diagnose.cc',
   'eval-cache.cc',
   'eval-error.cc',
   'eval-gc.cc',