feat(Cabal-syntax): Accept a trailing comma in comma-separated list fields

Cabal-syntax/src/Distribution/Parsec.hs

@@ -332,15 +332,23 @@ parsecLeadingCommaNonEmpty p = do
     lp = p <* P.spaces
     comma = P.char ',' *> P.spaces P.<?> "comma"
 
+-- | List of items separated by optional commas. A trailing comma is
+-- accepted, a leading comma is not.
+--
+-- @
+-- p*              -- no commas: many p
+-- p (comma p)*    -- p \`sepBy\` comma
+-- (p comma)*      -- trailing comma is accepted
+-- @
 parsecOptCommaList :: CabalParsing m => m a -> m [a]
-parsecOptCommaList p = P.sepBy (p <* P.spaces) (P.optional comma)
+parsecOptCommaList p = P.sepEndBy (p <* P.spaces) (P.optional comma)
   where
     comma = P.char ',' *> P.spaces
 
 -- | Like 'parsecOptCommaList' but
 --
 -- * require all or none commas
--- * accept leading or trailing comma.
+-- * additionally accept a leading comma.
 --
 -- @
 -- p (comma p)*  -- p `sepBy` comma

Cabal-tests/Cabal-tests.cabal

@@ -35,6 +35,7 @@ test-suite unit-tests
     UnitTests.Distribution.Compat.Time
     UnitTests.Distribution.Described
     UnitTests.Distribution.PackageDescription.Check
+    UnitTests.Distribution.Parsec
     UnitTests.Distribution.PkgconfigVersion
     UnitTests.Distribution.Simple.Command
     UnitTests.Distribution.Simple.Glob

Cabal-tests/tests/ParserTests.hs

@@ -115,7 +115,6 @@ errorTests = testGroup "errors"
     , errorTest "common3.cabal"
     , errorTest "leading-comma.cabal"
     , errorTest "leading-comma-2.cabal"
-    , errorTest "leading-comma-2b.cabal"
     , errorTest "leading-comma-2c.cabal"
     , errorTest "range-ge-wild.cabal"
     , errorTest "forward-compat.cabal"

Cabal-tests/tests/UnitTests.hs

@@ -25,6 +25,7 @@ import qualified UnitTests.Distribution.Utils.ShortText
 import qualified UnitTests.Distribution.Utils.Structured
 import qualified UnitTests.Distribution.Version (versionTests)
 import qualified UnitTests.Distribution.PkgconfigVersion (pkgconfigVersionTests)
+import qualified UnitTests.Distribution.Parsec
 import qualified UnitTests.Distribution.SPDX (spdxTests)
 import qualified UnitTests.Distribution.Described
 import qualified UnitTests.Distribution.CabalSpecVersion
@@ -67,6 +68,8 @@ tests =
         UnitTests.Distribution.Version.versionTests
     , testGroup "Distribution.Types.PkgconfigVersion(Range)"
         UnitTests.Distribution.PkgconfigVersion.pkgconfigVersionTests
+    , testGroup "Distribution.Parsec"
+        UnitTests.Distribution.Parsec.tests
     , testGroup "Distribution.SPDX"
         UnitTests.Distribution.SPDX.spdxTests
     , UnitTests.Distribution.Utils.CharSet.tests

Cabal-tests/tests/UnitTests/Distribution/Parsec.hs

@@ -0,0 +1,39 @@
+module UnitTests.Distribution.Parsec (tests) where
+
+import Distribution.Compat.Prelude
+import Prelude ()
+
+import Distribution.CabalSpecVersion
+import Distribution.Parsec
+  ( explicitEitherParsec'
+  , parsecOptCommaList
+  , parsecToken
+  )
+
+import Test.Tasty
+import Test.Tasty.HUnit
+
+tests :: [TestTree]
+tests =
+  [ testGroup "parsecOptCommaList"
+        [ testCase "trailing comma" $
+            parse "a, b," @?= Right ["a", "b"]
+        , testCase "trailing comma without spaces" $
+            parse "a,b," @?= Right ["a", "b"]
+        , testCase "no commas" $
+            parse "a b" @?= Right ["a", "b"]
+        , testCase "mixed commas" $
+            parse "a, b c" @?= Right ["a", "b", "c"]
+        , testCase "single item with trailing comma" $
+            parse "a," @?= Right ["a"]
+        , testCase "empty" $
+            parse "" @?= Right []
+        , testCase "leading comma is rejected" $
+            case parse ", a" of
+              Left _ -> pure ()
+              Right xs -> assertFailure $ "unexpectedly parsed: " ++ show xs
+        ]
+    ]
+  where
+    parse :: String -> Either String [String]
+    parse = explicitEitherParsec' CabalSpecV2_4 (parsecOptCommaList parsecToken)

cabal-testsuite/PackageTests/Regression/T10213/T10213.hs

@@ -0,0 +1 @@
+module T10213 where

cabal-testsuite/PackageTests/Regression/T10213/cabal.project

@@ -0,0 +1 @@
+packages: .

cabal-testsuite/PackageTests/Regression/T10213/cabal.test.hs

@@ -0,0 +1,35 @@
+import Test.Cabal.Prelude
+
+-- Regression test for #10213: parsing of @extra-lib-dirs@ (and other
+-- comma-separated list fields) must accept a trailing comma. Before the
+-- fix, a trailing comma caused a parse error for @cabal-version < 3.0@.
+main = cabalTest $ recordMode DoNotRecord $ do
+  env <- getTestEnv
+  let dir = testCurrentDir env
+      lib1 = dir </> "foo-lib"
+      lib2 = dir </> "bar-extra"
+
+  -- @extra-lib-dirs@ requires absolute paths, so we have to write the
+  -- @.cabal@ file at runtime rather than committing absolute paths.
+  liftIO $ writeFile (dir </> "t10213.cabal") $ unlines
+    [ "cabal-version: 2.4"
+    , "name:          t10213"
+    , "version:       0"
+    , "build-type:    Simple"
+    , ""
+    , "library"
+    , "  build-depends:    base"
+    , "  default-language: Haskell2010"
+    , "  exposed-modules:  T10213"
+    , "  extra-lib-dirs:   " ++ lib1 ++ ","
+    , "                    " ++ lib2 ++ ","
+    ]
+
+  res <- cabalG' [] "build" ["-v3"]
+
+  -- The trailing commas must not be merged into the paths: the two
+  -- directories should be parsed as two separate entries.
+  assertOutputContains "foo-lib" res
+  assertOutputContains "bar-extra" res
+  assertOutputDoesNotContain "foo-lib," res
+  assertOutputDoesNotContain "bar-extra," res

cabal-testsuite/PackageTests/Regression/T10213/t10213.cabal

@@ -0,0 +1,11 @@
+cabal-version: 2.4
+name:          t10213
+version:       0
+build-type:    Simple
+
+library
+  build-depends:    base
+  default-language: Haskell2010
+  exposed-modules:  T10213
+  extra-lib-dirs:   foo-lib,
+                    bar-extra,

changelog.d/12264.md

@@ -0,0 +1,11 @@
+---
+synopsis: Accept a trailing comma in comma-separated list fields
+packages: [Cabal-syntax]
+issues: 10213
+prs: 12264
+---
+
+The `.cabal` file parser now accepts a trailing comma in comma-separated list
+fields (such as `extra-lib-dirs`, `exposed-modules`, `extensions`, etc.) even
+when `cabal-version` is below `3.0`. Previously a trailing comma caused a parse
+error; it is now handled consistently with `cabal-version: 3.0` and later.