Fix #604 Where applicable, use elif in Cabal files

Author: mpilgrem

CHANGELOG.md

@@ -1,3 +1,7 @@
+## Changes in 0.39.0
+  - Where applicable, use `elif` in Cabal files (introduced in
+    `cabal-version: 2.2`).
+
 ## Changes in 0.38.0
   - Generate `build-tool-depends` instead of `build-tools` starting with
     `cabal-version: 2` (fixes #596)

README.md

@@ -477,6 +477,35 @@ becomes
     else
       ghc-options: -O0
 
+Conditionals with a `else` field that contains only one `when` field will make
+use of `elif` in Cabal files (introduced in `cabal-version: 2.2`).
+
+For example,
+
+    when:
+      - condition: os(windows)
+        then:
+          source-dirs: windows
+        else:
+          when:
+            - condition: "os(darwin) || os(linux)"
+              then:
+                source-dirs: unix-like
+              else:
+                source-dirs: unsupported-os
+
+becomes
+
+    if os(windows)
+      hs-source-dirs:
+          windows
+    elif os(darwin) || os(linux)
+      hs-source-dirs:
+          unix-like
+    else
+      hs-source-dirs:
+          unsupported-os
+
 **Note:** Conditionals with `condition: false` are omitted from the generated
 `.cabal` file.
 

hpack.cabal

@@ -1,6 +1,6 @@
 spec-version: 0.36.0
 name: hpack
-version: 0.38.0
+version: 0.39.0
 synopsis: A modern format for Haskell packages
 description: See README at <https://github.com/sol/hpack#readme>
 author: Simon Hengel <sol@typeful.net>

package.yaml

@@ -58,7 +58,9 @@ module Hpack.Config (
 , VerbatimValue(..)
 , verbatimValueToString
 , CustomSetup(..)
+, HasEmpty(..)
 , Section(..)
+, maybeSectionAConditional
 , Library(..)
 , Executable(..)
 , Conditional(..)
@@ -527,6 +529,9 @@ instance Monoid Empty where
   mempty = Empty
   mappend = (<>)
 
+instance HasEmpty Empty where
+  empty = Empty
+
 instance Semigroup Empty where
   Empty <> Empty = Empty
 
@@ -868,7 +873,7 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
     executableHasGeneratedModules :: Section Executable -> Bool
     executableHasGeneratedModules = any (not . null . executableGeneratedModules)
 
-    sectionCabalVersion :: (Section a -> [Module]) -> Section a -> Maybe CabalVersion
+    sectionCabalVersion :: (Eq a, HasEmpty a) => (Section a -> [Module]) -> Section a -> Maybe CabalVersion
     sectionCabalVersion getMentionedModules sect = maximum $ [
         makeVersion [2,2] <$ guard (sectionSatisfies (not . null . sectionCxxSources) sect)
       , makeVersion [2,2] <$ guard (sectionSatisfies (not . null . sectionCxxOptions) sect)
@@ -880,6 +885,7 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
               uses "RebindableSyntax"
           && (uses "OverloadedStrings" || uses "OverloadedLists")
           && pathsModule `elem` getMentionedModules sect)
+      , makeVersion [2,2] <$ guard (sectionSatisfies (isJust . maybeSectionAConditional) sect)
       ] ++ map versionFromSystemBuildTool systemBuildTools
       where
         defaultExtensions :: [String]
@@ -1040,6 +1046,10 @@ data CustomSetup = CustomSetup {
   customSetupDependencies :: Dependencies
 } deriving (Eq, Show)
 
+-- | A class for types that have a value corresponding to being \'empty\'.
+class HasEmpty a where
+  empty :: a
+
 data Library = Library {
   libraryExposed :: Maybe Bool
 , libraryVisibility :: Maybe String
@@ -1050,12 +1060,30 @@ data Library = Library {
 , librarySignatures :: [String]
 } deriving (Eq, Show)
 
+instance HasEmpty Library where
+  empty = Library
+    { libraryExposed = Nothing
+    , libraryVisibility = Nothing
+    , libraryExposedModules = []
+    , libraryOtherModules = []
+    , libraryGeneratedModules = []
+    , libraryReexportedModules = []
+    , librarySignatures = []
+    }
+
 data Executable = Executable {
   executableMain :: Maybe FilePath
 , executableOtherModules :: [Module]
 , executableGeneratedModules :: [Module]
 } deriving (Eq, Show)
 
+instance HasEmpty Executable where
+  empty = Executable
+    { executableMain = Nothing
+    , executableOtherModules = []
+    , executableGeneratedModules = []
+    }
+
 data BuildTool = BuildTool String String | LocalBuildTool String
   deriving (Show, Eq, Ord)
 
@@ -1093,6 +1121,41 @@ data Section a = Section {
 , sectionVerbatim :: [Verbatim]
 } deriving (Eq, Show, Functor, Foldable, Traversable)
 
+instance HasEmpty a => HasEmpty (Section a) where
+  empty = Section
+    { sectionData = Hpack.Config.empty
+    , sectionSourceDirs = []
+    , sectionDependencies = mempty
+    , sectionPkgConfigDependencies = []
+    , sectionDefaultExtensions = []
+    , sectionOtherExtensions = []
+    , sectionLanguage = Nothing
+    , sectionGhcOptions = []
+    , sectionGhcProfOptions = []
+    , sectionGhcSharedOptions = []
+    , sectionGhcjsOptions = []
+    , sectionCppOptions = []
+    , sectionAsmOptions = []
+    , sectionAsmSources = []
+    , sectionCcOptions = []
+    , sectionCSources = []
+    , sectionCxxOptions = []
+    , sectionCxxSources = []
+    , sectionJsSources = []
+    , sectionExtraLibDirs = []
+    , sectionExtraLibraries = []
+    , sectionExtraFrameworksDirs = []
+    , sectionFrameworks = []
+    , sectionIncludeDirs = []
+    , sectionInstallIncludes = []
+    , sectionLdOptions = []
+    , sectionBuildable = Nothing
+    , sectionConditionals = []
+    , sectionBuildTools = mempty
+    , sectionSystemBuildTools = mempty
+    , sectionVerbatim = []
+    }
+
 data Conditional a = Conditional {
   conditionalCondition :: Cond
 , conditionalThen :: a
@@ -1657,3 +1720,12 @@ pathsModuleFromPackageName name = Module ("Paths_" ++ map f name)
   where
     f '-' = '_'
     f x = x
+
+-- | If the given section contains only a single conditional, yields just that
+-- conditional, otherwise 'Nothing'.
+maybeSectionAConditional :: (Eq a, HasEmpty a) => Section a -> Maybe (Conditional (Section a))
+maybeSectionAConditional s@(Section { sectionConditionals = [c] }) =
+  if s == Hpack.Config.empty { sectionConditionals = sectionConditionals s }
+    then Just c
+    else Nothing
+maybeSectionAConditional _ = Nothing

src/Hpack/Config.hs

@@ -240,7 +240,7 @@ renderExposed = Field "exposed" . Literal . show
 renderVisibility :: String -> Element
 renderVisibility = Field "visibility" . Literal
 
-renderSection :: (a -> [Element]) -> [Element] -> Section a -> RenderM [Element]
+renderSection :: (Eq a, HasEmpty a) => (a -> [Element]) -> [Element] -> Section a -> RenderM [Element]
 renderSection renderSectionData extraFieldsStart Section{..} = do
   buildTools <- renderBuildTools sectionBuildTools sectionSystemBuildTools
   conditionals <- traverse (renderConditional renderSectionData) sectionConditionals
@@ -307,12 +307,18 @@ renderVerbatimObject = map renderPair . Map.toList
       [x] -> Field key (Literal x)
       xs -> Field key (LineSeparatedList xs)
 
-renderConditional :: (a -> [Element]) -> Conditional (Section a) -> RenderM Element
-renderConditional renderSectionData (Conditional condition sect mElse) = case mElse of
-  Nothing -> if_
-  Just else_ -> Group <$> if_ <*> (Stanza "else" <$> renderSection renderSectionData [] else_)
+renderConditional :: (Eq a, HasEmpty a) => (a -> [Element]) -> Conditional (Section a) -> RenderM Element
+renderConditional = renderConditional' "if "
   where
-    if_ = Stanza ("if " ++ renderCond condition) <$> renderSection renderSectionData [] sect
+    renderConditional' stanza renderSectionData (Conditional condition sect mElse) = case mElse of
+      Nothing -> stanza_
+      Just else_ -> Group <$> stanza_ <*> case maybeSectionAConditional else_ of
+        Just conditional ->
+          renderConditional' "elif " renderSectionData conditional
+        Nothing ->
+          Stanza "else" <$> renderSection renderSectionData [] else_
+      where
+        stanza_ = Stanza (stanza ++ renderCond condition) <$> renderSection renderSectionData [] sect
 
 renderCond :: Cond -> String
 renderCond = \ case

src/Hpack/Render.hs

@@ -1802,6 +1802,36 @@ spec = around_ (inTempDirectoryNamed "my-package") $ do
             build-depends:
                 unix
           |]
+        it "uses elif when applicable and infers cabal-version 2.2" $ do
+          [i|
+          when:
+            condition: os(windows)
+            then:
+              source-dirs: windows
+            else:
+              when:
+                condition: "os(darwin) || os(linux)"
+                then:
+                  source-dirs: unix-like
+                else:
+                  source-dirs: unsupported-os
+          executable: {}
+          |] `shouldRenderTo` (executable "my-package" [i|
+          other-modules:
+              Paths_my_package
+          autogen-modules:
+              Paths_my_package
+          default-language: Haskell2010
+          if os(windows)
+            hs-source-dirs:
+                windows
+          elif os(darwin) || os(linux)
+            hs-source-dirs:
+                unix-like
+          else
+            hs-source-dirs:
+                unsupported-os
+          |]) {packageCabalVersion = "2.2"}
 
         context "with empty then-branch" $ do
           it "provides a hint" $ do

test/EndToEndSpec.hs

@@ -246,6 +246,21 @@ spec = do
         , "      unix"
         ]
 
+    it "renders conditionals with else-branch using elif when applicable" $ do
+      let conditional = Conditional "os(windows)" (section Empty) {sectionSourceDirs = ["windows"]} $ Just (section Empty) {sectionConditionals = [innerConditional]}
+          innerConditional = Conditional "os(darwin) || os(linux)" (section Empty) {sectionSourceDirs = ["unix-like"]} $ Just (section Empty) {sectionSourceDirs = ["unsupported-os"]}
+      render defaultRenderSettings 0 (run $ renderConditional renderEmptySection conditional) `shouldBe` [
+          "if os(windows)"
+        , "  hs-source-dirs:"
+        , "      windows"
+        , "elif os(darwin) || os(linux)"
+        , "  hs-source-dirs:"
+        , "      unix-like"
+        , "else"
+        , "  hs-source-dirs:"
+        , "      unsupported-os"
+        ]
+
     it "renders nested conditionals" $ do
       let conditional = Conditional "arch(i386)" (section Empty) {sectionGhcOptions = ["-threaded"], sectionConditionals = [innerConditional]} Nothing
           innerConditional = Conditional "os(windows)" (section Empty) {sectionDependencies = deps ["Win32"]} Nothing