From b78ba9bc68b003288d56bab62693ea28e2cdfd76 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 28 Jan 2024 14:09:27 +0100
Subject: [PATCH] lib.types.unique: Check inner type deeply

This doesn't change uniq. Why not?

- In NixOS it seems that uniq is only used with
  simple types that are fully checked by t.check.

- It exists for much longer and is used more widely.

- I believe we should deprecate it, because unique was
  already better.

- unique can be a proving ground.
---
 lib/options.nix                    | 33 +++++++++++++++++++++++++-----
 lib/tests/modules.sh               | 10 +++++++++
 lib/tests/modules/types-unique.nix | 27 ++++++++++++++++++++++++
 lib/types.nix                      |  2 +-
 4 files changed, 66 insertions(+), 6 deletions(-)
 create mode 100644 lib/tests/modules/types-unique.nix

diff --git a/lib/options.nix b/lib/options.nix
index 9c10dfc8b36a..03ae32d22916 100644
--- a/lib/options.nix
+++ b/lib/options.nix
@@ -254,13 +254,36 @@ rec {
     else if all isInt list && all (x: x == head list) list then head list
     else throw "Cannot merge definitions of `${showOption loc}'. Definition values:${showDefs defs}";
 
+  /*
+    Require a single definition.
+
+    WARNING: Does not perform nested checks, as this does not run the merge function!
+    */
   mergeOneOption = mergeUniqueOption { message = ""; };
 
-  mergeUniqueOption = { message }: loc: defs:
-    if length defs == 1
-    then (head defs).value
-    else assert length defs > 1;
-      throw "The option `${showOption loc}' is defined multiple times while it's expected to be unique.\n${message}\nDefinition values:${showDefs defs}\n${prioritySuggestion}";
+  /*
+    Require a single definition.
+
+    NOTE: When the type is not checked completely by check, pass a merge function for further checking (of sub-attributes, etc).
+   */
+  mergeUniqueOption = args@{ message, merge ? null }:
+    let
+      notUnique = loc: defs:
+        assert length defs > 1;
+        throw "The option `${showOption loc}' is defined multiple times while it's expected to be unique.\n${message}\nDefinition values:${showDefs defs}\n${prioritySuggestion}";
+    in
+    if merge == null
+    # The inner conditional could be factored out, but this way we take advantage of partial application.
+    then
+      loc: defs:
+        if length defs == 1
+        then (head defs).value
+        else notUnique loc defs
+    else
+      loc: defs:
+        if length defs == 1
+        then merge loc defs
+        else notUnique loc defs;
 
   /* "Merge" option definitions by checking that they all have the same value. */
   mergeEqualOption = loc: defs:
diff --git a/lib/tests/modules.sh b/lib/tests/modules.sh
index a90ff4ad9a2f..1221ba7143f6 100755
--- a/lib/tests/modules.sh
+++ b/lib/tests/modules.sh
@@ -406,6 +406,16 @@ checkConfigOutput "{}" config.submodule.a ./emptyValues.nix
 checkConfigError 'The option .int.a. is used but not defined' config.int.a ./emptyValues.nix
 checkConfigError 'The option .nonEmptyList.a. is used but not defined' config.nonEmptyList.a ./emptyValues.nix
 
+# types.unique
+#   requires a single definition
+checkConfigError 'The option .examples\.merged. is defined multiple times while it.s expected to be unique' config.examples.merged.a ./types-unique.nix
+#   user message is printed
+checkConfigError 'We require a single definition, because seeing the whole value at once helps us maintain critical invariants of our system.' config.examples.merged.a ./types-unique.nix
+#   let the inner merge function check the values (on demand)
+checkConfigError 'A definition for option .examples\.badLazyType\.a. is not of type .string.' config.examples.badLazyType.a ./types-unique.nix
+#   overriding still works (unlike option uniqueness)
+checkConfigOutput '^"bee"$' config.examples.override.b ./types-unique.nix
+
 ## types.raw
 checkConfigOutput '^true$' config.unprocessedNestingEvaluates.success ./raw.nix
 checkConfigOutput "10" config.processedToplevel ./raw.nix
diff --git a/lib/tests/modules/types-unique.nix b/lib/tests/modules/types-unique.nix
new file mode 100644
index 000000000000..115be0126975
--- /dev/null
+++ b/lib/tests/modules/types-unique.nix
@@ -0,0 +1,27 @@
+{ lib, ... }:
+let
+  inherit (lib) mkOption types;
+in
+{
+  options.examples = mkOption {
+    type = types.lazyAttrsOf
+      (types.unique
+        { message = "We require a single definition, because seeing the whole value at once helps us maintain critical invariants of our system."; }
+        (types.attrsOf types.str));
+  };
+  imports = [
+    { examples.merged = { b = "bee"; }; }
+    { examples.override = lib.mkForce { b = "bee"; }; }
+  ];
+  config.examples = {
+    merged = {
+      a = "aye";
+    };
+    override = {
+      a = "aye";
+    };
+    badLazyType = {
+      a = true;
+    };
+  };
+}
diff --git a/lib/types.nix b/lib/types.nix
index cea63c598321..284c8df24f67 100644
--- a/lib/types.nix
+++ b/lib/types.nix
@@ -629,7 +629,7 @@ rec {
     unique = { message }: type: mkOptionType rec {
       name = "unique";
       inherit (type) description descriptionClass check;
-      merge = mergeUniqueOption { inherit message; };
+      merge = mergeUniqueOption { inherit message; inherit (type) merge; };
       emptyValue = type.emptyValue;
       getSubOptions = type.getSubOptions;
       getSubModules = type.getSubModules;