Merge pull request #242339 from hercules-ci/modules-catch-bare-type

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

lib/modules.nix

@@ -630,7 +630,13 @@ let
           loc = prefix ++ [name];
           defns = pushedDownDefinitionsByName.${name} or [];
           defns' = rawDefinitionsByName.${name} or [];
-          optionDecls = filter (m: isOption m.options) decls;
+          optionDecls = filter
+            (m: m.options?_type
+                && (m.options._type == "option"
+                    || throwDeclarationTypeError loc m.options._type
+                )
+            )
+            decls;
         in
           if length optionDecls == length decls then
             let opt = fixupOptionType loc (mergeOptionDecls loc decls);
@@ -692,6 +698,32 @@ let
           ) unmatchedDefnsByName);
     };
 
+  throwDeclarationTypeError = loc: actualTag:
+    let
+      name = lib.strings.escapeNixIdentifier (lib.lists.last loc);
+      path = showOption loc;
+      depth = length loc;
+
+      paragraphs = [
+        "Expected an option declaration at option path `${path}` but got an attribute set with type ${actualTag}"
+      ] ++ optional (actualTag == "option-type") ''
+          When declaring an option, you must wrap the type in a `mkOption` call. It should look somewhat like:
+              ${comment}
+              ${name} = lib.mkOption {
+                description = ...;
+                type = <the type you wrote for ${name}>;
+                ...
+              };
+        '';
+
+      # Ideally we'd know the exact syntax they used, but short of that,
+      # we can only reliably repeat the last. However, we repeat the
+      # full path in a non-misleading way here, in case they overlook
+      # the start of the message. Examples attract attention.
+      comment = optionalString (depth > 1) "\n    # ${showOption loc}";
+    in
+    throw (concatStringsSep "\n\n" paragraphs);
+
   /* Merge multiple option declarations into a single declaration.  In
      general, there should be only one declaration of each option.
      The exception is the ‘options’ attribute, which specifies

lib/tests/modules.sh

@@ -393,6 +393,11 @@ checkConfigError \
   config.set \
   ./declare-set.nix ./declare-enable-nested.nix
 
+# Options: accidental use of an option-type instead of option (or other tagged type; unlikely)
+checkConfigError 'Expected an option declaration at option path .result. but got an attribute set with type option-type' config.result ./options-type-error-typical.nix
+checkConfigError 'Expected an option declaration at option path .result.here. but got an attribute set with type option-type' config.result.here ./options-type-error-typical-nested.nix
+checkConfigError 'Expected an option declaration at option path .result. but got an attribute set with type configuration' config.result ./options-type-error-configuration.nix
+
 # Check that that merging of option collisions doesn't depend on type being set
 checkConfigError 'The option .group..*would be a parent of the following options, but its type .<no description>. does not support nested options.\n\s*- option.s. with prefix .group.enable..*' config.group.enable ./merge-typeless-option.nix
 

lib/tests/modules/options-type-error-configuration.nix

@@ -0,0 +1,6 @@
+{ lib, ... }: {
+  options = {
+    # unlikely mistake, but we can catch any attrset with _type
+    result = lib.evalModules { modules = []; };
+  };
+}

lib/tests/modules/options-type-error-typical-nested.nix

@@ -0,0 +1,5 @@
+{ lib, ... }: {
+  options = {
+    result.here = lib.types.str;
+  };
+}

lib/tests/modules/options-type-error-typical.nix

@@ -0,0 +1,5 @@
+{ lib, ... }: {
+  options = {
+    result = lib.types.str;
+  };
+}