Skip to content

Require type field when spec < 3.8 (#8115) #8138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions Cabal-syntax/src/Distribution/PackageDescription/FieldGrammar.hs
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,8 @@ testSuiteFieldGrammar = TestSuiteStanza
<*> monoidalFieldAla "code-generators" (alaList' CommaFSep Token) testStanzaCodeGenerators
^^^ availableSince CabalSpecV3_8 []

validateTestSuite :: Position -> TestSuiteStanza -> ParseResult TestSuite
validateTestSuite pos stanza = case testSuiteType of
validateTestSuite :: CabalSpecVersion -> Position -> TestSuiteStanza -> ParseResult TestSuite
validateTestSuite cabalSpecVersion pos stanza = case testSuiteType of
Nothing -> pure basicTestSuite

Just tt@(TestTypeUnknown _ _) ->
Expand Down Expand Up @@ -357,9 +357,10 @@ validateTestSuite pos stanza = case testSuiteType of
{ testInterface = TestSuiteLibV09 ver module_ }

where
testSuiteType =
_testStanzaTestType stanza
<|> testTypeExe <$ _testStanzaMainIs stanza
testSuiteType = _testStanzaTestType stanza <|> do
guard (cabalSpecVersion >= CabalSpecV3_8)

testTypeExe <$ _testStanzaMainIs stanza
<|> testTypeLib <$ _testStanzaTestModule stanza

missingField name tt = "The '" ++ name ++ "' field is required for the "
Expand Down Expand Up @@ -446,8 +447,8 @@ benchmarkFieldGrammar = BenchmarkStanza
<*> optionalField "benchmark-module" benchmarkStanzaBenchmarkModule
<*> blurFieldGrammar benchmarkStanzaBuildInfo buildInfoFieldGrammar

validateBenchmark :: Position -> BenchmarkStanza -> ParseResult Benchmark
validateBenchmark pos stanza = case benchmarkStanzaType of
validateBenchmark :: CabalSpecVersion -> Position -> BenchmarkStanza -> ParseResult Benchmark
validateBenchmark cabalSpecVersion pos stanza = case benchmarkStanzaType of
Nothing -> pure emptyBenchmark
{ benchmarkBuildInfo = _benchmarkStanzaBuildInfo stanza }

Expand All @@ -474,9 +475,10 @@ validateBenchmark pos stanza = case benchmarkStanzaType of
}

where
benchmarkStanzaType =
_benchmarkStanzaBenchmarkType stanza
<|> benchmarkTypeExe <$ _benchmarkStanzaMainIs stanza
benchmarkStanzaType = _benchmarkStanzaBenchmarkType stanza <|> do
guard (cabalSpecVersion >= CabalSpecV3_8)

benchmarkTypeExe <$ _benchmarkStanzaMainIs stanza

missingField name tt = "The '" ++ name ++ "' field is required for the "
++ prettyShow tt ++ " benchmark type."
Expand Down
31 changes: 25 additions & 6 deletions Cabal-syntax/src/Distribution/PackageDescription/Parsec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,23 @@ goSections specVer = traverse_ process
commonStanzas <- use stateCommonStanzas
name' <- parseUnqualComponentName pos args
testStanza <- lift $ parseCondTree' testSuiteFieldGrammar (fromBuildInfo' name') commonStanzas fields
testSuite <- lift $ traverse (validateTestSuite pos) testStanza
testSuite <- lift $ traverse (validateTestSuite specVer pos) testStanza

let hasType ts = testInterface ts /= testInterface mempty
unless (onAllBranches hasType testSuite) $ lift $ parseFailure pos $ concat
[ "Test suite " ++ show (prettyShow name')
, " is missing required field \"main-is\" or the field "
, "is not present in all conditional branches."

, concat $ case specVer of
v | v >= CabalSpecV3_8 ->
[ " is missing required field \"main-is\" or the field "
, "is not present in all conditional branches."
]
_ ->
[ " is missing required field \"type\" or the field "
, "is not present in all conditional branches. The "
, "available test types are: "
, intercalate ", " (map prettyShow knownTestTypes)
]
]

-- TODO check duplicate name here?
Expand All @@ -330,13 +340,22 @@ goSections specVer = traverse_ process
commonStanzas <- use stateCommonStanzas
name' <- parseUnqualComponentName pos args
benchStanza <- lift $ parseCondTree' benchmarkFieldGrammar (fromBuildInfo' name') commonStanzas fields
bench <- lift $ traverse (validateBenchmark pos) benchStanza
bench <- lift $ traverse (validateBenchmark specVer pos) benchStanza

let hasType ts = benchmarkInterface ts /= benchmarkInterface mempty
unless (onAllBranches hasType bench) $ lift $ parseFailure pos $ concat
[ "Benchmark " ++ show (prettyShow name')
, " is missing required field \"main-is\" or the field "
, "is not present in all conditional branches."
, concat $ case specVer of
v | v >= CabalSpecV3_8 ->
[ " is missing required field \"main-is\" or the field "
, "is not present in all conditional branches."
]
_ ->
[ " is missing required field \"type\" or the field "
, "is not present in all conditional branches. The "
, "available benchmark types are: "
, intercalate ", " (map prettyShow knownBenchmarkTypes)
]
]

-- TODO check duplicate name here?
Expand Down
2 changes: 1 addition & 1 deletion Cabal-tests/tests/ParserTests/errors/issue-5055-2.errors
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
VERSION: Just (mkVersion [2,0])
issue-5055-2.cabal:15:1: Test suite "flag-cabal-test" is missing required field "main-is" or the field is not present in all conditional branches.
issue-5055-2.cabal:15:1: Test suite "flag-cabal-test" is missing required field "type" or the field is not present in all conditional branches. The available test types are: exitcode-stdio-1.0, detailed-0.9
2 changes: 1 addition & 1 deletion Cabal-tests/tests/ParserTests/errors/issue-5055.errors
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
VERSION: Just (mkVersion [2,0])
issue-5055.cabal:15:1: Test suite "flag-cabal-test" is missing required field "main-is" or the field is not present in all conditional branches.
issue-5055.cabal:15:1: Test suite "flag-cabal-test" is missing required field "type" or the field is not present in all conditional branches. The available test types are: exitcode-stdio-1.0, detailed-0.9
8 changes: 5 additions & 3 deletions Cabal-tests/tests/ParserTests/regressions/issue-5055.expr
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,11 @@ GenericPackageDescription {
condTreeData = TestSuite {
testName = UnqualComponentName
"",
testInterface = TestSuiteExeV10
(mkVersion [1, 0])
"FirstMain.hs",
testInterface =
TestSuiteUnsupported
(TestTypeUnknown
""
(mkVersion [])),
testBuildInfo = BuildInfo {
buildable = True,
buildTools = [],
Expand Down
2 changes: 0 additions & 2 deletions Cabal-tests/tests/ParserTests/regressions/issue-5055.format
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,3 @@ test-suite flag-cabal-test
build-depends: base >=4.8 && <5

if os(windows)
type: exitcode-stdio-1.0
main-is: FirstMain.hs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cabal-version: 3.8
name: p
version: 0.1
build-type: Simple
cabal-version: >= 1.10

benchmark solver-disabled
type: exitcode-stdio-1.0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cabal-version: 3.8
name: p
version: 0.1
build-type: Simple
cabal-version: >= 1.10

test-suite p1
main-is: P1.hs
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cabal-version: 3.8
name: p
version: 0.1
build-type: Simple
cabal-version: >= 1.10

library
exposed-modules: P
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
cabal-version: 3.8
name: DuplicateModuleName
version: 0.1.0.0
license: BSD3
license: BSD-3-Clause
author: Edward Z. Yang
maintainer: [email protected]
build-type: Simple
cabal-version: >=1.10

library
exposed-modules: Foo
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cabal-version: 3.8
name: MultipleBenchmarks
version: 1.0
build-type: Simple
cabal-version: >= 1.10

benchmark foo
main-is: Foo.hs
Expand Down
2 changes: 1 addition & 1 deletion changelog.d/pr-8115
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ prs: #8115
issues: #7459
description: {

Allow the ommission of the `type` field in `test-suite` and `benchmark` stanzas
Allow the omission of the `type` field in `test-suite` and `benchmark` stanzas
when the type can be inferred by the presence of `main-is` or `test-module`.

}
2 changes: 2 additions & 0 deletions doc/file-format-changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ relative to the respective preceding *published* version.
allowed of the form ``foo/**/literalFile``. Prior, double-star
wildcards required the trailing filename itself be a wildcard.

* Allow the omission of the `type` field in `test-suite` and `benchmark` stanzas
when the type can be inferred by the presence of `main-is` or `test-module`.

``cabal-version: 3.6``
----------------------
Expand Down