New diagnostics infra by Ericson2314 · Pull Request #15326 · NixOS/nix

Ericson2314 · 2026-02-23T23:19:33Z

Motivation

Convert no-url-literals from experimental feature to Diagnose setting

Replace Xp::NoUrlLiterals with a new lint-url-literals setting that accepts ignore, warn, or fatal. This provides more flexibility than the binary experimental feature.
Convert warn-short-path-literals to use new lint infra

We now have lint-short-path-literals = ignore | warn | fatal instead.

Context

Fix #10048
Progress on #10281

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

xokdvium · 2026-02-24T17:46:50Z

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

-        false,
-        "warn-short-path-literals",
+        LintMode::allow,
+        "short-path-literals",


Isn't this too ambiguous of an option name?

Added lint- to both

src/libexpr/parser.y

src/libexpr/include/nix/expr/lint-mode.hh

roberth · 2026-02-24T17:51:59Z

src/libutil/experimental-features.cc

It's a stabilization 🎉

My understanding here:

this check works

it's something we can keep maintaining

neither we nor users have felt a need to make this opt-out, but could still be considered

Needs release note.

Should this experimental feature be kept around just to affect the option default? Maybe not because unlike a Nix-based configuration manager, the settings/flags system is tricky and hard to reason about.

Something actionable beyond "warning: unknown experimental feature 'no-url-literals'" would be nice

Improved the warning and added the release note

roberth · 2026-02-24T17:53:11Z

tests/functional/absolute-path-literals.sh

Could, but does not implement the command line options proposed and approved in

Add a warning infrastructure #10281

Need to drop from "Fixes #" or implement

Made it "progress on"

tests/functional/short-path-literals.sh

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

src/libexpr/parser.y

xokdvium · 2026-02-24T19:45:07Z

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

+        logWarning(mkInfo());
+        return;
+    case Diagnose::Fatal:
+        throw ParseError(mkInfo());


Seems like harding ParseError here isn't exactly correct. Maybe it should be the callback the decides which error type to throw?

roberth

Ideas. See comments.
Also would be nice to have compiler-style flag syntax from the start.
Other than that, lgtm.

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

roberth · 2026-02-25T13:02:26Z

tests/functional/no-url-literals.sh

We can pass options in the lang tests as well. These tests could perhaps be migrated. Characterisation tests give a more detailed coverage of the error reports and nicer DX.

roberth · 2026-02-25T13:18:21Z

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

+    /**
+     * Treat the feature as a fatal error.
+     */
+    Fatal,


Good rename!
We could add Error and a diagnostic state later.
class DiagnosticState { vector<Error>; warn(Error &&); saveError(Error &&); }
or
class DiagnosticState { bool mustThrow; }
and
throwingDiagnosticErrors :: (DiagnosticState -> IO r) -> IO r

Agreed we should collect all fatal errors and then throw. But best saved for later.

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

roberth · 2026-02-25T21:48:13Z

src/libexpr/parser.y

-              .msg = HintFmt("URL literals are disabled"),
+      diagnose(state->settings.urlLiterals, [&]() -> std::optional<ParseError> {
+          return ParseError({
+              .msg = HintFmt("URL literal '%s' is deprecated", std::string_view($1.p, $1.l)),


Like my earlier comment, this raises the question: which URL literals are allowed?
Instead it should say these independent things:

URL literals are disallowed / discouraged (depending on warn/fatal)

Here it is

Use a string literal instead

roberth · 2026-02-25T21:49:34Z

tests/functional/no-url-literals.sh


 source common.sh

-clearStoreIfPossible


Please look into this:

Removed test cases not fully covered by new lang tests

no-url-literals.sh went from 7 tests to 4:

Old Test 1 (default: unquoted URLs accepted) — REMOVED, not replaced. No new test verifies that lint-url-literals defaults to ignore and URLs work without flags.

Old Test 6 (CLI flag override: --extra-experimental-features works even with empty NIX_CONFIG) — REMOVED, not replaced.

Old Test 7 (--raw evaluation correctness for quoted URLs) — REMOVED. Partially covered by the new eval-okay-url-literal-warn.exp lang test, but that tests warn mode, not fatal mode with quoted URLs.

short-path-literals.sh went from 11 tests to 10 (8 old + 3 new):

Old Test 3 (multiple path patterns: foo/bar, a/b/c/d) — REMOVED. Only test/subdir is tested now. Reduces variation coverage.

Old Test 7 (position check: at «string»:1:1:) — REMOVED. Covered by the .err.exp lang tests.

Old Test 8 (evaluation correctness: result equals $PWD/test/subdir) — REMOVED. Covered by eval-okay-short-path-literal-warn.exp.

src/libexpr/eval-settings.cc

- Convert `no-url-literals` from experimental feature to `Diagnose` setting Replace `Xp::NoUrlLiterals` with a new `lint-url-literals` setting that accepts `ignore`, `warn`, or `fatal`. This provides more flexibility than the binary experimental feature. - Convert `warn-short-path-literals` to use new lint infra We now have `lint-short-path-literals = ignore | warn | fatal` instead. - Convert some of the tests to lang tests Fix NixOS#10048 Progress on NixOS#10281

github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Feb 23, 2026

Ericson2314 force-pushed the no-abs-paths-in-eval branch 2 times, most recently from bca0e3d to 56a03e9 Compare February 24, 2026 17:32

Ericson2314 marked this pull request as ready for review February 24, 2026 17:33

Ericson2314 requested a review from edolstra as a code owner February 24, 2026 17:33

Ericson2314 force-pushed the no-abs-paths-in-eval branch from 56a03e9 to d1949c4 Compare February 24, 2026 17:34