diff --git a/Cabal-syntax/src/Distribution/Fields/LexerMonad.hs b/Cabal-syntax/src/Distribution/Fields/LexerMonad.hs index ac414c18e31..5e505cd9dea 100644 --- a/Cabal-syntax/src/Distribution/Fields/LexerMonad.hs +++ b/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 diff --git a/Cabal-syntax/src/Distribution/Fields/Parser.hs b/Cabal-syntax/src/Distribution/Fields/Parser.hs index 9061117b04e..f1b3bd4b4c3 100644 --- a/Cabal-syntax/src/Distribution/Fields/Parser.hs +++ b/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 = diff --git a/Cabal-syntax/src/Distribution/Parsec/Warning.hs b/Cabal-syntax/src/Distribution/Parsec/Warning.hs index eedf5545cf7..266d361d4d3 100644 --- a/Cabal-syntax/src/Distribution/Parsec/Warning.hs +++ b/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) diff --git a/Cabal-tests/tests/CheckTests.hs b/Cabal-tests/tests/CheckTests.hs index 01fe7ae749f..ad9a93feebe 100644 --- a/Cabal-tests/tests/CheckTests.hs +++ b/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 diff --git a/Cabal-tests/tests/ParserTests/regressions/decreasing-indentation.cabal b/Cabal-tests/tests/ParserTests/regressions/decreasing-indentation.cabal new file mode 100644 index 00000000000..ae9297e088a --- /dev/null +++ b/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 +maintainer: Adam Wick +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 \ No newline at end of file diff --git a/Cabal-tests/tests/ParserTests/regressions/decreasing-indentation.check b/Cabal-tests/tests/ParserTests/regressions/decreasing-indentation.check new file mode 100644 index 00000000000..89fbc7db9c4 --- /dev/null +++ b/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/ diff --git a/changelog.d/inconsistent-indentation b/changelog.d/inconsistent-indentation new file mode 100644 index 00000000000..d900c61c4f3 --- /dev/null +++ b/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.