From 9698ef7fd059fc6ccf4f747a3164b3d425943193 Mon Sep 17 00:00:00 2001 From: piegames Date: Wed, 19 Apr 2023 00:46:03 +0200 Subject: [PATCH] Rework if statements - Disallow single-line if statements - Handle multi-line conditions differently (indent instead of absorb) --- src/Nixfmt/Pretty.hs | 10 +- test/diff/idioms_lib_1/out.nix | 7 +- test/diff/idioms_lib_2/out.nix | 130 ++++++++++++--- test/diff/idioms_lib_3/out.nix | 123 +++++++++++--- test/diff/idioms_nixos_1/out.nix | 4 +- test/diff/idioms_pkgs_3/out.nix | 4 +- test/diff/if_else/out.nix | 272 ++++++++++++++++++++++++++----- 7 files changed, 455 insertions(+), 95 deletions(-) diff --git a/src/Nixfmt/Pretty.hs b/src/Nixfmt/Pretty.hs index 036b8cb5..f5c5e443 100644 --- a/src/Nixfmt/Pretty.hs +++ b/src/Nixfmt/Pretty.hs @@ -184,9 +184,11 @@ absorb left right (Just level) x absorbSet :: Expression -> Doc absorbSet = absorb line mempty Nothing +-- Don't absorb the if body, always force content on new line absorbThen :: Expression -> Doc +-- XXX this should be removed, but does not appear to work anyways? absorbThen (Term t) | isAbsorbable t = hardspace <> prettyTerm t <> hardspace -absorbThen x = line <> nest 2 (group x) <> line +absorbThen x = hardline <> nest 2 (group x) <> hardline -- What is allowed to come on the same line as `in`? -- Absorbable terms like sets @@ -198,12 +200,13 @@ absorbIn x@(With _ _ _ _) = group x absorbIn x@(Let _ _ _ _) = group x absorbIn x = line <> nest 2 (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 <> pretty then_ <> absorbThen expr0 <> pretty else_ <> absorbElse expr1 - +-- XXX Same as for Then absorbElse (Term t) | isAbsorbable t = hardspace <> prettyTerm t absorbElse x = line <> nest 2 (group x) @@ -238,7 +241,8 @@ instance Pretty Expression where pretty (If if_ cond then_ expr0 else_ expr1) = base $ group $ - pretty if_ <> hardspace <> group cond <> hardspace + -- `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' <> 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 3c4520d9..310240a7 100644 --- a/test/diff/idioms_lib_1/out.nix +++ b/test/diff/idioms_lib_1/out.nix @@ -6,5 +6,10 @@ msg: # Value to return x: - if pred then trace msg x else x; + if + pred + then + trace msg x + else + x; } diff --git a/test/diff/idioms_lib_2/out.nix b/test/diff/idioms_lib_2/out.nix index f570eda5..e2fa6c58 100644 --- a/test/diff/idioms_lib_2/out.nix +++ b/test/diff/idioms_lib_2/out.nix @@ -93,16 +93,31 @@ rec { and = x: y: x && y; # bitwise “and” - bitAnd = builtins.bitAnd or (import ./zip-int-bits.nix - (a: b: if a == 1 && b == 1 then 1 else 0)); + bitAnd = builtins.bitAnd or (import ./zip-int-bits.nix (a: b: + if + a == 1 && b == 1 + then + 1 + else + 0)); # bitwise “or” - bitOr = builtins.bitOr or (import ./zip-int-bits.nix - (a: b: if a == 1 || b == 1 then 1 else 0)); + bitOr = builtins.bitOr or (import ./zip-int-bits.nix (a: b: + if + a == 1 || b == 1 + then + 1 + else + 0)); # bitwise “xor” - bitXor = builtins.bitXor or (import ./zip-int-bits.nix - (a: b: if a != b then 1 else 0)); + bitXor = builtins.bitXor or (import ./zip-int-bits.nix (a: b: + if + a != b + then + 1 + else + 0)); # bitwise “not” bitNot = builtins.sub (-1); @@ -115,7 +130,13 @@ rec { Type: boolToString :: bool -> string */ - boolToString = b: if b then "true" else "false"; + boolToString = b: + if + b + then + "true" + else + "false"; /* Merge two attribute sets shallowly, right side trumps left @@ -155,7 +176,12 @@ rec { f: # Argument to check for null before passing it to `f` a: - if a == null then a else f a; + if + a == null + then + a + else + f a; # Pull in some builtins not included elsewhere. inherit (builtins) @@ -180,7 +206,9 @@ rec { # Returns the current nixpkgs version suffix as string. versionSuffix = let suffixFile = ../.version-suffix; - in if pathExists suffixFile then + in if + pathExists suffixFile + then lib.strings.fileContents suffixFile else "pre-git"; @@ -196,7 +224,9 @@ rec { let revisionFile = "${toString ./..}/.git-revision"; gitRepo = "${toString ./..}/.git"; - in if lib.pathIsGitRepo gitRepo then + in if + lib.pathIsGitRepo gitRepo + then lib.commitIdFromGitRepo gitRepo else if lib.pathExists revisionFile then lib.fileContents revisionFile @@ -216,10 +246,22 @@ rec { ## Integer operations # Return minimum of two numbers. - min = x: y: if x < y then x else y; + min = x: y: + if + x < y + then + x + else + y; # Return maximum of two numbers. - max = x: y: if x > y then x else y; + max = x: y: + if + x > y + then + x + else + y; /* Integer modulus @@ -239,7 +281,15 @@ rec { a == b, compare a b => 0 a > b, compare a b => 1 */ - compare = a: b: if a < b then -1 else if a > b then 1 else 0; + compare = a: b: + if + a < b + then + -1 + else if a > b then + 1 + else + 0; /* Split type into two subtypes by predicate `p`, take all elements of the first subtype to be less than all the elements of the @@ -270,7 +320,19 @@ rec { a: # Second value to compare b: - if p a then if p b then yes a b else -1 else if p b then 1 else no a b; + if + p a + then + if + p b + then + yes a b + else + -1 + else if p b then + 1 + else + no a b; /* Reads a JSON file. @@ -308,11 +370,13 @@ rec { Type: string -> a -> a */ - warn = if lib.elem (builtins.getEnv "NIX_ABORT_ON_WARN") [ - "1" - "true" - "yes" - ] then + warn = if + lib.elem (builtins.getEnv "NIX_ABORT_ON_WARN") [ + "1" + "true" + "yes" + ] + then msg: builtins.trace "warning: ${msg}" (abort "NIX_ABORT_ON_WARN=true; warnings are treated as unrecoverable errors.") @@ -323,7 +387,13 @@ rec { Type: bool -> string -> a -> a */ - warnIf = cond: msg: if cond then warn msg else id; + warnIf = cond: msg: + if + cond + then + warn msg + else + id; /* Like the `assert b; e` expression, but with a custom error message and without the semicolon. @@ -343,7 +413,13 @@ rec { lib.foldr (x: throwIfNot (lib.isFunction x) "All overlays passed to nixpkgs must be functions.") (r: r) overlays pkgs */ - throwIfNot = cond: msg: if cond then x: x else throw msg; + throwIfNot = cond: msg: + if + cond + then + x: x + else + throw msg; /* Check if the elements in a list are valid values from a enum, returning the identity function, or throwing an error message otherwise. @@ -396,7 +472,9 @@ rec { setFunctionArgs : (a → b) → Map String Bool. */ functionArgs = f: - if f ? __functor then + if + f ? __functor + then f.__functionArgs or (lib.functionArgs (f.__functor f)) else builtins.functionArgs f; @@ -419,7 +497,9 @@ rec { toHexString = i: let toHexDigit = d: - if d < 10 then + if + d < 10 + then toString d else { @@ -446,7 +526,9 @@ rec { toBaseDigits = base: i: let go = i: - if i < base then [ i ] else + if + i < base + then [ i ] else let r = i - ((i / base) * base); q = (i - r) / base; diff --git a/test/diff/idioms_lib_3/out.nix b/test/diff/idioms_lib_3/out.nix index 8761e1c9..a21db911 100644 --- a/test/diff/idioms_lib_3/out.nix +++ b/test/diff/idioms_lib_3/out.nix @@ -36,7 +36,9 @@ in rec { err = t: v: abort ("generators.mkValueStringDefault: " + "${t} not supported: ${toPretty { } v}"); - in if isInt v then + in if + isInt v + then toString v # convert derivations to store paths else if lib.isDerivation v then @@ -94,8 +96,15 @@ in rec { }: let mkLine = k: v: mkKeyValue k v + "\n"; - mkLines = if listsAsDuplicateKeys then - k: v: map (mkLine k) (if lib.isList v then v else [ v ]) + mkLines = if + listsAsDuplicateKeys + then + k: v: + map (mkLine k) (if + lib.isList v + then + v + else [ v ]) else k: v: [ (mkLine k v) ]; in @@ -195,7 +204,9 @@ in rec { globalSection, sections, }: - (if globalSection == { } then + (if + globalSection == { } + then "" else (toKeyValue { inherit mkKeyValue listsAsDuplicateKeys; } globalSection) @@ -230,7 +241,9 @@ in rec { section = head sections; subsections = tail sections; subsection = concatStringsSep "." subsections; - in if containsQuote || subsections == [ ] then + in if + containsQuote || subsections == [ ] + then name else ''${section} "${subsection}"''; @@ -246,7 +259,9 @@ 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 then + if + isAttrs value && !lib.isDerivation value + then lib.mapAttrsToList (name: value: recurse ([ name ] ++ path) value) value else if length path > 1 then { @@ -291,10 +306,19 @@ in rec { "__pretty" ]; stepIntoAttr = evalNext: name: - if builtins.elem name specialAttrs then id else evalNext; + if + builtins.elem name specialAttrs + then + id + else + evalNext; transform = depth: - if depthLimit != null && depth > depthLimit then - if throwOnDepthLimit then + if + depthLimit != null && depth > depthLimit + then + if + throwOnDepthLimit + then throw "Exceeded maximum eval-depth limit of ${ toString depthLimit } while trying to evaluate with `generators.withRecursion'!" @@ -306,7 +330,9 @@ in rec { depth: v: let evalNext = x: mapAny (depth + 1) (transform (depth + 1) x); - in if isAttrs v then + in if + isAttrs v + then mapAttrs (stepIntoAttr evalNext) v else if isList v then map evalNext v @@ -337,15 +363,21 @@ in rec { with builtins; let isPath = v: typeOf v == "path"; - introSpace = if multiline then '' + introSpace = if + multiline + then '' ${indent} '' else " "; - outroSpace = if multiline then '' + outroSpace = if + multiline + then '' ${indent}'' else " "; - in if isInt v then + 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 @@ -376,11 +408,16 @@ in rec { lastLine = lib.last escapedLines; in "''" + introSpace - + concatStringsSep introSpace (lib.init escapedLines) - + (if lastLine == "" then outroSpace else introSpace + lastLine) - + "''" + + concatStringsSep introSpace (lib.init escapedLines) + (if + lastLine == "" + then + outroSpace + else + introSpace + lastLine) + "''" ; - in if multiline && length lines > 1 then + in if + multiline && length lines > 1 + then multilineResult else singlelineResult @@ -393,7 +430,9 @@ in rec { else if isPath v then toString v else if isList v then - if v == [ ] then + if + v == [ ] + then "[ ]" else "[" + introSpace @@ -403,14 +442,24 @@ in rec { let fna = lib.functionArgs v; showFnas = concatStringsSep ", " (libAttr.mapAttrsToList - (name: hasDefVal: if hasDefVal then name + "?" else name) fna); - in if fna == { } then + (name: hasDefVal: + if + hasDefVal + then + name + "?" + else + name) fna); + in if + fna == { } + then "" else "" else if isAttrs v then # apply pretty values if allowed - if allowPrettyValues && v ? __pretty && v ? val then + if + allowPrettyValues && v ? __pretty && v ? val + then v.__pretty v.val else if v == { } then "{ }" @@ -437,7 +486,9 @@ in rec { isFloat = builtins.isFloat or (x: false); expr = ind: x: with builtins; - if x == null then + if + x == null + then "" else if isBool x then bool ind x @@ -456,7 +507,13 @@ in rec { literal = ind: x: ind + x; - bool = ind: x: literal ind (if x then "" else ""); + bool = ind: x: + literal ind (if + x + then + "" + else + ""); int = ind: x: literal ind "${toString x}"; str = ind: x: literal ind "${x}"; key = ind: x: literal ind "${x}"; @@ -507,7 +564,9 @@ in rec { with builtins; let concatItems = lib.strings.concatStringsSep ", "; - in if isAttrs v then + in if + isAttrs v + then "{ ${ concatItems (lib.attrsets.mapAttrsToList (key: value: "${key} = ${toDhall args value}") v) @@ -515,9 +574,21 @@ in rec { else if isList v then "[ ${concatItems (map (toDhall args) v)} ]" else if isInt v then - "${if v < 0 then "" else "+"}${toString v}" + "${ + if + v < 0 + then + "" + else + "+" + }${toString v}" else if isBool v then - (if v then "True" else "False") + (if + v + then + "True" + else + "False") else if isFunction v then abort "generators.toDhall: cannot convert a function to Dhall" else if v == null then diff --git a/test/diff/idioms_nixos_1/out.nix b/test/diff/idioms_nixos_1/out.nix index 9b14d57e..5d9d3b91 100644 --- a/test/diff/idioms_nixos_1/out.nix +++ b/test/diff/idioms_nixos_1/out.nix @@ -356,7 +356,9 @@ in { ] ++ (optional (randstructSeed != "") (isYes "GCC_PLUGIN_RANDSTRUCT")); # nixpkgs kernels are assumed to have all required features - assertions = if config.boot.kernelPackages.kernel ? features then + assertions = if + config.boot.kernelPackages.kernel ? features + then [ ] else let diff --git a/test/diff/idioms_pkgs_3/out.nix b/test/diff/idioms_pkgs_3/out.nix index 9b14d57e..5d9d3b91 100644 --- a/test/diff/idioms_pkgs_3/out.nix +++ b/test/diff/idioms_pkgs_3/out.nix @@ -356,7 +356,9 @@ in { ] ++ (optional (randstructSeed != "") (isYes "GCC_PLUGIN_RANDSTRUCT")); # nixpkgs kernels are assumed to have all required features - assertions = if config.boot.kernelPackages.kernel ? features then + assertions = if + config.boot.kernelPackages.kernel ? features + then [ ] else let diff --git a/test/diff/if_else/out.nix b/test/diff/if_else/out.nix index 628141ab..5f912f4b 100644 --- a/test/diff/if_else/out.nix +++ b/test/diff/if_else/out.nix @@ -1,72 +1,266 @@ [ (if true then { version = "1.2.3"; } else { version = "3.2.1"; }) - (if true then '' + (if + true + then '' some text '' else '' other text '') - (if ./a then b else c) - (if a then b else c) + (if + ./a + then + b + else + c) + (if + a + then + b + else + c) (if # test - a # test + a # test then # test b # test else # test c) (if # test - a # test + a # test then # test b # test else # test c) - (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 + (if + if + a + then + b + else + c + then b else if a then b - else # x - if a then + else if a then b else c) - (if (if (if (if a then b else c) then - (if a then b else c) - else - (if a then b else c)) then - (if (if a then b else c) then - (if a then b else c) + (if + if + a + then + b else - (if a then b else c)) + c + then + b + else if a then + b + else # x + if a then + b else - (if (if a then b else c) then - (if a then b else c) - else - (if a then b else c))) then - (if (if (if a then b else c) then - (if a then b else c) + c) + (if + (if + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) + else + (if + a + then + b + else + c)) + then + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) + else + (if + a + then + b + else + c)) else - (if a then b else c)) then - (if (if a then b else c) then - (if a then b else c) + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) + else + (if + a + then + b + else + c))) + then + (if + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) else - (if a then b else c)) + (if + a + then + b + else + c)) + then + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) + else + (if + a + then + b + else + c)) else - (if (if a then b else c) then - (if a then b else c) + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) else - (if a then b else c))) + (if + a + then + b + else + c))) else - (if (if (if a then b else c) then - (if a then b else c) - else - (if a then b else c)) then - (if (if a then b else c) then - (if a then b else c) + (if + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) + else + (if + a + then + b + else + c)) + then + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) else - (if a then b else c)) + (if + a + then + b + else + c)) else - (if (if a then b else c) then - (if a then b else c) + (if + (if + a + then + b + else + c) + then + (if + a + then + b + else + c) else - (if a then b else c)))) + (if + a + then + b + else + c)))) ]