Create lint for absolute path literals

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.

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

@@ -14,7 +14,7 @@ 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`.
+ * `Setting<Diagnose>` setting to `Warn`.
  */
 class DeprecatedWarnSetting : public BaseSetting<bool>
 {
@@ -388,6 +388,30 @@ struct EvalSettings : Config
         )",
     };
 
+    Setting<Diagnose> 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<Diagnose> lintUrlLiterals{
         this,
         Diagnose::Ignore,

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<ParseError> {
-        if (literal.front() != '/' && literal.front() != '.')
+    if (literal.front() == '/') {
+        diagnose(state->settings.lintAbsolutePathLiterals, [&](bool) -> std::optional<ParseError> {
             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<ExprPath>(state->exprs.alloc, state->rootFS, path)
-        : state->exprs.add<ExprPath>(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<ExprPath>(state->exprs.alloc, state->rootFS, path);
+    } else {
+        /* check for short path literals */
+        diagnose(state->settings.lintShortPathLiterals, [&](bool) -> std::optional<ParseError> {
+            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<ExprPath>(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<ParseError> {
+        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<ExprPath>(state->exprs.alloc, ref<SourceAccessor>(state->rootFS), path);
+    $$ = state->exprs.add<ExprPath>(state->exprs.alloc, state->rootFS, path);
   }
   ;
 

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!"

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|

tests/functional/lang/eval-fail-abs-path-fatal.flags

@@ -0,0 +1 @@
+--lint-absolute-path-literals fatal

tests/functional/lang/eval-fail-abs-path-fatal.nix

@@ -0,0 +1 @@
+/tmp/foo

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|

tests/functional/lang/eval-fail-home-path-fatal.flags

@@ -0,0 +1 @@
+--impure --lint-absolute-path-literals fatal

tests/functional/lang/eval-fail-home-path-fatal.nix

@@ -0,0 +1 @@
+~/foo

tests/functional/lang/eval-okay-abs-path-default.exp

@@ -0,0 +1 @@
+/tmp/foo

tests/functional/lang/eval-okay-abs-path-default.nix

@@ -0,0 +1,2 @@
+# Test: By default, absolute path literals are allowed
+/tmp/foo

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|

tests/functional/lang/eval-okay-abs-path-warn.exp

@@ -0,0 +1 @@
+/tmp/foo

tests/functional/lang/eval-okay-abs-path-warn.flags

@@ -0,0 +1 @@
+--lint-absolute-path-literals warn

tests/functional/lang/eval-okay-abs-path-warn.nix

@@ -0,0 +1 @@
+/tmp/foo

tests/functional/lang/eval-okay-dotdotslash-abs-fatal.exp

@@ -0,0 +1 @@
+true

tests/functional/lang/eval-okay-dotdotslash-abs-fatal.flags

@@ -0,0 +1 @@
+--lint-absolute-path-literals fatal

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

tests/functional/lang/eval-okay-dotslash-abs-fatal.exp

@@ -0,0 +1 @@
+true

tests/functional/lang/eval-okay-dotslash-abs-fatal.flags

@@ -0,0 +1 @@
+--lint-absolute-path-literals fatal

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

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|

tests/functional/lang/eval-okay-home-path-warn.exp

@@ -0,0 +1 @@
+true

tests/functional/lang/eval-okay-home-path-warn.flags

@@ -0,0 +1 @@
+--impure --lint-absolute-path-literals warn

tests/functional/lang/eval-okay-home-path-warn.nix

@@ -0,0 +1 @@
+builtins.isPath ~/foo

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',