nixos/doc: render option values using `lib.generators.toPretty`

Render un`_type`d defaults and examples as `literalExpression`s using
`lib.generators.toPretty` so that consumers don't have to reinvent Nix
pretty-printing. `renderOptionValue` is kept internal for now intentionally.

Make `toPretty` print floats as valid Nix values (without a tilde).

Get rid of the now-obsolete `substSpecial` function.

Move towards disallowing evaluation of packages in the manual by
raising a warning on `pkgs.foo.{outPath,drvPath}`; later, this should
throw an error. Instead, module authors should use `literalExpression`
and `mkPackageOption`.

5 files changed, 37 insertions, 30 deletions

diff --git a/lib/generators.nix b/lib/generators.nix
index 06823d215e360..c0fe69389e00d 100644
--- a/lib/generators.nix
+++ b/lib/generators.nix
@@ -278,8 +278,11 @@ rec {
         mapAny 0;
 
   /* Pretty print a value, akin to `builtins.trace`.
-    * Should probably be a builtin as well.
-    */
+   * Should probably be a builtin as well.
+   * The pretty-printed string should be suitable for rendering default values
+   * in the NixOS manual. In particular, it should be as close to a valid Nix expression
+   * as possible.
+   */
   toPretty = {
     /* If this option is true, attrsets like { __pretty = fn; val = …; }
        will use fn to convert val to a pretty printed representation.
@@ -294,7 +297,10 @@ rec {
             introSpace = if multiline then "\n${indent}  " else " ";
             outroSpace = if multiline then "\n${indent}" else " ";
     in if   isInt      v then toString v
-    else if isFloat    v then "~${toString v}"
+    # toString loses precision on floats, so we use toJSON instead. This isn't perfect
+    # as the resulting string may not parse back as a float (e.g. 42, 1e-06), but for
+    # pretty-printing purposes this is acceptable.
+    else if isFloat    v then builtins.toJSON v
     else if isString   v then
       let
         lines = filter (v: ! isList v) (builtins.split "\n" v);
@@ -328,7 +334,7 @@ rec {
                          else "<function, args: {${showFnas}}>"
     else if isAttrs    v then
       # apply pretty values if allowed
-      if attrNames v == [ "__pretty" "val" ] && allowPrettyValues
+      if allowPrettyValues && v ? __pretty && v ? val
          then v.__pretty v.val
       else if v == {} then "{ }"
       else if v ? type && v.type == "derivation" then
diff --git a/lib/options.nix b/lib/options.nix
index c80256c0e653b..b13687576e810 100644
--- a/lib/options.nix
+++ b/lib/options.nix
@@ -218,7 +218,7 @@ rec {
   # the set generated with filterOptionSets.
   optionAttrSetToDocList = optionAttrSetToDocList' [];
 
-  optionAttrSetToDocList' = prefix: options:
+  optionAttrSetToDocList' = _: options:
     concatMap (opt:
       let
         docOption = rec {
@@ -234,9 +234,8 @@ rec {
           readOnly = opt.readOnly or false;
           type = opt.type.description or "unspecified";
         }
-        // optionalAttrs (opt ? example) { example = scrubOptionValue opt.example; }
-        // optionalAttrs (opt ? default) { default = scrubOptionValue opt.default; }
-        // optionalAttrs (opt ? defaultText) { default = opt.defaultText; }
+        // optionalAttrs (opt ? example) { example = renderOptionValue opt.example; }
+        // optionalAttrs (opt ? default) { default = renderOptionValue (opt.defaultText or opt.default); }
         // optionalAttrs (opt ? relatedPackages && opt.relatedPackages != null) { inherit (opt) relatedPackages; };
 
         subOptions =
@@ -256,6 +255,9 @@ rec {
      efficient: the XML representation of derivations is very large
      (on the order of megabytes) and is not actually used by the
      manual generator.
+
+     This function was made obsolete by renderOptionValue and is kept for
+     compatibility with out-of-tree code.
   */
   scrubOptionValue = x:
     if isDerivation x then
@@ -265,6 +267,17 @@ rec {
     else x;
 
 
+  /* Ensures that the given option value (default or example) is a `_type`d string
+     by rendering Nix values to `literalExpression`s.
+  */
+  renderOptionValue = v:
+    if v ? _type && v ? text then v
+    else literalExpression (lib.generators.toPretty {
+      multiline = true;
+      allowPrettyValues = true;
+    } v);
+
+
   /* For use in the `defaultText` and `example` option attributes. Causes the
      given string to be rendered verbatim in the documentation as Nix code. This
      is necessary for complex values, e.g. functions, or values that depend on
diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix
index cfad5a3cd92a0..648c05ab35720 100644
--- a/lib/tests/misc.nix
+++ b/lib/tests/misc.nix
@@ -741,7 +741,7 @@ runTests {
     };
     expected = rec {
       int = "42";
-      float = "~0.133700";
+      float = "0.1337";
       bool = "true";
       emptystring = ''""'';
       string = ''"fn\''${o}\"r\\d"'';
diff --git a/nixos/lib/make-options-doc/default.nix b/nixos/lib/make-options-doc/default.nix
index 694512115d40a..dea3eec5bd6d7 100644
--- a/nixos/lib/make-options-doc/default.nix
+++ b/nixos/lib/make-options-doc/default.nix
@@ -26,7 +26,7 @@
   # If you include more than one option list into a document, you need to
   # provide different ids.
 , variablelistId ? "configuration-variable-list"
-  # Strig to prefix to the option XML/HTML id attributes.
+  # String to prefix to the option XML/HTML id attributes.
 , optionIdPrefix ? "opt-"
 , revision ? "" # Specify revision for the options
 # a set of options the docs we are generating will be merged into, as if by recursiveUpdate.
@@ -45,28 +45,11 @@
 }:
 
 let
-  # Make a value safe for JSON. Functions are replaced by the string "<function>",
-  # derivations are replaced with an attrset
-  # { _type = "derivation"; name = <name of that derivation>; }.
-  # We need to handle derivations specially because consumers want to know about them,
-  # but we can't easily use the type,name subset of keys (since type is often used as
-  # a module option and might cause confusion). Use _type,name instead to the same
-  # effect, since _type is already used by the module system.
-  substSpecial = x:
-    if lib.isDerivation x then { _type = "derivation"; name = x.name; }
-    else if builtins.isAttrs x then lib.mapAttrs (name: substSpecial) x
-    else if builtins.isList x then map substSpecial x
-    else if lib.isFunction x then "<function>"
-    else x;
-
   rawOpts = lib.optionAttrSetToDocList options;
   transformedOpts = map transformOptions rawOpts;
   filteredOpts = lib.filter (opt: opt.visible && !opt.internal) transformedOpts;
   optionsList = lib.flip map filteredOpts
    (opt: opt
-    // lib.optionalAttrs (opt ? example) { example = substSpecial opt.example; }
-    // lib.optionalAttrs (opt ? default) { default = substSpecial opt.default; }
-    // lib.optionalAttrs (opt ? type) { type = substSpecial opt.type; }
     // lib.optionalAttrs (opt ? relatedPackages && opt.relatedPackages != []) { relatedPackages = genRelatedPackages opt.relatedPackages opt.name; }
    );
 
diff --git a/nixos/modules/misc/documentation.nix b/nixos/modules/misc/documentation.nix
index aa6e40f43705e..64a8f7846b463 100644
--- a/nixos/modules/misc/documentation.nix
+++ b/nixos/modules/misc/documentation.nix
@@ -48,10 +48,15 @@ let
         };
         scrubDerivations = namePrefix: pkgSet: mapAttrs
           (name: value:
-            let wholeName = "${namePrefix}.${name}"; in
-            if isAttrs value then
+            let
+              wholeName = "${namePrefix}.${name}";
+              guard = lib.warn "Attempt to evaluate package ${wholeName} in option documentation; this is not supported and will eventually be an error. Use `mkPackageOption` or `literalExpression` instead.";
+            in if isAttrs value then
               scrubDerivations wholeName value
-              // (optionalAttrs (isDerivation value) { outPath = "\${${wholeName}}"; })
+              // optionalAttrs (isDerivation value) {
+                outPath = guard "\${${wholeName}}";
+                drvPath = guard drvPath;
+              }
             else value
           )
           pkgSet;