diff --git a/doc/manual/rl-next/lint-url-literals.md b/doc/manual/rl-next/lint-url-literals.md new file mode 100644 index 00000000000..dedf07a5673 --- /dev/null +++ b/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. diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 5de5a5c91ad..f742a744002 100644 --- a/maintainers/flake-module.nix +++ b/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 = { diff --git a/src/libexpr/diagnose.cc b/src/libexpr/diagnose.cc new file mode 100644 index 00000000000..0013281abef --- /dev/null +++ b/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 + +namespace nix { + +template<> +Diagnose BaseSetting::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::trait +{ + static constexpr bool appendable = false; +}; + +template<> +std::string BaseSetting::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; + +} // namespace nix diff --git a/src/libexpr/eval-settings.cc b/src/libexpr/eval-settings.cc index c0acede70a1..5cf0ae04304 100644 --- a/src/libexpr/eval-settings.cc +++ b/src/libexpr/eval-settings.cc @@ -1,4 +1,5 @@ #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" @@ -6,6 +7,26 @@ 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) diff --git a/src/libexpr/include/nix/expr/diagnose.hh b/src/libexpr/include/nix/expr/diagnose.hh new file mode 100644 index 00000000000..8dfe052134a --- /dev/null +++ b/src/libexpr/include/nix/expr/diagnose.hh @@ -0,0 +1,76 @@ +#pragma once +///@file + +#include + +#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::parse(const std::string & str) const; + +template<> +std::string BaseSetting::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 +void diagnose(const Setting & 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 diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index 5dbef9272b9..33ea5f97db7 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/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`. + * When set to true, it emits a deprecation warning and sets the target + * `Setting` setting to `warn`. + */ +class DeprecatedWarnSetting : public BaseSetting +{ + Setting & target; + const char * targetName; + +public: + DeprecatedWarnSetting( + Config * options, + Setting & target, + const char * targetName, + const std::string & name, + const std::string & description, + const StringSet & aliases = {}) + : BaseSetting(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 warnShortPathLiterals{ + Setting 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 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 bindingsUpdateLayerRhsSizeThreshold{ this, diff --git a/src/libexpr/include/nix/expr/meson.build b/src/libexpr/include/nix/expr/meson.build index 2b0fbc40603..4213476fe73 100644 --- a/src/libexpr/include/nix/expr/meson.build +++ b/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', diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index a98319e4af1..0b594dc0cf1 100644 --- a/src/libexpr/meson.build +++ b/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', diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 3358cc1b599..117f678afb1 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -30,6 +30,7 @@ #include "nix/expr/nixexpr.hh" #include "nix/expr/eval.hh" #include "nix/expr/eval-settings.hh" +#include "nix/expr/diagnose.hh" #include "nix/expr/parser-state.hh" #define YY_DECL \ @@ -337,12 +338,14 @@ expr_simple state->exprs.add(state->exprs.alloc, path)}); } | URI { - static bool noURLLiterals = experimentalFeatureSettings.isEnabled(Xp::NoUrlLiterals); - if (noURLLiterals) - throw ParseError({ - .msg = HintFmt("URL literals are disabled"), + diagnose(state->settings.lintUrlLiterals, [&](bool fatal) -> std::optional { + return ParseError({ + .msg = HintFmt("URL literals are %s. Consider using a string literal \"%s\" instead", + fatal ? "disallowed" : "discouraged", + std::string_view($1.p, $1.l)), .pos = state->positions[CUR_POS] }); + }); $$ = state->exprs.add(state->exprs.alloc, $1); } | '(' expr ')' { $$ = $2; } @@ -381,12 +384,14 @@ path_start std::string_view literal({$1.p, $1.l}); /* check for short path literals */ - if (state->settings.warnShortPathLiterals && literal.front() != '/' && literal.front() != '.') { - logWarning({ - .msg = HintFmt("relative path literal '%s' should be prefixed with '.' for clarity: './%s'. (" ANSI_BOLD "warn-short-path-literals" ANSI_NORMAL " = true)", literal, literal), - .pos = state->positions[CUR_POS] - }); - } + diagnose(state->settings.lintShortPathLiterals, [&](bool) -> std::optional { + if (literal.front() != '/' && literal.front() != '.') + return ParseError({ + .msg = HintFmt("relative path literal '%s' should be prefixed with '.' for clarity: './%s'", literal, literal), + .pos = state->positions[CUR_POS] + }); + return std::nullopt; + }); auto basePath = std::filesystem::path(state->basePath.path.abs()); Path path(absPath(literal, &basePath).string()); diff --git a/src/libutil/configuration.cc b/src/libutil/configuration.cc index a18ef3da584..2fe000e4d92 100644 --- a/src/libutil/configuration.cc +++ b/src/libutil/configuration.cc @@ -403,7 +403,11 @@ std::set BaseSetting>::parse( res.insert(thisXpFeature.value()); if (thisXpFeature.value() == Xp::Flakes) res.insert(Xp::FetchTree); - } else + } else if (s == "no-url-literals") + warn( + "experimental feature '%s' has been stabilized and renamed; use 'lint-url-literals = fatal' setting instead", + s); + else warn("unknown experimental feature '%s'", s); } return res; diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 6f9f7d3a51d..9e5b80e76c9 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -27,18 +27,20 @@ void throwExceptionSelfCheck() "C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded."); } +void BaseError::recalcWhat() const +{ + std::ostringstream oss; + showErrorInfo(oss, err, loggerSettings.showTrace); + what_ = oss.str(); +} + // c++ std::exception descendants must have a 'const char* what()' function. // This stringifies the error and caches it for use by what(), or similarly by msg(). const std::string & BaseError::calcWhat() const { - if (what_.has_value()) - return *what_; - else { - std::ostringstream oss; - showErrorInfo(oss, err, loggerSettings.showTrace); - what_ = oss.str(); - return *what_; - } + if (!what_.has_value()) + recalcWhat(); + return *what_; } bool BaseError::hasPos() const diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 727365833fe..5ce11836b0b 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -156,48 +156,6 @@ constexpr std::array xpFeatureDetails )", .trackingUrl = "https://github.com/NixOS/nix/milestone/47", }, - { - .tag = Xp::NoUrlLiterals, - .name = "no-url-literals", - .description = R"( - Disallow unquoted URLs as part of the Nix language syntax. The Nix - language allows for URL literals, like so: - - ``` - $ nix repl - Welcome to Nix 2.15.0. Type :? for help. - - nix-repl> http://foo - "http://foo" - ``` - - But enabling this experimental feature will cause the Nix parser to - throw an error when encountering a URL literal: - - ``` - $ nix repl --extra-experimental-features 'no-url-literals' - Welcome to Nix 2.15.0. Type :? for help. - - nix-repl> http://foo - error: URL literals are disabled - - at «string»:1:1: - - 1| http://foo - | ^ - - ``` - - While this is currently an experimental feature, 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 parameters have to be quoted anyway, and unquoted URLs - may confuse external tooling. - )", - .trackingUrl = "https://github.com/NixOS/nix/milestone/44", - }, { .tag = Xp::FetchClosure, .name = "fetch-closure", diff --git a/src/libutil/include/nix/util/error.hh b/src/libutil/include/nix/util/error.hh index ca54523b666..30af3270f7f 100644 --- a/src/libutil/include/nix/util/error.hh +++ b/src/libutil/include/nix/util/error.hh @@ -225,10 +225,22 @@ public: return !err.traces.empty(); } - const ErrorInfo & info() + /** + * Returns a mutable reference to the error info. + * + * @warning After modifying the returned ErrorInfo, you must call + * recalcWhat() to update the cached formatted message. + */ + ErrorInfo & unsafeInfo() { return err; - }; + } + + /** + * Recalculate the cached formatted error message. + * Must be called after modifying the error info via unsafeInfo(). + */ + void recalcWhat() const; [[noreturn]] virtual void throwClone() const = 0; }; diff --git a/src/libutil/include/nix/util/experimental-features.hh b/src/libutil/include/nix/util/experimental-features.hh index 1a4c9b6b574..1a97f269144 100644 --- a/src/libutil/include/nix/util/experimental-features.hh +++ b/src/libutil/include/nix/util/experimental-features.hh @@ -24,7 +24,6 @@ enum struct ExperimentalFeature { NixCommand, GitHashing, RecursiveNix, - NoUrlLiterals, FetchClosure, AutoAllocateUids, Cgroups, diff --git a/tests/functional/lang/eval-fail-short-path-literal.err.exp b/tests/functional/lang/eval-fail-short-path-literal.err.exp new file mode 100644 index 00000000000..447c29db0f7 --- /dev/null +++ b/tests/functional/lang/eval-fail-short-path-literal.err.exp @@ -0,0 +1,5 @@ +error: relative path literal 'test/subdir' should be prefixed with '.' for clarity: './test/subdir' (lint-short-path-literals) + at /pwd/lang/eval-fail-short-path-literal.nix:1:1: + 1| test/subdir + | ^ + 2| diff --git a/tests/functional/lang/eval-fail-short-path-literal.flags b/tests/functional/lang/eval-fail-short-path-literal.flags new file mode 100644 index 00000000000..674d96dbeb7 --- /dev/null +++ b/tests/functional/lang/eval-fail-short-path-literal.flags @@ -0,0 +1 @@ +--lint-short-path-literals fatal diff --git a/tests/functional/lang/eval-fail-short-path-literal.nix b/tests/functional/lang/eval-fail-short-path-literal.nix new file mode 100644 index 00000000000..481e220c4d7 --- /dev/null +++ b/tests/functional/lang/eval-fail-short-path-literal.nix @@ -0,0 +1 @@ +test/subdir diff --git a/tests/functional/lang/eval-fail-url-literal.err.exp b/tests/functional/lang/eval-fail-url-literal.err.exp new file mode 100644 index 00000000000..1e7fd0db7ec --- /dev/null +++ b/tests/functional/lang/eval-fail-url-literal.err.exp @@ -0,0 +1,5 @@ +error: URL literals are disallowed. Consider using a string literal "http://example.com" instead (lint-url-literals) + at /pwd/lang/eval-fail-url-literal.nix:1:1: + 1| http://example.com + | ^ + 2| diff --git a/tests/functional/lang/eval-fail-url-literal.flags b/tests/functional/lang/eval-fail-url-literal.flags new file mode 100644 index 00000000000..518207573ad --- /dev/null +++ b/tests/functional/lang/eval-fail-url-literal.flags @@ -0,0 +1 @@ +--lint-url-literals fatal diff --git a/tests/functional/lang/eval-fail-url-literal.nix b/tests/functional/lang/eval-fail-url-literal.nix new file mode 100644 index 00000000000..7bc94d03292 --- /dev/null +++ b/tests/functional/lang/eval-fail-url-literal.nix @@ -0,0 +1 @@ +http://example.com diff --git a/tests/functional/lang/eval-okay-dotdotslash-path-fatal.exp b/tests/functional/lang/eval-okay-dotdotslash-path-fatal.exp new file mode 100644 index 00000000000..6cdb45ac7af --- /dev/null +++ b/tests/functional/lang/eval-okay-dotdotslash-path-fatal.exp @@ -0,0 +1 @@ +/pwd/test/subdir diff --git a/tests/functional/lang/eval-okay-dotdotslash-path-fatal.flags b/tests/functional/lang/eval-okay-dotdotslash-path-fatal.flags new file mode 100644 index 00000000000..674d96dbeb7 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotdotslash-path-fatal.flags @@ -0,0 +1 @@ +--lint-short-path-literals fatal diff --git a/tests/functional/lang/eval-okay-dotdotslash-path-fatal.nix b/tests/functional/lang/eval-okay-dotdotslash-path-fatal.nix new file mode 100644 index 00000000000..d29a8981c3f --- /dev/null +++ b/tests/functional/lang/eval-okay-dotdotslash-path-fatal.nix @@ -0,0 +1,2 @@ +# Test: ../ paths are not affected by lint-short-path-literals=fatal +../test/subdir diff --git a/tests/functional/lang/eval-okay-dotslash-path-fatal.exp b/tests/functional/lang/eval-okay-dotslash-path-fatal.exp new file mode 100644 index 00000000000..c2926da802c --- /dev/null +++ b/tests/functional/lang/eval-okay-dotslash-path-fatal.exp @@ -0,0 +1 @@ +/pwd/lang/test/subdir diff --git a/tests/functional/lang/eval-okay-dotslash-path-fatal.flags b/tests/functional/lang/eval-okay-dotslash-path-fatal.flags new file mode 100644 index 00000000000..674d96dbeb7 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotslash-path-fatal.flags @@ -0,0 +1 @@ +--lint-short-path-literals fatal diff --git a/tests/functional/lang/eval-okay-dotslash-path-fatal.nix b/tests/functional/lang/eval-okay-dotslash-path-fatal.nix new file mode 100644 index 00000000000..500b32375b9 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotslash-path-fatal.nix @@ -0,0 +1,2 @@ +# Test: ./ paths are not affected by lint-short-path-literals=fatal +./test/subdir diff --git a/tests/functional/lang/eval-okay-short-path-literal-warn.err.exp b/tests/functional/lang/eval-okay-short-path-literal-warn.err.exp new file mode 100644 index 00000000000..f6b986ac32e --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-literal-warn.err.exp @@ -0,0 +1,5 @@ +warning: relative path literal 'test/subdir' should be prefixed with '.' for clarity: './test/subdir' (lint-short-path-literals) + at /pwd/lang/eval-okay-short-path-literal-warn.nix:1:1: + 1| test/subdir + | ^ + 2| diff --git a/tests/functional/lang/eval-okay-short-path-literal-warn.exp b/tests/functional/lang/eval-okay-short-path-literal-warn.exp new file mode 100644 index 00000000000..c2926da802c --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-literal-warn.exp @@ -0,0 +1 @@ +/pwd/lang/test/subdir diff --git a/tests/functional/lang/eval-okay-short-path-literal-warn.flags b/tests/functional/lang/eval-okay-short-path-literal-warn.flags new file mode 100644 index 00000000000..e514f53eed7 --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-literal-warn.flags @@ -0,0 +1 @@ +--lint-short-path-literals warn diff --git a/tests/functional/lang/eval-okay-short-path-literal-warn.nix b/tests/functional/lang/eval-okay-short-path-literal-warn.nix new file mode 100644 index 00000000000..481e220c4d7 --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-literal-warn.nix @@ -0,0 +1 @@ +test/subdir diff --git a/tests/functional/lang/eval-okay-short-path-variation.err.exp b/tests/functional/lang/eval-okay-short-path-variation.err.exp new file mode 100644 index 00000000000..64f58bd62ff --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-variation.err.exp @@ -0,0 +1,12 @@ +warning: relative path literal 'foo/bar' should be prefixed with '.' for clarity: './foo/bar' (lint-short-path-literals) + at /pwd/lang/eval-okay-short-path-variation.nix:4:3: + 3| [ + 4| foo/bar + | ^ + 5| a/b/c/d +warning: relative path literal 'a/b/c/d' should be prefixed with '.' for clarity: './a/b/c/d' (lint-short-path-literals) + at /pwd/lang/eval-okay-short-path-variation.nix:5:3: + 4| foo/bar + 5| a/b/c/d + | ^ + 6| ] diff --git a/tests/functional/lang/eval-okay-short-path-variation.exp b/tests/functional/lang/eval-okay-short-path-variation.exp new file mode 100644 index 00000000000..45524cce3f6 --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-variation.exp @@ -0,0 +1 @@ +[ /pwd/lang/foo/bar /pwd/lang/a/b/c/d ] diff --git a/tests/functional/lang/eval-okay-short-path-variation.flags b/tests/functional/lang/eval-okay-short-path-variation.flags new file mode 100644 index 00000000000..e514f53eed7 --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-variation.flags @@ -0,0 +1 @@ +--lint-short-path-literals warn diff --git a/tests/functional/lang/eval-okay-short-path-variation.nix b/tests/functional/lang/eval-okay-short-path-variation.nix new file mode 100644 index 00000000000..173c10d9e50 --- /dev/null +++ b/tests/functional/lang/eval-okay-short-path-variation.nix @@ -0,0 +1,6 @@ +# Test: Different short path literals should produce warnings +# This tests variation in path patterns +[ + foo/bar + a/b/c/d +] diff --git a/tests/functional/lang/eval-okay-url-literal-default.exp b/tests/functional/lang/eval-okay-url-literal-default.exp new file mode 100644 index 00000000000..0573d059428 --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-default.exp @@ -0,0 +1 @@ +"http://example.com" diff --git a/tests/functional/lang/eval-okay-url-literal-default.nix b/tests/functional/lang/eval-okay-url-literal-default.nix new file mode 100644 index 00000000000..ad07bc804c3 --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-default.nix @@ -0,0 +1,2 @@ +# Test: By default (no flags), unquoted URL literals are accepted +http://example.com diff --git a/tests/functional/lang/eval-okay-url-literal-quoted-fatal.exp b/tests/functional/lang/eval-okay-url-literal-quoted-fatal.exp new file mode 100644 index 00000000000..0573d059428 --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-quoted-fatal.exp @@ -0,0 +1 @@ +"http://example.com" diff --git a/tests/functional/lang/eval-okay-url-literal-quoted-fatal.flags b/tests/functional/lang/eval-okay-url-literal-quoted-fatal.flags new file mode 100644 index 00000000000..518207573ad --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-quoted-fatal.flags @@ -0,0 +1 @@ +--lint-url-literals fatal diff --git a/tests/functional/lang/eval-okay-url-literal-quoted-fatal.nix b/tests/functional/lang/eval-okay-url-literal-quoted-fatal.nix new file mode 100644 index 00000000000..9b4f20755ec --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-quoted-fatal.nix @@ -0,0 +1,2 @@ +# Test: Quoted URLs are always accepted even with fatal setting +"http://example.com" diff --git a/tests/functional/lang/eval-okay-url-literal-warn.err.exp b/tests/functional/lang/eval-okay-url-literal-warn.err.exp new file mode 100644 index 00000000000..f3636f403ee --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-warn.err.exp @@ -0,0 +1,5 @@ +warning: URL literals are discouraged. Consider using a string literal "http://example.com" instead (lint-url-literals) + at /pwd/lang/eval-okay-url-literal-warn.nix:1:1: + 1| http://example.com + | ^ + 2| diff --git a/tests/functional/lang/eval-okay-url-literal-warn.exp b/tests/functional/lang/eval-okay-url-literal-warn.exp new file mode 100644 index 00000000000..0573d059428 --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-warn.exp @@ -0,0 +1 @@ +"http://example.com" diff --git a/tests/functional/lang/eval-okay-url-literal-warn.flags b/tests/functional/lang/eval-okay-url-literal-warn.flags new file mode 100644 index 00000000000..7ee85110e8e --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-warn.flags @@ -0,0 +1 @@ +--lint-url-literals warn diff --git a/tests/functional/lang/eval-okay-url-literal-warn.nix b/tests/functional/lang/eval-okay-url-literal-warn.nix new file mode 100644 index 00000000000..7bc94d03292 --- /dev/null +++ b/tests/functional/lang/eval-okay-url-literal-warn.nix @@ -0,0 +1 @@ +http://example.com diff --git a/tests/functional/no-url-literals.sh b/tests/functional/no-url-literals.sh index fbc6e1cec24..7b86d79b5a2 100644 --- a/tests/functional/no-url-literals.sh +++ b/tests/functional/no-url-literals.sh @@ -2,27 +2,18 @@ source common.sh -clearStoreIfPossible +# Tests covered by lang tests: +# - Default: unquoted URLs accepted → eval-okay-url-literal-default +# - Fatal: unquoted URLs rejected → eval-fail-url-literal +# - Warn: produces warning → eval-okay-url-literal-warn +# - Quoted URLs accepted with fatal → eval-okay-url-literal-quoted-fatal -# Test 1: By default, unquoted URLs are accepted -nix eval --expr 'http://example.com' 2>&1 | grepQuietInverse "error: URL literals are disabled" +# Test: URLs with parameters (which must be quoted) are accepted +nix eval --lint-url-literals fatal --expr '"http://example.com?foo=bar"' 2>&1 | grepQuietInverse "error:" -# Test 2: With the experimental feature enabled, unquoted URLs are rejected -expect 1 nix eval --extra-experimental-features 'no-url-literals' --expr 'http://example.com' 2>&1 | grepQuiet "error: URL literals are disabled" +# Test: The setting can be enabled via NIX_CONFIG +expect 1 env NIX_CONFIG='lint-url-literals = fatal' nix eval --expr 'http://example.com' 2>&1 | grepQuiet "error: URL literal" -# Test 3: Quoted URLs are always accepted -nix eval --extra-experimental-features 'no-url-literals' --expr '"http://example.com"' 2>&1 | grepQuietInverse "error: URL literals are disabled" - -# Test 4: URLs with parameters (which must be quoted) are accepted -nix eval --extra-experimental-features 'no-url-literals' --expr '"http://example.com?foo=bar"' 2>&1 | grepQuietInverse "error: URL literals are disabled" - -# Test 5: The feature can be enabled via NIX_CONFIG -expect 1 env NIX_CONFIG='extra-experimental-features = no-url-literals' nix eval --expr 'http://example.com' 2>&1 | grepQuiet "error: URL literals are disabled" - -# Test 6: The feature can be enabled via CLI even if not set in config -expect 1 env NIX_CONFIG='' nix eval --extra-experimental-features 'no-url-literals' --expr 'http://example.com' 2>&1 | grepQuiet "error: URL literals are disabled" - -# Test 7: Evaluation still works for quoted URLs -nix eval --raw --extra-experimental-features no-url-literals --expr '"http://example.com"' | grepQuiet "^http://example.com$" - -echo "no-url-literals test passed!" +# Test: Using old experimental feature name produces helpful warning +nix eval --extra-experimental-features no-url-literals --expr '"test"' 2>&1 \ + | grepQuiet "experimental feature 'no-url-literals' has been stabilized and renamed; use 'lint-url-literals = fatal' setting instead" diff --git a/tests/functional/short-path-literals.sh b/tests/functional/short-path-literals.sh index f74044ddad7..5ee864f0cb0 100644 --- a/tests/functional/short-path-literals.sh +++ b/tests/functional/short-path-literals.sh @@ -2,54 +2,53 @@ source common.sh -clearStoreIfPossible +# Tests covered by lang tests: +# - Warning for short paths → eval-okay-short-path-literal-warn +# - Fatal for short paths → eval-fail-short-path-literal +# - Variation paths (foo/bar, a/b/c/d) → eval-okay-short-path-variation +# - ./ paths with fatal → eval-okay-dotslash-path-fatal +# - ../ paths with fatal → eval-okay-dotdotslash-path-fatal -# Test 1: Without the setting (default), no warnings should be produced +# Tests for the deprecated --warn-short-path-literals boolean setting + +# Test: Without the setting (default), no warnings should be produced nix eval --expr 'test/subdir' 2>"$TEST_ROOT"/stderr grepQuietInverse < "$TEST_ROOT/stderr" -E "relative path|path literal" || fail "Should not produce warnings by default" -# Test 2: With the setting enabled, warnings should be produced for short path literals -nix eval --warn-short-path-literals --expr 'test/subdir' 2>"$TEST_ROOT"/stderr -grepQuiet "relative path literal 'test/subdir' should be prefixed with '.' for clarity: './test/subdir'" "$TEST_ROOT/stderr" - -# Test 3: Different short path literals should all produce warnings -nix eval --warn-short-path-literals --expr 'foo/bar' 2>"$TEST_ROOT"/stderr -grepQuiet "relative path literal 'foo/bar' should be prefixed with '.' for clarity: './foo/bar'" "$TEST_ROOT/stderr" - -nix eval --warn-short-path-literals --expr 'a/b/c/d' 2>"$TEST_ROOT"/stderr -grepQuiet "relative path literal 'a/b/c/d' should be prefixed with '.' for clarity: './a/b/c/d'" "$TEST_ROOT/stderr" - -# Test 4: Paths starting with ./ should NOT produce warnings +# Test: Paths starting with ./ should NOT produce warnings nix eval --warn-short-path-literals --expr './test/subdir' 2>"$TEST_ROOT"/stderr grepQuietInverse "relative path literal" "$TEST_ROOT/stderr" -# Test 5: Paths starting with ../ should NOT produce warnings +# Test: Paths starting with ../ should NOT produce warnings nix eval --warn-short-path-literals --expr '../test/subdir' 2>"$TEST_ROOT"/stderr grepQuietInverse "relative path literal" "$TEST_ROOT/stderr" -# Test 6: Absolute paths should NOT produce warnings +# Test: Absolute paths should NOT produce warnings nix eval --warn-short-path-literals --expr '/absolute/path' 2>"$TEST_ROOT"/stderr grepQuietInverse "relative path literal" "$TEST_ROOT/stderr" -# Test 7: Test that the warning is at the correct position -nix eval --warn-short-path-literals --expr 'foo/bar' 2>"$TEST_ROOT"/stderr -grepQuiet "at «string»:1:1:" "$TEST_ROOT/stderr" - -# Test 8: Test that evaluation still works correctly despite the warning -result=$(nix eval --warn-short-path-literals --expr 'test/subdir' 2>/dev/null) -expected="$PWD/test/subdir" -[[ "$result" == "$expected" ]] || fail "Evaluation result should be correct despite warning" - -# Test 9: Test with nix-instantiate as well +# Test: Test with nix-instantiate as well nix-instantiate --warn-short-path-literals --eval -E 'foo/bar' 2>"$TEST_ROOT"/stderr grepQuiet "relative path literal 'foo/bar' should be prefixed" "$TEST_ROOT/stderr" -# Test 10: Test that the setting can be set via configuration +# Test: Test that the deprecated setting can be set via configuration NIX_CONFIG='warn-short-path-literals = true' nix eval --expr 'test/file' 2>"$TEST_ROOT"/stderr grepQuiet "relative path literal 'test/file' should be prefixed" "$TEST_ROOT/stderr" -# Test 11: Test that command line flag overrides config +# Test: Test that command line flag overrides config NIX_CONFIG='warn-short-path-literals = true' nix eval --no-warn-short-path-literals --expr 'test/file' 2>"$TEST_ROOT"/stderr grepQuietInverse "relative path literal" "$TEST_ROOT/stderr" -echo "short-path-literals test passed!" +# Tests for NIX_CONFIG and setting precedence + +# Test: New setting via NIX_CONFIG +NIX_CONFIG='lint-short-path-literals = warn' nix eval --expr 'test/file' 2>"$TEST_ROOT"/stderr +grepQuiet "relative path literal 'test/file' should be prefixed" "$TEST_ROOT/stderr" + +# Test: New setting overrides deprecated setting +NIX_CONFIG='warn-short-path-literals = true' nix eval --lint-short-path-literals ignore --expr 'test/file' 2>"$TEST_ROOT"/stderr +grepQuietInverse "relative path literal" "$TEST_ROOT/stderr" + +# Test: Explicit new setting takes precedence (error over deprecated warn) +NIX_CONFIG='warn-short-path-literals = true' expectStderr 1 nix eval --lint-short-path-literals fatal --expr 'test/subdir' \ + | grepQuiet "error:"