Merge pull request #249243 from lf-/jade/declarationsWithLocations

nixos/modules: Add declarationPositions

3 files changed, 79 insertions, 5 deletions

diff --git a/lib/modules.nix b/lib/modules.nix
index 5c2fb48868c..4acbce39e94 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -537,7 +537,7 @@ let
     mergeModules' prefix modules
       (concatMap (m: map (config: { file = m._file; inherit config; }) (pushDownProperties m.config)) modules);
 
-  mergeModules' = prefix: options: configs:
+  mergeModules' = prefix: modules: configs:
     let
       # an attrset 'name' => list of submodules that declare ‘name’.
       declsByName =
@@ -554,11 +554,11 @@ let
               else
                 mapAttrs
                   (n: option:
-                    [{ inherit (module) _file; options = option; }]
+                    [{ inherit (module) _file; pos = builtins.unsafeGetAttrPos n subtree; options = option; }]
                   )
                   subtree
               )
-            options);
+            modules);
 
       # The root of any module definition must be an attrset.
       checkedConfigs =
@@ -762,9 +762,16 @@ let
             else res.options;
         in opt.options // res //
           { declarations = res.declarations ++ [opt._file];
+            # In the case of modules that are generated dynamically, we won't
+            # have exact declaration lines; fall back to just the file being
+            # evaluated.
+            declarationPositions = res.declarationPositions
+              ++ (if opt.pos != null
+                then [opt.pos]
+                else [{ file = opt._file; line = null; column = null; }]);
             options = submodules;
           } // typeSet
-    ) { inherit loc; declarations = []; options = []; } opts;
+    ) { inherit loc; declarations = []; declarationPositions = []; options = []; } opts;
 
   /* Merge all the definitions of an option to produce the final
      config value. */
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index 2c5e4cdbcec..93fb1df9332 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -39,7 +39,7 @@ reportFailure() {
 checkConfigOutput() {
     local outputContains=$1
     shift
-    if evalConfig "$@" 2>/dev/null | grep --silent "$outputContains" ; then
+    if evalConfig "$@" 2>/dev/null | grep -E --silent "$outputContains" ; then
         ((++pass))
     else
         echo 2>&1 "error: Expected result matching '$outputContains', while evaluating"
@@ -444,6 +444,24 @@ checkConfigOutput '^"The option `a\.b. defined in `.*/doRename-warnings\.nix. ha
 checkConfigOutput '^"pear"$' config.once.raw ./merge-module-with-key.nix
 checkConfigOutput '^"pear\\npear"$' config.twice.raw ./merge-module-with-key.nix
 
+# Declaration positions
+# Line should be present for direct options
+checkConfigOutput '^10$' options.imported.line10.declarationPositions.0.line ./declaration-positions.nix
+checkConfigOutput '/declaration-positions.nix"$' options.imported.line10.declarationPositions.0.file ./declaration-positions.nix
+# Generated options may not have line numbers but they will at least get the
+# right file
+checkConfigOutput '/declaration-positions.nix"$' options.generated.line18.declarationPositions.0.file ./declaration-positions.nix
+checkConfigOutput '^null$' options.generated.line18.declarationPositions.0.line ./declaration-positions.nix
+# Submodules don't break it
+checkConfigOutput '^39$' config.submoduleLine34.submodDeclLine39.0.line ./declaration-positions.nix
+checkConfigOutput '/declaration-positions.nix"$' config.submoduleLine34.submodDeclLine39.0.file ./declaration-positions.nix
+# New options under freeform submodules get collected into the parent submodule
+# (consistent with .declarations behaviour, but weird; notably appears in system.build)
+checkConfigOutput '^34|23$' options.submoduleLine34.declarationPositions.0.line ./declaration-positions.nix
+checkConfigOutput '^34|23$' options.submoduleLine34.declarationPositions.1.line ./declaration-positions.nix
+# nested options work
+checkConfigOutput '^30$' options.nested.nestedLine30.declarationPositions.0.line ./declaration-positions.nix
+
 cat <<EOF
 ====== module tests ======
 $pass Pass
diff --git a/lib/tests/modules/declaration-positions.nix b/lib/tests/modules/declaration-positions.nix
new file mode 100644
index 00000000000..cefd3b4e516
--- /dev/null
+++ b/lib/tests/modules/declaration-positions.nix
@@ -0,0 +1,49 @@
+{ lib, options, ... }:
+let discardPositions = lib.mapAttrs (k: v: v);
+in
+# unsafeGetAttrPos is unspecified best-effort behavior, so we only want to consider this test on an evaluator that satisfies some basic assumptions about this function.
+assert builtins.unsafeGetAttrPos "a" { a = true; } != null;
+assert builtins.unsafeGetAttrPos "a" (discardPositions { a = true; }) == null;
+{
+  imports = [
+    {
+      options.imported.line10 = lib.mkOption {
+        type = lib.types.int;
+      };
+
+      # Simulates various patterns of generating modules such as
+      # programs.firefox.nativeMessagingHosts.ff2mpv. We don't expect to get
+      # line numbers for these, but we can fall back on knowing the file.
+      options.generated = discardPositions {
+        line18 = lib.mkOption {
+          type = lib.types.int;
+        };
+      };
+
+      options.submoduleLine34.extraOptLine23 = lib.mkOption {
+        default = 1;
+        type = lib.types.int;
+      };
+    }
+  ];
+
+  options.nested.nestedLine30 = lib.mkOption {
+    type = lib.types.int;
+  };
+
+  options.submoduleLine34 = lib.mkOption {
+    default = { };
+    type = lib.types.submoduleWith {
+      modules = [
+        ({ options, ... }: {
+          options.submodDeclLine39 = lib.mkOption { };
+        })
+        { freeformType = with lib.types; lazyAttrsOf (uniq unspecified); }
+      ];
+    };
+  };
+
+  config = {
+    submoduleLine34.submodDeclLine39 = (options.submoduleLine34.type.getSubOptions [ ]).submodDeclLine39.declarationPositions;
+  };
+}