Skip to content

Commit

Permalink
nixos/modules: Add declarationPositions
Browse files Browse the repository at this point in the history
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:
- NixOS#65024

  This one does it for all *definitions* rather than declarations, and
  it was not followed through with due to performance worries.
- NixOS#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.
  • Loading branch information
lf- committed Aug 28, 2023
1 parent 450d643 commit c745c0b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 5 deletions.
15 changes: 11 additions & 4 deletions lib/modules.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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 =
Expand Down Expand Up @@ -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. */
Expand Down
15 changes: 14 additions & 1 deletion lib/tests/modules.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -439,6 +439,19 @@ 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 '^7$' options.imported.line7.declarationPositions.0.line ./declaration-positions.nix
checkConfigOutput '/declaration-positions.nix"$' options.imported.line7.declarationPositions.0.file ./declaration-positions.nix
checkConfigOutput '^22$' options.submoduleLine22.declarationPositions.0.line ./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.line16.declarationPositions.0.file ./declaration-positions.nix
checkConfigOutput '^null$' options.generated.line16.declarationPositions.0.line ./declaration-positions.nix
# Submodules don't break it
checkConfigOutput '^27$' config.submoduleLine22.submodDeclLine27.0.line ./declaration-positions.nix
checkConfigOutput '/declaration-positions.nix"$' config.submoduleLine22.submodDeclLine27.0.file ./declaration-positions.nix

cat <<EOF
====== module tests ======
$pass Pass
Expand Down
36 changes: 36 additions & 0 deletions lib/tests/modules/declaration-positions.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{ lib, options, ... }:
let discardPositions = lib.mapAttrs (k: v: v);
in
{
imports = [
{
options.imported.line7 = 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 {
line16 = lib.mkOption {
type = lib.types.int;
};
};
}
];

options.submoduleLine22 = lib.mkOption {
default = { };
type = lib.types.submoduleWith {
modules = [
({ options, ... }: {
options.submodDeclLine27 = lib.mkOption { };
})
];
};
};

config = {
submoduleLine22.submodDeclLine27 = (options.submoduleLine22.type.getSubOptions []).submodDeclLine27.declarationPositions;
};
}

0 comments on commit c745c0b

Please sign in to comment.