diff --git a/doc/manual/rl-next/lint-url-literals.md b/doc/manual/rl-next/lint-url-literals.md index dedf07a5673a..01bd8283d2d4 100644 --- a/doc/manual/rl-next/lint-url-literals.md +++ b/doc/manual/rl-next/lint-url-literals.md @@ -1,7 +1,7 @@ --- -synopsis: "New diagnostics infrastructure, with `lint-url-literals` and `lint-short-path-literals` settings" +synopsis: "New diagnostics infrastructure, with `lint-url-literals`, `lint-short-path-literals`, and `lint-absolute-path-literals` settings" prs: [15326] -issues: [10048, 10281] +issues: [8738, 10048, 10281] --- A new diagnostics infrastructure has been added for controlling language features that we are considering deprecating. @@ -32,11 +32,15 @@ with: lint-short-path-literals = warn ``` +## [`lint-absolute-path-literals`](@docroot@/command-ref/conf-file.md#conf-lint-absolute-path-literals) + +A new [`lint-absolute-path-literals`](@docroot@/command-ref/conf-file.md#conf-lint-absolute-path-literals) setting has been added to control handling of absolute path literals (paths starting with `/`) and home path literals (paths starting with `~/`). + ## Setting values -Both settings accept three values: -- `ignore`: Allow the feature without any diagnostic (default) +All three settings accept three values: +- `ignore`: Allow the feature without emitting 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. +The defaults may change in future versions. diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index 33ea5f97db77..ceddff9ef824 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/src/libexpr/include/nix/expr/eval-settings.hh @@ -14,7 +14,7 @@ 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`. + * `Setting` setting to `Warn`. */ class DeprecatedWarnSetting : public BaseSetting { @@ -388,6 +388,30 @@ struct EvalSettings : Config )", }; + Setting lintAbsolutePathLiterals{ + this, + Diagnose::Ignore, + "lint-absolute-path-literals", + R"( + Controls handling of absolute path literals (paths starting with `/`) and home path literals (paths starting with `~/`). + + - `ignore`: Ignore without warning (default) + - `warn`: Emit a warning about non-portability + - `fatal`: Treat as a parse error + + It is true that some files are more difficult to reference with relative paths, + because they would require lots of `../../..` upward traversing to reach them. + But firstly, it is probably not a good idea to reference these files --- + such paths often make Nix expressions less portable and reproducible, + as they depend on the file system layout of the machine evaluating the expression. + + Secondly, with [pure evaluation mode](#conf-pure-eval), most such files are prohibited to access anyway, + whether by absolute or relative paths. + In that case, enabling this lint in fatal mode is less disruptive, + because the paths pure eval allows are usually not the ones that would be ergonomically expressed with absolute paths anyway. + )", + }; + Setting lintUrlLiterals{ this, Diagnose::Ignore, diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 27f9fb824e04..772a20e81a4b 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -415,38 +415,56 @@ path_start : PATH { std::string_view literal({$1.p, $1.l}); - /* check for short path literals */ - diagnose(state->settings.lintShortPathLiterals, [&](bool) -> std::optional { - if (literal.front() != '/' && literal.front() != '.') + if (literal.front() == '/') { + diagnose(state->settings.lintAbsolutePathLiterals, [&](bool) -> std::optional { return ParseError({ - .msg = HintFmt("relative path literal '%s' should be prefixed with '.' for clarity: './%s'", literal, literal), + .msg = HintFmt("absolute path literals are not portable. Consider replacing path literal '%s' by a string, relative path, or parameter", literal), .pos = state->positions[CUR_POS] }); - return std::nullopt; - }); - - auto basePath = std::filesystem::path(state->basePath.path.abs()); - Path path(absPath(literal, &basePath).string()); - /* add back in the trailing '/' to the first segment */ - if (literal.size() > 1 && literal.back() == '/') - path += '/'; - $$ = + }); /* Absolute paths are always interpreted relative to the root filesystem accessor, rather than the accessor of the current Nix expression. */ - literal.front() == '/' - ? state->exprs.add(state->exprs.alloc, state->rootFS, path) - : state->exprs.add(state->exprs.alloc, state->basePath.accessor, path); + Path path(canonPath(literal)); + /* add back in the trailing '/' to the first segment */ + if (literal.size() > 1 && literal.back() == '/') + path += '/'; + $$ = state->exprs.add(state->exprs.alloc, state->rootFS, path); + } else { + /* check for short path literals */ + diagnose(state->settings.lintShortPathLiterals, [&](bool) -> std::optional { + if (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()); + /* add back in the trailing '/' to the first segment */ + if (literal.size() > 1 && literal.back() == '/') + path += '/'; + $$ = state->exprs.add(state->exprs.alloc, state->basePath.accessor, path); + } } | HPATH { + std::string_view literal($1.p, $1.l); if (state->settings.pureEval) { throw Error( "the path '%s' can not be resolved in pure mode", - std::string_view($1.p, $1.l) + literal ); } + diagnose(state->settings.lintAbsolutePathLiterals, [&](bool) -> std::optional { + return ParseError({ + .msg = HintFmt("home path literals are not portable. Consider replacing path literal '%s' by a string, relative path, or parameter", literal), + .pos = state->positions[CUR_POS] + }); + }); Path path(getHome().string() + std::string($1.p + 1, $1.l - 1)); - $$ = state->exprs.add(state->exprs.alloc, ref(state->rootFS), path); + $$ = state->exprs.add(state->exprs.alloc, state->rootFS, path); } ; diff --git a/tests/functional/absolute-path-literals.sh b/tests/functional/absolute-path-literals.sh new file mode 100755 index 000000000000..8cb31ffa4666 --- /dev/null +++ b/tests/functional/absolute-path-literals.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash + +source common.sh + +# Tests for absolute path literals that require NIX_CONFIG or grepQuietInverse. +# Basic warn/fatal/default behavior tests are in lang/eval-*-abs-path-*.nix + +clearStoreIfPossible + +# Test: Setting via NIX_CONFIG +NIX_CONFIG='lint-absolute-path-literals = warn' nix eval --expr '/tmp/bar' 2>"$TEST_ROOT"/stderr +grepQuiet "absolute path literals are not portable" "$TEST_ROOT/stderr" + +# Test: Command line overrides config +NIX_CONFIG='lint-absolute-path-literals = warn' nix eval --lint-absolute-path-literals ignore --expr '/tmp/bar' 2>"$TEST_ROOT"/stderr +grepQuietInverse "absolute path literal" "$TEST_ROOT/stderr" + +echo "absolute-path-literals test passed!" diff --git a/tests/functional/lang/eval-fail-abs-path-fatal.err.exp b/tests/functional/lang/eval-fail-abs-path-fatal.err.exp new file mode 100644 index 000000000000..698af03909a2 --- /dev/null +++ b/tests/functional/lang/eval-fail-abs-path-fatal.err.exp @@ -0,0 +1,5 @@ +error: absolute path literals are not portable. Consider replacing path literal '/tmp/foo' by a string, relative path, or parameter (lint-absolute-path-literals) + at /pwd/lang/eval-fail-abs-path-fatal.nix:1:1: + 1| /tmp/foo + | ^ + 2| diff --git a/tests/functional/lang/eval-fail-abs-path-fatal.flags b/tests/functional/lang/eval-fail-abs-path-fatal.flags new file mode 100644 index 000000000000..1615019e589c --- /dev/null +++ b/tests/functional/lang/eval-fail-abs-path-fatal.flags @@ -0,0 +1 @@ +--lint-absolute-path-literals fatal diff --git a/tests/functional/lang/eval-fail-abs-path-fatal.nix b/tests/functional/lang/eval-fail-abs-path-fatal.nix new file mode 100644 index 000000000000..7ddb87177c45 --- /dev/null +++ b/tests/functional/lang/eval-fail-abs-path-fatal.nix @@ -0,0 +1 @@ +/tmp/foo diff --git a/tests/functional/lang/eval-fail-home-path-fatal.err.exp b/tests/functional/lang/eval-fail-home-path-fatal.err.exp new file mode 100644 index 000000000000..c4f3ea3b8632 --- /dev/null +++ b/tests/functional/lang/eval-fail-home-path-fatal.err.exp @@ -0,0 +1,5 @@ +error: home path literals are not portable. Consider replacing path literal '~/foo' by a string, relative path, or parameter (lint-absolute-path-literals) + at /pwd/lang/eval-fail-home-path-fatal.nix:1:1: + 1| ~/foo + | ^ + 2| diff --git a/tests/functional/lang/eval-fail-home-path-fatal.flags b/tests/functional/lang/eval-fail-home-path-fatal.flags new file mode 100644 index 000000000000..a78e3b680857 --- /dev/null +++ b/tests/functional/lang/eval-fail-home-path-fatal.flags @@ -0,0 +1 @@ +--impure --lint-absolute-path-literals fatal diff --git a/tests/functional/lang/eval-fail-home-path-fatal.nix b/tests/functional/lang/eval-fail-home-path-fatal.nix new file mode 100644 index 000000000000..827774311d93 --- /dev/null +++ b/tests/functional/lang/eval-fail-home-path-fatal.nix @@ -0,0 +1 @@ +~/foo diff --git a/tests/functional/lang/eval-okay-abs-path-default.exp b/tests/functional/lang/eval-okay-abs-path-default.exp new file mode 100644 index 000000000000..7ddb87177c45 --- /dev/null +++ b/tests/functional/lang/eval-okay-abs-path-default.exp @@ -0,0 +1 @@ +/tmp/foo diff --git a/tests/functional/lang/eval-okay-abs-path-default.nix b/tests/functional/lang/eval-okay-abs-path-default.nix new file mode 100644 index 000000000000..3727c0af4e16 --- /dev/null +++ b/tests/functional/lang/eval-okay-abs-path-default.nix @@ -0,0 +1,2 @@ +# Test: By default, absolute path literals are allowed +/tmp/foo diff --git a/tests/functional/lang/eval-okay-abs-path-warn.err.exp b/tests/functional/lang/eval-okay-abs-path-warn.err.exp new file mode 100644 index 000000000000..1b93c9798fd9 --- /dev/null +++ b/tests/functional/lang/eval-okay-abs-path-warn.err.exp @@ -0,0 +1,5 @@ +warning: absolute path literals are not portable. Consider replacing path literal '/tmp/foo' by a string, relative path, or parameter (lint-absolute-path-literals) + at /pwd/lang/eval-okay-abs-path-warn.nix:1:1: + 1| /tmp/foo + | ^ + 2| diff --git a/tests/functional/lang/eval-okay-abs-path-warn.exp b/tests/functional/lang/eval-okay-abs-path-warn.exp new file mode 100644 index 000000000000..7ddb87177c45 --- /dev/null +++ b/tests/functional/lang/eval-okay-abs-path-warn.exp @@ -0,0 +1 @@ +/tmp/foo diff --git a/tests/functional/lang/eval-okay-abs-path-warn.flags b/tests/functional/lang/eval-okay-abs-path-warn.flags new file mode 100644 index 000000000000..93b76fc26a1b --- /dev/null +++ b/tests/functional/lang/eval-okay-abs-path-warn.flags @@ -0,0 +1 @@ +--lint-absolute-path-literals warn diff --git a/tests/functional/lang/eval-okay-abs-path-warn.nix b/tests/functional/lang/eval-okay-abs-path-warn.nix new file mode 100644 index 000000000000..7ddb87177c45 --- /dev/null +++ b/tests/functional/lang/eval-okay-abs-path-warn.nix @@ -0,0 +1 @@ +/tmp/foo diff --git a/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.exp b/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.exp new file mode 100644 index 000000000000..27ba77ddaf61 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.exp @@ -0,0 +1 @@ +true diff --git a/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.flags b/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.flags new file mode 100644 index 000000000000..1615019e589c --- /dev/null +++ b/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.flags @@ -0,0 +1 @@ +--lint-absolute-path-literals fatal diff --git a/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.nix b/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.nix new file mode 100644 index 000000000000..5c01768f9e93 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotdotslash-abs-fatal.nix @@ -0,0 +1,2 @@ +# Test: ../ relative paths work even when absolute paths are fatal +builtins.isPath ../lang/lang.nix diff --git a/tests/functional/lang/eval-okay-dotslash-abs-fatal.exp b/tests/functional/lang/eval-okay-dotslash-abs-fatal.exp new file mode 100644 index 000000000000..27ba77ddaf61 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotslash-abs-fatal.exp @@ -0,0 +1 @@ +true diff --git a/tests/functional/lang/eval-okay-dotslash-abs-fatal.flags b/tests/functional/lang/eval-okay-dotslash-abs-fatal.flags new file mode 100644 index 000000000000..1615019e589c --- /dev/null +++ b/tests/functional/lang/eval-okay-dotslash-abs-fatal.flags @@ -0,0 +1 @@ +--lint-absolute-path-literals fatal diff --git a/tests/functional/lang/eval-okay-dotslash-abs-fatal.nix b/tests/functional/lang/eval-okay-dotslash-abs-fatal.nix new file mode 100644 index 000000000000..dda55fefd078 --- /dev/null +++ b/tests/functional/lang/eval-okay-dotslash-abs-fatal.nix @@ -0,0 +1,2 @@ +# Test: ./ relative paths work even when absolute paths are fatal +builtins.isPath ./lang.nix diff --git a/tests/functional/lang/eval-okay-home-path-warn.err.exp b/tests/functional/lang/eval-okay-home-path-warn.err.exp new file mode 100644 index 000000000000..b7a3a88cdf61 --- /dev/null +++ b/tests/functional/lang/eval-okay-home-path-warn.err.exp @@ -0,0 +1,5 @@ +warning: home path literals are not portable. Consider replacing path literal '~/foo' by a string, relative path, or parameter (lint-absolute-path-literals) + at /pwd/lang/eval-okay-home-path-warn.nix:1:17: + 1| builtins.isPath ~/foo + | ^ + 2| diff --git a/tests/functional/lang/eval-okay-home-path-warn.exp b/tests/functional/lang/eval-okay-home-path-warn.exp new file mode 100644 index 000000000000..27ba77ddaf61 --- /dev/null +++ b/tests/functional/lang/eval-okay-home-path-warn.exp @@ -0,0 +1 @@ +true diff --git a/tests/functional/lang/eval-okay-home-path-warn.flags b/tests/functional/lang/eval-okay-home-path-warn.flags new file mode 100644 index 000000000000..7be81df22710 --- /dev/null +++ b/tests/functional/lang/eval-okay-home-path-warn.flags @@ -0,0 +1 @@ +--impure --lint-absolute-path-literals warn diff --git a/tests/functional/lang/eval-okay-home-path-warn.nix b/tests/functional/lang/eval-okay-home-path-warn.nix new file mode 100644 index 000000000000..42f317720ca5 --- /dev/null +++ b/tests/functional/lang/eval-okay-home-path-warn.nix @@ -0,0 +1 @@ +builtins.isPath ~/foo diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 68639a409088..b1a903164562 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -114,6 +114,7 @@ suites = [ 'pure-eval.sh', 'eval.sh', 'short-path-literals.sh', + 'absolute-path-literals.sh', 'no-url-literals.sh', 'repl.sh', 'binary-cache-build-remote.sh',