Rebase of #5370 (fix for #4683) (#7409)

* Introduce checks for upper dependencies in setup.

setup depends are now checked for having upper dependencies.
Upper bounds are mandatory for base and Cabal libraries, and
are optional (but emit warning) for other libraries.
Implicit dependencies are not being checked.

* Add tests.

* Fix tests.

* add changelog

* fix test for the newer cabal_raw'

* scale down alerts: no warnings on any package, only errors on base+Cabal

* address review comments

* typo

* improve changelog (add Cabal)

* remove boundedAbove and use Distribution.Version.hasUpperBound instead

Co-authored-by: Alexander Vershilov <alexander.vershilov@gmail.com>
Co-authored-by: Emily Pillmore <emilypi@cohomolo.gy>
Co-authored-by: Mikolaj Konarski <mikolaj@well-typed.com>

Cabal/src/Distribution/PackageDescription/Check.hs

@@ -154,6 +154,7 @@ checkPackage gpkg mpkg =
   ++ checkUnusedFlags gpkg
   ++ checkUnicodeXFields gpkg
   ++ checkPathsModuleExtensions pkg
+  ++ checkSetupVersions gpkg
   where
     pkg = fromMaybe (flattenPackageDescription gpkg) mpkg
 
@@ -1416,7 +1417,7 @@ checkPackageVersions pkg =
     -- For example this bans "build-depends: base >= 3".
     -- It should probably be "build-depends: base >= 3 && < 4"
     -- which is the same as  "build-depends: base == 3.*"
-    check (not (boundedAbove baseDependency)) $
+    check (not (hasUpperBound baseDependency)) $
       PackageDistInexcusable $
            "The dependency 'build-depends: base' does not specify an upper "
         ++ "bound on the version number. Each major release of the 'base' "
@@ -1431,21 +1432,7 @@ checkPackageVersions pkg =
 
   ]
   where
-    -- TODO: What we really want to do is test if there exists any
-    -- configuration in which the base version is unbounded above.
-    -- However that's a bit tricky because there are many possible
-    -- configurations. As a cheap easy and safe approximation we will
-    -- pick a single "typical" configuration and check if that has an
-    -- open upper bound. To get a typical configuration we finalise
-    -- using no package index and the current platform.
-    finalised = finalizePD
-                              mempty defaultComponentRequestedSpec (const True)
-                              buildPlatform
-                              (unknownCompilerInfo
-                                (CompilerId buildCompilerFlavor nullVersion)
-                                NoAbiTag)
-                              [] pkg
-    baseDependency = case finalised of
+    baseDependency = case typicalPkg pkg of
       Right (pkg', _) | not (null baseDeps) ->
           foldr intersectVersionRanges anyVersion baseDeps
         where
@@ -1455,18 +1442,9 @@ checkPackageVersions pkg =
 
       -- Just in case finalizePD fails for any reason,
       -- or if the package doesn't depend on the base package at all,
-      -- then we will just skip the check, since boundedAbove noVersion = True
+      -- then we will just skip the check, since hasUpperBound noVersion = True
       _          -> noVersion
 
-    -- TODO: move to Distribution.Version
-    boundedAbove :: VersionRange -> Bool
-    boundedAbove vr = case asVersionIntervals vr of
-      []     -> True -- this is the inconsistent version range.
-      (x:xs) -> case last (x:|xs) of
-        VersionInterval _ UpperBound {} -> True
-        VersionInterval _ NoUpperBound  -> False
-
-
 checkConditionals :: GenericPackageDescription -> [PackageCheck]
 checkConditionals pkg =
   catMaybes [
@@ -2151,6 +2129,35 @@ checkGlobFiles verbosity pkg root =
           ++ " directory by that name."
       ]
 
+-- | Check that setup dependencies, have proper bounds.
+-- In particular, @base@ and @Cabal@ upper bounds are mandatory.
+checkSetupVersions :: GenericPackageDescription -> [PackageCheck]
+checkSetupVersions pkg =
+    [ emitError nameStr
+    | (name, vr) <- Map.toList deps
+    , not (hasUpperBound vr)
+    , let nameStr = unPackageName name
+    , nameStr `elem` criticalPkgs
+    ]
+  where
+    criticalPkgs = ["Cabal", "base"]
+    deps = case typicalPkg pkg of
+      Right (pkgs', _) ->
+        Map.fromListWith intersectVersionRanges
+          [ (pname, vr)
+          | sbi <- maybeToList $ setupBuildInfo pkgs'
+          , Dependency pname vr _ <- setupDepends sbi
+          ]
+      _ -> Map.empty
+    emitError nm =
+      PackageDistInexcusable $
+           "The dependency 'setup-depends: '"++nm++"' does not specify an "
+        ++ "upper bound on the version number. Each major release of the "
+        ++ "'"++nm++"' package changes the API in various ways and most "
+        ++ "packages will need some changes to compile with it. If you are "
+        ++ "not sure what upper bound to use then use the next major "
+        ++ "version."
+
 -- ------------------------------------------------------------
 -- * Utils
 -- ------------------------------------------------------------
@@ -2384,3 +2391,21 @@ isGoodRelativeDirectoryPath = state0
 --     | x <= CSlash         -> 1
 --     | otherwise           -> 4
 -- @
+
+--
+-- TODO: What we really want to do is test if there exists any
+-- configuration in which the base version is unbounded above.
+-- However that's a bit tricky because there are many possible
+-- configurations. As a cheap easy and safe approximation we will
+-- pick a single "typical" configuration and check if that has an
+-- open upper bound. To get a typical configuration we finalise
+-- using no package index and the current platform.
+typicalPkg :: GenericPackageDescription
+           -> Either [Dependency] (PackageDescription, FlagAssignment)
+typicalPkg = finalizePD
+  mempty defaultComponentRequestedSpec (const True)
+  buildPlatform
+  (unknownCompilerInfo
+    (CompilerId buildCompilerFlavor nullVersion)
+      NoAbiTag)
+  []

cabal-testsuite/PackageTests/CheckSetup/LICENSE

@@ -0,0 +1 @@
+LICENSE

cabal-testsuite/PackageTests/CheckSetup/MyLibrary.hs

@@ -0,0 +1 @@
+module MyLibrary () where

cabal-testsuite/PackageTests/CheckSetup/Setup.hs

@@ -0,0 +1,2 @@
+import Distribution.Simple
+main = defaultMain

cabal-testsuite/PackageTests/CheckSetup/my.cabal

@@ -0,0 +1,25 @@
+name: CheckSetup
+version: 0.1
+license: BSD3
+license-file: LICENSE
+author: Alexander Vershilov
+maintainer: Alexander Vershilov
+synopsis: Check setup
+category: PackageTests
+build-type: Custom
+cabal-version: 2.0
+
+description:
+    Check that Cabal recognizes problems with setup module.
+
+custom-setup
+  setup-depends:
+    base,
+    Cabal,
+    bytestring
+
+Library
+    default-language: Haskell2010
+    build-depends: base <5.0
+    exposed-modules:
+        MyLibrary

cabal-testsuite/PackageTests/CheckSetup/setup.test.hs

@@ -0,0 +1,20 @@
+import Test.Cabal.Prelude
+
+-- Test that setup shows all the 'autogen-modules' warnings.
+main = cabalTest $ do
+
+        checkResult <- fails $ cabal_raw' ["check"] Nothing
+
+        -- Package check messages.
+        let libError1 =
+              "The dependency 'setup-depends: 'Cabal' does not specify "
+              ++ "an upper bound on the version number"
+            libError2 =
+              "The dependency 'setup-depends: 'base' does not specify "
+              ++ "an upper bound on the version number"
+
+        -- Asserts for the desired check messages after configure.
+        assertOutputContains libError1 checkResult
+        assertOutputContains libError2 checkResult
+
+        return ()

changelog.d/issue-4683

@@ -0,0 +1,4 @@
+synopsis: 'cabal check' to fail when no upper bounds for base or Cabal are present in setup dependencies
+packages: Cabal, cabal-install
+issues: #4683
+prs: #5370, #7409