diff --git a/ReadMe.md b/ReadMe.md new file mode 100644 --- /dev/null +++ b/ReadMe.md @@ -0,0 +1,187 @@ +You must run the tool in a directory containing a Cabal package. + + $ check-pvp + +This requires that the package is configured, +since only then the association of packages to modules is known. +If you want to run the tool on a non-configured package +you may just check all imports for addition-proof style. + + $ check-pvp --include-all + +It follows a detailed description of the procedure +and the rationale behind it. + +First the program classifies all dependencies +in the Cabal file of the package. +You can show all classifications with the `--classify-dependencies` option, +otherwise only problematic dependencies are shown. + +A dependency like `containers >=0.5.0.3 && <0.5.1` +does not allow changes of the API of `containers` +and thus the program does not check its imports. +Clashing import abbreviations are an exception. + +The dependency `containers >=0.5.1 && <0.7` +requires more care when importing modules from `containers` +and this is what the program is going to check next. +This is the main purpose of the program! +I warmly recommend this kind of dependency range +since it greatly reduces the work +to keep your package going together with its imported packages. + +Dependencies like `containers >=0.5` or `containers >=0.5 && <1` +are always problematic, +since within the specified version ranges identifier can disappear. +There is no import style that protects against removed identifiers. + +An inclusive upper bound as in `containers >=0.5 && <=0.6` +will also cause a warning, because it is unnecessarily strict. +If you know that `containers-0.6` works for you, +then `containers-0.6.0.1` or `containers-0.6.1` will also work, +depending on your import style. +A special case of inclusive upper bounds are specific versions +like in `containers ==0.6`. +The argument for the warning remains the same. + +Please note that the check of ranges +is performed entirely on the package description. +The program will not inspect the imported module contents. +E.g. if you depend on `containers >=0.5 && <0.7` +but import in a way that risks name clashes, +then you may just extend the dependency to `containers >=0.5 && <0.7.1` +in order to let the checker fall silent. +If you use the dependency `containers >=0.5 && <0.7.1` +then the checker expects that you have verified +that your package works with all versions of kind `0.5.x` +and the version `0.6.0`. +Other versions would then work, too, +due to the constraints imposed by package versioning policy. + +Let us now look at imports +that must be protected against identifier additions. + +The program may complain about a lax import. +This means you have imported like + + import Data.Map as Map + +Additions to `Data.Map` may clash with other identifiers, +thus you must import either + + import qualified Data.Map as Map + +or + + import Data.Map (Map) + +The program emits an error on clashing module abbreviations like + + import qualified Data.Map.Lazy as Map + import qualified Data.Map.Strict as Map + +This error is raised +whenever multiple modules are imported with the same abbreviation, +where at least one module is open for additions. +Our test is overly strict in the sense that it also blames + + import qualified Data.Map as Map + import qualified Data.Map as Map + +but I think it is good idea to avoid redundant imports anyway. + +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) + +is not bad in the sense of the PVP, +but this way you depend on the existence of the identifier `insert` +although you do not need it. +If it is removed in a later version of `containers`, +then your import breaks although you did not use the identifier. + +Finally you can control what items are checked. +First of all you can select the imports that are checked. +Normally the imports are checked that belong to lax dependencies +like `containers >=0.5 && <0.7`. +However this requires the package to be configured +in order to know which import belongs to which dependency. +E.g. `Data.Map` belongs to `containers`. +You can just check all imports for being addition-proof +using the `--include-all` option. +Following you can write the options +`--include-import`, +`--exclude-import`, +`--include-dependency`, +`--exclude-dependency` +that allow to additionally check or ignore imports +from certain modules or packages. +These modifiers are applied from left to right. +E.g. `--exclude-import=Prelude` will accept any import style for `Prelude` +and `--exclude-dependency=foobar` will ignore the package `foobar`, +say, because it does not conform to the PVP. + +Secondly, you may ignore certain modules or components of the package +using the options +`--exclude-module`, +`--exclude-library`, +`--exclude-executables`, +`--exclude-testsuites`, +`--exclude-benchmarks`. +E.g. `--exclude-module=Paths_PKG` will exclude the Paths module +that is generated by Cabal. +I assume that it will always be free of name clashes. + +Known problems: + +* The program cannot automatically filter out the `Paths` module. + +* The program cannot find and check preprocessed modules. + +* The program may yield wrong results in the presence of Cabal conditions. + +If this program proves to be useful +it might eventually be integrated in the `check` command of `cabal-install`. +See . + +Alternative: +If you want to allow exclusively large version ranges, i.e. `>=x.y && . +Unfortunately there is no GHC warning on clashing module abbreviations. +See . 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.2 +Version: 0.0.2.1 License: BSD3 License-File: LICENSE Author: Henning Thielemann @@ -20,194 +20,6 @@ are protected against name clashes that could be caused by addition of identifiers. . - You must run the tool in a directory containing a Cabal package. - . - > $ check-pvp - . - This requires that the package is configured, - since only then the association of packages to modules is known. - If you want to run the tool on a non-configured package - you may just check all imports for addition-proof style. - . - > $ check-pvp --include-all - . - It follows a detailed description of the procedure - and the rationale behind it. - . - First the program classifies all dependencies - in the Cabal file of the package. - You can show all classifications with the @--classify-dependencies@ option, - otherwise only problematic dependencies are shown. - . - A dependency like @containers >=0.5.0.3 && <0.5.1@ - does not allow changes of the API of @containers@ - and thus the program does not check its imports. - Clashing import abbreviations are an exception. - . - The dependency @containers >=0.5.1 && <0.7@ - requires more care when importing modules from @containers@ - and this is what the program is going to check next. - This is the main purpose of the program! - I warmly recommend this kind of dependency range - since it greatly reduces the work - to keep your package going together with its imported packages. - . - Dependencies like @containers >=0.5@ or @containers >=0.5 && <1@ - are always problematic, - since within the specified version ranges identifier can disappear. - There is no import style that protects against removed identifiers. - . - An inclusive upper bound as in @containers >=0.5 && <=0.6@ - will also cause a warning, because it is unnecessarily strict. - If you know that @containers-0.6@ works for you, - then @containers-0.6.0.1@ or @containers-0.6.1@ will also work, - depending on your import style. - A special case of inclusive upper bounds are specific versions - like in @containers ==0.6@. - The argument for the warning remains the same. - . - Please note that the check of ranges - is performed entirely on the package description. - The program will not inspect the imported module contents. - E.g. if you depend on @containers >=0.5 && <0.7@ - but import in a way that risks name clashes, - then you may just extend the dependency to @containers >=0.5 && <0.7.1@ - in order to let the checker fall silent. - If you use the dependency @containers >=0.5 && <0.7.1@ - then the checker expects that you have verified - that your package works with all versions of kind @0.5.x@ - and the version @0.6.0@. - Other versions would then work, too, - due to the constraints imposed by package versioning policy. - . - Let us now look at imports - that must be protected against identifier additions. - . - The program may complain about a lax import. - This means you have imported like - . - > import Data.Map as Map - . - Additions to @Data.Map@ may clash with other identifiers, - thus you must import either - . - > import qualified Data.Map as Map - . - or - . - > import Data.Map (Map) - . - The program emits an error on clashing module abbreviations like - . - > import qualified Data.Map.Lazy as Map - > import qualified Data.Map.Strict as Map - . - This error is raised - whenever multiple modules are imported with the same abbreviation, - where at least one module is open for additions. - Our test is overly strict in the sense that it also blames - . - > import qualified Data.Map as Map - > import qualified Data.Map as Map - . - but I think it is good idea to avoid redundant imports anyway. - . - 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) - . - is not bad in the sense of the PVP, - but this way you depend on the existence of the identifier @insert@ - although you do not need it. - If it is removed in a later version of @containers@, - then your import breaks although you did not use the identifier. - . - Finally you can control what items are checked. - First of all you can select the imports that are checked. - Normally the imports are checked that belong to lax dependencies - like @containers >=0.5 && <0.7@. - However this requires the package to be configured - in order to know which import belongs to which dependency. - E.g. @Data.Map@ belongs to @containers@. - You can just check all imports for being addition-proof - using the @--include-all@ option. - Following you can write the options - @--include-import@, - @--exclude-import@, - @--include-dependency@, - @--exclude-dependency@ - that allow to additionally check or ignore imports - from certain modules or packages. - These modifiers are applied from left to right. - E.g. @--exclude-import=Prelude@ will accept any import style for @Prelude@ - and @--exclude-dependency=foobar@ will ignore the package @foobar@, - say, because it does not conform to the PVP. - . - Secondly, you may ignore certain modules or components of the package - using the options - @--exclude-module@, - @--exclude-library@, - @--exclude-executables@, - @--exclude-testsuites@, - @--exclude-benchmarks@. - E.g. @--exclude-module=Paths_PKG@ will exclude the Paths module - that is generated by Cabal. - I assume that it will always be free of name clashes. - . - Known problems: - . - * The program cannot automatically filter out the @Paths@ module. - . - * The program cannot find and check preprocessed modules. - . - * The program may yield wrong results in the presence of Cabal conditions. - . - If this program proves to be useful - it might eventually be integrated in the @check@ command of @cabal-install@. - See . - . - Alternative: - If you want to allow exclusively large version ranges, i.e. @>=x.y && . - Unfortunately there is no GHC warning on clashing module abbreviations. - See . - . Related: There are programs that check PVP compliance of exports: . @@ -220,6 +32,9 @@ Tested-With: GHC==7.4.2 Cabal-Version: >=1.6 Build-Type: Simple +Extra-Source-Files: + ReadMe.md + Source-Repository head type: darcs location: http://hub.darcs.net/thielema/check-pvp/ @@ -227,7 +42,7 @@ Source-Repository this type: darcs location: http://hub.darcs.net/thielema/check-pvp/ - tag: 0.0.2 + tag: 0.0.2.1 Flag advanced description: Build executable based on haskell-package @@ -243,13 +58,15 @@ explicit-exception >=0.1.4 && <0.2, transformers >=0.2 && <0.6, non-empty >=0.1.3 && <0.4, - utility-ht >=0.0.10 && <0.1, + utility-ht >=0.0.12 && <0.1, base >=4 && <4.13 GHC-Options: -Wall Hs-Source-Dirs: src Main-Is: Basic.hs - Other-Modules: CheckPVP + Other-Modules: + CheckPVP + ModuleSet Executable check-pvp-compiler @@ -265,7 +82,7 @@ explicit-exception >=0.1.4 && <0.2, transformers >=0.2 && <0.6, non-empty >=0.1.3 && <0.4, - utility-ht >=0.0.10 && <0.1, + utility-ht >=0.0.12 && <0.1, base >=4 && <4.13 Else Buildable: False @@ -273,4 +90,7 @@ GHC-Options: -Wall Hs-Source-Dirs: src Main-Is: Advanced.hs - Other-Modules: CheckPVP + Other-Modules: + CheckPVP + ModuleSet + Paths_check_pvp diff --git a/src/CheckPVP.hs b/src/CheckPVP.hs --- a/src/CheckPVP.hs +++ b/src/CheckPVP.hs @@ -40,6 +40,7 @@ import qualified Data.NonEmpty as NonEmpty import qualified Data.Foldable as Fold import qualified Data.Monoid.HT as Mn +import qualified Data.List.Reverse.StrictSpine as ListRev import qualified Data.List.HT as ListHT import qualified Data.List as List import qualified Data.Map as Map @@ -453,7 +454,7 @@ Nothing -> [] Just xs -> NonEmpty.minimumKey length $ - fmap (ListHT.dropWhileRev (0==) . + fmap (ListRev.dropWhile (0==) . Version.versionBranch . fst) xs boundClass = diff --git a/src/ModuleSet.hs b/src/ModuleSet.hs new file mode 100644 --- /dev/null +++ b/src/ModuleSet.hs @@ -0,0 +1,41 @@ +module ModuleSet where + +import qualified Language.Haskell.Exts.Syntax as Syntax + +import qualified Data.Set as Set +import Data.Set (Set, ) + + +data T = + Content (Set Syntax.ModuleName) + | Complement (Set Syntax.ModuleName) + + +fromSet :: Set Syntax.ModuleName -> T +fromSet = Content + +full :: T +full = Complement Set.empty + + +member :: Syntax.ModuleName -> T -> Bool +member modu (Content set) = Set.member modu set +member modu (Complement set) = not $ Set.member modu set + + +insert :: Syntax.ModuleName -> T -> T +insert modu (Content set) = Content $ Set.insert modu set +insert modu (Complement set) = Complement $ Set.delete modu set + +delete :: Syntax.ModuleName -> T -> T +delete modu (Content set) = Content $ Set.delete modu set +delete modu (Complement set) = Complement $ Set.insert modu set + + +insertSet :: Set Syntax.ModuleName -> T -> T +insertSet new (Content set) = Content $ Set.union new set +insertSet new (Complement set) = Complement $ Set.difference set new + +deleteSet :: Set Syntax.ModuleName -> T -> T +deleteSet new (Content set) = Content $ Set.difference set new +deleteSet new (Complement set) = Complement $ Set.union new set