From a1d38823079bdf7836dd44392e5e1029087d8c85 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Wed, 28 Dec 2022 23:39:05 +0100 Subject: [PATCH] nixos/modules: Add declarationPositions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/modules.nix | 15 +++++-- lib/tests/modules.sh | 20 ++++++++- lib/tests/modules/declaration-positions.nix | 49 +++++++++++++++++++++ 3 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 lib/tests/modules/declaration-positions.nix diff --git a/lib/modules.nix b/lib/modules.nix index 4966619f6630..a6f8b77deb2e 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 = @@ -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. */ diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh index b933a24a57a1..08535b189ee8 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" @@ -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 <