Allow imports in conditional sections

- Warn about missplaced import (on top level)
- Warn in trees

Author: phadej

Cabal/Cabal.cabal

@@ -84,6 +84,9 @@ extra-source-files:
   tests/ParserTests/regressions/bad-glob-syntax.check
   tests/ParserTests/regressions/cc-options-with-optimization.cabal
   tests/ParserTests/regressions/cc-options-with-optimization.check
+  tests/ParserTests/regressions/common-conditional.cabal
+  tests/ParserTests/regressions/common-conditional.expr
+  tests/ParserTests/regressions/common-conditional.format
   tests/ParserTests/regressions/common.cabal
   tests/ParserTests/regressions/common.expr
   tests/ParserTests/regressions/common.format

Cabal/Distribution/PackageDescription/Parsec.hs

@@ -419,30 +419,35 @@ parseFields v fields grammar = do
 
 warnInvalidSubsection :: Section Position -> ParseResult ()
 warnInvalidSubsection (MkSection (Name pos name) _ _) =
-    void (parseFailure pos $ "invalid subsection " ++ show name)
+    void $ parseFailure pos $ "invalid subsection " ++ show name
 
 parseCondTree
-    :: forall a c.
-       CabalSpecVersion
-    -> HasElif                  -- ^ accept @elif@
-    -> ParsecFieldGrammar' a  -- ^ grammar
-    -> (a -> c)                 -- ^ condition extractor
+    :: forall a. FromBuildInfo a
+    => CabalSpecVersion
+    -> HasElif                        -- ^ accept @elif@
+    -> ParsecFieldGrammar' a          -- ^ grammar
+    -> Map String CondTreeBuildInfo   -- ^ common stanzas
+    -> (a -> [Dependency])            -- ^ condition extractor
     -> [Field Position]
-    -> ParseResult (CondTree ConfVar c a)
-parseCondTree v hasElif grammar cond = go
+    -> ParseResult (CondTree ConfVar [Dependency] a)
+parseCondTree v hasElif grammar commonStanzas cond = go
   where
-    go fields = do
+    go fields0 = do
+        (fields, endo) <-
+            if v >= CabalSpecV3_0
+            then processImports v commonStanzas fields0
+            else traverse_ (warnImport v) fields0 >> return (fields0, id)
+
         let (fs, ss) = partitionFields fields
         x <- parseFieldGrammar v fs grammar
         branches <- concat <$> traverse parseIfs ss
-        return (CondNode x (cond x) branches) -- TODO: branches
+        return $ endo $ CondNode x (cond x) branches
 
-    parseIfs :: [Section Position] -> ParseResult [CondBranch ConfVar c a]
+    parseIfs :: [Section Position] -> ParseResult [CondBranch ConfVar [Dependency] a]
     parseIfs [] = return []
     parseIfs (MkSection (Name _ name) test fields : sections) | name == "if" = do
         test' <- parseConditionConfVar test
         fields' <- go fields
-        -- TODO: else
         (elseFields, sections') <- parseElseIfs sections
         return (CondBranch test' fields' elseFields : sections')
     parseIfs (MkSection (Name pos name) _ _ : sections) = do
@@ -451,7 +456,7 @@ parseCondTree v hasElif grammar cond = go
 
     parseElseIfs
         :: [Section Position]
-        -> ParseResult (Maybe (CondTree ConfVar c a), [CondBranch ConfVar c a])
+        -> ParseResult (Maybe (CondTree ConfVar [Dependency] a), [CondBranch ConfVar [Dependency] a])
     parseElseIfs [] = return (Nothing, [])
     parseElseIfs (MkSection (Name pos name) args fields : sections) | name == "else" = do
         unless (null args) $
@@ -460,10 +465,7 @@ parseCondTree v hasElif grammar cond = go
         sections' <- parseIfs sections
         return (Just elseFields, sections')
 
-
-
     parseElseIfs (MkSection (Name _ name) test fields : sections) | hasElif == HasElif, name == "elif" = do
-        -- TODO: check cabal-version
         test' <- parseConditionConfVar test
         fields' <- go fields
         (elseFields, sections') <- parseElseIfs sections
@@ -560,21 +562,31 @@ parseCondTreeWithCommonStanzas
     -> Map String CondTreeBuildInfo  -- ^ common stanzas
     -> [Field Position]
     -> ParseResult (CondTree ConfVar [Dependency] a)
-parseCondTreeWithCommonStanzas v grammar commonStanzas = goImports []
+parseCondTreeWithCommonStanzas v grammar commonStanzas fields = do
+    (fields', endo) <- processImports v commonStanzas fields
+    x <- parseCondTree v hasElif grammar commonStanzas (view L.targetBuildDepends) fields'
+    return (endo x)
   where
     hasElif = specHasElif v
+
+processImports
+    :: forall a. FromBuildInfo a
+    => CabalSpecVersion
+    -> Map String CondTreeBuildInfo  -- ^ common stanzas
+    -> [Field Position]
+    -> ParseResult ([Field Position], CondTree ConfVar [Dependency] a -> CondTree ConfVar [Dependency] a)
+processImports v commonStanzas = go []
+  where
     hasCommonStanzas = specHasCommonStanzas v
 
     getList' :: List CommaFSep Token String -> [String]
     getList' = Newtype.unpack
 
-    -- parse leading imports
-    -- not supported:
-    goImports acc (Field (Name pos name) _ : fields) | name == "import", hasCommonStanzas == NoCommonStanzas = do
+    go acc (Field (Name pos name) _ : fields) | name == "import", hasCommonStanzas == NoCommonStanzas = do
         parseWarning pos PWTUnknownField "Unknown field: import. You should set cabal-version: 2.2 or larger to use common stanzas"
-        goImports acc fields
+        go acc fields
     -- supported:
-    goImports acc (Field (Name pos name) fls : fields) | name == "import" = do
+    go acc (Field (Name pos name) fls : fields) | name == "import" = do
         names <- getList' <$> runFieldParser pos parsec v fls
         names' <- for names $ \commonName ->
             case Map.lookup commonName commonStanzas of
@@ -584,16 +596,19 @@ parseCondTreeWithCommonStanzas v grammar commonStanzas = goImports []
                 Just commonTree ->
                     pure (Just commonTree)
 
-        goImports (acc ++ catMaybes names') fields
+        go (acc ++ catMaybes names') fields
 
-    -- Go to parsing condTree after first non-import 'Field'.
-    goImports acc fields = go acc fields
+    -- TODO: and filter!
+    go acc fields = do
+        traverse_ (warnImport v) fields
+        return (fields, \x -> foldr mergeCommonStanza x acc)
 
-    -- parse actual CondTree
-    go :: [CondTreeBuildInfo] -> [Field Position] -> ParseResult (CondTree ConfVar [Dependency] a)
-    go bis fields = do
-        x <- parseCondTree v hasElif grammar (view L.targetBuildDepends) fields
-        pure $ foldr mergeCommonStanza x bis
+warnImport :: CabalSpecVersion -> Field Position -> ParseResult ()
+warnImport v (Field (Name pos name) _) | name ==  "import" =
+    if specHasCommonStanzas v == NoCommonStanzas
+    then parseWarning pos PWTUnknownField "Unknown field: import. You should set cabal-version: 2.2 or larger to use common stanzas"
+    else parseWarning pos PWTUnknownField "Unknown field: import. Common stanza imports should be at the top of the enclosing section"
+warnImport _ _ = pure ()
 
 mergeCommonStanza
     :: forall a. FromBuildInfo a

Cabal/tests/ParserTests.hs

@@ -140,6 +140,7 @@ regressionTests = testGroup "regressions"
     , regressionTest "shake.cabal"
     , regressionTest "common.cabal"
     , regressionTest "common2.cabal"
+    , regressionTest "common-conditional.cabal"
     , regressionTest "leading-comma.cabal"
     , regressionTest "wl-pprint-indef.cabal"
     , regressionTest "th-lift-instances.cabal"

Cabal/tests/ParserTests/regressions/common-conditional.cabal

@@ -0,0 +1,49 @@
+cabal-version:       2.6
+name:                common-conditional
+version:             0
+synopsis:            Common-stanza demo demo
+build-type:          Simple
+
+source-repository head
+  Type:     git
+  Location: https://github.com/hvr/-.git
+
+flag foo
+  manual: True
+  default: True
+
+common win-dows
+  if os(windows)
+    build-depends: Win32
+
+common deps
+  import: win-dows
+  buildable: True
+  build-depends:
+    base >=4.10 && <4.11,
+    containers
+
+library
+  if flag(foo)
+    import: deps
+
+  default-language: Haskell2010
+  exposed-modules:  ElseIf
+
+  build-depends:
+    ghc-prim
+
+test-suite tests
+  -- buildable fields verify that we don't have duplicate field warnings
+  buildable: True
+  if os(windows)
+    buildable: False
+
+  if flag(foo)
+    import: deps, win-dows
+
+  type: exitcode-stdio-1.0
+  main-is: Tests.hs
+
+  build-depends:
+    HUnit