Make readFields warn about inconsistent indentation.

This is affect of using indentOfAtLeast method:
any indentation greater than current offset is fine.

That behavior is desirable to parsing multiline field contents,
but it is a bit surprising for fields, which we expect to be aligned.

Such insonsistency seems to be always a mistake, and it's easy to fix once
a machine points it out.

Cabal-syntax/src/Distribution/Fields/LexerMonad.hs

@@ -35,7 +35,7 @@ module Distribution.Fields.LexerMonad (
 import qualified Data.ByteString              as B
 import qualified Data.List.NonEmpty           as NE
 import           Distribution.Compat.Prelude
-import           Distribution.Parsec.Position (Position (..), showPos)
+import           Distribution.Parsec.Position (Position (..), showPos, positionRow)
 import           Distribution.Parsec.Warning  (PWarnType (..), PWarning (..))
 import           Prelude ()
 
@@ -65,9 +65,10 @@ instance Monad Lex where
 data LexResult a = LexResult {-# UNPACK #-} !LexState a
 
 data LexWarningType
-    = LexWarningNBSP  -- ^ Encountered non breaking space
-    | LexWarningBOM   -- ^ BOM at the start of the cabal file
-    | LexWarningTab   -- ^ Leading tags
+    = LexWarningNBSP             -- ^ Encountered non breaking space
+    | LexWarningBOM              -- ^ BOM at the start of the cabal file
+    | LexWarningTab              -- ^ Leading tags
+    | LexInconsistentIndentation -- ^ indentation decreases
   deriving (Eq, Ord, Show)
 
 data LexWarning = LexWarning                !LexWarningType
@@ -78,7 +79,7 @@ toPWarnings :: [LexWarning] -> [PWarning]
 toPWarnings
     = map (uncurry toWarning)
     . Map.toList
-    . Map.fromListWith (<>)
+    . Map.fromListWith (flip (<>)) -- fromListWith gives existing element first.
     . map (\(LexWarning t p) -> (t, pure p))
   where
     toWarning LexWarningBOM poss =
@@ -87,6 +88,8 @@ toPWarnings
         PWarning PWTLexNBSP (NE.head poss) $ "Non breaking spaces at " ++ intercalate ", " (NE.toList $ fmap showPos poss)
     toWarning LexWarningTab poss =
         PWarning PWTLexTab (NE.head poss) $ "Tabs used as indentation at " ++ intercalate ", " (NE.toList $ fmap showPos poss)
+    toWarning LexInconsistentIndentation poss =
+        PWarning PWTInconsistentIndentation (NE.head poss) $ "Inconsistent indentation at lines " ++ intercalate ", " (NE.toList $ fmap (show . positionRow) poss)
 
 data LexState = LexState {
         curPos   :: {-# UNPACK #-} !Position,        -- ^ position at current input location

Cabal-syntax/src/Distribution/Fields/Parser.hs

@@ -33,8 +33,8 @@ import           Distribution.Compat.Prelude
 import           Distribution.Fields.Field
 import           Distribution.Fields.Lexer
 import           Distribution.Fields.LexerMonad
-                 (LexResult (..), LexState (..), LexWarning (..), unLex)
-import           Distribution.Parsec.Position   (Position (..))
+                 (LexResult (..), LexState (..), LexWarning (..), LexWarningType (..), unLex)
+import           Distribution.Parsec.Position   (Position (..), positionCol)
 import           Prelude ()
 import           Text.Parsec.Combinator         hiding (eof, notFollowedBy)
 import           Text.Parsec.Error
@@ -319,10 +319,36 @@ readFields' s = do
     parser = do
         fields <- cabalStyleFile
         ws     <- getLexerWarnings
-        pure (fields, ws)
+        pure (fields, ws ++ checkIndentation fields [])
 
     lexSt = mkLexState' (mkLexState s)
 
+-- | Check (recursively) that all fields inside a block are indented the same.
+--
+-- We have to do this as a post-processing check.
+-- As the parser uses indentOfAtLeast approach, we don't know what is the "correct"
+-- indentation for following fields.
+--
+-- To catch during parsing we would need to parse first field/section of a section
+-- and then parse the following ones (softly) requiring the exactly the same indentation.
+--
+checkIndentation :: [Field Position] -> [LexWarning] -> [LexWarning]
+checkIndentation []                        = id
+checkIndentation (Field   name _    : fs') =                       checkIndentation' (nameAnn name) fs'
+checkIndentation (Section name _ fs : fs') = checkIndentation fs . checkIndentation' (nameAnn name) fs'
+
+-- | First field/section determines expected indentation for this block.
+checkIndentation' :: Position -> [Field Position] -> [LexWarning] -> [LexWarning]
+checkIndentation' _   []                        = id
+checkIndentation' pos (Field   name _    : fs') = checkIndentation'' pos (nameAnn name) .                       checkIndentation' pos fs'
+checkIndentation' pos (Section name _ fs : fs') = checkIndentation'' pos (nameAnn name) . checkIndentation fs . checkIndentation' pos fs'
+
+-- | Check that positions' columns are the same.
+checkIndentation'' :: Position -> Position -> [LexWarning] -> [LexWarning]
+checkIndentation'' a b
+    | positionCol a == positionCol b = id
+    | otherwise                      = (LexWarning LexInconsistentIndentation b :)
+
 #ifdef CABAL_PARSEC_DEBUG
 parseTest' :: Show a => Parsec LexState' () a -> SourceName -> B8.ByteString -> IO ()
 parseTest' p fname s =

Cabal-syntax/src/Distribution/Parsec/Warning.hs

@@ -43,6 +43,8 @@ data PWarnType
 
     | PWTEmptyFilePath         -- ^ Empty filepath, i.e. literally ""
 
+    | PWTInconsistentIndentation -- ^ sections contents (sections and fields) are indented inconsistently
+
     | PWTExperimental          -- ^ Experimental feature
     deriving (Eq, Ord, Show, Enum, Bounded, Generic)
 

Cabal-tests/tests/CheckTests.hs

@@ -57,6 +57,7 @@ checkTests = testGroup "regressions"
     , checkTest "issue-7776-b.cabal"
     , checkTest "issue-7776-c.cabal"
     , checkTest "issue-8646.cabal"
+    , checkTest "decreasing-indentation.cabal"
     ]
 
 checkTest :: FilePath -> TestTree

Cabal-tests/tests/ParserTests/regressions/decreasing-indentation.cabal

@@ -0,0 +1,73 @@
+name:       RSA
+category:   Cryptography, Codec
+version:    1.0.0
+license:    BSD3
+license-file: LICENSE
+author:     Adam Wick <awick@galois.com>
+maintainer: Adam Wick <awick@galois.com>
+stability:  stable
+build-type: Simple
+cabal-version: >= 1.2
+tested-with: GHC ==6.8.0
+synopsis: Implementation of RSA, using the padding schemes of PKCS#1 v2.1.
+description: This library implements the RSA encryption and signature 
+             algorithms for arbitrarily-sized ByteStrings. While the 
+             implementations work, they are not necessarily the fastest ones
+             on the planet. Particularly key generation. The algorithms
+             included are based of RFC 3447, or the Public-Key Cryptography
+             Standard for RSA, version 2.1 (a.k.a, PKCS#1 v2.1).   
+
+Flag IncludeMD5
+  Description: Include support for using MD5 in the various crypto routines.
+
+Flag UseBinary
+  Description: Use the binary package for serializing keys.
+
+Library
+ build-depends: base >= 3, bytestring, SHA, random
+ GHC-Options: -O2 -Wall -fno-ignore-asserts -fno-warn-orphans 
+ if flag(UseBinary)
+   build-depends: binary
+   CPP-Options: -DUSE_BINARY
+ if flag(IncludeMD5) && flag(UseBinary)
+   build-depends: pureMD5
+   CPP-Options: -DINCLUDE_MD5
+ exposed-modules: Codec.Crypto.RSA
+ extensions: CPP, BangPatterns, PatternSignatures
+
+Executable test_rsa
+    build-depends: base >= 3, bytestring, QuickCheck, SHA
+  GHC-Options: -O2 -Wall -fno-ignore-asserts -fno-warn-orphans 
+  CPP-Options: -DRSA_TEST
+  Main-Is: Test.hs
+  Other-Modules: Codec.Crypto.RSA
+  extensions: CPP, BangPatterns, PatternSignatures
+
+-- The above is actual RSA-1.0.0 cabal file. The following up is just some weird stuff
+-- Note that here main-is is part of GHC-Options field.
+Executable warnings
+    build-depends: base < 5
+  GHC-Options: -Wall
+    main-is: warnings.hs
+  Other-Modules: FooBar
+
+-- Increasing indentation is also possible if we use braces to delimit field contents.
+Executable warnings2
+  build-depends: { base <5 }
+    main-is: { warnings2.hs }
+      Other-Modules: FooBar
+
+-- another common mistake is something like below,
+-- where a sub-section is over-indented
+flag splitBase
+
+Executable warnings3
+    if flag(splitBase)
+        build-depends: base >= 3, parsec, array, directory, old-time
+    else
+        build-depends: base <  3, haskell98, parsec
+
+ Main-Is: warnings3.hs 
+ Other-Modules:
+  Graphics.UI.WXCore
+  Graphics.UI.WXCore.Wx

Cabal-tests/tests/ParserTests/regressions/decreasing-indentation.check

@@ -0,0 +1,16 @@
+decreasing-indentation.cabal:40:3: Inconsistent indentation at lines 40, 41, 42, 43, 44, 50, 52, 57, 58, 70, 71
+No 'main-is' field found for executable warnings
+Deprecated extensions: 'PatternSignatures'. Instead of 'PatternSignatures' use 'ScopedTypeVariables'.
+'ghc-options: -O2' is rarely needed. Check that it is giving a real benefit and not just imposing longer compile times on your users.
+These packages miss upper bounds:
+  - QuickCheck
+  - SHA
+  - array
+  - binary
+  - bytestring
+  - directory
+  - old-time
+  - parsec
+  - pureMD5
+  - random
+Please add them, using `cabal gen-bounds` for suggestions. For more information see:  https://pvp.haskell.org/

changelog.d/inconsistent-indentation

@@ -0,0 +1,12 @@
+synopsis: Warn about inconsistent indentation
+packages: Cabal-syntax
+prs: #TODO
+
+description:
+    Make readFields warn about inconsistent indentation.
+
+    This is affect of using indentOfAtLeast method: any indentation greater than current offset is fine.
+
+    That behavior is desirable to parsing multiline field contents, but it is a bit surprising for fields, which we expect to be aligned.
+
+    Such insonsistency seems to be always a mistake, and it's easy to fix once a machine points it out.