diff --git a/check-pvp.cabal b/check-pvp.cabal --- a/check-pvp.cabal +++ b/check-pvp.cabal @@ -1,5 +1,5 @@ Name: check-pvp -Version: 0.0 +Version: 0.0.1 License: BSD3 License-File: LICENSE Author: Henning Thielemann @@ -97,19 +97,6 @@ . > import Data.Map (Map) . - The program may complain about an open list of constructors as in - . - > import Data.Sequence (ViewL(..)) - . - Additions of constructors to @ViewL@ may also conflict with other identifiers. - You must instead import like - . - > import Data.Sequence (ViewL(EmptyL, (:<))) - . - or - . - > import qualified Data.Sequence as Seq - . The program emits an error on clashing module abbreviations like . > import qualified Data.Map.Lazy as Map @@ -125,8 +112,40 @@ . but I think it is good idea to avoid redundant imports anyway. . - Additionally you can enable a test for hiding imports - with the @--pedantic@ option. + Additionally there are warnings on imports + that are consistent with large version ranges, + but complicate API changing updates of your dependencies. + You can disable these warnings with @--disable-warnings@. + . + The program warns about an open list of constructors as in + . + > import Data.Sequence (ViewL(..)) + . + Additions of constructors to @ViewL@ may also conflict with other identifiers, + but additions of constructors are considered API changes + since they may turn a complete case analysis into an incomplete one. + Similarly additionally class methods can turn a complete class instance + into a partial one. + Thus addition of constructors and class methods + require a version bump from @x.y.z@ to @x.y+1@. + Nonetheless it is a good idea to import either + . + > import Data.Sequence (ViewL(EmptyL, (:<))) + . + or + . + > import qualified Data.Sequence as Seq + . + because you document the origin of identifiers this way. + This is especially important when the imported identifiers + are moved or removed in the future. + If you use constructors only for constructions and not for pattern matches + or if you only call class methods but do not define instances, + then with explicit imports or qualified imports + your modules survive such additions in your dependent packages + without modifications. + . + More warnings are issued for hiding imports. The import . > import Data.Map hiding (insert) @@ -188,6 +207,13 @@ See . Unfortunately there is no GHC warning on clashing module abbreviations. See . + . + Related: + There are programs that check PVP compliance of exports: + . + * @precis@: + . + * @apidiff@: Tested-With: GHC==7.4.2 Cabal-Version: >=1.6 Build-Type: Simple @@ -198,7 +224,7 @@ Source-Repository this type: darcs location: http://code.haskell.org/~thielema/check-pvp/ - tag: 0.0 + tag: 0.0.1 Executable check-pvp diff --git a/src/Main.hs b/src/Main.hs --- a/src/Main.hs +++ b/src/Main.hs @@ -60,7 +60,8 @@ flagVerbosity :: Verbosity.Verbosity, flagBuildDir :: FilePath, flagClassifyDependencies :: Bool, - flagPedantic :: Bool, + flagClassifyGrouped :: Bool, + flagWarnings :: Bool, flagCheckLibrary, flagCheckExecutables, flagCheckTestSuites, @@ -77,7 +78,8 @@ flagVerbosity = Verbosity.silent, flagBuildDir = Setup.defaultDistPref, flagClassifyDependencies = False, - flagPedantic = False, + flagClassifyGrouped = False, + flagWarnings = True, flagCheckLibrary = True, flagCheckExecutables = True, flagCheckTestSuites = True, @@ -110,9 +112,9 @@ Option [] ["classify-dependencies"] (NoArg (\flags -> return $ flags{flagClassifyDependencies = True})) "print diagnostics of version ranges in Build-Depends fields" : - Option [] ["pedantic"] - (NoArg (\flags -> return $ flags{flagPedantic = True})) - "also check for hiding imports" : + Option [] ["disable-warnings"] + (NoArg (\flags -> return $ flags{flagWarnings = False})) + "suppress warnings" : Option [] ["include-all"] (NoArg @@ -248,7 +250,7 @@ data CheckFlags = CheckFlags { criticalModule :: Syntax.ModuleName -> Bool, - checkPedantic :: Bool + showWarnings :: Bool } run :: Flags -> IO () @@ -261,9 +263,7 @@ notice verbosity "Package description" let classified = classifyDependencies $ P.buildDepends desc - mapM_ - (printUpperBoundDiagnostics $ flagClassifyDependencies flags) - classified + mapM_ (printUpperBoundDiagnostics flags) classified pkgIdx <- if flagLoadPackageIndex flags @@ -273,7 +273,7 @@ let checkFlags = CheckFlags { criticalModule = flip memberModule modIdx, - checkPedantic = flagPedantic flags + showWarnings = flagWarnings flags } when (flagCheckLibrary flags) $ do @@ -395,21 +395,24 @@ Parser.ParseFailed loc msg -> hPrintf IO.stderr "\n%s\n %s\n" (formatSrcLoc loc) msg - Parser.ParseOk modu -> checkModule flags modu + Parser.ParseOk + (Syntax.Module _loc _name _pragma _warn _export imports _decls) -> + checkImports flags imports -checkModule :: CheckFlags -> Syntax.Module -> IO () -checkModule flags - (Syntax.Module _loc _name _pragma _warn _export imports _decls) = do +checkImports :: CheckFlags -> [Syntax.ImportDecl] -> IO () +checkImports flags imports = do forM_ (filter (criticalModule flags . Syntax.importModule) imports) $ \imp -> do let problems = Mn.when (not $ strictImport imp) ["lax import"] ++ - (Mn.when - (checkPedantic flags && Fold.any fst (Syntax.importSpecs imp)) - ["hiding import"]) - ++ - (flip map (implicitSpecs imp) $ \name -> - "open constructor or method list for " ++ prettyPrint name) + Mn.when (showWarnings flags) + (Mn.when + (Fold.any fst $ Syntax.importSpecs imp) + ["Warning: hiding import"] + ++ + (flip map (implicitSpecs imp) $ \name -> + "Warning: open constructor or method list for " ++ + prettyPrint name)) when (not $ null problems) $ do void $ @@ -491,8 +494,8 @@ data BoundClass = Open | Lax Int | Generous Int Int | Tight [Int] -printUpperBoundDiagnostics :: Bool -> DepAttrs -> IO () -printUpperBoundDiagnostics classifyAll depAttrs = +printUpperBoundDiagnostics :: Flags -> DepAttrs -> IO () +printUpperBoundDiagnostics flags depAttrs = let warn = (,) True info = (,) False @@ -520,13 +523,18 @@ filteredMsgs = map snd $ - if classifyAll + if flagClassifyDependencies flags then msgs else filter fst msgs - in when (not $ null filteredMsgs) $ do - putStrLn $ unpackPkgName $ depPkgName depAttrs - putStrLn $ unlines $ map (replicate 4 ' ' ++) filteredMsgs + in if flagClassifyGrouped flags + then when (not $ null filteredMsgs) $ + putStr $ unlines $ + "" : + unpackPkgName (depPkgName depAttrs) : + map (replicate 4 ' ' ++) filteredMsgs + else forM_ filteredMsgs $ \msg -> + printf "%s: %s\n" (unpackPkgName $ depPkgName depAttrs) msg classifyDependencies :: [Pkg.Dependency] -> [DepAttrs] classifyDependencies deps =