From 1cbe950384a24f0b8272e3cc8813662292896821 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 15 Sep 2022 17:29:38 +0100 Subject: [PATCH] lib.types: Add parentheses where description is ambiguous --- lib/tests/misc.nix | 53 ++++++++++++++++++++++++++++++ lib/types.nix | 82 ++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 122 insertions(+), 13 deletions(-) diff --git a/lib/tests/misc.nix b/lib/tests/misc.nix index 584a946e92cc..9b1397a7915a 100644 --- a/lib/tests/misc.nix +++ b/lib/tests/misc.nix @@ -1206,4 +1206,57 @@ runTests { expr = strings.levenshteinAtMost 3 "hello" "Holla"; expected = true; }; + + testTypeDescriptionInt = { + expr = (with types; int).description; + expected = "signed integer"; + }; + testTypeDescriptionListOfInt = { + expr = (with types; listOf int).description; + expected = "list of signed integer"; + }; + testTypeDescriptionListOfListOfInt = { + expr = (with types; listOf (listOf int)).description; + expected = "list of list of signed integer"; + }; + testTypeDescriptionListOfEitherStrOrBool = { + expr = (with types; listOf (either str bool)).description; + expected = "list of (string or boolean)"; + }; + testTypeDescriptionEitherListOfStrOrBool = { + expr = (with types; either (listOf bool) str).description; + expected = "(list of boolean) or string"; + }; + testTypeDescriptionEitherStrOrListOfBool = { + expr = (with types; either str (listOf bool)).description; + expected = "string or list of boolean"; + }; + testTypeDescriptionOneOfListOfStrOrBool = { + expr = (with types; oneOf [ (listOf bool) str ]).description; + expected = "(list of boolean) or string"; + }; + testTypeDescriptionOneOfListOfStrOrBoolOrNumber = { + expr = (with types; oneOf [ (listOf bool) str number ]).description; + expected = "(list of boolean) or string or signed integer or floating point number"; + }; + testTypeDescriptionEitherListOfBoolOrEitherStringOrNumber = { + expr = (with types; either (listOf bool) (either str number)).description; + expected = "(list of boolean) or string or signed integer or floating point number"; + }; + testTypeDescriptionEitherEitherListOfBoolOrStringOrNumber = { + expr = (with types; either (either (listOf bool) str) number).description; + expected = "(list of boolean) or string or signed integer or floating point number"; + }; + testTypeDescriptionEitherNullOrBoolOrString = { + expr = (with types; either (nullOr bool) str).description; + expected = "null or boolean or string"; + }; + testTypeDescriptionEitherListOfEitherBoolOrStrOrInt = { + expr = (with types; either (listOf (either bool str)) int).description; + expected = "(list of (boolean or string)) or signed integer"; + }; + testTypeDescriptionEitherIntOrListOrEitherBoolOrStr = { + expr = (with types; either int (listOf (either bool str))).description; + expected = "signed integer or list of (boolean or string)"; + }; } diff --git a/lib/types.nix b/lib/types.nix index f235e3419926..3750ba965558 100644 --- a/lib/types.nix +++ b/lib/types.nix @@ -113,6 +113,12 @@ rec { name , # Description of the type, defined recursively by embedding the wrapped type if any. description ? null + # A hint for whether or not this description needs parentheses. Possible values: + # - "noun": a simple noun phrase such as "positive integer" + # - "conjunction": a phrase with a potentially ambiguous "or" connective. + # - "composite": a phrase with an "of" connective + # See the `optionDescriptionPhrase` function. + , descriptionClass ? null , # Function applied to each definition that should return true if # its type-correct, false otherwise. check ? (x: true) @@ -158,10 +164,36 @@ rec { nestedTypes ? {} }: { _type = "option-type"; - inherit name check merge emptyValue getSubOptions getSubModules substSubModules typeMerge functor deprecationMessage nestedTypes; + inherit + name check merge emptyValue getSubOptions getSubModules substSubModules + typeMerge functor deprecationMessage nestedTypes descriptionClass; description = if description == null then name else description; }; + # optionDescriptionPhrase :: (str -> bool) -> optionType -> str + # + # Helper function for producing unambiguous but readable natural language + # descriptions of types. + # + # Parameters + # + # optionDescriptionPhase unparenthesize optionType + # + # `unparenthesize`: A function from descriptionClass string to boolean. + # It must return true when the class of phrase will fit unambiguously into + # the description of the caller. + # + # `optionType`: The option type to parenthesize or not. + # The option whose description we're returning. + # + # Return value + # + # The description of the `optionType`, with parentheses if there may be an + # ambiguity. + optionDescriptionPhrase = unparenthesize: t: + if unparenthesize (t.descriptionClass or null) + then t.description + else "(${t.description})"; # When adding new types don't forget to document them in # nixos/doc/manual/development/option-types.xml! @@ -170,6 +202,7 @@ rec { raw = mkOptionType rec { name = "raw"; description = "raw value"; + descriptionClass = "noun"; check = value: true; merge = mergeOneOption; }; @@ -177,6 +210,7 @@ rec { anything = mkOptionType { name = "anything"; description = "anything"; + descriptionClass = "noun"; check = value: true; merge = loc: defs: let @@ -216,12 +250,14 @@ rec { }; unspecified = mkOptionType { - name = "unspecified"; + name = "unspecified value"; + descriptionClass = "noun"; }; bool = mkOptionType { name = "bool"; description = "boolean"; + descriptionClass = "noun"; check = isBool; merge = mergeEqualOption; }; @@ -229,6 +265,7 @@ rec { int = mkOptionType { name = "int"; description = "signed integer"; + descriptionClass = "noun"; check = isInt; merge = mergeEqualOption; }; @@ -294,6 +331,7 @@ rec { float = mkOptionType { name = "float"; description = "floating point number"; + descriptionClass = "noun"; check = isFloat; merge = mergeEqualOption; }; @@ -325,6 +363,7 @@ rec { str = mkOptionType { name = "str"; description = "string"; + descriptionClass = "noun"; check = isString; merge = mergeEqualOption; }; @@ -332,6 +371,7 @@ rec { nonEmptyStr = mkOptionType { name = "nonEmptyStr"; description = "non-empty string"; + descriptionClass = "noun"; check = x: str.check x && builtins.match "[ \t\n]*" x == null; inherit (str) merge; }; @@ -344,6 +384,7 @@ rec { mkOptionType { name = "singleLineStr"; description = "(optionally newline-terminated) single-line string"; + descriptionClass = "noun"; inherit check; merge = loc: defs: lib.removeSuffix "\n" (merge loc defs); @@ -352,6 +393,7 @@ rec { strMatching = pattern: mkOptionType { name = "strMatching ${escapeNixString pattern}"; description = "string matching the pattern ${pattern}"; + descriptionClass = "noun"; check = x: str.check x && builtins.match pattern x != null; inherit (str) merge; }; @@ -364,6 +406,7 @@ rec { then "Concatenated string" # for types.string. else "strings concatenated with ${builtins.toJSON sep}" ; + descriptionClass = "noun"; check = isString; merge = loc: defs: concatStringsSep sep (getValues defs); functor = (defaultFunctor name) // { @@ -387,7 +430,7 @@ rec { passwdEntry = entryType: addCheck entryType (str: !(hasInfix ":" str || hasInfix "\n" str)) // { name = "passwdEntry ${entryType.name}"; - description = "${entryType.description}, not containing newlines or colons"; + description = "${optionDescriptionPhrase (class: class == "noun") entryType}, not containing newlines or colons"; }; attrs = mkOptionType { @@ -407,6 +450,7 @@ rec { # ("/nix/store/hash-foo"). These get a context added to them using builtins.storePath. package = mkOptionType { name = "package"; + descriptionClass = "noun"; check = x: isDerivation x || isStorePath x; merge = loc: defs: let res = mergeOneOption loc defs; @@ -427,7 +471,8 @@ rec { listOf = elemType: mkOptionType rec { name = "listOf"; - description = "list of ${elemType.description}"; + description = "list of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; + descriptionClass = "composite"; check = isList; merge = loc: defs: map (x: x.value) (filter (x: x ? value) (concatLists (imap1 (n: def: @@ -450,13 +495,14 @@ rec { nonEmptyListOf = elemType: let list = addCheck (types.listOf elemType) (l: l != []); in list // { - description = "non-empty " + list.description; + description = "non-empty ${optionDescriptionPhrase (class: class == "noun") list}"; emptyValue = { }; # no .value attr, meaning unset }; attrsOf = elemType: mkOptionType rec { name = "attrsOf"; - description = "attribute set of ${elemType.description}"; + description = "attribute set of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; + descriptionClass = "composite"; check = isAttrs; merge = loc: defs: mapAttrs (n: v: v.value) (filterAttrs (n: v: v ? value) (zipAttrsWith (name: defs: @@ -479,7 +525,8 @@ rec { # error that it's not defined. Use only if conditional definitions don't make sense. lazyAttrsOf = elemType: mkOptionType rec { name = "lazyAttrsOf"; - description = "lazy attribute set of ${elemType.description}"; + description = "lazy attribute set of ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; + descriptionClass = "composite"; check = isAttrs; merge = loc: defs: zipAttrsWith (name: defs: @@ -509,7 +556,7 @@ rec { # Value of given type but with no merging (i.e. `uniq list`s are not concatenated). uniq = elemType: mkOptionType rec { name = "uniq"; - inherit (elemType) description check; + inherit (elemType) description descriptionClass check; merge = mergeOneOption; emptyValue = elemType.emptyValue; getSubOptions = elemType.getSubOptions; @@ -521,7 +568,7 @@ rec { unique = { message }: type: mkOptionType rec { name = "unique"; - inherit (type) description check; + inherit (type) description descriptionClass check; merge = mergeUniqueOption { inherit message; }; emptyValue = type.emptyValue; getSubOptions = type.getSubOptions; @@ -534,7 +581,8 @@ rec { # Null or value of ... nullOr = elemType: mkOptionType rec { name = "nullOr"; - description = "null or ${elemType.description}"; + description = "null or ${optionDescriptionPhrase (class: class == "noun" || class == "conjunction") elemType}"; + descriptionClass = "conjunction"; check = x: x == null || elemType.check x; merge = loc: defs: let nrNulls = count (def: def.value == null) defs; in @@ -552,7 +600,8 @@ rec { functionTo = elemType: mkOptionType { name = "functionTo"; - description = "function that evaluates to a(n) ${elemType.description}"; + description = "function that evaluates to a(n) ${optionDescriptionPhrase (class: class == "noun" || class == "composite") elemType}"; + descriptionClass = "composite"; check = isFunction; merge = loc: defs: fnArgs: (mergeDefinitions (loc ++ [ "[function body]" ]) elemType (map (fn: { inherit (fn) file; value = fn.value fnArgs; }) defs)).mergedValue; @@ -578,6 +627,7 @@ rec { deferredModuleWith = attrs@{ staticModules ? [] }: mkOptionType { name = "deferredModule"; description = "module"; + descriptionClass = "noun"; check = x: isAttrs x || isFunction x || path.check x; merge = loc: defs: { imports = staticModules ++ map (def: lib.setDefaultModuleLocation "${def.file}, via option ${showOption loc}" def.value) defs; @@ -603,6 +653,7 @@ rec { optionType = mkOptionType { name = "optionType"; description = "optionType"; + descriptionClass = "noun"; check = value: value._type or null == "option-type"; merge = loc: defs: if length defs == 1 @@ -749,6 +800,10 @@ rec { "value ${show (builtins.head values)} (singular enum)" else "one of ${concatMapStringsSep ", " show values}"; + descriptionClass = + if builtins.length values < 2 + then "noun" + else "conjunction"; check = flip elem values; merge = mergeEqualOption; functor = (defaultFunctor name) // { payload = values; binOp = a: b: unique (a ++ b); }; @@ -757,7 +812,8 @@ rec { # Either value of type `t1` or `t2`. either = t1: t2: mkOptionType rec { name = "either"; - description = "${t1.description} or ${t2.description}"; + description = "${optionDescriptionPhrase (class: class == "noun" || class == "conjunction") t1} or ${optionDescriptionPhrase (class: class == "noun" || class == "conjunction" || class == "composite") t2}"; + descriptionClass = "conjunction"; check = x: t1.check x || t2.check x; merge = loc: defs: let @@ -795,7 +851,7 @@ rec { coercedType.description})"; mkOptionType rec { name = "coercedTo"; - description = "${finalType.description} or ${coercedType.description} convertible to it"; + description = "${optionDescriptionPhrase (class: class == "noun") finalType} or ${optionDescriptionPhrase (class: class == "noun") coercedType} convertible to it"; check = x: (coercedType.check x && finalType.check (coerceFunc x)) || finalType.check x; merge = loc: defs: let