From d030e2109fd491e32cb48df54d100aa608551298 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 24 Jan 2022 15:58:17 +0100
Subject: [PATCH 01/13] lib.modules: Let module declare options directly in
 bare submodule

... where a bare submodule is an option that has a type like
`submoduleWith x`, as opposed to `attrsOf (submoduleWith x)`.

This makes migration unnecessary when introducing a freeform type
in an existing option tree.

Closes #146882
---
 lib/modules.nix                               | 22 ++++++++++++++++++-
 lib/tests/modules.sh                          |  5 +++++
 .../declare-bare-submodule-deep-option.nix    | 10 +++++++++
 .../declare-bare-submodule-nested-option.nix  | 18 +++++++++++++++
 lib/tests/modules/declare-bare-submodule.nix  | 12 ++++++++++
 .../modules/define-bare-submodule-values.nix  |  4 ++++
 lib/types.nix                                 |  5 +++++
 7 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 lib/tests/modules/declare-bare-submodule-deep-option.nix
 create mode 100644 lib/tests/modules/declare-bare-submodule-nested-option.nix
 create mode 100644 lib/tests/modules/declare-bare-submodule.nix
 create mode 100644 lib/tests/modules/define-bare-submodule-values.nix

diff --git a/lib/modules.nix b/lib/modules.nix
index 79d54e4a5387..bde89123cd98 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -474,6 +474,18 @@ rec {
           [{ inherit (module) file; inherit value; }]
         ) configs;
 
+      # Convert an option tree decl to a submodule option decl
+      optionTreeToOption = decl:
+        if isOption decl.options
+        then decl
+        else decl // {
+            options = mkOption {
+              type = types.submoduleWith {
+                modules = [ { options = decl.options; } ];
+              };
+            };
+          };
+
       resultsByName = mapAttrs (name: decls:
         # We're descending into attribute ‘name’.
         let
@@ -493,7 +505,15 @@ rec {
               firstOption = findFirst (m: isOption m.options) "" decls;
               firstNonOption = findFirst (m: !isOption m.options) "" decls;
             in
-              throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'."
+              if firstOption.options.type?isSubmodule && firstOption.options.type.isSubmodule
+              then
+                let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
+                in {
+                  matchedOptions = evalOptionValue loc opt defns';
+                  unmatchedDefns = [];
+                }
+              else
+                throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'."
           else
             mergeModules' loc decls defns) declsByName;
 
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index e4bb7ad21900..3591b8f1e6fc 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -62,6 +62,11 @@ checkConfigError() {
 checkConfigOutput '^false$' config.enable ./declare-enable.nix
 checkConfigError 'The option .* does not exist. Definition values:\n\s*- In .*: true' config.enable ./define-enable.nix
 
+checkConfigOutput '^1$' config.bare-submodule.nested ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix
+checkConfigOutput '^2$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-deep-option.nix
+checkConfigOutput '^42$' config.bare-submodule.nested ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix ./declare-bare-submodule-deep-option.nix ./define-bare-submodule-values.nix
+checkConfigOutput '^420$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix ./declare-bare-submodule-deep-option.nix ./define-bare-submodule-values.nix
+
 # Check integer types.
 # unsigned
 checkConfigOutput '^42$' config.value ./declare-int-unsigned-value.nix ./define-value-int-positive.nix
diff --git a/lib/tests/modules/declare-bare-submodule-deep-option.nix b/lib/tests/modules/declare-bare-submodule-deep-option.nix
new file mode 100644
index 000000000000..06ad1f6e0a51
--- /dev/null
+++ b/lib/tests/modules/declare-bare-submodule-deep-option.nix
@@ -0,0 +1,10 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+in
+{
+  options.bare-submodule.deep = mkOption {
+    type = types.int;
+    default = 2;
+  };
+}
diff --git a/lib/tests/modules/declare-bare-submodule-nested-option.nix b/lib/tests/modules/declare-bare-submodule-nested-option.nix
new file mode 100644
index 000000000000..009dd4d6cb37
--- /dev/null
+++ b/lib/tests/modules/declare-bare-submodule-nested-option.nix
@@ -0,0 +1,18 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+in
+{
+  options.bare-submodule = mkOption {
+    type = types.submoduleWith {
+      modules = [
+        {
+          options.nested = mkOption {
+            type = types.int;
+            default = 1;
+          };
+        }
+      ];
+    };
+  };
+}
diff --git a/lib/tests/modules/declare-bare-submodule.nix b/lib/tests/modules/declare-bare-submodule.nix
new file mode 100644
index 000000000000..298c71e3ca0b
--- /dev/null
+++ b/lib/tests/modules/declare-bare-submodule.nix
@@ -0,0 +1,12 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+in
+{
+  options.bare-submodule = mkOption {
+    type = types.submoduleWith {
+      modules = [ ];
+    };
+    default = {};
+  };
+}
diff --git a/lib/tests/modules/define-bare-submodule-values.nix b/lib/tests/modules/define-bare-submodule-values.nix
new file mode 100644
index 000000000000..00ede929ee66
--- /dev/null
+++ b/lib/tests/modules/define-bare-submodule-values.nix
@@ -0,0 +1,4 @@
+{
+  bare-submodule.nested = 42;
+  bare-submodule.deep = 420;
+}
diff --git a/lib/types.nix b/lib/types.nix
index 3fcac9c31b31..51046c2c31b6 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -642,6 +642,11 @@ rec {
               else throw "A submoduleWith option is declared multiple times with conflicting shorthandOnlyDefinesConfig values";
           };
         };
+      } // {
+        # Submodule-typed options get special treatment in order to facilitate
+        # certain migrations, such as the addition of freeformTypes onto
+        # existing option trees.
+        isSubmodule = true;
       };
 
     # A value from a set of allowed ones.

From 58a8a48e9d14cf397181d1223eabeb001f499049 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 24 Feb 2022 14:11:53 +0100
Subject: [PATCH 02/13] lib.types.submodule: Remove redundant isSubmodule attr

---
 lib/modules.nix | 2 +-
 lib/types.nix   | 5 -----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/modules.nix b/lib/modules.nix
index bde89123cd98..540eba1dd3dc 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -505,7 +505,7 @@ rec {
               firstOption = findFirst (m: isOption m.options) "" decls;
               firstNonOption = findFirst (m: !isOption m.options) "" decls;
             in
-              if firstOption.options.type?isSubmodule && firstOption.options.type.isSubmodule
+              if firstOption.options.type.name == "submodule"
               then
                 let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
                 in {
diff --git a/lib/types.nix b/lib/types.nix
index 51046c2c31b6..3fcac9c31b31 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -642,11 +642,6 @@ rec {
               else throw "A submoduleWith option is declared multiple times with conflicting shorthandOnlyDefinesConfig values";
           };
         };
-      } // {
-        # Submodule-typed options get special treatment in order to facilitate
-        # certain migrations, such as the addition of freeformTypes onto
-        # existing option trees.
-        isSubmodule = true;
       };
 
     # A value from a set of allowed ones.

From 0c09eb343dfa186c0fbf2bb6cbc36e7a8f369ea5 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 24 Feb 2022 14:23:02 +0100
Subject: [PATCH 03/13] lib.modules: Refactor option scanning slightly

This scans the options with fewer function calls, improving performance.

It also removes a let Env from the happy flow of the new logic.
---
 lib/modules.nix | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/lib/modules.nix b/lib/modules.nix
index 540eba1dd3dc..e6812625f989 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -9,7 +9,6 @@ let
     catAttrs
     concatLists
     concatMap
-    count
     elem
     filter
     findFirst
@@ -492,20 +491,16 @@ rec {
           loc = prefix ++ [name];
           defns = defnsByName.${name} or [];
           defns' = defnsByName'.${name} or [];
-          nrOptions = count (m: isOption m.options) decls;
+          optionDecls = filter (m: isOption m.options) decls;
         in
-          if nrOptions == length decls then
+          if length optionDecls == length decls then
             let opt = fixupOptionType loc (mergeOptionDecls loc decls);
             in {
               matchedOptions = evalOptionValue loc opt defns';
               unmatchedDefns = [];
             }
-          else if nrOptions != 0 then
-            let
-              firstOption = findFirst (m: isOption m.options) "" decls;
-              firstNonOption = findFirst (m: !isOption m.options) "" decls;
-            in
-              if firstOption.options.type.name == "submodule"
+          else if optionDecls != [] then
+              if (lib.head optionDecls).options.type.name == "submodule"
               then
                 let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
                 in {
@@ -513,7 +508,10 @@ rec {
                   unmatchedDefns = [];
                 }
               else
-                throw "The option `${showOption loc}' in `${firstOption._file}' is a prefix of options in `${firstNonOption._file}'."
+                let
+                  firstNonOption = findFirst (m: !isOption m.options) "" decls;
+                in
+                throw "The option `${showOption loc}' in `${(lib.head optionDecls)._file}' is a prefix of options in `${firstNonOption._file}'."
           else
             mergeModules' loc decls defns) declsByName;
 

From 81f342d1f3a6bab4a57e195ce99a153b82b1ef86 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 24 Feb 2022 14:50:40 +0100
Subject: [PATCH 04/13] lib.modules: Explain why options can only be merged
 into submodules

---
 lib/modules.nix | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/modules.nix b/lib/modules.nix
index e6812625f989..62e9615d25b3 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -501,6 +501,15 @@ rec {
             }
           else if optionDecls != [] then
               if (lib.head optionDecls).options.type.name == "submodule"
+              # Raw options can only be merged into submodules. Merging into
+              # attrsets might be nice, but ambiguous. Suppose we have
+              # attrset as a `attrsOf submodule`. User declares option
+              # attrset.foo.bar, this could mean:
+              #  a. option `bar` is only available in `attrset.foo`
+              #  b. option `foo.bar` is available in all `attrset.*`
+              #  c. reject and require "<name>" as a reminder that it behaves like (b).
+              #  d. magically combine (a) and (c).
+              # All options are merely syntax sugar though.
               then
                 let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
                 in {

From 11537c9c0239dc4ae52477faa78a4a0a7bdf206c Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 28 Feb 2022 22:39:56 +0100
Subject: [PATCH 05/13] lib.modules: Improve option-is-prefix error message

---
 lib/modules.nix                   | 24 +++++++++++++++++++++---
 lib/tests/modules.sh              |  6 ++++++
 lib/tests/modules/declare-set.nix | 12 ++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 lib/tests/modules/declare-set.nix

diff --git a/lib/modules.nix b/lib/modules.nix
index 62e9615d25b3..7d9c55f9a158 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -9,6 +9,7 @@ let
     catAttrs
     concatLists
     concatMap
+    concatStringsSep
     elem
     filter
     findFirst
@@ -46,6 +47,20 @@ let
     showOption
     unknownModule
     ;
+
+  showDeclPrefix = loc: decl: prefix:
+    " - option(s) with prefix `${showOption (loc ++ [prefix])}' in module `${decl._file}'";
+  showRawDecls = loc: decls:
+    concatStringsSep "\n"
+      (sort (a: b: a < b)
+        (concatMap
+          (decl: map
+            (showDeclPrefix loc decl)
+            (attrNames decl.options)
+          )
+          decls
+      ));
+
 in
 
 rec {
@@ -500,7 +515,7 @@ rec {
               unmatchedDefns = [];
             }
           else if optionDecls != [] then
-              if (lib.head optionDecls).options.type.name == "submodule"
+              if all (x: x.options.type.name == "submodule") optionDecls
               # Raw options can only be merged into submodules. Merging into
               # attrsets might be nice, but ambiguous. Suppose we have
               # attrset as a `attrsOf submodule`. User declares option
@@ -509,7 +524,7 @@ rec {
               #  b. option `foo.bar` is available in all `attrset.*`
               #  c. reject and require "<name>" as a reminder that it behaves like (b).
               #  d. magically combine (a) and (c).
-              # All options are merely syntax sugar though.
+              # All of the above are merely syntax sugar though.
               then
                 let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
                 in {
@@ -519,8 +534,11 @@ rec {
               else
                 let
                   firstNonOption = findFirst (m: !isOption m.options) "" decls;
+                  nonOptions = filter (m: !isOption m.options) decls;
                 in
-                throw "The option `${showOption loc}' in `${(lib.head optionDecls)._file}' is a prefix of options in `${firstNonOption._file}'."
+                throw "The option `${showOption loc}' in module `${(lib.head optionDecls)._file}' would be a parent of the following options, but its type `${(lib.head optionDecls).options.type.description or "<no description>"}' does not support nested options.\n${
+                  showRawDecls loc nonOptions
+                }"
           else
             mergeModules' loc decls defns) declsByName;
 
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index 3591b8f1e6fc..e903714a7209 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -309,6 +309,12 @@ checkConfigOutput "10" config.processedToplevel ./raw.nix
 checkConfigError "The option .multiple. is defined multiple times" config.multiple ./raw.nix
 checkConfigOutput "bar" config.priorities ./raw.nix
 
+## Option collision
+checkConfigError \
+  'The option .set. in module .*/declare-set.nix. would be a parent of the following options, but its type .attribute set of signed integers. does not support nested options.\n\s*- option[(]s[)] with prefix .set.enable. in module .*/declare-enable-nested.nix.' \
+  config.set \
+  ./declare-set.nix ./declare-enable-nested.nix
+
 # Test that types.optionType merges types correctly
 checkConfigOutput '^10$' config.theOption.int ./optionTypeMerging.nix
 checkConfigOutput '^"hello"$' config.theOption.str ./optionTypeMerging.nix
diff --git a/lib/tests/modules/declare-set.nix b/lib/tests/modules/declare-set.nix
new file mode 100644
index 000000000000..853418531a81
--- /dev/null
+++ b/lib/tests/modules/declare-set.nix
@@ -0,0 +1,12 @@
+{ lib, ... }:
+
+{
+  options.set = lib.mkOption {
+    default = { };
+    example = { a = 1; };
+    type = lib.types.attrsOf lib.types.int;
+    description = ''
+      Some descriptive text
+    '';
+  };
+}

From 8baea8b82cc80c6a2843045d5b554f7f65acbc4f Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 28 Feb 2022 22:57:03 +0100
Subject: [PATCH 06/13] lib.modules: Make option injection work when
 shorthandOnlyDefinesConfig

---
 lib/modules.nix                                           | 1 +
 lib/tests/modules.sh                                      | 1 +
 .../modules/declare-bare-submodule-nested-option.nix      | 3 ++-
 lib/tests/modules/declare-bare-submodule.nix              | 8 +++++++-
 .../modules/define-shorthandOnlyDefinesConfig-true.nix    | 1 +
 lib/types.nix                                             | 6 +++++-
 6 files changed, 17 insertions(+), 3 deletions(-)
 create mode 100644 lib/tests/modules/define-shorthandOnlyDefinesConfig-true.nix

diff --git a/lib/modules.nix b/lib/modules.nix
index 7d9c55f9a158..cc045391fcb1 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -496,6 +496,7 @@ rec {
             options = mkOption {
               type = types.submoduleWith {
                 modules = [ { options = decl.options; } ];
+                shorthandOnlyDefinesConfig = null;
               };
             };
           };
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index e903714a7209..3950d83f584b 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -66,6 +66,7 @@ checkConfigOutput '^1$' config.bare-submodule.nested ./declare-bare-submodule.ni
 checkConfigOutput '^2$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-deep-option.nix
 checkConfigOutput '^42$' config.bare-submodule.nested ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix ./declare-bare-submodule-deep-option.nix ./define-bare-submodule-values.nix
 checkConfigOutput '^420$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix ./declare-bare-submodule-deep-option.nix ./define-bare-submodule-values.nix
+checkConfigOutput '^2$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-deep-option.nix ./define-shorthandOnlyDefinesConfig-true.nix
 
 # Check integer types.
 # unsigned
diff --git a/lib/tests/modules/declare-bare-submodule-nested-option.nix b/lib/tests/modules/declare-bare-submodule-nested-option.nix
index 009dd4d6cb37..da125c84b25d 100644
--- a/lib/tests/modules/declare-bare-submodule-nested-option.nix
+++ b/lib/tests/modules/declare-bare-submodule-nested-option.nix
@@ -1,10 +1,11 @@
-{ lib, ... }:
+{ config, lib, ... }:
 let
   inherit (lib) mkOption types;
 in
 {
   options.bare-submodule = mkOption {
     type = types.submoduleWith {
+      shorthandOnlyDefinesConfig = config.shorthandOnlyDefinesConfig;
       modules = [
         {
           options.nested = mkOption {
diff --git a/lib/tests/modules/declare-bare-submodule.nix b/lib/tests/modules/declare-bare-submodule.nix
index 298c71e3ca0b..5402f4ff5a50 100644
--- a/lib/tests/modules/declare-bare-submodule.nix
+++ b/lib/tests/modules/declare-bare-submodule.nix
@@ -1,4 +1,4 @@
-{ lib, ... }:
+{ config, lib, ... }:
 let
   inherit (lib) mkOption types;
 in
@@ -6,7 +6,13 @@ in
   options.bare-submodule = mkOption {
     type = types.submoduleWith {
       modules = [ ];
+      shorthandOnlyDefinesConfig = config.shorthandOnlyDefinesConfig;
     };
     default = {};
   };
+
+  # config-dependent options: won't recommend, but useful for making this test parameterized
+  options.shorthandOnlyDefinesConfig = mkOption {
+    default = false;
+  };
 }
diff --git a/lib/tests/modules/define-shorthandOnlyDefinesConfig-true.nix b/lib/tests/modules/define-shorthandOnlyDefinesConfig-true.nix
new file mode 100644
index 000000000000..bd3a73dce340
--- /dev/null
+++ b/lib/tests/modules/define-shorthandOnlyDefinesConfig-true.nix
@@ -0,0 +1 @@
+{ shorthandOnlyDefinesConfig = true; }
diff --git a/lib/types.nix b/lib/types.nix
index 3fcac9c31b31..bd4062d555aa 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -637,7 +637,11 @@ rec {
               then lhs.specialArgs // rhs.specialArgs
               else throw "A submoduleWith option is declared multiple times with the same specialArgs \"${toString (attrNames intersecting)}\"";
             shorthandOnlyDefinesConfig =
-              if lhs.shorthandOnlyDefinesConfig == rhs.shorthandOnlyDefinesConfig
+              if lhs.shorthandOnlyDefinesConfig == null
+              then rhs.shorthandOnlyDefinesConfig
+              else if rhs.shorthandOnlyDefinesConfig == null
+              then lhs.shorthandOnlyDefinesConfig
+              else if lhs.shorthandOnlyDefinesConfig == rhs.shorthandOnlyDefinesConfig
               then lhs.shorthandOnlyDefinesConfig
               else throw "A submoduleWith option is declared multiple times with conflicting shorthandOnlyDefinesConfig values";
           };

From 6b077c47ff14cb9a4a8f5cb8986fa83ff626c732 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 28 Feb 2022 23:29:15 +0100
Subject: [PATCH 07/13] lib.modules: Remove redundant fixupOptionType in option
 injection

---
 lib/modules.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/modules.nix b/lib/modules.nix
index cc045391fcb1..470e3818820f 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -527,7 +527,7 @@ rec {
               #  d. magically combine (a) and (c).
               # All of the above are merely syntax sugar though.
               then
-                let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
+                let opt = mergeOptionDecls loc (map optionTreeToOption decls);
                 in {
                   matchedOptions = evalOptionValue loc opt defns';
                   unmatchedDefns = [];

From 28aeae21269a69ae9721c9c8f9194877799ead69 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Tue, 1 Mar 2022 10:37:10 +0100
Subject: [PATCH 08/13] lib.modules: Default shorthandOnlyDefinesConfig to true
 when null

---
 lib/types.nix | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/types.nix b/lib/types.nix
index bd4062d555aa..73f271103fc4 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -562,9 +562,13 @@ rec {
       let
         inherit (lib.modules) evalModules;
 
+        shorthandToModule = if shorthandOnlyDefinesConfig == false
+          then value: value
+          else value: { config = value; };
+
         coerce = unify: value: if isFunction value
           then setFunctionArgs (args: unify (value args)) (functionArgs value)
-          else unify (if shorthandOnlyDefinesConfig then { config = value; } else value);
+          else unify (shorthandToModule value);
 
         allModules = defs: imap1 (n: { value, file }:
           if isAttrs value || isFunction value then

From 20506699226b0dac5d423c6f6249f3cb15565169 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Tue, 1 Mar 2022 10:45:46 +0100
Subject: [PATCH 09/13] lib.modules: Inline a private function

This should save about four calls per module.
---
 lib/types.nix | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/types.nix b/lib/types.nix
index 73f271103fc4..d2c109cabe81 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -566,14 +566,14 @@ rec {
           then value: value
           else value: { config = value; };
 
-        coerce = unify: value: if isFunction value
-          then setFunctionArgs (args: unify (value args)) (functionArgs value)
-          else unify (shorthandToModule value);
-
         allModules = defs: imap1 (n: { value, file }:
-          if isAttrs value || isFunction value then
-            # Annotate the value with the location of its definition for better error messages
-            coerce (lib.modules.unifyModuleSyntax file "${toString file}-${toString n}") value
+          if isFunction value
+          then setFunctionArgs
+                (args: lib.modules.unifyModuleSyntax file "${toString file}-${toString n}" (value args))
+                (functionArgs value)
+          else if isAttrs value
+          then
+            lib.modules.unifyModuleSyntax file "${toString file}-${toString n}" (shorthandToModule value)
           else value
         ) defs;
 

From db08290453ae0eb7622648435bf7af187929b153 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 7 Mar 2022 10:59:03 +0100
Subject: [PATCH 10/13] Revert "lib.modules: Remove redundant fixupOptionType
 in option injection"

This reverts commit 6b077c47ff14cb9a4a8f5cb8986fa83ff626c732.

Thanks Infinisil for discovering this problem:

> After a lot of trial and error, trying to prove why fixupOptionType should
> be used here or not, I figured it out: It's needed for the sake of file
> locations in error messages.
---
 lib/modules.nix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/modules.nix b/lib/modules.nix
index 470e3818820f..cc045391fcb1 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -527,7 +527,7 @@ rec {
               #  d. magically combine (a) and (c).
               # All of the above are merely syntax sugar though.
               then
-                let opt = mergeOptionDecls loc (map optionTreeToOption decls);
+                let opt = fixupOptionType loc (mergeOptionDecls loc (map optionTreeToOption decls));
                 in {
                   matchedOptions = evalOptionValue loc opt defns';
                   unmatchedDefns = [];

From e162ed8a142d6ff53b7d03018bfd95a3a044bd06 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 7 Mar 2022 11:02:02 +0100
Subject: [PATCH 11/13] lib/modules.nix: Move comment to the actual legacy code

---
 lib/modules.nix | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/modules.nix b/lib/modules.nix
index cc045391fcb1..5d1532af623e 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -799,13 +799,14 @@ rec {
       compare = a: b: (a.priority or 1000) < (b.priority or 1000);
     in sort compare defs';
 
-  /* Hack for backward compatibility: convert options of type
-     optionSet to options of type submodule.  FIXME: remove
-     eventually. */
   fixupOptionType = loc: opt:
     let
       options = opt.options or
         (throw "Option `${showOption loc}' has type optionSet but has no option attribute, in ${showFiles opt.declarations}.");
+
+      # Hack for backward compatibility: convert options of type
+      # optionSet to options of type submodule.  FIXME: remove
+      # eventually.
       f = tp:
         let optionSetIn = type: (tp.name == type) && (tp.functor.wrapped.name == "optionSet");
         in

From c90844aeb97c0d57f3dbb5774f56cddbf5b2a16d Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 7 Mar 2022 11:20:30 +0100
Subject: [PATCH 12/13] lib/tests/modules: Add test case for duplicate option
 error file location

---
 lib/tests/modules.sh                                   |  1 +
 .../declare-bare-submodule-deep-option-duplicate.nix   | 10 ++++++++++
 2 files changed, 11 insertions(+)
 create mode 100644 lib/tests/modules/declare-bare-submodule-deep-option-duplicate.nix

diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index 3950d83f584b..e86b545c4666 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -67,6 +67,7 @@ checkConfigOutput '^2$' config.bare-submodule.deep ./declare-bare-submodule.nix
 checkConfigOutput '^42$' config.bare-submodule.nested ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix ./declare-bare-submodule-deep-option.nix ./define-bare-submodule-values.nix
 checkConfigOutput '^420$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-nested-option.nix ./declare-bare-submodule-deep-option.nix ./define-bare-submodule-values.nix
 checkConfigOutput '^2$' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-deep-option.nix ./define-shorthandOnlyDefinesConfig-true.nix
+checkConfigError 'The option .bare-submodule.deep. in .*/declare-bare-submodule-deep-option.nix. is already declared in .*/declare-bare-submodule-deep-option-duplicate.nix' config.bare-submodule.deep ./declare-bare-submodule.nix ./declare-bare-submodule-deep-option.nix  ./declare-bare-submodule-deep-option-duplicate.nix
 
 # Check integer types.
 # unsigned
diff --git a/lib/tests/modules/declare-bare-submodule-deep-option-duplicate.nix b/lib/tests/modules/declare-bare-submodule-deep-option-duplicate.nix
new file mode 100644
index 000000000000..06ad1f6e0a51
--- /dev/null
+++ b/lib/tests/modules/declare-bare-submodule-deep-option-duplicate.nix
@@ -0,0 +1,10 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+in
+{
+  options.bare-submodule.deep = mkOption {
+    type = types.int;
+    default = 2;
+  };
+}

From c4b38702e59ea156924d3297e3f7ec80f7f816cb Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 7 Mar 2022 11:23:24 +0100
Subject: [PATCH 13/13] lib/modules.nix: Add comment about internal shorthand
 null value

---
 lib/modules.nix | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/modules.nix b/lib/modules.nix
index 5d1532af623e..25cd5921decb 100644
--- a/lib/modules.nix
+++ b/lib/modules.nix
@@ -496,6 +496,9 @@ rec {
             options = mkOption {
               type = types.submoduleWith {
                 modules = [ { options = decl.options; } ];
+                # `null` is not intended for use by modules. It is an internal
+                # value that means "whatever the user has declared elsewhere".
+                # This might become obsolete with https://github.com/NixOS/nixpkgs/issues/162398
                 shorthandOnlyDefinesConfig = null;
               };
             };