Rework if statements

- Disallow single-line if statements - Handle multi-line conditions differently (indent instead of absorb)
NixOS · May 5, 2023 · 9698ef7 · 9698ef7
1 parent 2c1e36e
commit 9698ef7
Show file tree

Hide file tree

Showing 7 changed files with 455 additions and 95 deletions.
diff --git 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
@@ -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
@@ -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 "[1;31mwarning: ${msg}[0m" (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;