From f112421b123a6b6bd741d35851786bdd8d4ffb2a Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Mon, 4 Nov 2024 14:48:50 -0800 Subject: [PATCH] Make a trailing colon for stanzas a parse failure Here's a mistake I make semi-regularly: source-repository-package: type: git location: https://github.com/parsonsmatt/foundation tag: 688c32ccd9a951bc96dd09423a6e6684f091d510 subdir: basement subdir: foundation Cabal treats this as a warning, so it prints: Warning: cabal.project: Unrecognized field 'source-repository-package' on line 52 This is fine (if you already know the mistake you've made, at least!), but it's very easy to miss amidst lots of output. I often re-run `cabal` when I see a ton of output to attempt to get a smaller error message. (Usually it works and I get an error message that's got less "compiling module such and such" noise in it.) However, re-running `cabal` will discard this warning entirely! Let's make it a hard error instead. This is a backwards-compatibility break. --- .../src/Distribution/Client/ParseUtils.hs | 20 +++++++---- .../src/Distribution/Deprecated/ParseUtils.hs | 5 +++ .../FieldStanzaConfusion/cabal.out | 4 +++ .../FieldStanzaConfusion/cabal.project | 6 ++++ .../FieldStanzaConfusion/cabal.test.hs | 6 ++++ .../FieldStanzaConfusion/src/MyLib.hs | 4 +++ .../FieldStanzaConfusion/test.cabal | 13 +++++++ changelog.d/pr-10525 | 34 +++++++++++++++++++ 8 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.out create mode 100644 cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.project create mode 100644 cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.test.hs create mode 100644 cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/src/MyLib.hs create mode 100644 cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/test.cabal create mode 100644 changelog.d/pr-10525 diff --git a/cabal-install/src/Distribution/Client/ParseUtils.hs b/cabal-install/src/Distribution/Client/ParseUtils.hs index 18062b7428f..96c702a2970 100644 --- a/cabal-install/src/Distribution/Client/ParseUtils.hs +++ b/cabal-install/src/Distribution/Client/ParseUtils.hs @@ -1,6 +1,8 @@ {-# LANGUAGE ExistentialQuantification #-} {-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE RankNTypes #-} +{-# LANGUAGE ScopedTypeVariables #-} +{-# LANGUAGE TypeApplications #-} ----------------------------------------------------------------------------- @@ -53,6 +55,7 @@ import Distribution.Deprecated.ParseUtils ( Field (..) , FieldDescr (..) , LineNo + , PError (..) , ParseResult (..) , liftField , lineNo @@ -292,13 +295,16 @@ parseFieldsAndSections fieldDescrs sectionDescrs fgSectionDescrs = setField a (F line name value) = case Map.lookup name fieldMap of Just (FieldDescr _ _ set) -> set line value a - Nothing -> do - warning $ - "Unrecognized field '" - ++ name - ++ "' on line " - ++ show line - return a + Nothing -> + case Left <$> Map.lookup name sectionMap <|> Right <$> Map.lookup name fgSectionMap of + Just _ -> ParseFailed $ FieldShouldBeStanza name line + Nothing -> do + warning $ + "Unrecognized field '" + ++ name + ++ "' on line " + ++ show line + return a setField a (Section line name param fields) = case Left <$> Map.lookup name sectionMap <|> Right <$> Map.lookup name fgSectionMap of Just (Left (SectionDescr _ fieldDescrs' sectionDescrs' _ set sectionEmpty)) -> do diff --git a/cabal-install/src/Distribution/Deprecated/ParseUtils.hs b/cabal-install/src/Distribution/Deprecated/ParseUtils.hs index e1d389ac9aa..4743213fde9 100644 --- a/cabal-install/src/Distribution/Deprecated/ParseUtils.hs +++ b/cabal-install/src/Distribution/Deprecated/ParseUtils.hs @@ -91,6 +91,7 @@ data PError = AmbiguousParse String LineNo | NoParse String LineNo | TabsError LineNo + | FieldShouldBeStanza String LineNo | FromString String (Maybe LineNo) deriving (Eq, Show) @@ -186,6 +187,10 @@ locatedErrorMsg (NoParse f n) = , "Parse of field '" ++ f ++ "' failed." ) locatedErrorMsg (TabsError n) = (Just n, "Tab used as indentation.") +locatedErrorMsg (FieldShouldBeStanza name lineNumber) = + ( Just lineNumber + , "'" ++ name ++ "' is a stanza, not a field. Remove the trailing ':' to parse a stanza." + ) locatedErrorMsg (FromString s n) = (n, s) syntaxError :: LineNo -> String -> ParseResult a diff --git a/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.out b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.out new file mode 100644 index 00000000000..60680b86db3 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.out @@ -0,0 +1,4 @@ +# cabal build +Error: [Cabal-7090] +Error parsing project file /cabal.project:4: +'source-repository-package' is a stanza, not a field. Remove the trailing ':' to parse a stanza. diff --git a/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.project b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.project new file mode 100644 index 00000000000..518ac39f5fb --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.project @@ -0,0 +1,6 @@ +packages: . + +-- This is an error; a trailing `:` is syntax for a field, not a stanza! +source-repository-package: + type: git + location: https://github.com/haskell-hvr/Only diff --git a/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.test.hs b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.test.hs new file mode 100644 index 00000000000..39636819157 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/cabal.test.hs @@ -0,0 +1,6 @@ +import Test.Cabal.Prelude + +main = cabalTest $ do + result <- fails $ cabal' "build" [] + assertOutputContains "Error parsing project file" result + assertOutputContains "'source-repository-package' is a stanza, not a field." result diff --git a/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/src/MyLib.hs b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/src/MyLib.hs new file mode 100644 index 00000000000..e657c4403f6 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/src/MyLib.hs @@ -0,0 +1,4 @@ +module MyLib (someFunc) where + +someFunc :: IO () +someFunc = putStrLn "someFunc" diff --git a/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/test.cabal b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/test.cabal new file mode 100644 index 00000000000..86374a457c7 --- /dev/null +++ b/cabal-testsuite/PackageTests/ProjectConfig/FieldStanzaConfusion/test.cabal @@ -0,0 +1,13 @@ +cabal-version: 3.0 +name: test +version: 0.1.0.0 +license: NONE +author: rbt@sent.as +maintainer: Rebecca Turner +build-type: Simple + +library + exposed-modules: MyLib + build-depends: base + hs-source-dirs: src + default-language: Haskell2010 diff --git a/changelog.d/pr-10525 b/changelog.d/pr-10525 new file mode 100644 index 00000000000..7235d0bec74 --- /dev/null +++ b/changelog.d/pr-10525 @@ -0,0 +1,34 @@ +--- +synopsis: "A trailing colon after a stanza name in `cabal.project` is now an error" +packages: [cabal-install] +prs: 10525 +--- + +It is now a hard error to use a trailing colon after a stanza name in +`cabal.project` or `*.cabal` files: + +``` +packages: . + +source-repository-package: + type: git + location: https://github.com/haskell/cabal + tag: f34aba976a60940295f41b6649674e9568893894 +``` + +``` +$ cabal build +Error parsing project file cabal.project:3: +'source-repository-package' is a stanza, not a field. Remove the trailing ':' to parse a stanza. +``` + +Previously, the warning message was easily ignored and somewhat misleading, as +the difference between a stanza and a field is not immediately obvious to +Haskellers used to config languages like JSON and YAML (which don't distinguish +between fields which have string or list values and stanzas which have nested +fields): + +``` +Warning: cabal.project: Unrecognized field +'source-repository-package' on line 3 +```