Check.hs 90.6 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
-- 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
Jan Hrček's avatar
Jan Hrček committed
17
-- we consider only basic problems. The higher standard is uses 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
import Data.List                                     (group)
40
import Distribution.CabalSpecVersion
41
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
import Distribution.Simple.Utils                     hiding (findPackageDesc, notice)
import Distribution.System
54
import Distribution.Types.ComponentRequestedSpec
Oleg Grenrus's avatar
Oleg Grenrus committed
55
import Distribution.Types.ModuleReexport
56
import Distribution.Utils.Generic                    (isAscii)
57
import Distribution.Verbosity
58
import Distribution.Version
59
import Language.Haskell.Extension
60 61
import System.FilePath
       (splitDirectories, splitExtension, splitPath, takeExtension, takeFileName, (<.>), (</>))
62

63 64 65 66 67
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
68

69 70
import qualified System.Directory        (getDirectoryContents)
import qualified System.FilePath.Windows as FilePath.Windows (isValid)
71

Oleg Grenrus's avatar
Oleg Grenrus committed
72
import qualified Data.Set as Set
73
import qualified Distribution.Utils.ShortText as ShortText
74

75
import qualified Distribution.Types.BuildInfo.Lens                 as L
76
import qualified Distribution.Types.GenericPackageDescription.Lens as L
77
import qualified Distribution.Types.PackageDescription.Lens        as L
78

79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97
-- | 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
98
       -- might be annoying or detrimental when the package is distributed to
99 100 101 102 103
       -- 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 }

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

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

instance Show PackageCheck where
    show notice = explanation notice

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

122
checkSpecVersion :: PackageDescription -> CabalSpecVersion -> Bool -> PackageCheck
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
123
                 -> Maybe PackageCheck
124
checkSpecVersion pkg specver cond pc
125 126
  | specVersion pkg >= specver  = Nothing
  | otherwise                   = check cond pc
127

128 129 130 131 132 133
-- ------------------------------------------------------------
-- * Standard checks
-- ------------------------------------------------------------

-- | Check for common mistakes and problems in package descriptions.
--
Ian D. Bollinger's avatar
Ian D. Bollinger committed
134
-- This is the standard collection of checks covering all aspects except
135 136 137
-- for checks that require looking at files within the package. For those
-- see 'checkPackageFiles'.
--
138 139 140 141 142 143 144 145 146
-- 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
147
  ++ checkConditionals gpkg
148
  ++ checkPackageVersions gpkg
149
  ++ checkDevelopmentOnlyFlags gpkg
Oleg Grenrus's avatar
Oleg Grenrus committed
150
  ++ checkFlagNames gpkg
Oleg Grenrus's avatar
Oleg Grenrus committed
151
  ++ checkUnusedFlags gpkg
Oleg Grenrus's avatar
Oleg Grenrus committed
152
  ++ checkUnicodeXFields gpkg
153
  ++ checkPathsModuleExtensions pkg
154 155 156 157
  where
    pkg = fromMaybe (flattenPackageDescription gpkg) mpkg

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


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

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

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

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

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

197
  , check (any (== LMainLibName) (map libName $ subLibraries pkg)) $
198 199 200
      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
201
  , check (not (null duplicateNames)) $
202 203
      PackageBuildImpossible $ "Duplicate sections: "
        ++ commaSep (map unUnqualComponentName duplicateNames)
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
204 205
        ++ ". The name of every library, executable, test suite,"
        ++ " and benchmark section in"
tibbe's avatar
tibbe committed
206
        ++ " the package must be unique."
207 208

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

221
  ++ concatMap (checkLibrary    pkg) (allLibraries pkg)
222 223 224
  ++ concatMap (checkExecutable pkg) (executables pkg)
  ++ concatMap (checkTestSuite  pkg) (testSuites pkg)
  ++ concatMap (checkBenchmark  pkg) (benchmarks pkg)
225

226
  where
227
    -- The public 'library' gets special dispensation, because it
228
    -- is common practice to export a library and name the executable
229
    -- the same as the package.
230
    subLibNames = mapMaybe (libraryNameString . libName) $ subLibraries pkg
231
    exeNames = map exeName $ executables pkg
232
    testNames = map testName $ testSuites pkg
tibbe's avatar
tibbe committed
233
    bmNames = map benchmarkName $ benchmarks pkg
234
    duplicateNames = dups $ subLibNames ++ exeNames ++ testNames ++ bmNames
235

236
checkLibrary :: PackageDescription -> Library -> [PackageCheck]
237
checkLibrary pkg lib =
238 239
  catMaybes [

240
    check (not (null moduleDuplicates)) $
241
       PackageBuildImpossible $
242
            "Duplicate modules in library: "
243
         ++ commaSep (map prettyShow moduleDuplicates)
244

245 246
  -- TODO: This check is bogus if a required-signature was passed through
  , check (null (explicitLibModules lib) && null (reexportedModules lib)) $
247
      PackageDistSuspiciousWarn $
248
           showLibraryName (libName lib) ++ " does not expose any modules"
249

250
    -- check use of signatures sections
251
  , checkVersion CabalSpecV2_0 (not (null (signatures lib))) $
252
      PackageDistInexcusable $
253
           "To use the 'signatures' field the package needs to specify "
254
        ++ "at least 'cabal-version: 2.0'."
fmaste's avatar
fmaste committed
255 256 257

    -- check that all autogen-modules appear on other-modules or exposed-modules
  , check
258
      (not $ and $ map (flip elem (explicitLibModules lib)) (libModulesAutogen lib)) $
fmaste's avatar
fmaste committed
259 260 261 262
      PackageBuildImpossible $
           "An 'autogen-module' is neither on 'exposed-modules' or "
        ++ "'other-modules'."

Oleg Grenrus's avatar
Oleg Grenrus committed
263 264 265 266 267 268
    -- check that all autogen-includes appear on includes or install-includes
  , check
      (not $ and $ map (flip elem (allExplicitIncludes lib)) (view L.autogenIncludes lib)) $
      PackageBuildImpossible $
           "An include in 'autogen-includes' is neither in 'includes' or "
        ++ "'install-includes'."
269 270
  ]

271
  where
272
    checkVersion :: CabalSpecVersion -> Bool -> PackageCheck -> Maybe PackageCheck
273
    checkVersion ver cond pc
274 275
      | specVersion pkg >= ver = Nothing
      | otherwise              = check cond pc
276

277 278
    -- TODO: not sure if this check is always right in Backpack
    moduleDuplicates = dups (explicitLibModules lib ++
279
                             map moduleReexportName (reexportedModules lib))
Duncan Coutts's avatar
Duncan Coutts committed
280

Oleg Grenrus's avatar
Oleg Grenrus committed
281 282 283
allExplicitIncludes :: L.HasBuildInfo a => a -> [FilePath]
allExplicitIncludes x = view L.includes x ++ view L.installIncludes x

284 285
checkExecutable :: PackageDescription -> Executable -> [PackageCheck]
checkExecutable pkg exe =
286 287 288 289
  catMaybes [

    check (null (modulePath exe)) $
      PackageBuildImpossible $
290
        "No 'main-is' field found for executable " ++ prettyShow (exeName exe)
291 292

  , check (not (null (modulePath exe))
293
       && (not $ fileExtensionSupportedLanguage $ modulePath exe)) $
294
      PackageBuildImpossible $
295 296 297
           "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
298

299
  , checkSpecVersion pkg CabalSpecV1_18
300 301 302 303 304 305
          (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
306
  , check (not (null moduleDuplicates)) $
307
       PackageBuildImpossible $
308 309
            "Duplicate modules in executable '" ++ prettyShow (exeName exe) ++ "': "
         ++ commaSep (map prettyShow moduleDuplicates)
fmaste's avatar
fmaste committed
310 311 312 313 314

    -- check that all autogen-modules appear on other-modules
  , check
      (not $ and $ map (flip elem (exeModules exe)) (exeModulesAutogen exe)) $
      PackageBuildImpossible $
315
           "On executable '" ++ prettyShow (exeName exe) ++ "' an 'autogen-module' is not "
fmaste's avatar
fmaste committed
316
        ++ "on 'other-modules'"
Oleg Grenrus's avatar
Oleg Grenrus committed
317 318 319 320 321

    -- check that all autogen-includes appear on includes
  , check
      (not $ and $ map (flip elem (view L.includes exe)) (view L.autogenIncludes exe)) $
      PackageBuildImpossible "An include in 'autogen-includes' is not in 'includes'."
322
  ]
323 324
  where
    moduleDuplicates = dups (exeModules exe)
325

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

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

      TestSuiteUnsupported tt -> Just $
        PackageBuildWarning $
339
             quote (prettyShow tt) ++ " is not a supported test suite version. "
340
          ++ "The known test suite types are: "
341
          ++ commaSep (map prettyShow knownTestTypes)
342 343 344
      _ -> Nothing

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

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

355
  , checkSpecVersion pkg CabalSpecV1_18 (mainIsNotHsExt && not mainIsWrongExt) $
356 357 358
      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
359 360 361

    -- check that all autogen-modules appear on other-modules
  , check
Oleg Grenrus's avatar
Oleg Grenrus committed
362
      (not $ and $ map (flip elem (testModules test)) (testModulesAutogen test)) $
fmaste's avatar
fmaste committed
363
      PackageBuildImpossible $
364
           "On test suite '" ++ prettyShow (testName test) ++ "' an 'autogen-module' is not "
fmaste's avatar
fmaste committed
365
        ++ "on 'other-modules'"
Oleg Grenrus's avatar
Oleg Grenrus committed
366 367 368 369 370

    -- check that all autogen-includes appear on includes
  , check
      (not $ and $ map (flip elem (view L.includes test)) (view L.autogenIncludes test)) $
      PackageBuildImpossible "An include in 'autogen-includes' is not in 'includes'."
371
  ]
372 373
  where
    moduleDuplicates = dups $ testModules test
374 375

    mainIsWrongExt = case testInterface test of
376
      TestSuiteExeV10 _ f -> not $ fileExtensionSupportedLanguage f
377 378
      _                   -> False

379 380 381 382
    mainIsNotHsExt = case testInterface test of
      TestSuiteExeV10 _ f -> takeExtension f `notElem` [".hs", ".lhs"]
      _                   -> False

tibbe's avatar
tibbe committed
383
checkBenchmark :: PackageDescription -> Benchmark -> [PackageCheck]
384
checkBenchmark _pkg bm =
tibbe's avatar
tibbe committed
385 386 387 388 389
  catMaybes [

    case benchmarkInterface bm of
      BenchmarkUnsupported tt@(BenchmarkTypeUnknown _ _) -> Just $
        PackageBuildWarning $
390
             quote (prettyShow tt) ++ " is not a known type of benchmark. "
tibbe's avatar
tibbe committed
391
          ++ "The known benchmark types are: "
392
          ++ commaSep (map prettyShow knownBenchmarkTypes)
tibbe's avatar
tibbe committed
393 394 395

      BenchmarkUnsupported tt -> Just $
        PackageBuildWarning $
396
             quote (prettyShow tt) ++ " is not a supported benchmark version. "
tibbe's avatar
tibbe committed
397
          ++ "The known benchmark types are: "
398
          ++ commaSep (map prettyShow knownBenchmarkTypes)
tibbe's avatar
tibbe committed
399 400 401
      _ -> Nothing

  , check (not $ null moduleDuplicates) $
402
      PackageBuildImpossible $
403 404
           "Duplicate modules in benchmark '" ++ prettyShow (benchmarkName bm) ++ "': "
        ++ commaSep (map prettyShow moduleDuplicates)
tibbe's avatar
tibbe committed
405 406 407 408 409

  , 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
410 411 412

    -- check that all autogen-modules appear on other-modules
  , check
Oleg Grenrus's avatar
Oleg Grenrus committed
413
      (not $ and $ map (flip elem (benchmarkModules bm)) (benchmarkModulesAutogen bm)) $
fmaste's avatar
fmaste committed
414
      PackageBuildImpossible $
415
             "On benchmark '" ++ prettyShow (benchmarkName bm) ++ "' an 'autogen-module' is "
fmaste's avatar
fmaste committed
416
          ++ "not on 'other-modules'"
Oleg Grenrus's avatar
Oleg Grenrus committed
417 418 419 420 421

    -- check that all autogen-includes appear on includes
  , check
      (not $ and $ map (flip elem (view L.includes bm)) (view L.autogenIncludes bm)) $
      PackageBuildImpossible "An include in 'autogen-includes' is not in 'includes'."
tibbe's avatar
tibbe committed
422 423 424 425 426 427 428 429
  ]
  where
    moduleDuplicates = dups $ benchmarkModules bm

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

430 431 432 433 434 435 436 437
-- ------------------------------------------------------------
-- * Additional pure checks
-- ------------------------------------------------------------

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

438
    check (not . FilePath.Windows.isValid . prettyShow . packageName $ pkg) $
439
      PackageDistInexcusable $
440
           "Unfortunately, the package name '" ++ prettyShow (packageName pkg)
441 442 443 444
        ++ "' 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."

445
  , check ((isPrefixOf "z-") . prettyShow . packageName $ pkg) $
446 447 448 449
      PackageDistInexcusable $
           "Package names with the prefix 'z-' are reserved by Cabal and "
        ++ "cannot be used."

450
  , check (isNothing (buildTypeRaw pkg) && specVersion pkg < CabalSpecV2_2) $
451 452 453 454
      PackageBuildWarning $
           "No 'build-type' specified. If you do not need a custom Setup.hs or "
        ++ "./configure script then use 'build-type: Simple'."

455
  , check (isJust (setupBuildInfo pkg) && buildType pkg /= Custom) $
456 457 458 459 460
      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."

461 462
  , check (not (null unknownCompilers)) $
      PackageBuildWarning $
463
        "Unknown compiler " ++ commaSep (map quote unknownCompilers)
464 465
                            ++ " in 'tested-with' field."

466 467 468 469
  , check (not (null unknownLanguages)) $
      PackageBuildWarning $
        "Unknown languages: " ++ commaSep unknownLanguages

470 471
  , check (not (null unknownExtensions)) $
      PackageBuildWarning $
472
        "Unknown extensions: " ++ commaSep unknownExtensions
473

474 475 476 477 478 479 480
  , 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."

481
  , check (not (null ourDeprecatedExtensions)) $
482 483
      PackageDistSuspicious $
           "Deprecated extensions: "
484
        ++ commaSep (map (quote . prettyShow . fst) ourDeprecatedExtensions)
EyalLotem's avatar
EyalLotem committed
485
        ++ ". " ++ unwords
486 487
             [ "Instead of '" ++ prettyShow ext
            ++ "' use '" ++ prettyShow replacement ++ "'."
488
             | (ext, Just replacement) <- ourDeprecatedExtensions ]
489

490
  , check (ShortText.null (category pkg)) $
491 492
      PackageDistSuspicious "No 'category' field."

493
  , check (ShortText.null (maintainer pkg)) $
494 495
      PackageDistSuspicious "No 'maintainer' field."

496
  , check (ShortText.null (synopsis pkg) && ShortText.null (description pkg)) $
EyalLotem's avatar
EyalLotem committed
497
      PackageDistInexcusable "No 'synopsis' or 'description' field."
498

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

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

Ian D. Bollinger's avatar
Ian D. Bollinger committed
505
    --TODO: recommend the bug reports URL, author and homepage fields
506 507 508
    --TODO: recommend not using the stability field
    --TODO: recommend specifying a source repo

509
  , check (ShortText.length (synopsis pkg) >= 80) $
510 511
      PackageDistSuspicious
        "The 'synopsis' field is rather long (max 80 chars is recommended)."
512

513
    -- See also https://github.com/haskell/cabal/pull/3479
514 515
  , check (not (ShortText.null (description pkg))
           && ShortText.length (description pkg) <= ShortText.length (synopsis pkg)) $
516 517 518 519 520 521 522 523 524 525 526 527 528
      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."

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: "
533
        ++ commaSep (map prettyShow testedWithImpossibleRanges)
534 535 536 537
        ++ ". 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'."
538

John Ericson's avatar
John Ericson committed
539
  , check (not (null depInternalLibraryWithExtraVersion)) $
540
      PackageBuildWarning $
John Ericson's avatar
John Ericson committed
541
           "The package has an extraneous version range for a dependency on an "
542
        ++ "internal library: "
543
        ++ commaSep (map prettyShow depInternalLibraryWithExtraVersion)
John Ericson's avatar
John Ericson committed
544 545 546 547 548 549 550
        ++ ". 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: "
551
        ++ commaSep (map prettyShow depInternalLibraryWithImpossibleVersion)
John Ericson's avatar
John Ericson committed
552 553 554 555 556 557 558
        ++ ". 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: "
559
        ++ commaSep (map prettyShow depInternalExecutableWithExtraVersion)
John Ericson's avatar
John Ericson committed
560 561 562 563 564 565 566
        ++ ". 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: "
567
        ++ commaSep (map prettyShow depInternalExecutableWithImpossibleVersion)
John Ericson's avatar
John Ericson committed
568 569 570 571 572 573
        ++ ". 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: "
574
        ++ commaSep (map prettyShow depInternalExecutableWithImpossibleVersion)
575
  ]
576 577
  where
    unknownCompilers  = [ name | (OtherCompiler name, _) <- testedWith pkg ]
578 579
    unknownLanguages  = [ name | bi <- allBuildInfo pkg
                               , UnknownLanguage name <- allLanguages bi ]
580
    unknownExtensions = [ name | bi <- allBuildInfo pkg
581
                               , UnknownExtension name <- allExtensions bi
582
                               , name `notElem` map prettyShow knownLanguages ]
583 584
    ourDeprecatedExtensions = nub $ catMaybes
      [ find ((==ext) . fst) deprecatedExtensions
585
      | bi <- allBuildInfo pkg
Ian Lynagh's avatar
Ian Lynagh committed
586
      , ext <- allExtensions bi ]
587 588 589
    languagesUsedAsExtensions =
      [ name | bi <- allBuildInfo pkg
             , UnknownExtension name <- allExtensions bi
590
             , name `elem` map prettyShow knownLanguages ]
591

592
    testedWithImpossibleRanges =
593
      [ Dependency (mkPackageName (prettyShow compiler)) vr mainLibSet
594 595 596
      | (compiler, vr) <- testedWith pkg
      , isNoVersion vr ]

597
    internalLibraries =
598
        map (maybe (packageName pkg) (unqualComponentNameToPackageName) . libraryNameString . libName)
599
            (allLibraries pkg)
John Ericson's avatar
John Ericson committed
600 601 602 603

    internalExecutables = map exeName $ executables pkg

    internalLibDeps =
604 605
      [ dep
      | bi <- allBuildInfo pkg
606
      , dep@(Dependency name _ _) <- targetBuildDepends bi
607 608 609
      , name `elem` internalLibraries
      ]

John Ericson's avatar
John Ericson committed
610 611 612
    internalExeDeps =
      [ dep
      | bi <- allBuildInfo pkg
613 614
      , dep <- getAllToolDependencies pkg bi
      , isInternal pkg dep
John Ericson's avatar
John Ericson committed
615 616 617 618
      ]

    depInternalLibraryWithExtraVersion =
      [ dep
619
      | dep@(Dependency _ versionRange _) <- internalLibDeps
John Ericson's avatar
John Ericson committed
620 621 622 623 624 625
      , not $ isAnyVersion versionRange
      , packageVersion pkg `withinRange` versionRange
      ]

    depInternalLibraryWithImpossibleVersion =
      [ dep
626
      | dep@(Dependency _ versionRange _) <- internalLibDeps
John Ericson's avatar
John Ericson committed
627 628 629 630 631 632 633 634 635 636 637 638 639 640 641 642 643 644 645 646 647 648
      , 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
      ]

649

650
checkLicense :: PackageDescription -> [PackageCheck]
651 652 653 654 655 656 657 658 659 660
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."
    ]
661

662 663 664
checkOldLicense :: PackageDescription -> License -> [PackageCheck]
checkOldLicense pkg lic = catMaybes
  [ check (lic == UnspecifiedLicense) $
665
      PackageDistInexcusable
666
        "The 'license' field is missing."
667

668
  , check (lic == AllRightsReserved) $
669 670
      PackageDistSuspicious
        "The 'license' is AllRightsReserved. Is that really what you want?"
671

672
  , checkVersion CabalSpecV1_4 (lic `notElem` compatLicenses) $
673 674 675 676 677 678 679
      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
680 681
      UnknownLicense l -> Just $
        PackageBuildWarning $
682 683
             quote ("license: " ++ l) ++ " is not a recognised license. The "
          ++ "known licenses are: "
684
          ++ commaSep (map prettyShow knownLicenses)
685 686
      _ -> Nothing

687
  , check (lic == BSD4) $
688 689
      PackageDistSuspicious $
           "Using 'license: BSD4' is almost always a misunderstanding. 'BSD4' "
690 691
        ++ "refers to the old 4-clause BSD license with the advertising "
        ++ "clause. 'BSD3' refers the new 3-clause BSD license."
692

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

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

725
    checkVersion :: CabalSpecVersion -> Bool -> PackageCheck -> Maybe PackageCheck
726
    checkVersion ver cond pc
727 728
      | specVersion pkg >= ver  = Nothing
      | otherwise               = check cond pc
729 730 731 732 733

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

734 735 736 737 738 739 740 741 742 743
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
744
  , check (isNothing (repoType repo)) $
745 746 747
      PackageDistInexcusable
        "The source-repository 'type' is a required field."

EyalLotem's avatar
EyalLotem committed
748
  , check (isNothing (repoLocation repo)) $
749 750 751
      PackageDistInexcusable
        "The source-repository 'location' is a required field."

752
  , check (repoType repo == Just (KnownRepoType CVS) && isNothing (repoModule repo)) $
753 754 755
      PackageDistInexcusable
        "For a CVS source-repository, the 'module' is a required field."

EyalLotem's avatar
EyalLotem committed
756
  , check (repoKind repo == RepoThis && isNothing (repoTag repo)) $
757 758 759 760 761
      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."

762
  , check (maybe False isAbsoluteOnAnyPlatform (repoSubdir repo)) $
763 764 765 766 767 768 769
      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.

770 771 772 773 774 775 776 777 778 779 780 781 782
-- | 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 =
783 784
  catMaybes [

785
    checkFlags ["-fasm"] $
786
      PackageDistInexcusable $
787
           "'" ++ fieldName ++ ": -fasm' is unnecessary and will not work on CPU "
788
        ++ "architectures other than x86, x86-64, ppc or sparc."
789

Duncan Coutts's avatar
Duncan Coutts committed
790 791
  , checkFlags ["-fvia-C"] $
      PackageDistSuspicious $
792
           "'" ++ fieldName ++": -fvia-C' is usually unnecessary. If your package "
793 794 795
        ++ "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
796 797 798

  , checkFlags ["-fhpc"] $
      PackageDistInexcusable $
Jan Hrček's avatar
Jan Hrček committed
799
           "'" ++ fieldName ++ ": -fhpc' is not necessary. Use the configure flag "
800
        ++ " --enable-coverage instead."
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
801

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

  , checkFlags ["-o"] $
809
      PackageBuildWarning $
810
           "'" ++ fieldName ++ ": -o' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
811
        ++ "The output files are named automatically."
Duncan Coutts's avatar
Duncan Coutts committed
812 813

  , checkFlags ["-hide-package"] $
814
      PackageBuildWarning $
815
      "'" ++ fieldName ++ ": -hide-package' is never needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
816
      ++ "Cabal hides all packages."
Duncan Coutts's avatar
Duncan Coutts committed
817

Duncan Coutts's avatar
Duncan Coutts committed
818
  , checkFlags ["--make"] $
819
      PackageBuildWarning $
820
      "'" ++ fieldName ++ ": --make' is never needed. Cabal uses this automatically."
Duncan Coutts's avatar
Duncan Coutts committed
821

Duncan Coutts's avatar
Duncan Coutts committed
822 823
  , checkFlags ["-main-is"] $
      PackageDistSuspicious $
824
      "'" ++ fieldName ++ ": -main-is' is not portable."
Duncan Coutts's avatar
Duncan Coutts committed
825

826
  , checkNonTestAndBenchmarkFlags ["-O0", "-Onot"] $
827
      PackageDistSuspicious $
828
      "'" ++ fieldName ++ ": -O0' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
829
      ++ "Use the --disable-optimization configure flag."
Duncan Coutts's avatar
Duncan Coutts committed
830

831 832
  , checkTestAndBenchmarkFlags ["-O0", "-Onot"] $
      PackageDistSuspiciousWarn $
833
      "'" ++ fieldName ++ ": -O0' is not needed. "
834 835
      ++ "Use the --disable-optimization configure flag."

Duncan Coutts's avatar
Duncan Coutts committed
836
  , checkFlags [ "-O", "-O1"] $
837
      PackageDistInexcusable $
838
      "'" ++ fieldName ++ ": -O' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
839 840
      ++ "Cabal automatically adds the '-O' flag. "
      ++ "Setting it yourself interferes with the --disable-optimization flag."
841

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

Ben Gamari's avatar
Ben Gamari committed
848 849
  , checkFlags ["-split-sections"] $
      PackageBuildWarning $
850
        "'" ++ fieldName ++ ": -split-sections' is not needed. "
Ben Gamari's avatar
Ben Gamari committed
851 852
        ++ "Use the --enable-split-sections configure flag."

Duncan Coutts's avatar
Duncan Coutts committed
853
  , checkFlags ["-split-objs"] $
854
      PackageBuildWarning $
855
        "'" ++ fieldName ++ ": -split-objs' is not needed. "
Mikhail Glushenkov's avatar
Mikhail Glushenkov committed
856
        ++ "Use the --enable-split-objs configure flag."