From 0d472a62012364d064f0b75f1da491242c6ae9c6 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sat, 8 Jul 2023 22:18:36 +0200
Subject: [PATCH] lib/modules: Report a good error when option tree has bare
 type

Note that this removes the possibility of declaring an option
named `_type`.
---
 lib/modules.nix                               | 34 ++++++++++++++++++-
 lib/tests/modules.sh                          |  5 +++
 .../options-type-error-configuration.nix      |  6 ++++
 .../options-type-error-typical-nested.nix     |  5 +++
 .../modules/options-type-error-typical.nix    |  5 +++
 5 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 lib/tests/modules/options-type-error-configuration.nix
 create mode 100644 lib/tests/modules/options-type-error-typical-nested.nix
 create mode 100644 lib/tests/modules/options-type-error-typical.nix

diff --git a/lib/modules.nix b/lib/modules.nix
index 4966619f6630..9371ba4b27a4 100644
--- a/lib/modules.nix
+++ b/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
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index b933a24a57a1..5f2e3f2a3114 100755
--- a/lib/tests/modules.sh
+++ b/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
 
diff --git a/lib/tests/modules/options-type-error-configuration.nix b/lib/tests/modules/options-type-error-configuration.nix
new file mode 100644
index 000000000000..bcd6db89487a
--- /dev/null
+++ b/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 = []; };
+  };
+}
diff --git a/lib/tests/modules/options-type-error-typical-nested.nix b/lib/tests/modules/options-type-error-typical-nested.nix
new file mode 100644
index 000000000000..2c07e19fb8ae
--- /dev/null
+++ b/lib/tests/modules/options-type-error-typical-nested.nix
@@ -0,0 +1,5 @@
+{ lib, ... }: {
+  options = {
+    result.here = lib.types.str;
+  };
+}
diff --git a/lib/tests/modules/options-type-error-typical.nix b/lib/tests/modules/options-type-error-typical.nix
new file mode 100644
index 000000000000..416f436e0ad7
--- /dev/null
+++ b/lib/tests/modules/options-type-error-typical.nix
@@ -0,0 +1,5 @@
+{ lib, ... }: {
+  options = {
+    result = lib.types.str;
+  };
+}