Commit a1d38823 authored by Jade Lovelace's avatar Jade Lovelace
Browse files

nixos/modules: Add declarationPositions 

What it does: line and column level *declaration* position information:

$ nix repl .
nix-repl> :p nixosConfigurations.micro.options.environment.systemPackages.declarationPositions
[ { column = 7; file = "/nix/store/24aj3k7fgqv3ly7qkbf98qvphasrw9nb-source/nixos/modules/config/system-path.nix"; line = 63; } ]

Use cases:
- ctags over NixOS options, as will be presented at NixCon 2023 ;)
- improving the documentation pages to go to the exact line of the
  declarations.

Related work:
- https://github.com/NixOS/nixpkgs/pull/65024

  This one does it for all *definitions* rather than declarations, and
  it was not followed through with due to performance worries.
- https://github.com/NixOS/nixpkgs/pull/208173

  The basis for this change. This change is just a rebase of that one.
  I split it out to add the capability before adding users of it, in
  order to simplify review. However, the ctags script in there is a
  sample user of this feature.

Benchmarks: conducted by evaluating my own reasonably complex NixOS
configuration with the command:
`hyperfine -S none -w 1 -- "nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath"`

```
Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath
  Time (mean ± σ):      8.971 s ±  0.254 s    [User: 5.872 s, System: 1.388 s]
  Range (min … max):    8.574 s …  9.327 s    10 runs

Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath
  Time (mean ± σ):      8.766 s ±  0.160 s    [User: 5.873 s, System: 1.346 s]
  Range (min … max):    8.496 s …  9.033 s    10 runs
```

Summary of results: it seems to be in the noise, this does not cause any
visible regression in times.
parent 450d6437

lib/modules.nix

+11 −4
Original line number Diff line number Diff line
@@ -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 =
@@ -730,9 +730,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. */

lib/tests/modules.sh

+19 −1
Original line number Diff line number Diff line
@@ -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"
@@ -439,6 +439,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

lib/tests/modules/declaration-positions.nix

0 → 100644
+49 −0
Original line number Diff line number Diff line 
{ 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;

  };

}