lib/modules: Report a good error when option tree has bare type

[roberth]

Description of changes

Catch a common mistake.

Example message

       error: Expected an option declaration for option `services.foo.extraConfig` but got an attribute set with type option-type

       When declaring an option, you must wrap the type in a `mkOption` call. It should look somewhat like:

           # services.foo.extraConfig
           extraConfig = lib.mkOption {
             description = ...;
             type = <the type you wrote for here>;
             ...
           };

This removes the possibility of creating an option named _type.

Excuse my clever use of && and ||. It's performance critical code, so I prefer to avoid unnecessary function calls, and to a degree unnecessary AST nodes. Speaking of, this code should be an ever so slight improvement, for avoiding the isType function call with its associated Env allocation (and other complexity).

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[Shawn8901]

I am not sure to give that feedback here or not, but i give it a try.

First of all thanks for that check, it did find a very nasty error in one of my custom modules (didnt use the module atm, so but still it was an error).

The module was the following code basically

{
  lib,
  ...
}: let
  cfg = config.stuff;

in {
  options = {
     stuff = { enable = lib.mkEnableOption "does somehting useful"; }
     config = mkIf cfg.enable { 
....
   };
};

So one may see that the config is defined inside the options, which is the error. And super nice that the module system now complains when that occurs. 👍

So the error message occuring is correct, but sadly does not really help the user to find the problem, as with the module the error is

 error: Expected an option declaration at option path `config`, config but got an attribute set with type if

To actually help the user it would be great if we could either add to the output the configuration path, so in that case config.stuff.config or stuff.config (depending if we would add the config at the beginning), or could give a hint from which actual module that error is produced (the file could help here?).

I found the problematic module at the end by commenting out code until eval unbroke again.

The stacktrace that was produced was the following:

error:
       … while checking flake output 'nixosConfigurations'

         at «none»:0: (source not available)

       … while checking the NixOS configuration 'nixosConfigurations.pointalpha'

         at «none»:0: (source not available)

       … while calling the 'seq' builtin

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:320:18:

          319|         options = checked options;
          320|         config = checked (removeAttrs config [ "_module" ]);
             |                  ^
          321|         _module = checked (config._module);

       … while evaluating a branch condition

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:261:9:

          260|       checkUnmatched =
          261|         if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then
             |         ^
          262|           let

       … in the right operand of the AND (&&) operator

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:261:72:

          260|       checkUnmatched =
          261|         if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then
             |                                                                        ^
          262|           let

       … while evaluating the attribute 'unmatchedDefns'

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:686:7:

          685|       # Transforms unmatchedDefnsByName into a list of definitions
          686|       unmatchedDefns =
             |       ^
          687|         if configs == []

       … while calling the 'concatLists' builtin

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:693:11:

          692|         else
          693|           concatLists (mapAttrsToList (name: defs:
             |           ^
          694|             map (def: def // {

       … while calling anonymous lambda

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/attrsets.nix:543:10:

          542|     attrs:
          543|     map (name: f name attrs.${name}) (attrNames attrs);
             |          ^
          544|

       … from call site

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/attrsets.nix:543:16:

          542|     attrs:
          543|     map (name: f name attrs.${name}) (attrNames attrs);
             |                ^
          544|

       … while calling anonymous lambda

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:693:46:

          692|         else
          693|           concatLists (mapAttrsToList (name: defs:
             |                                              ^
          694|             map (def: def // {

       … while calling anonymous lambda

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:679:22:

          678|         # Propagate all unmatched definitions from nested option sets
          679|         mapAttrs (n: v: v.unmatchedDefns) resultsByName
             |                      ^
          680|         # Plus the definitions for the current prefix that don't have a matching option

       … while calling anonymous lambda

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:627:39:

          626|
          627|       resultsByName = mapAttrs (name: decls:
             |                                       ^
          628|         # We're descending into attribute ‘name’.

       … while calling anonymous lambda

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:634:14:

          633|           optionDecls = filter
          634|             (m: m.options?_type
             |              ^
          635|                 && (m.options._type == "option"

       … from call site

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:636:24:

          635|                 && (m.options._type == "option"
          636|                     || throwDeclarationTypeError loc m.options._type
             |                        ^
          637|                 )

       … while calling 'throwDeclarationTypeError'

         at /nix/store/i4jp11qs189sjpxqpi4b5kmcrx12w2bm-source/lib/modules.nix:701:36:

          700|
          701|   throwDeclarationTypeError = loc: actualTag:
             |                                    ^
          702|     let

       error: Expected an option declaration at option path `config`, config but got an attribute set with type if

The original problematic module code is https://github.com/Shawn8901/nix-configuration/blob/2d13dcbb4a5e4b88038d36e9bfd47dda293aa239/modules/nixos/_private/backup-rclone.nix#L21 and the machines pointalphas eval does cause the trace, just in case it helps to understand how to produce the error, if the pseudo module above is unclear