Better list argument handling

main/Main.hs

@@ -49,7 +49,8 @@ data Nixfmt = Nixfmt
     quiet :: Bool,
     strict :: Bool,
     verify :: Bool,
-    ast :: Bool
+    ast :: Bool,
+    ir :: Bool
   }
   deriving (Show, Data, Typeable)
 
@@ -76,7 +77,11 @@ options =
         ast =
           False
             &= help
-              "Pretty print the internal AST, only for debugging"
+              "Pretty print the internal AST, only for debugging",
+        ir =
+          False
+            &= help
+              "Pretty print the internal intermediate representation, only for debugging"
       }
       &= summary ("nixfmt " ++ versionFromFile)
       &= help "Format Nix source code"
@@ -157,6 +162,7 @@ type Formatter = FilePath -> Text -> Either String Text
 
 toFormatter :: Nixfmt -> Formatter
 toFormatter Nixfmt{ast = True} = Nixfmt.printAst
+toFormatter Nixfmt{ir = True} = Nixfmt.printIR
 toFormatter Nixfmt{width, verify = True, strict} = Nixfmt.formatVerify (layout width strict)
 toFormatter Nixfmt{width, verify = False, strict} = Nixfmt.format (layout width strict)
 

src/Nixfmt.hs

@@ -7,6 +7,7 @@ module Nixfmt (
   format,
   formatVerify,
   printAst,
+  printIR,
 )
 where
 
@@ -15,7 +16,7 @@ import Data.Either (fromRight)
 import Data.Text (Text, unpack)
 import Data.Text.Lazy (toStrict)
 import qualified Nixfmt.Parser as Parser
-import Nixfmt.Predoc (Pretty)
+import Nixfmt.Predoc (Pretty, fixup, pretty)
 import Nixfmt.Pretty ()
 import Nixfmt.Types (Expression, LanguageElement, ParseErrorBundle, Whole (..), walkSubprograms)
 import qualified Text.Megaparsec as Megaparsec (parse)
@@ -41,6 +42,12 @@ printAst path unformatted = do
   Whole unformattedParsed' _ <- first errorBundlePretty . Megaparsec.parse Parser.file path $ unformatted
   Left (unpack $ toStrict $ pShow unformattedParsed')
 
+-- | Pretty print the internal IR for debugging
+printIR :: FilePath -> Text -> Either String Text
+printIR filename =
+  bimap errorBundlePretty (toStrict . pShow . fixup . pretty)
+    . Megaparsec.parse Parser.file filename
+
 -- Same functionality as `format`, but add sanity checks to guarantee the following properties of the formatter:
 -- - Correctness: The formatted output parses, and the parse tree is identical to the input's
 -- - Idempotency: Formatting the output again will not modify it

src/Nixfmt/Pretty.hs

@@ -381,6 +381,12 @@ prettyApp indentFunction pre hasPost f a =
       -- because if they get expanded before anything else,
       -- only the `.`-and-after part gets to a new line, which looks very odd
       absorbApp (Application f' a'@(Term Selection{})) = group' Transparent (absorbApp f') <> line <> nest (group' RegularG $ absorbInner a')
+      -- If two consecutive arguments are lists, treat them specially: Don't priority expand, and also
+      -- if one does not fit onto the line then put both on a new line each.
+      -- Note that this does not handle the case where the two last arguments are lists, as the last argument
+      -- is handled elsewhere and cannot be pattern-matched here.
+      absorbApp (Application (Application f' l1@(Term List{})) l2@(Term List{})) =
+        group' Transparent (group' Transparent (absorbApp f') <> nest (group' RegularG $ line <> group (absorbInner l1) <> line <> group (absorbInner l2)))
       absorbApp (Application f' a') = group' Transparent (absorbApp f') <> line <> nest (group' Priority $ absorbInner a')
       -- First argument
       absorbApp expr
@@ -451,15 +457,35 @@ prettyApp indentFunction pre hasPost f a =
           ((\a'@Ann{preTrivia} -> (a'{preTrivia = []}, preTrivia)) . moveTrailingCommentUp)
           f
 
-      renderedF = pre <> group' Transparent (absorbApp fWithoutComment)
-      renderedFUnexpanded = unexpandSpacing' Nothing renderedF
+      -- renderSimple will take a document to render, and call one of two callbacks depending on whether
+      -- it can take a simplified layout (with removed line breaks) or not.
+      renderSimple :: Doc -> (Doc -> Doc) -> (Doc -> Doc) -> Doc
+      renderSimple toRender renderIfSimple renderOtherwise =
+        let renderedF = pre <> group' Transparent toRender
+            renderedFUnexpanded = unexpandSpacing' Nothing renderedF
+        in if isSimple (Application f a) && isJust renderedFUnexpanded
+            then renderIfSimple (fromJust renderedFUnexpanded)
+            else renderOtherwise renderedF
 
       post = if hasPost then line' else mempty
   in pretty comment'
-      <> ( if isSimple (Application f a) && isJust renderedFUnexpanded
-            then group' RegularG $ fromJust renderedFUnexpanded <> hardspace <> absorbLast a
-            else group' RegularG $ renderedF <> line <> absorbLast a <> post
-         )
+      <> case (fWithoutComment, a) of
+        -- When the two last arguments are lists, render these specially (same as above)
+        -- Also no need to wrap in renderSimple here, because we know that these kinds of arguments
+        -- are never "simple" by definition.
+        (Application fWithoutCommandAndWithoutArg l1@(Term List{}), l2@(Term List{})) ->
+          group' RegularG $
+            (pre <> group' Transparent (absorbApp fWithoutCommandAndWithoutArg))
+              <> line
+              <> nest (group (absorbInner l1))
+              <> line
+              <> nest (group (absorbInner l2))
+              <> post
+        _ ->
+          renderSimple
+            (absorbApp fWithoutComment)
+            (\fRendered -> group' RegularG $ fRendered <> hardspace <> absorbLast a)
+            (\fRendered -> group' RegularG $ fRendered <> line <> absorbLast a <> post)
       <> (if hasPost && not (null comment') then hardline else mempty)
 
 prettyWith :: Bool -> Expression -> Doc

test/diff/apply/out-pure.nix

@@ -160,10 +160,7 @@
       ''"''
       "\${"
     ];
-    escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [
-      "''\${"
-      "'''"
-    ];
+    escapeMultiline = libStr.replaceStrings [ "\${" "''" ] [ "''\${" "'''" ];
     test =
       foo
         [

test/diff/apply_with_lists/in.nix

@@ -0,0 +1,78 @@
+# This file contains an assortment of test cases involving list-heavy function calls
+
+[
+  (f [ ] [ rhs lhs ])
+  (lib.mkMerge [ false false ])
+  (replaceStrings
+    [ "\${" "''" ]
+    #force multiline
+    [ "''\${" "'''" ])
+  (replaceStrings [ ''"'' "\\" ] [ ''\"'' "\\\\" ] name)
+  (replaceStrings
+    [ ''"'' "\\" ]
+    # force multiline
+    [ ''\"'' "\\\\" ]
+    name)
+  (replaceStrings
+      [ "@" ":" "\\" "[" "]" ]
+      [ "-" "-" "-" "" "" ])
+  (lists.removePrefix [ 1 2 ] [ ])
+  (lists.removePrefix aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa [ 1 2 ] [ ])
+  (builtins.replaceStrings
+    [ "@NIX_STORE_VERITY@" ]
+    [partitionTypes.usr-verity]
+    (builtins.readFile ./assert_uki_repart_match.py))
+  (replaceStrings [ "-" ] [ "_" ] (toUpper final.rust.cargoShortTarget))
+  (lib.mkChangedOptionModule
+    [ "security" "acme" "validMin" ]
+    [ "security" "acme" "defaults" "validMinDays" ]
+    (config: config.security.acme.validMin / (24 * 3600)))
+  (lib.replaceStrings
+    [ "https://registry" ".io/providers" ]
+    [ "registry" ".io" ]
+    homepage)
+  (lib.mkRenamedOptionModule [ "boot" "extraTTYs" ] [ "console" "extraTTYs" ])
+  # This line is engineered to exactly hit the line length limit
+  (lib.mkRenamedOptionModule [ "hardware" "package234" ] [ "hardware" "graphics" ])
+  (mkRenamedOptionModule [
+    "services"
+    "xserver"
+    "displayManager"
+    "sddm"
+    "enable"
+  ] [ "services" "displayManager" "sddm" "enable" ])
+  (map (buildAllowCommand "allow" [ "snapshot" "mount" "destroy" ]))
+  (map (x: "${x} ${escapeShellArgs [ stateDir workDir logsDir ]}") [
+    "+${unconfigureRunner}" # runs as root
+    configureRunner
+    setupWorkDir
+  ])
+  (lib.checkListOfEnum "${pname}: theme accent"
+    [
+      "Blue"
+      "Flamingo"
+      "Green"
+    ]
+    [ accent ]
+    lib.checkListOfEnum
+    "${pname}: color variant"
+    [ "Latte" "Frappe" "Macchiato" "Mocha" ]
+    [ variant ]
+  )
+  (lib.switch [ coq.coq-version ssreflect.version ] [
+    {
+      cases = [
+        (lib.versions.range "8.15" "8.20")
+        lib.pred.true
+      ];
+      out = "2.0.4";
+    }
+    {
+      cases = [
+        "8.5"
+        lib.pred.true
+      ];
+      out = "20170512";
+    }
+  ] null)
+]

test/diff/apply_with_lists/out-pure.nix

@@ -0,0 +1,131 @@
+# This file contains an assortment of test cases involving list-heavy function calls
+
+[
+  (f [ ] [ rhs lhs ])
+  (lib.mkMerge [
+    false
+    false
+  ])
+  (replaceStrings
+    [ "\${" "''" ]
+    #force multiline
+    [ "''\${" "'''" ]
+  )
+  (replaceStrings [ ''"'' "\\" ] [ ''\"'' "\\\\" ] name)
+  (replaceStrings
+    [ ''"'' "\\" ]
+    # force multiline
+    [ ''\"'' "\\\\" ]
+    name
+  )
+  (replaceStrings
+    [
+      "@"
+      ":"
+      "\\"
+      "["
+      "]"
+    ]
+    [
+      "-"
+      "-"
+      "-"
+      ""
+      ""
+    ]
+  )
+  (lists.removePrefix
+    [
+      1
+      2
+    ]
+    [ ]
+  )
+  (lists.removePrefix aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+    [
+      1
+      2
+    ]
+    [ ]
+  )
+  (builtins.replaceStrings [ "@NIX_STORE_VERITY@" ] [ partitionTypes.usr-verity ]
+    (builtins.readFile ./assert_uki_repart_match.py)
+  )
+  (replaceStrings [ "-" ] [ "_" ] (toUpper final.rust.cargoShortTarget))
+  (lib.mkChangedOptionModule
+    [ "security" "acme" "validMin" ]
+    [ "security" "acme" "defaults" "validMinDays" ]
+    (config: config.security.acme.validMin / (24 * 3600))
+  )
+  (lib.replaceStrings [ "https://registry" ".io/providers" ] [ "registry" ".io" ]
+    homepage
+  )
+  (lib.mkRenamedOptionModule [ "boot" "extraTTYs" ] [ "console" "extraTTYs" ])
+  # This line is engineered to exactly hit the line length limit
+  (lib.mkRenamedOptionModule
+    [ "hardware" "package234" ]
+    [ "hardware" "graphics" ]
+  )
+  (mkRenamedOptionModule
+    [
+      "services"
+      "xserver"
+      "displayManager"
+      "sddm"
+      "enable"
+    ]
+    [ "services" "displayManager" "sddm" "enable" ]
+  )
+  (map (
+    buildAllowCommand "allow" [
+      "snapshot"
+      "mount"
+      "destroy"
+    ]
+  ))
+  (map
+    (
+      x:
+      "${x} ${
+        escapeShellArgs [
+          stateDir
+          workDir
+          logsDir
+        ]
+      }"
+    )
+    [
+      "+${unconfigureRunner}" # runs as root
+      configureRunner
+      setupWorkDir
+    ]
+  )
+  (lib.checkListOfEnum "${pname}: theme accent"
+    [ "Blue" "Flamingo" "Green" ]
+    [ accent ]
+    lib.checkListOfEnum
+    "${pname}: color variant"
+    [ "Latte" "Frappe" "Macchiato" "Mocha" ]
+    [ variant ]
+  )
+  (lib.switch
+    [ coq.coq-version ssreflect.version ]
+    [
+      {
+        cases = [
+          (lib.versions.range "8.15" "8.20")
+          lib.pred.true
+        ];
+        out = "2.0.4";
+      }
+      {
+        cases = [
+          "8.5"
+          lib.pred.true
+        ];
+        out = "20170512";
+      }
+    ]
+    null
+  )
+]