Check.hs 96.5 KB
Newer Older
1 2 3 4
-----------------------------------------------------------------------------
-- |
-- Module      :  Distribution.PackageDescription.Check
-- Copyright   :  Lennart Kolmodin 2008
5
-- License     :  BSD3
6
--
Duncan Coutts's avatar
Duncan Coutts committed
7
-- Maintainer  :  cabal-devel@haskell.org
8 9
-- Portability :  portable
--
Duncan Coutts's avatar
Duncan Coutts committed
10 11 12 13 14 15 16 17
-- This has code for checking for various problems in packages. There is one
-- set of checks that just looks at a 'PackageDescription' in isolation and
-- another set of checks that also looks at files in the package. Some of the
-- checks are basic sanity checks, others are portability standards that we'd
-- like to encourage. There is a 'PackageCheck' type that distinguishes the
-- different kinds of check so we can see which ones are appropriate to report
-- in different situations. This code gets uses when configuring a package when
-- we consider only basic problems. The higher standard is uses when when
Ian D. Bollinger's avatar
Ian D. Bollinger committed
18
-- preparing a source tarball and by Hackage when uploading new packages. The
Duncan Coutts's avatar
Duncan Coutts committed
19 20 21
-- reason for this is that we want to hold packages that are expected to be
-- distributed to a higher standard than packages that are only ever expected
-- to be used on the author's own environment.
22 23 24 25 26

module Distribution.PackageDescription.Check (
        -- * Package Checking
        PackageCheck(..),
        checkPackage,
27
        checkConfiguredPackage,
28 29 30 31 32

        -- ** Checking package contents
        checkPackageFiles,
        checkPackageContent,
        CheckPackageContentOps(..),
33
        checkPackageFileNames,
34 35
  ) where

36
import Distribution.Compat.Prelude
37
import Prelude ()
38

39 40 41
import Control.Monad                                 (mapM)
import Data.List                                     (group)
import Distribution.Compat.Lens
42 43
import Distribution.Compiler
import Distribution.License
44 45 46 47 48
import Distribution.Package
import Distribution.PackageDescription
import Distribution.PackageDescription.Configuration
import Distribution.Pretty                           (prettyShow)
import Distribution.Simple.BuildPaths                (autogenPathsModuleName)
John Ericson's avatar
John Ericson committed
49
import Distribution.Simple.BuildToolDepends
50
import Distribution.Simple.CCompiler
51
import Distribution.Simple.Glob
52 53 54
import Distribution.Simple.Utils                     hiding (findPackageDesc, notice)
import Distribution.System
import Distribution.Text
55
import Distribution.Types.ComponentRequestedSpec
56
import Distribution.Types.CondTree
57
import Distribution.Types.ExeDependency
58
import Distribution.Types.UnqualComponentName
59
import Distribution.Utils.Generic                    (isAscii)
60
import Distribution.Verbosity
61
import Distribution.Version
62
import Language.Haskell.Extension
63 64
import System.FilePath
       (splitDirectories, splitExtension, splitPath, takeExtension, takeFileName, (<.>), (</>))
65

66 67 68 69 70
import qualified Data.ByteString.Lazy      as BS
import qualified Data.Map                  as Map
import qualified Distribution.Compat.DList as DList
import qualified Distribution.SPDX         as SPDX
import qualified System.Directory          as System
71

72 73
import qualified System.Directory        (getDirectoryContents)
import qualified System.FilePath.Windows as FilePath.Windows (isValid)
74

Oleg Grenrus's avatar
Oleg Grenrus committed
75
import qualified Data.Set as Set
76

77
import qualified Distribution.Types.BuildInfo.Lens                 as L
78
import qualified Distribution.Types.GenericPackageDescription.Lens as L
79
import qualified Distribution.Types.PackageDescription.Lens        as L
80

81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99
-- | Results of some kind of failed package check.
--
-- There are a range of severities, from merely dubious to totally insane.
-- All of them come with a human readable explanation. In future we may augment
-- them with more machine readable explanations, for example to help an IDE
-- suggest automatic corrections.
--
data PackageCheck =

       -- | This package description is no good. There's no way it's going to
       -- build sensibly. This should give an error at configure time.
       PackageBuildImpossible { explanation :: String }

       -- | A problem that is likely to affect building the package, or an
       -- issue that we'd like every package author to be aware of, even if
       -- the package is never distributed.
     | PackageBuildWarning { explanation :: String }

       -- | An issue that might not be a problem for the package author but
Ian D. Bollinger's avatar
Ian D. Bollinger committed
100
       -- might be annoying or detrimental when the package is distributed to
101 102 103 104 105
       -- users. We should encourage distributed packages to be free from these
       -- issues, but occasionally there are justifiable reasons so we cannot
       -- ban them entirely.
     | PackageDistSuspicious { explanation :: String }

106
       -- | Like PackageDistSuspicious but will only display warnings
107
       -- rather than causing abnormal exit when you run 'cabal check'.
108 109
     | PackageDistSuspiciousWarn { explanation :: String }

Ian D. Bollinger's avatar
Ian D. Bollinger committed
110
       -- | An issue that is OK in the author's environment but is almost
111 112 113 114
       -- certain to be a portability problem for other environments. We can
       -- quite legitimately refuse to publicly distribute packages with these
       -- problems.
     | PackageDistInexcusable { explanation :: String }
115
  deriving (Eq, Ord)
116 117 118 119 120 121 122 123

instance Show PackageCheck where
    show notice = explanation notice

check :: Bool -> PackageCheck -> Maybe PackageCheck
check False _  = Nothing
check True  pc = Just pc

Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
124 125
checkSpecVersion :: PackageDescription -> [Int] -> Bool -> PackageCheck
                 -> Maybe PackageCheck
126
checkSpecVersion pkg specver cond pc
127
  | specVersion pkg >= mkVersion specver  = Nothing
128 129
  | otherwise                             = check cond pc

130 131 132 133 134 135
-- ------------------------------------------------------------
-- * Standard checks
-- ------------------------------------------------------------

-- | Check for common mistakes and problems in package descriptions.
--
Ian D. Bollinger's avatar
Ian D. Bollinger committed
136
-- This is the standard collection of checks covering all aspects except
137 138 139
-- for checks that require looking at files within the package. For those
-- see 'checkPackageFiles'.
--
140 141 142 143 144 145 146 147 148
-- It requires the 'GenericPackageDescription' and optionally a particular
-- configuration of that package. If you pass 'Nothing' then we just check
-- a version of the generic description using 'flattenPackageDescription'.
--
checkPackage :: GenericPackageDescription
             -> Maybe PackageDescription
             -> [PackageCheck]
checkPackage gpkg mpkg =
     checkConfiguredPackage pkg
149
  ++ checkConditionals gpkg
150
  ++ checkPackageVersions gpkg
151
  ++ checkDevelopmentOnlyFlags gpkg
Oleg Grenrus's avatar
Oleg Grenrus committed
152
  ++ checkFlagNames gpkg
Oleg Grenrus's avatar
Oleg Grenrus committed
153
  ++ checkUnusedFlags gpkg
Oleg Grenrus's avatar
Oleg Grenrus committed
154
  ++ checkUnicodeXFields gpkg
155
  ++ checkPathsModuleExtensions pkg
156 157 158 159
  where
    pkg = fromMaybe (flattenPackageDescription gpkg) mpkg

--TODO: make this variant go away
refold's avatar
Typo.  
refold committed
160
--      we should always know the GenericPackageDescription
161 162
checkConfiguredPackage :: PackageDescription -> [PackageCheck]
checkConfiguredPackage pkg =
163 164 165
    checkSanity pkg
 ++ checkFields pkg
 ++ checkLicense pkg
166
 ++ checkSourceRepos pkg
167
 ++ checkAllGhcOptions pkg
Duncan Coutts's avatar
Duncan Coutts committed
168
 ++ checkCCOptions pkg
169
 ++ checkCxxOptions pkg
170
 ++ checkCPPOptions pkg
171
 ++ checkPaths pkg
172
 ++ checkCabalVersion pkg
173 174 175 176 177 178 179 180 181 182 183 184


-- ------------------------------------------------------------
-- * Basic sanity checks
-- ------------------------------------------------------------

-- | Check that this package description is sane.
--
checkSanity :: PackageDescription -> [PackageCheck]
checkSanity pkg =
  catMaybes [

185
    check (null . unPackageName . packageName $ pkg) $
186 187
      PackageBuildImpossible "No 'name' field."

188
  , check (nullVersion == packageVersion pkg) $
189 190
      PackageBuildImpossible "No 'version' field."

191 192 193
  , check (all ($ pkg) [ null . executables
                       , null . testSuites
                       , null . benchmarks
194 195
                       , null . allLibraries
                       , null . foreignLibs ]) $
196
      PackageBuildImpossible
197
        "No executables, libraries, tests, or benchmarks found. Nothing to do."
198

199 200 201 202
  , check (any isNothing (map libName $ subLibraries pkg)) $
      PackageBuildImpossible $ "Found one or more unnamed internal libraries. "
        ++ "Only the non-internal library can have the same name as the package."

tibbe's avatar
tibbe committed
203
  , check (not (null duplicateNames)) $
204 205
      PackageBuildImpossible $ "Duplicate sections: "
        ++ commaSep (map unUnqualComponentName duplicateNames)
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
206 207
        ++ ". The name of every library, executable, test suite,"
        ++ " and benchmark section in"
tibbe's avatar
tibbe committed
208
        ++ " the package must be unique."
209 210

  -- NB: but it's OK for executables to have the same name!
211 212
  -- TODO shouldn't need to compare on the string level
  , check (any (== display (packageName pkg)) (display <$> subLibNames)) $
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
213 214 215 216 217
      PackageBuildImpossible $ "Illegal internal library name "
        ++ display (packageName pkg)
        ++ ". Internal libraries cannot have the same name as the package."
        ++ " Maybe you wanted a non-internal library?"
        ++ " If so, rewrite the section stanza"
218
        ++ " from 'library: '" ++ display (packageName pkg) ++ "' to 'library'."
219
  ]
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
220 221
  --TODO: check for name clashes case insensitively: windows file systems cannot
  --cope.
222

223
  ++ concatMap (checkLibrary    pkg) (allLibraries pkg)
224 225 226
  ++ concatMap (checkExecutable pkg) (executables pkg)
  ++ concatMap (checkTestSuite  pkg) (testSuites pkg)
  ++ concatMap (checkBenchmark  pkg) (benchmarks pkg)
227 228 229

  ++ catMaybes [

230
    check (specVersion pkg > cabalVersion) $
231
      PackageBuildImpossible $
232 233 234
           "This package description follows version "
        ++ display (specVersion pkg) ++ " of the Cabal specification. This "
        ++ "tool only supports up to version " ++ display cabalVersion ++ "."
235
  ]
236
  where
237
    -- The public 'library' gets special dispensation, because it
238
    -- is common practice to export a library and name the executable
239 240
    -- the same as the package.
    subLibNames = catMaybes . map libName $ subLibraries pkg
241
    exeNames = map exeName $ executables pkg
242
    testNames = map testName $ testSuites pkg
tibbe's avatar
tibbe committed
243
    bmNames = map benchmarkName $ benchmarks pkg
244
    duplicateNames = dups $ subLibNames ++ exeNames ++ testNames ++ bmNames
245

246
checkLibrary :: PackageDescription -> Library -> [PackageCheck]
247
checkLibrary pkg lib =
248 249
  catMaybes [

250
    check (not (null moduleDuplicates)) $
251
       PackageBuildImpossible $
252 253
            "Duplicate modules in library: "
         ++ commaSep (map display moduleDuplicates)
254

255 256
  -- TODO: This check is bogus if a required-signature was passed through
  , check (null (explicitLibModules lib) && null (reexportedModules lib)) $
257
      PackageDistSuspiciousWarn $
258 259
           "Library " ++ (case libName lib of
                            Nothing -> ""
260
                            Just n -> display n
261
                            ) ++ "does not expose any modules"
262

263 264
    -- check use of signatures sections
  , checkVersion [1,25] (not (null (signatures lib))) $
265
      PackageDistInexcusable $
266
           "To use the 'signatures' field the package needs to specify "
267
        ++ "at least 'cabal-version: 2.0'."
fmaste's avatar
fmaste committed
268 269 270

    -- check that all autogen-modules appear on other-modules or exposed-modules
  , check
271
      (not $ and $ map (flip elem (explicitLibModules lib)) (libModulesAutogen lib)) $
fmaste's avatar
fmaste committed
272 273 274 275
      PackageBuildImpossible $
           "An 'autogen-module' is neither on 'exposed-modules' or "
        ++ "'other-modules'."

276 277
  ]

278
  where
279 280
    checkVersion :: [Int] -> Bool -> PackageCheck -> Maybe PackageCheck
    checkVersion ver cond pc
281
      | specVersion pkg >= mkVersion ver       = Nothing
282 283
      | otherwise                              = check cond pc

284 285
    -- TODO: not sure if this check is always right in Backpack
    moduleDuplicates = dups (explicitLibModules lib ++
286
                             map moduleReexportName (reexportedModules lib))
Duncan Coutts's avatar
Duncan Coutts committed
287

288 289
checkExecutable :: PackageDescription -> Executable -> [PackageCheck]
checkExecutable pkg exe =
290 291 292 293
  catMaybes [

    check (null (modulePath exe)) $
      PackageBuildImpossible $
294
        "No 'main-is' field found for executable " ++ display (exeName exe)
295 296

  , check (not (null (modulePath exe))
297
       && (not $ fileExtensionSupportedLanguage $ modulePath exe)) $
298
      PackageBuildImpossible $
299 300 301
           "The 'main-is' field must specify a '.hs' or '.lhs' file "
        ++ "(even if it is generated by a preprocessor), "
        ++ "or it may specify a C/C++/obj-C source file."
Duncan Coutts's avatar
Duncan Coutts committed
302

303 304 305 306 307 308 309
  , checkSpecVersion pkg [1,17]
          (fileExtensionSupportedLanguage (modulePath exe)
        && takeExtension (modulePath exe) `notElem` [".hs", ".lhs"]) $
      PackageDistInexcusable $
           "The package uses a C/C++/obj-C source file for the 'main-is' field. "
        ++ "To use this feature you must specify 'cabal-version: >= 1.18'."

Duncan Coutts's avatar
Duncan Coutts committed
310
  , check (not (null moduleDuplicates)) $
311
       PackageBuildImpossible $
312
            "Duplicate modules in executable '" ++ display (exeName exe) ++ "': "
313
         ++ commaSep (map display moduleDuplicates)
fmaste's avatar
fmaste committed
314 315 316 317 318

    -- check that all autogen-modules appear on other-modules
  , check
      (not $ and $ map (flip elem (exeModules exe)) (exeModulesAutogen exe)) $
      PackageBuildImpossible $
319
           "On executable '" ++ display (exeName exe) ++ "' an 'autogen-module' is not "
fmaste's avatar
fmaste committed
320
        ++ "on 'other-modules'"
321
  ]
322 323
  where
    moduleDuplicates = dups (exeModules exe)
324

325 326
checkTestSuite :: PackageDescription -> TestSuite -> [PackageCheck]
checkTestSuite pkg test =
327 328
  catMaybes [

329 330 331 332 333 334 335 336 337 338 339 340 341 342 343
    case testInterface test of
      TestSuiteUnsupported tt@(TestTypeUnknown _ _) -> Just $
        PackageBuildWarning $
             quote (display tt) ++ " is not a known type of test suite. "
          ++ "The known test suite types are: "
          ++ commaSep (map display knownTestTypes)

      TestSuiteUnsupported tt -> Just $
        PackageBuildWarning $
             quote (display tt) ++ " is not a supported test suite version. "
          ++ "The known test suite types are: "
          ++ commaSep (map display knownTestTypes)
      _ -> Nothing

  , check (not $ null moduleDuplicates) $
344
      PackageBuildImpossible $
345
           "Duplicate modules in test suite '" ++ display (testName test) ++ "': "
346
        ++ commaSep (map display moduleDuplicates)
347

348 349 350
  , check mainIsWrongExt $
      PackageBuildImpossible $
           "The 'main-is' field must specify a '.hs' or '.lhs' file "
351 352
        ++ "(even if it is generated by a preprocessor), "
        ++ "or it may specify a C/C++/obj-C source file."
353

354 355 356 357
  , checkSpecVersion pkg [1,17] (mainIsNotHsExt && not mainIsWrongExt) $
      PackageDistInexcusable $
           "The package uses a C/C++/obj-C source file for the 'main-is' field. "
        ++ "To use this feature you must specify 'cabal-version: >= 1.18'."
fmaste's avatar
fmaste committed
358 359 360 361 362 363 364 365

    -- check that all autogen-modules appear on other-modules
  , check
      (not $ and $ map
        (flip elem (testModules test))
        (testModulesAutogen test)
      ) $
      PackageBuildImpossible $
366
           "On test suite '" ++ display (testName test) ++ "' an 'autogen-module' is not "
fmaste's avatar
fmaste committed
367
        ++ "on 'other-modules'"
368
  ]
369 370
  where
    moduleDuplicates = dups $ testModules test
371 372

    mainIsWrongExt = case testInterface test of
373
      TestSuiteExeV10 _ f -> not $ fileExtensionSupportedLanguage f
374 375
      _                   -> False

376 377 378 379
    mainIsNotHsExt = case testInterface test of
      TestSuiteExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"]
      _                   -> False

tibbe's avatar
tibbe committed
380
checkBenchmark :: PackageDescription -> Benchmark -> [PackageCheck]
381
checkBenchmark _pkg bm =
tibbe's avatar
tibbe committed
382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398
  catMaybes [

    case benchmarkInterface bm of
      BenchmarkUnsupported tt@(BenchmarkTypeUnknown _ _) -> Just $
        PackageBuildWarning $
             quote (display tt) ++ " is not a known type of benchmark. "
          ++ "The known benchmark types are: "
          ++ commaSep (map display knownBenchmarkTypes)

      BenchmarkUnsupported tt -> Just $
        PackageBuildWarning $
             quote (display tt) ++ " is not a supported benchmark version. "
          ++ "The known benchmark types are: "
          ++ commaSep (map display knownBenchmarkTypes)
      _ -> Nothing

  , check (not $ null moduleDuplicates) $
399
      PackageBuildImpossible $
400
           "Duplicate modules in benchmark '" ++ display (benchmarkName bm) ++ "': "
tibbe's avatar
tibbe committed
401 402 403 404 405 406
        ++ commaSep (map display moduleDuplicates)

  , check mainIsWrongExt $
      PackageBuildImpossible $
           "The 'main-is' field must specify a '.hs' or '.lhs' file "
        ++ "(even if it is generated by a preprocessor)."
fmaste's avatar
fmaste committed
407 408 409 410 411 412 413 414

    -- check that all autogen-modules appear on other-modules
  , check
      (not $ and $ map
        (flip elem (benchmarkModules bm))
        (benchmarkModulesAutogen bm)
      ) $
      PackageBuildImpossible $
415
             "On benchmark '" ++ display (benchmarkName bm) ++ "' an 'autogen-module' is "
fmaste's avatar
fmaste committed
416
          ++ "not on 'other-modules'"
tibbe's avatar
tibbe committed
417 418 419 420 421 422 423 424
  ]
  where
    moduleDuplicates = dups $ benchmarkModules bm

    mainIsWrongExt = case benchmarkInterface bm of
      BenchmarkExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"]
      _                   -> False

425 426 427 428 429 430 431 432
-- ------------------------------------------------------------
-- * Additional pure checks
-- ------------------------------------------------------------

checkFields :: PackageDescription -> [PackageCheck]
checkFields pkg =
  catMaybes [

433 434 435 436 437 438 439
    check (not . FilePath.Windows.isValid . display . packageName $ pkg) $
      PackageDistInexcusable $
           "Unfortunately, the package name '" ++ display (packageName pkg)
        ++ "' is one of the reserved system file names on Windows. Many tools "
        ++ "need to convert package names to file names so using this name "
        ++ "would cause problems."

440 441 442 443 444
  , check ((isPrefixOf "z-") . display . packageName $ pkg) $
      PackageDistInexcusable $
           "Package names with the prefix 'z-' are reserved by Cabal and "
        ++ "cannot be used."

445
  , check (isNothing (buildTypeRaw pkg) && specVersion pkg < mkVersion [2,1]) $
446 447 448 449
      PackageBuildWarning $
           "No 'build-type' specified. If you do not need a custom Setup.hs or "
        ++ "./configure script then use 'build-type: Simple'."

450
  , check (isJust (setupBuildInfo pkg) && buildType pkg /= Custom) $
451 452 453 454 455
      PackageBuildWarning $
           "Ignoring the 'custom-setup' section because the 'build-type' is "
        ++ "not 'Custom'. Use 'build-type: Custom' if you need to use a "
        ++ "custom Setup.hs script."

456 457
  , check (not (null unknownCompilers)) $
      PackageBuildWarning $
458
        "Unknown compiler " ++ commaSep (map quote unknownCompilers)
459 460
                            ++ " in 'tested-with' field."

461 462 463 464
  , check (not (null unknownLanguages)) $
      PackageBuildWarning $
        "Unknown languages: " ++ commaSep unknownLanguages

465 466
  , check (not (null unknownExtensions)) $
      PackageBuildWarning $
467
        "Unknown extensions: " ++ commaSep unknownExtensions
468

469 470 471 472 473 474 475
  , check (not (null languagesUsedAsExtensions)) $
      PackageBuildWarning $
           "Languages listed as extensions: "
        ++ commaSep languagesUsedAsExtensions
        ++ ". Languages must be specified in either the 'default-language' "
        ++ " or the 'other-languages' field."

476
  , check (not (null ourDeprecatedExtensions)) $
477 478
      PackageDistSuspicious $
           "Deprecated extensions: "
479
        ++ commaSep (map (quote . display . fst) ourDeprecatedExtensions)
EyalLotem's avatar
EyalLotem committed
480
        ++ ". " ++ unwords
481 482
             [ "Instead of '" ++ display ext
            ++ "' use '" ++ display replacement ++ "'."
483
             | (ext, Just replacement) <- ourDeprecatedExtensions ]
484

485 486 487 488 489 490
  , check (null (category pkg)) $
      PackageDistSuspicious "No 'category' field."

  , check (null (maintainer pkg)) $
      PackageDistSuspicious "No 'maintainer' field."

491
  , check (null (synopsis pkg) && null (description pkg)) $
EyalLotem's avatar
EyalLotem committed
492
      PackageDistInexcusable "No 'synopsis' or 'description' field."
493 494 495 496 497

  , check (null (description pkg) && not (null (synopsis pkg))) $
      PackageDistSuspicious "No 'description' field."

  , check (null (synopsis pkg) && not (null (description pkg))) $
498 499
      PackageDistSuspicious "No 'synopsis' field."

Ian D. Bollinger's avatar
Ian D. Bollinger committed
500
    --TODO: recommend the bug reports URL, author and homepage fields
501 502 503
    --TODO: recommend not using the stability field
    --TODO: recommend specifying a source repo

504 505 506
  , check (length (synopsis pkg) >= 80) $
      PackageDistSuspicious
        "The 'synopsis' field is rather long (max 80 chars is recommended)."
507

508 509 510 511 512 513 514 515 516 517 518 519 520 521 522 523
    -- See also https://github.com/haskell/cabal/pull/3479
  , check (not (null (description pkg))
           && length (description pkg) <= length (synopsis pkg)) $
      PackageDistSuspicious $
           "The 'description' field should be longer than the 'synopsis' "
        ++ "field. "
        ++ "It's useful to provide an informative 'description' to allow "
        ++ "Haskell programmers who have never heard about your package to "
        ++ "understand the purpose of your package. "
        ++ "The 'description' field content is typically shown by tooling "
        ++ "(e.g. 'cabal info', Haddock, Hackage) below the 'synopsis' which "
        ++ "serves as a headline. "
        ++ "Please refer to <https://www.haskell.org/"
        ++ "cabal/users-guide/developing-packages.html#package-properties>"
        ++ " for more details."

524 525 526 527 528 529 530 531 532
    -- check use of impossible constraints "tested-with: GHC== 6.10 && ==6.12"
  , check (not (null testedWithImpossibleRanges)) $
      PackageDistInexcusable $
           "Invalid 'tested-with' version range: "
        ++ commaSep (map display testedWithImpossibleRanges)
        ++ ". To indicate that you have tested a package with multiple "
        ++ "different versions of the same compiler use multiple entries, "
        ++ "for example 'tested-with: GHC==6.10.4, GHC==6.12.3' and not "
        ++ "'tested-with: GHC==6.10.4 && ==6.12.3'."
533

John Ericson's avatar
John Ericson committed
534
  , check (not (null depInternalLibraryWithExtraVersion)) $
535
      PackageBuildWarning $
John Ericson's avatar
John Ericson committed
536
           "The package has an extraneous version range for a dependency on an "
537
        ++ "internal library: "
John Ericson's avatar
John Ericson committed
538 539 540 541 542 543 544 545 546 547 548 549 550 551 552 553 554 555 556 557 558 559 560 561 562 563 564 565 566 567 568 569
        ++ commaSep (map display depInternalLibraryWithExtraVersion)
        ++ ". This version range includes the current package but isn't needed "
        ++ "as the current package's library will always be used."

  , check (not (null depInternalLibraryWithImpossibleVersion)) $
      PackageBuildImpossible $
           "The package has an impossible version range for a dependency on an "
        ++ "internal library: "
        ++ commaSep (map display depInternalLibraryWithImpossibleVersion)
        ++ ". This version range does not include the current package, and must "
        ++ "be removed as the current package's library will always be used."

  , check (not (null depInternalExecutableWithExtraVersion)) $
      PackageBuildWarning $
           "The package has an extraneous version range for a dependency on an "
        ++ "internal executable: "
        ++ commaSep (map display depInternalExecutableWithExtraVersion)
        ++ ". This version range includes the current package but isn't needed "
        ++ "as the current package's executable will always be used."

  , check (not (null depInternalExecutableWithImpossibleVersion)) $
      PackageBuildImpossible $
           "The package has an impossible version range for a dependency on an "
        ++ "internal executable: "
        ++ commaSep (map display depInternalExecutableWithImpossibleVersion)
        ++ ". This version range does not include the current package, and must "
        ++ "be removed as the current package's executable will always be used."

  , check (not (null depMissingInternalExecutable)) $
      PackageBuildImpossible $
           "The package depends on a missing internal executable: "
        ++ commaSep (map display depInternalExecutableWithImpossibleVersion)
570
  ]
571 572
  where
    unknownCompilers  = [ name | (OtherCompiler name, _) <- testedWith pkg ]
573 574
    unknownLanguages  = [ name | bi <- allBuildInfo pkg
                               , UnknownLanguage name <- allLanguages bi ]
575
    unknownExtensions = [ name | bi <- allBuildInfo pkg
576 577
                               , UnknownExtension name <- allExtensions bi
                               , name `notElem` map display knownLanguages ]
578 579
    ourDeprecatedExtensions = nub $ catMaybes
      [ find ((==ext) . fst) deprecatedExtensions
580
      | bi <- allBuildInfo pkg
Ian Lynagh's avatar
Ian Lynagh committed
581
      , ext <- allExtensions bi ]
582 583 584 585
    languagesUsedAsExtensions =
      [ name | bi <- allBuildInfo pkg
             , UnknownExtension name <- allExtensions bi
             , name `elem` map display knownLanguages ]
586

587
    testedWithImpossibleRanges =
588
      [ Dependency (mkPackageName (display compiler)) vr
589 590 591
      | (compiler, vr) <- testedWith pkg
      , isNoVersion vr ]

592
    internalLibraries =
593
        map (maybe (packageName pkg) (unqualComponentNameToPackageName) . libName)
594
            (allLibraries pkg)
John Ericson's avatar
John Ericson committed
595 596 597 598

    internalExecutables = map exeName $ executables pkg

    internalLibDeps =
599 600
      [ dep
      | bi <- allBuildInfo pkg
John Ericson's avatar
John Ericson committed
601
      , dep@(Dependency name _) <- targetBuildDepends bi
602 603 604
      , name `elem` internalLibraries
      ]

John Ericson's avatar
John Ericson committed
605 606 607
    internalExeDeps =
      [ dep
      | bi <- allBuildInfo pkg
608 609
      , dep <- getAllToolDependencies pkg bi
      , isInternal pkg dep
John Ericson's avatar
John Ericson committed
610 611 612 613 614 615 616 617 618 619 620 621 622 623 624 625 626 627 628 629 630 631 632 633 634 635 636 637 638 639 640 641 642 643
      ]

    depInternalLibraryWithExtraVersion =
      [ dep
      | dep@(Dependency _ versionRange) <- internalLibDeps
      , not $ isAnyVersion versionRange
      , packageVersion pkg `withinRange` versionRange
      ]

    depInternalLibraryWithImpossibleVersion =
      [ dep
      | dep@(Dependency _ versionRange) <- internalLibDeps
      , not $ packageVersion pkg `withinRange` versionRange
      ]

    depInternalExecutableWithExtraVersion =
      [ dep
      | dep@(ExeDependency _ _ versionRange) <- internalExeDeps
      , not $ isAnyVersion versionRange
      , packageVersion pkg `withinRange` versionRange
      ]

    depInternalExecutableWithImpossibleVersion =
      [ dep
      | dep@(ExeDependency _ _ versionRange) <- internalExeDeps
      , not $ packageVersion pkg `withinRange` versionRange
      ]

    depMissingInternalExecutable =
      [ dep
      | dep@(ExeDependency _ eName _) <- internalExeDeps
      , not $ eName `elem` internalExecutables
      ]

644

645
checkLicense :: PackageDescription -> [PackageCheck]
646 647 648 649 650 651 652 653 654 655
checkLicense pkg = case licenseRaw pkg of
    Right l -> checkOldLicense pkg l
    Left  l -> checkNewLicense pkg l

checkNewLicense :: PackageDescription -> SPDX.License -> [PackageCheck]
checkNewLicense _pkg lic = catMaybes
    [ check (lic == SPDX.NONE) $
        PackageDistInexcusable
            "The 'license' field is missing or is NONE."
    ]
656

657 658 659
checkOldLicense :: PackageDescription -> License -> [PackageCheck]
checkOldLicense pkg lic = catMaybes
  [ check (lic == UnspecifiedLicense) $
660
      PackageDistInexcusable
661
        "The 'license' field is missing."
662

663
  , check (lic == AllRightsReserved) $
664 665
      PackageDistSuspicious
        "The 'license' is AllRightsReserved. Is that really what you want?"
666 667 668 669 670 671 672 673 674

  , checkVersion [1,4] (lic `notElem` compatLicenses) $
      PackageDistInexcusable $
           "Unfortunately the license " ++ quote (prettyShow (license pkg))
        ++ " messes up the parser in earlier Cabal versions so you need to "
        ++ "specify 'cabal-version: >= 1.4'. Alternatively if you require "
        ++ "compatibility with earlier Cabal versions then use 'OtherLicense'."

  , case lic of
675 676
      UnknownLicense l -> Just $
        PackageBuildWarning $
677 678 679
             quote ("license: " ++ l) ++ " is not a recognised license. The "
          ++ "known licenses are: "
          ++ commaSep (map display knownLicenses)
680 681
      _ -> Nothing

682
  , check (lic == BSD4) $
683 684
      PackageDistSuspicious $
           "Using 'license: BSD4' is almost always a misunderstanding. 'BSD4' "
685 686
        ++ "refers to the old 4-clause BSD license with the advertising "
        ++ "clause. 'BSD3' refers the new 3-clause BSD license."
687

688
  , case unknownLicenseVersion (lic) of
689 690
      Just knownVersions -> Just $
        PackageDistSuspicious $
691
             "'license: " ++ display (lic) ++ "' is not a known "
692 693 694 695 696 697
          ++ "version of that license. The known versions are "
          ++ commaSep (map display knownVersions)
          ++ ". If this is not a mistake and you think it should be a known "
          ++ "version then please file a ticket."
      _ -> Nothing

698
  , check (lic `notElem` [ AllRightsReserved
699
                                 , UnspecifiedLicense, PublicDomain]
700
           -- AllRightsReserved and PublicDomain are not strictly
701
           -- licenses so don't need license files.
Duncan Coutts's avatar
Duncan Coutts committed
702
        && null (licenseFiles pkg)) $
703 704
      PackageDistSuspicious "A 'license-file' is not specified."
  ]
705 706 707 708 709 710 711
  where
    unknownLicenseVersion (GPL  (Just v))
      | v `notElem` knownVersions = Just knownVersions
      where knownVersions = [ v' | GPL  (Just v') <- knownLicenses ]
    unknownLicenseVersion (LGPL (Just v))
      | v `notElem` knownVersions = Just knownVersions
      where knownVersions = [ v' | LGPL (Just v') <- knownLicenses ]
712 713 714
    unknownLicenseVersion (AGPL (Just v))
      | v `notElem` knownVersions = Just knownVersions
      where knownVersions = [ v' | AGPL (Just v') <- knownLicenses ]
715 716 717
    unknownLicenseVersion (Apache  (Just v))
      | v `notElem` knownVersions = Just knownVersions
      where knownVersions = [ v' | Apache  (Just v') <- knownLicenses ]
718
    unknownLicenseVersion _ = Nothing
719

720 721 722 723 724 725 726 727 728
    checkVersion :: [Int] -> Bool -> PackageCheck -> Maybe PackageCheck
    checkVersion ver cond pc
      | specVersion pkg >= mkVersion ver       = Nothing
      | otherwise                              = check cond pc

    compatLicenses = [ GPL Nothing, LGPL Nothing, AGPL Nothing, BSD3, BSD4
                     , PublicDomain, AllRightsReserved
                     , UnspecifiedLicense, OtherLicense ]

729 730 731 732 733 734 735 736 737 738
checkSourceRepos :: PackageDescription -> [PackageCheck]
checkSourceRepos pkg =
  catMaybes $ concat [[

    case repoKind repo of
      RepoKindUnknown kind -> Just $ PackageDistInexcusable $
        quote kind ++ " is not a recognised kind of source-repository. "
                   ++ "The repo kind is usually 'head' or 'this'"
      _ -> Nothing

EyalLotem's avatar
EyalLotem committed
739
  , check (isNothing (repoType repo)) $
740 741 742
      PackageDistInexcusable
        "The source-repository 'type' is a required field."

EyalLotem's avatar
EyalLotem committed
743
  , check (isNothing (repoLocation repo)) $
744 745 746
      PackageDistInexcusable
        "The source-repository 'location' is a required field."

EyalLotem's avatar
EyalLotem committed
747
  , check (repoType repo == Just CVS && isNothing (repoModule repo)) $
748 749 750
      PackageDistInexcusable
        "For a CVS source-repository, the 'module' is a required field."

EyalLotem's avatar
EyalLotem committed
751
  , check (repoKind repo == RepoThis && isNothing (repoTag repo)) $
752 753 754 755 756
      PackageDistInexcusable $
           "For the 'this' kind of source-repository, the 'tag' is a required "
        ++ "field. It should specify the tag corresponding to this version "
        ++ "or release of the package."

757
  , check (maybe False isAbsoluteOnAnyPlatform (repoSubdir repo)) $
758 759 760 761 762 763 764
      PackageDistInexcusable
        "The 'subdir' field of a source-repository must be a relative path."
  ]
  | repo <- sourceRepos pkg ]

--TODO: check location looks like a URL for some repo types.

765 766 767 768 769 770 771 772 773 774 775 776 777
-- | Checks GHC options from all ghc-*-options fields in the given
-- PackageDescription and reports commonly misused or non-portable flags
checkAllGhcOptions :: PackageDescription -> [PackageCheck]
checkAllGhcOptions pkg =
    checkGhcOptions "ghc-options" (hcOptions GHC) pkg
 ++ checkGhcOptions "ghc-prof-options" (hcProfOptions GHC) pkg
 ++ checkGhcOptions "ghc-shared-options" (hcSharedOptions GHC) pkg

-- | Extracts GHC options belonging to the given field from the given
-- PackageDescription using given function and checks them for commonly misused
-- or non-portable flags
checkGhcOptions :: String -> (BuildInfo -> [String]) -> PackageDescription -> [PackageCheck]
checkGhcOptions fieldName getOptions pkg =
778 779
  catMaybes [

780
    checkFlags ["-fasm"] $
781
      PackageDistInexcusable $
782
           "'" ++ fieldName ++ ": -fasm' is unnecessary and will not work on CPU "
783
        ++ "architectures other than x86, x86-64, ppc or sparc."
784

Duncan Coutts's avatar
Duncan Coutts committed
785 786
  , checkFlags ["-fvia-C"] $
      PackageDistSuspicious $
787
           "'" ++ fieldName ++": -fvia-C' is usually unnecessary. If your package "
788 789 790
        ++ "needs -via-C for correctness rather than performance then it "
        ++ "is using the FFI incorrectly and will probably not work with GHC "
        ++ "6.10 or later."
Duncan Coutts's avatar
Duncan Coutts committed
791 792 793

  , checkFlags ["-fhpc"] $
      PackageDistInexcusable $
794
           "'" ++ fieldName ++ ": -fhpc' is not not necessary. Use the configure flag "
795
        ++ " --enable-coverage instead."
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
796

Duncan Coutts's avatar
Duncan Coutts committed
797
  , checkFlags ["-prof"] $
798
      PackageBuildWarning $
799
           "'" ++ fieldName ++ ": -prof' is not necessary and will lead to problems "
800
        ++ "when used on a library. Use the configure flag "
gershomb's avatar
gershomb committed
801
        ++ "--enable-library-profiling and/or --enable-profiling."
Duncan Coutts's avatar
Duncan Coutts committed
802 803

  , checkFlags ["-o"] $
804
      PackageBuildWarning $
805
           "'" ++ fieldName ++ ": -o' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
806
        ++ "The output files are named automatically."
Duncan Coutts's avatar
Duncan Coutts committed
807 808

  , checkFlags ["-hide-package"] $
809
      PackageBuildWarning $
810
      "'" ++ fieldName ++ ": -hide-package' is never needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
811
      ++ "Cabal hides all packages."
Duncan Coutts's avatar
Duncan Coutts committed
812

Duncan Coutts's avatar
Duncan Coutts committed
813
  , checkFlags ["--make"] $
814
      PackageBuildWarning $
815
      "'" ++ fieldName ++ ": --make' is never needed. Cabal uses this automatically."
Duncan Coutts's avatar
Duncan Coutts committed
816

Duncan Coutts's avatar
Duncan Coutts committed
817 818
  , checkFlags ["-main-is"] $
      PackageDistSuspicious $
819
      "'" ++ fieldName ++ ": -main-is' is not portable."
Duncan Coutts's avatar
Duncan Coutts committed
820

821
  , checkNonTestAndBenchmarkFlags ["-O0", "-Onot"] $
822
      PackageDistSuspicious $
823
      "'" ++ fieldName ++ ": -O0' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
824
      ++ "Use the --disable-optimization configure flag."
Duncan Coutts's avatar
Duncan Coutts committed
825

826 827
  , checkTestAndBenchmarkFlags ["-O0", "-Onot"] $
      PackageDistSuspiciousWarn $
828
      "'" ++ fieldName ++ ": -O0' is not needed. "
829 830
      ++ "Use the --disable-optimization configure flag."

Duncan Coutts's avatar
Duncan Coutts committed
831
  , checkFlags [ "-O", "-O1"] $
832
      PackageDistInexcusable $
833
      "'" ++ fieldName ++ ": -O' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
834 835
      ++ "Cabal automatically adds the '-O' flag. "
      ++ "Setting it yourself interferes with the --disable-optimization flag."
836

Duncan Coutts's avatar
Duncan Coutts committed
837
  , checkFlags ["-O2"] $
838
      PackageDistSuspiciousWarn $
839
      "'" ++ fieldName ++ ": -O2' is rarely needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
840 841
      ++ "Check that it is giving a real benefit "
      ++ "and not just imposing longer compile times on your users."
842

Ben Gamari's avatar
Ben Gamari committed
843 844
  , checkFlags ["-split-sections"] $
      PackageBuildWarning $
845
        "'" ++ fieldName ++ ": -split-sections' is not needed. "
Ben Gamari's avatar
Ben Gamari committed
846 847
        ++ "Use the --enable-split-sections configure flag."

Duncan Coutts's avatar
Duncan Coutts committed
848
  , checkFlags ["-split-objs"] $
849
      PackageBuildWarning $
850
        "'" ++ fieldName ++ ": -split-objs' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
851
        ++ "Use the --enable-split-objs configure flag."
Duncan Coutts's avatar
Duncan Coutts committed
852

853
  , checkFlags ["-optl-Wl,-s", "-optl-s"] $
854
      PackageDistInexcusable $
855
           "'" ++ fieldName ++ ": -optl-Wl,-s' is not needed and is not portable to all"
856 857 858 859 860
        ++ " operating systems. Cabal 1.4 and later automatically strip"
        ++ " executables. Cabal also has a flag --disable-executable-stripping"
        ++ " which is necessary when building packages for some Linux"
        ++ " distributions and using '-optl-Wl,-s' prevents that from working."

Duncan Coutts's avatar
Duncan Coutts committed
861 862
  , checkFlags ["-fglasgow-exts"] $
      PackageDistSuspicious $
863
        "Instead of '" ++ fieldName ++ ": -fglasgow-exts' it is preferable to use "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
864
        ++ "the 'extensions' field."
Duncan Coutts's avatar
Duncan Coutts committed
865

866
  , check ("-threaded" `elem` lib_ghc_options) $
867
      PackageBuildWarning $
868
           "'" ++ fieldName ++ ": -threaded' has no effect for libraries. It should "
869 870
        ++ "only be used for executables."

871 872
  , check ("-rtsopts" `elem` lib_ghc_options) $
      PackageBuildWarning $
873
           "'" ++ fieldName ++ ": -rtsopts' has no effect for libraries. It should "
874 875 876 877
        ++ "only be used for executables."

  , check (any (\opt -> "-with-rtsopts" `isPrefixOf` opt) lib_ghc_options) $
      PackageBuildWarning $
878
           "'" ++ fieldName ++ ": -with-rtsopts' has no effect for libraries. It "
879 880
        ++ "should only be used for executables."

881
  , checkAlternatives fieldName "extensions"
882 883
      [ (flag, display extension) | flag <- all_ghc_options
                                  , Just extension <- [ghcExtension flag] ]
Duncan Coutts's avatar
Duncan Coutts committed
884

885
  , checkAlternatives fieldName "extensions"
886
      [ (flag, extension) | flag@('-':'X':extension) <- all_ghc_options ]
Duncan Coutts's avatar
Duncan Coutts committed
887

888
  , checkAlternatives fieldName "cpp-options" $
Duncan Coutts's avatar
Duncan Coutts committed
889 890 891
         [ (flag, flag) | flag@('-':'D':_) <- all_ghc_options ]
      ++ [ (flag, flag) | flag@('-':'U':_) <- all_ghc_options ]

892
  , checkAlternatives fieldName "include-dirs"
Duncan Coutts's avatar
Duncan Coutts committed
893 894
      [ (flag, dir) | flag@('-':'I':dir) <- all_ghc_options ]

895
  , checkAlternatives fieldName "extra-libraries"
Duncan Coutts's avatar
Duncan Coutts committed
896 897
      [ (flag, lib) | flag@('-':'l':lib) <- all_ghc_options ]

898
  , checkAlternatives fieldName "extra-lib-dirs"
Duncan Coutts's avatar
Duncan Coutts committed
899
      [ (flag, dir) | flag@('-':'L':dir) <- all_ghc_options ]