From 818f63cd4e9986a99ca67f5d56ec1a1d4e4536af Mon Sep 17 00:00:00 2001 From: piegames Date: Fri, 5 May 2023 14:29:02 +0200 Subject: [PATCH] fixup! Rework if statements --- src/Nixfmt/Pretty.hs | 5 +- test/diff/idioms_lib_1/out.nix | 3 +- test/diff/idioms_lib_2/out.nix | 60 +++++------ test/diff/idioms_lib_3/out.nix | 172 ++++++++++++++++--------------- test/diff/idioms_nixos_1/out.nix | 3 +- test/diff/idioms_pkgs_3/out.nix | 3 +- test/diff/if_else/in.nix | 25 +++++ test/diff/if_else/out.nix | 138 +++++++++++++------------ 8 files changed, 215 insertions(+), 194 deletions(-) diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs index a9a078fd..3b834029 100644 --- a/src/Nixfmt/Pretty.hs +++ b/src/Nixfmt/Pretty.hs @@ -206,7 +206,8 @@ absorbIn x = line <> group x <> line -- Only absorb "else if" absorbElse :: Expression -> Doc absorbElse (If if_ cond then_ expr0 else_ expr1) - = hardspace <> pretty if_ <> hardspace <> group cond <> hardspace + -- `if cond then` if it fits on one line, otherwise `if\n cond\nthen` (with cond indented) + = hardspace <> (group (pretty if_ <> hardspace <> line' <> nest 2 (group cond) <> hardspace <> line')) <> pretty then_ <> absorbThen expr0 <> pretty else_ <> absorbElse expr1 -- XXX Same as for Then @@ -245,7 +246,7 @@ instance Pretty Expression where pretty (If if_ cond then_ expr0 else_ expr1) = base $ group $ -- `if cond then` if it fits on one line, otherwise `if\n cond\nthen` (with cond indented) - pretty if_ <> hardspace <> line' <> nest 2 (group cond) <> hardspace <> line' + (group (pretty if_ <> hardspace <> line' <> nest 2 (group cond) <> hardspace <> line')) <> pretty then_ <> absorbThen expr0 <> pretty else_ <> absorbElse expr1 diff --git a/test/diff/idioms_lib_1/out.nix b/test/diff/idioms_lib_1/out.nix index 310240a7..ac24ae6f 100644 --- a/test/diff/idioms_lib_1/out.nix +++ b/test/diff/idioms_lib_1/out.nix @@ -6,8 +6,7 @@ msg: # Value to return x: - if - pred + if pred then trace msg x else diff --git a/test/diff/idioms_lib_2/out.nix b/test/diff/idioms_lib_2/out.nix index 8b1e85f8..97d9b980 100644 --- a/test/diff/idioms_lib_2/out.nix +++ b/test/diff/idioms_lib_2/out.nix @@ -94,8 +94,7 @@ rec { # bitwise “and” bitAnd = builtins.bitAnd or (import ./zip-int-bits.nix (a: b: - if - a == 1 && b == 1 + if a == 1 && b == 1 then 1 else @@ -103,8 +102,7 @@ rec { # bitwise “or” bitOr = builtins.bitOr or (import ./zip-int-bits.nix (a: b: - if - a == 1 || b == 1 + if a == 1 || b == 1 then 1 else @@ -112,8 +110,7 @@ rec { # bitwise “xor” bitXor = builtins.bitXor or (import ./zip-int-bits.nix (a: b: - if - a != b + if a != b then 1 else @@ -131,8 +128,7 @@ rec { Type: boolToString :: bool -> string */ boolToString = b: - if - b + if b then "true" else @@ -176,8 +172,7 @@ rec { f: # Argument to check for null before passing it to `f` a: - if - a == null + if a == null then a else @@ -217,8 +212,7 @@ rec { # Returns the current nixpkgs version suffix as string. versionSuffix = let suffixFile = ../.version-suffix; - in if - pathExists suffixFile + in if pathExists suffixFile then lib.strings.fileContents suffixFile else @@ -235,11 +229,11 @@ rec { let revisionFile = "${toString ./..}/.git-revision"; gitRepo = "${toString ./..}/.git"; - in if - lib.pathIsGitRepo gitRepo + in if lib.pathIsGitRepo gitRepo then lib.commitIdFromGitRepo gitRepo - else if lib.pathExists revisionFile then + else if lib.pathExists revisionFile + then lib.fileContents revisionFile else default; @@ -258,8 +252,7 @@ rec { # Return minimum of two numbers. min = x: y: - if - x < y + if x < y then x else @@ -267,8 +260,7 @@ rec { # Return maximum of two numbers. max = x: y: - if - x > y + if x > y then x else @@ -293,11 +285,11 @@ rec { a > b, compare a b => 1 */ compare = a: b: - if - a < b + if a < b then -1 - else if a > b then + else if a > b + then 1 else 0; @@ -331,16 +323,15 @@ rec { a: # Second value to compare b: - if - p a + if p a then - if - p b + if p b then yes a b else -1 - else if p b then + else if p b + then 1 else no a b; @@ -399,8 +390,7 @@ rec { Type: bool -> string -> a -> a */ warnIf = cond: msg: - if - cond + if cond then warn msg else @@ -425,8 +415,7 @@ rec { pkgs */ throwIfNot = cond: msg: - if - cond + if cond then x: x else @@ -482,8 +471,7 @@ rec { setFunctionArgs : (a → b) → Map String Bool. */ functionArgs = f: - if - f ? __functor + if f ? __functor then f.__functionArgs or (lib.functionArgs (f.__functor f)) else @@ -507,8 +495,7 @@ rec { toHexString = i: let toHexDigit = d: - if - d < 10 + if d < 10 then toString d else @@ -536,8 +523,7 @@ rec { toBaseDigits = base: i: let go = i: - if - i < base + if i < base then [ i ] else let r = i - ((i / base) * base); diff --git a/test/diff/idioms_lib_3/out.nix b/test/diff/idioms_lib_3/out.nix index 9409840a..ae861283 100644 --- a/test/diff/idioms_lib_3/out.nix +++ b/test/diff/idioms_lib_3/out.nix @@ -36,37 +36,45 @@ in rec { err = t: v: abort ("generators.mkValueStringDefault: " + "${t} not supported: ${toPretty { } v}"); - in if - isInt v + in if isInt v then toString v # convert derivations to store paths - else if lib.isDerivation v then + else if lib.isDerivation v + then toString v # we default to not quoting strings - else if isString v then + else if isString v + then v # isString returns "1", which is not a good default - else if true == v then + else if true == v + then "true" # here it returns to "", which is even less of a good default - else if false == v then + else if false == v + then "false" - else if null == v then + else if null == v + then "null" # if you have lists you probably want to replace this - else if isList v then + else if isList v + then err "lists" v # same as for lists, might want to replace - else if isAttrs v then + else if isAttrs v + then err "attrsets" v # functions can’t be printed of course - else if isFunction v then + else if isFunction v + then err "functions" v # Floats currently can't be converted to precise strings, # condition warning on nix version once this isn't a problem anymore # See https://github.com/NixOS/nix/pull/3480 - else if isFloat v then + else if isFloat v + then libStr.floatToString v else err "this value is" (toString v); @@ -96,12 +104,10 @@ in rec { }: let mkLine = k: v: mkKeyValue k v + "\n"; - mkLines = if - listsAsDuplicateKeys + mkLines = if listsAsDuplicateKeys then k: v: - map (mkLine k) (if - lib.isList v + map (mkLine k) (if lib.isList v then v else [ v ]) @@ -204,8 +210,7 @@ in rec { globalSection, sections, }: - (if - globalSection == { } + (if globalSection == { } then "" else @@ -241,8 +246,7 @@ in rec { section = head sections; subsections = tail sections; subsection = concatStringsSep "." subsections; - in if - containsQuote || subsections == [ ] + in if containsQuote || subsections == [ ] then name else @@ -259,12 +263,12 @@ in rec { # converts { a.b.c = 5; } to { "a.b".c = 5; } for toINI gitFlattenAttrs = let recurse = path: value: - if - isAttrs value && !lib.isDerivation value + if isAttrs value && !lib.isDerivation value then lib.mapAttrsToList (name: value: recurse ([ name ] ++ path) value) value - else if length path > 1 then { + else if length path > 1 + then { ${concatStringsSep "." (lib.reverseList (tail path))}.${head path} = value; } else { @@ -305,18 +309,15 @@ in rec { "__pretty" ]; stepIntoAttr = evalNext: name: - if - builtins.elem name specialAttrs + if builtins.elem name specialAttrs then id else evalNext; transform = depth: - if - depthLimit != null && depth > depthLimit + if depthLimit != null && depth > depthLimit then - if - throwOnDepthLimit + if throwOnDepthLimit then throw "Exceeded maximum eval-depth limit of ${ toString depthLimit @@ -329,11 +330,11 @@ in rec { depth: v: let evalNext = x: mapAny (depth + 1) (transform (depth + 1) x); - in if - isAttrs v + in if isAttrs v then mapAttrs (stepIntoAttr evalNext) v - else if isList v then + else if isList v + then map evalNext v else transform (depth + 1) v; @@ -362,28 +363,27 @@ in rec { with builtins; let isPath = v: typeOf v == "path"; - introSpace = if - multiline + introSpace = if multiline then '' ${indent} '' else " "; - outroSpace = if - multiline + outroSpace = if multiline then '' ${indent}'' else " "; - in if - isInt v + in if isInt 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 + else if isFloat v + then builtins.toJSON v - else if isString v then + else if isString v + then let lines = filter (v: !isList v) (builtins.split "\n" v); escapeSingleline = libStr.escape [ @@ -407,62 +407,66 @@ in rec { lastLine = lib.last escapedLines; in "''" + introSpace - + concatStringsSep introSpace (lib.init escapedLines) + (if - lastLine == "" + + concatStringsSep introSpace (lib.init escapedLines) + + (if lastLine == "" then outroSpace else introSpace + lastLine) + "''" ; - in if - multiline && length lines > 1 + in if multiline && length lines > 1 then multilineResult else singlelineResult - else if true == v then + else if true == v + then "true" - else if false == v then + else if false == v + then "false" - else if null == v then + else if null == v + then "null" - else if isPath v then + else if isPath v + then toString v - else if isList v then - if - v == [ ] + else if isList v + then + if v == [ ] then "[ ]" else "[" + introSpace + libStr.concatMapStringsSep introSpace (go (indent + " ")) v + outroSpace + "]" - else if isFunction v then + else if isFunction v + then let fna = lib.functionArgs v; showFnas = concatStringsSep ", " (libAttr.mapAttrsToList (name: hasDefVal: - if - hasDefVal + if hasDefVal then name + "?" else name) fna); - in if - fna == { } + in if fna == { } then "" else "" - else if isAttrs v then + else if isAttrs v + then # apply pretty values if allowed - if - allowPrettyValues && v ? __pretty && v ? val + if allowPrettyValues && v ? __pretty && v ? val then v.__pretty v.val - else if v == { } then + else if v == { } + then "{ }" - else if v ? type && v.type == "derivation" then + else if v ? type && v.type == "derivation" + then "" else "{" + introSpace + libStr.concatStringsSep introSpace @@ -485,21 +489,26 @@ in rec { isFloat = builtins.isFloat or (x: false); expr = ind: x: with builtins; - if - x == null + if x == null then "" - else if isBool x then + else if isBool x + then bool ind x - else if isInt x then + else if isInt x + then int ind x - else if isString x then + else if isString x + then str ind x - else if isList x then + else if isList x + then list ind x - else if isAttrs x then + else if isAttrs x + then attrs ind x - else if isFloat x then + else if isFloat x + then float ind x else abort "generators.toPlist: should never happen (v = ${v})"; @@ -507,8 +516,7 @@ in rec { literal = ind: x: ind + x; bool = ind: x: - literal ind (if - x + literal ind (if x then "" else @@ -563,34 +571,36 @@ in rec { with builtins; let concatItems = lib.strings.concatStringsSep ", "; - in if - isAttrs v + in if isAttrs v then "{ ${ concatItems (lib.attrsets.mapAttrsToList (key: value: "${key} = ${toDhall args value}") v) } }" - else if isList v then + else if isList v + then "[ ${concatItems (map (toDhall args) v)} ]" - else if isInt v then + else if isInt v + then "${ - if - v < 0 + if v < 0 then "" else "+" }${toString v}" - else if isBool v then - (if - v + else if isBool v + then + (if v then "True" else "False") - else if isFunction v then + else if isFunction v + then abort "generators.toDhall: cannot convert a function to Dhall" - else if v == null then + else if v == null + then abort "generators.toDhall: cannot convert a null to Dhall" else builtins.toJSON v; diff --git a/test/diff/idioms_nixos_1/out.nix b/test/diff/idioms_nixos_1/out.nix index 4e0bcc08..189413da 100644 --- a/test/diff/idioms_nixos_1/out.nix +++ b/test/diff/idioms_nixos_1/out.nix @@ -356,8 +356,7 @@ in { ] ++ (optional (randstructSeed != "") (isYes "GCC_PLUGIN_RANDSTRUCT")); # nixpkgs kernels are assumed to have all required features - assertions = if - config.boot.kernelPackages.kernel ? features + assertions = if config.boot.kernelPackages.kernel ? features then [ ] else diff --git a/test/diff/idioms_pkgs_3/out.nix b/test/diff/idioms_pkgs_3/out.nix index 4e0bcc08..189413da 100644 --- a/test/diff/idioms_pkgs_3/out.nix +++ b/test/diff/idioms_pkgs_3/out.nix @@ -356,8 +356,7 @@ in { ] ++ (optional (randstructSeed != "") (isYes "GCC_PLUGIN_RANDSTRUCT")); # nixpkgs kernels are assumed to have all required features - assertions = if - config.boot.kernelPackages.kernel ? features + assertions = if config.boot.kernelPackages.kernel ? features then [ ] else diff --git a/test/diff/if_else/in.nix b/test/diff/if_else/in.nix index 5212aa73..eb3cec76 100644 --- a/test/diff/if_else/in.nix +++ b/test/diff/if_else/in.nix @@ -31,6 +31,31 @@ else # test /**/ c) + (if [ + multiline + # tmp + condition + ] then + foo + else if [ + more + multi + line + ] then + bar + else + baz + ) + (if unabsorbable # comment + == multiline + then + foo + else if unabsorbable # comment + == multiline then + bar + else + baz + ) (if if a then b else c then b else if a then b else if a then b else c) (if if a then b else c then b else if a then b else /*x*/ if a then b else c) (if diff --git a/test/diff/if_else/out.nix b/test/diff/if_else/out.nix index 5f912f4b..1062e54d 100644 --- a/test/diff/if_else/out.nix +++ b/test/diff/if_else/out.nix @@ -1,20 +1,17 @@ [ (if true then { version = "1.2.3"; } else { version = "3.2.1"; }) - (if - true + (if true then '' some text '' else '' other text '') - (if - ./a + (if ./a then b else c) - (if - a + (if a then b else @@ -32,99 +29,122 @@ else # test c) (if - if - a + [ + multiline + # tmp + condition + ] + then + foo + else if + [ + more + multi + line + ] + then + bar + else + baz) + (if + unabsorbable # comment + == multiline + then + foo + else if + unabsorbable # comment + == multiline + then + bar + else + baz) + (if + if a then b else c then b - else if a then + else if a + then b - else if a then + else if a + then b else c) (if - if - a + if a then b else c then b - else if a then + else if a + then b else # x - if a then + if a + then b else c) (if (if (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else c)) then (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else c)) else (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else @@ -132,66 +152,57 @@ then (if (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else c)) then (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else c)) else (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else @@ -199,66 +210,57 @@ else (if (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else c)) then (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else c)) else (if - (if - a + (if a then b else c) then - (if - a + (if a then b else c) else - (if - a + (if a then b else