diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ `smuggler2` uses [PVP Versioning][1]. The change log is available [on GitHub][2]. +## [0.3.6.1]: -- 19 June 2020 +- improve handling of pattern synonyms +- remove ghc-smuggler2 from build. Provide a scipt instead, to appease cabal + ## [0.3.5.2]: -- 15 June 2020 - tweaks to allow `smuggler2` to run under Windows diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -10,7 +10,12 @@ # environments (eg, travis). Use with care as it can lead to unexpected # results if you are not aware that ghc is using it; it is a normally hidden # dot file. - cabal build # --write-ghc-environment-files=always + cabal outdated + cabal build all:libs + cabal build all:exes + cabal build all:tests + + #cabal build # --write-ghc-environment-files=always debug: cabal build -fdebug diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -30,9 +30,6 @@ Install `smuggler2` using `cabal install --lib smuggler2`. -If you also want the `ghc` wrapper, install it using -`cabal install exe:smuggler2`. - ### Adding Smuggler2 to your dependencies Add `smuggler2` to the dependencies of your project and to your compiler flags. @@ -69,15 +66,16 @@ ### Alternatively, using a local version If you have installed `smuggler2` from a local copy of this repository, you may -need to add `-package smuggler2` to your `ghc-options` if you did not install -using the `--lib` flag to `cabal install`. +need to add `-package-env default -package smuggler2` to your `ghc-options` if +you did not install using the `--lib` flag to `cabal install`. (This will depend +on your setup and your version of `cabal`. ### Or use a `ghc` wrapper -The `smuggler2` package provides an executable `ghc-smuggler2` that calls `ghc` -with the `-fplugin=Smuggler2.Plugin` argument (followed by any others that you -supply). This allows you to run the plugin over your sources without modifying -your `.cabal` file: +The repostory also has a very simple `ghc` wrapper `ghc-smuggler2` in the `app` +folder that you can tweak to accomodate your local build environment. This +allows you to run the plugin over your sources without modifying your `.cabal` +file: ```bash $ cabal build -with-compiler=ghc-smuggler2 @@ -153,24 +151,16 @@ Because `cabal` and `ghc` don't have full support for distinguishing dependent packages from plug-ins you will probably want to ensure that the build the -dependencies for your project tha are installed into your local package db -first, before enabling sumuggler, otherwise they will all be processed by it -too, as your project builds, which should do no harm, but will increase your -build time. - -`Smuggler2` is robust -- it can chew through the -[Agda](https://github.com/agda/agda) codebase of over 370 modules with complex -interdependencies and be tripped over by only - -- a couple of ambiguous exports (are we trying to export something defined in - the current module or something with the same name from an imported module) -- and a couple of imports where both qualifed and unqualifed version of the - module are imported and there are references to both qualified and unqualifed - version of the same names -- some qualified record fields are overlooked +dependencies for your project that are installed into your local package db +first, before enabling `Smuggler2`, or `ghc-smuggler2` otherwise they will all +be processed by it too, as your project builds, which should do no harm, but +will increase your build time: -But there are some caveats, most of which are either easy enough to work around -(and still offer the benefit of a great reduction in keyboard work): +```bash +$ cabal build +$ cabal clean +$ cabal -w ghc-smuggler2 +``` - `Smuggler2` rewrites the existing imports, rather than attempting to prune them. (This is a more aggressive approach than `smuggler` which focuses on @@ -182,6 +172,23 @@ disdvantage is that imports may be reordered, comments and blank lines dropped, external imports mixed with external, etc. +- if you import patterns synonyms from a library without naming them explicitly + in an import list, you do not need the `PatternSynonyms` language extension. + If you import them explicitly, using the `pattern` keyword, the language + extension is required (otherwise you will just get a syntax error on + compilation). `Smuggler2.Plugin` will not add that for you. + +`Smuggler2` is robust -- it can chew through the +[Agda](https://github.com/agda/agda) codebase of over 370 modules with complex +interdependencies and be tripped over by only + +- a couple of ambiguous exports (are we trying to export something defined in + the current module or something with the same name from an imported module) + +- and a couple of imports where both qualifed and unqualifed version of the + module are imported and there are references to both qualified and unqualifed + version of the same names + - By default `Smuggler2` does not remove imports completely because an import may be being used to only import instances of typeclasses, So it will leave stubs like @@ -217,11 +224,11 @@ - Literate Haskell `.lhs` files will procssed into ordinary haskell files wth a `-lhs` suffix. -* `hiding` clauses may not be properly analysed. So hiding things that are not - used may not be spotted. +* `hiding` clauses are not be analysed. So hiding things that are not used will + not be spotted. In fact, hiding imports will be discarded. -* The plugin does not seem to run reliably on Windows. This is probably more - of an issue with the way that the tests are run, than `Smuggler2` itself. +* The plugin does not seem to run reliably on Windows. This is probably more of + an issue with the way that the tests are run, than `Smuggler2` itself. * Currently `cabal` does not have a particular way of specifying plugins. (See, eg, https://gitlab.haskell.org/ghc/ghc/issues/11244 and diff --git a/TODO.md b/TODO.md --- a/TODO.md +++ b/TODO.md @@ -35,4 +35,12 @@ - [ ]- Figure out fhy github workflow uses ghc 8.10.1 when it should be running 8.8.3 -- [ ] #1 Add options for handling `NoImplictPrelude` and keeping it pristine? +- [X] Add options for handling `NoImplictPrelude` and keeping it pristine? + +- [ ] Add option to import using widcards only + +- [ ] Add the `PatternSynonym` language extension when it is required. + +- [ ] Rexporting of patterns + +- [ ] Hidings should be retained diff --git a/app/Main.hs b/app/Main.hs deleted file mode 100644 --- a/app/Main.hs +++ /dev/null @@ -1,23 +0,0 @@ -module Main where - -import GHC.Paths (ghc) -import System.Environment (getArgs) -import System.Exit (exitWith) -import System.Process.Typed - ( proc, - runProcess, - setEnvInherit, - setWorkingDirInherit, - ) - -main :: IO () -main = do - args <- getArgs - - runProcess - ( setWorkingDirInherit . setEnvInherit $ - proc - ghc - ("-fplugin=Smuggler2.Plugin" : args) - ) - >>= exitWith diff --git a/smuggler2.cabal b/smuggler2.cabal --- a/smuggler2.cabal +++ b/smuggler2.cabal @@ -1,6 +1,6 @@ cabal-version: 3.0 name: smuggler2 -version: 0.3.5.2 +version: 0.3.6.1 synopsis: GHC Source Plugin that helps to minimise imports and generate explicit exports @@ -66,8 +66,7 @@ ghc-options: -O2 -Wall -Wextra -Wincomplete-uni-patterns -Wincomplete-record-updates -Wcompat -Widentities - -Wredundant-constraints -fhide-source-paths - -fobject-code + -Wredundant-constraints -fhide-source-paths -fobject-code if impl(ghc >=8.10.0) ghc-options: -Wunused-packages -fwrite-ide-info @@ -87,15 +86,6 @@ MultiParamTypeClasses TupleSections -common executable-options - ghc-options: -rtsopts - - if flag(debug) - ghc-options: -debug - - if flag(threaded) - ghc-options: -threaded -with-rtsopts=-N - library import: common-options hs-source-dirs: src @@ -122,19 +112,31 @@ if flag(debug) build-depends: text -executable ghc-smuggler2 - import: common-options - import: executable-options - hs-source-dirs: app - main-is: Main.hs - build-depends: - , ghc-paths ^>=0.1.0 - , typed-process ^>=0.2.6 +common executable-options + ghc-options: -rtsopts + if flag(debug) + ghc-options: -debug + + if flag(threaded) + ghc-options: -threaded -with-rtsopts=-N + +--executable ghc-smuggler2 +-- import: common-options +-- import: executable-options +-- hs-source-dirs: app +-- main-is: Main.hs +-- build-depends: +-- , ghc-paths ^>=0.1.0 +-- , typed-process ^>=0.2.6 +-- , smuggler2 -any + test-suite smuggler2-test import: common-options + import: executable-options type: exitcode-stdio-1.0 hs-source-dirs: test + main-is: Test.hs build-depends: , containers , directory @@ -145,9 +147,3 @@ , tasty , tasty-golden ^>=2.3.4 , typed-process - - main-is: Test.hs - ghc-options: -rtsopts - - if flag(threaded) - ghc-options: -threaded -with-rtsopts=-N diff --git a/src/Smuggler2/Imports.hs b/src/Smuggler2/Imports.hs --- a/src/Smuggler2/Imports.hs +++ b/src/Smuggler2/Imports.hs @@ -100,36 +100,36 @@ -- The main trick here is that if we're importing all the constructors -- we want to say "T(..)", but if we're importing only a subset we want -- to say "T(A,B,C)". So we have to find out what the module exports. - to_ie _ (Avail n) -- An ordinary identifier (eg, var, data constructor) + to_ie _ (Avail n) -- An ordinary identifier (eg, var, data constructor) = [IEVar noExt (to_ie_post_rn_var $ noLoc n)] to_ie _ (AvailTC n [m] []) -- type or class with absent () list - | n==m = [IEThingAbs noExt (to_ie_post_rn $ noLoc n)] + | n==m = [IEThingAbs noExt (to_ie_post_rn_name $ noLoc n)] to_ie iface (AvailTC n ns fs) = case [(xs,gs) | AvailTC x xs gs <- mi_exports iface , x == n , x `elem` xs -- Note [Partial export] ] of - -- class / type with methods / constructors - [xs] | all_used xs -> [IEThingAll noExt (to_ie_post_rn_var $ noLoc n)] -- (..) + -- class / type with methods / constructors s + [xs] | all_used xs -> [IEThingAll noExt (to_ie_post_rn_name $ noLoc n)] -- (..) - | isTcOcc (occName n) -> -- class - [IEThingWith noExt (to_ie_post_rn $ noLoc n) NoIEWildcard - (map (to_ie_post_rn_tc . noLoc) (filter (/= n) ns)) + | isTcOcc (occName n) -> -- typeclass -- @class Functor ... + [IEThingWith noExt (to_ie_post_rn_name $ noLoc n) NoIEWildcard + (map (to_ie_post_rn_varn . noLoc) (filter (/= n) ns)) (map noLoc fs)] -- Note [Overloaded field import] - | otherwise -> -- type - [IEThingWith noExt (to_ie_post_rn $ noLoc n) NoIEWildcard - (map (to_ie_post_rn . noLoc) (filter (/= n) ns)) + | otherwise -> -- type constructor (ie, @data X =@) + [IEThingWith noExt (to_ie_post_rn_name $ noLoc n) NoIEWildcard + (map (to_ie_post_rn_cname . noLoc) (filter (/= n) ns)) (map noLoc fs)] -- record type _other | all_non_overloaded fs - -> map (IEVar noExt . to_ie_post_rn . noLoc) $ ns + -> map (IEVar noExt . to_ie_post_rn_name . noLoc) $ ns ++ map flSelector fs | otherwise -> -- DuplicateRecordFields is applicable - [IEThingWith noExt (to_ie_post_rn $ noLoc n) NoIEWildcard - (map (to_ie_post_rn . noLoc) (filter (/= n) ns)) + [IEThingWith noExt (to_ie_post_rn_name $ noLoc n) NoIEWildcard + (map (to_ie_post_rn_cname . noLoc) (filter (/= n) ns)) (map noLoc fs)] where @@ -141,11 +141,32 @@ all_non_overloaded = not . any flIsOverloaded +to_ie_post_rn_name :: Located name -> LIEWrappedName name +to_ie_post_rn_name (L l n) = L l (IEName (L l n)) +to_ie_post_rn_var :: (HasOccName name) => Located name -> LIEWrappedName name +to_ie_post_rn_var (L l n) + | isDataOcc $ occName n = L l (IEPattern (L l n)) + | otherwise = L l (IEName (L l n)) + +to_ie_post_rn_varn :: (HasOccName name) => Located name -> LIEWrappedName name +to_ie_post_rn_varn (L l n) + | isTcOcc $ occName n = L l (IEType (L l n)) + | otherwise = L l (IEName (L l n)) + +to_ie_post_rn_cname :: (HasOccName name) => Located name -> LIEWrappedName name +to_ie_post_rn_cname (L l n) + | isTcOcc $ occName n = L l (IEType (L l n)) + | otherwise = L l (IEName (L l n)) + +-- Notes +-- +-- https://gitlab.haskell.org/ghc/ghc/-/wikis/pattern-synonyms/associating-synonyms + -- An import is -- a var --- a tycon -> [ (..) | ( cname1 , … , cnamen )] --- a tycls -> [(..) | ( var1 , … , varn )] +-- a tycls -> [(..) | ( var1 , … , varn )] -- class, etc +-- a tycon -> [ (..) | ( cname1 , … , cnamen )] -- data -- cname -> var | con -- var -> varid | ( varsym ) -- (does not start with :) @@ -155,7 +176,7 @@ -- The name of the pattern synonym is in the same namespace as proper data constructors. -- Like normal data constructors, pattern synonyms can be imported through associations -- with a type constructor or independently. --- To export them on their own, in an export or import specification, +-- To export them *on their own*, in an export or import specification, -- you must prefix pattern names with the pattern keyword -- -- GHC User Guide 9.9.5 @@ -167,19 +188,15 @@ --data family type constructor is exported along with the new data constructors, regardless of --whether the data family is defined locally or in another module. -to_ie_post_rn_var :: (HasOccName name) => Located name -> LIEWrappedName name -to_ie_post_rn_var (L l n) - | isDataOcc $ occName n = L l (IEPattern (L l n)) - | otherwise = L l (IEName (L l n)) - -to_ie_post_rn :: (HasOccName name) => Located name -> LIEWrappedName name -to_ie_post_rn (L l n) - | isTcOcc occ && isSymOcc occ = L l (IEType (L l n)) -- starts with :, ->, etc - | otherwise = L l (IEName (L l n)) - where occ = occName n - -to_ie_post_rn_tc :: (HasOccName name) => Located name -> LIEWrappedName name -to_ie_post_rn_tc (L l n) - | isTcOcc occ = L l (IEType (L l n)) - | otherwise = L l (IEName (L l n)) - where occ = occName n +-- isVarOcc -> variable name +-- isTvOcc -> is type variable +-- isTcOcc -> is type class name +-- isValOcc -- either in the variable or data constructor namespaces +-- isDataOcc -- Data constructor +-- isDataSymOcc -> Data contructuctor starting with a symbol +-- isSymOcc -> operator(data constructor, variable, etc) +-- +-- So there are +-- var -> IEName +-- tycon -> can have IEPattern +-- tyclas -> can have IEType type (:+:) diff --git a/src/Smuggler2/Plugin.hs b/src/Smuggler2/Plugin.hs --- a/src/Smuggler2/Plugin.hs +++ b/src/Smuggler2/Plugin.hs @@ -13,7 +13,13 @@ import Data.List (intersect) import Data.Maybe (fromMaybe, isJust, isNothing) import Data.Version (showVersion) -import DynFlags (DynFlags (dumpDir), HasDynFlags (getDynFlags), setUnsafeGlobalDynFlags, xopt) +import DynFlags + ( DynFlags (dumpDir), + HasDynFlags (getDynFlags), + setUnsafeGlobalDynFlags, + xopt, + xopt_set, + ) import ErrUtils (compilationProgressMsg, fatalErrorMsg, warningMsg) import GHC ( GenLocated (L), @@ -33,7 +39,7 @@ unLoc, ) import GHC.IO.Encoding (setLocaleEncoding, utf8) -import GHC.LanguageExtensions (Extension (Cpp)) +import GHC.LanguageExtensions (Extension (Cpp, PatternSynonyms)) import IOEnv (MonadIO (liftIO), readMutVar) import Language.Haskell.GHC.ExactPrint ( Anns, @@ -66,7 +72,7 @@ import Smuggler2.Parser (runParser) import StringBuffer (StringBuffer (StringBuffer), lexemeToString) import System.Directory (removeFile) -import System.FilePath (isExtensionOf, takeExtension, (-<.>), ()) +import System.FilePath ((-<.>), (), isExtensionOf, takeExtension) import System.IO (IOMode (WriteMode), withFile) import TcRnExports (exports_from_avail) import TcRnTypes @@ -153,6 +159,7 @@ -- were givem. tcEnv <$ smuggling dflags minImpFilePath where + -- The original imports imports :: [LImportDecl GhcRn] imports = tcg_rn_imports tcEnv @@ -181,11 +188,20 @@ -- Read the dumped file of minimal imports minImpFileContents <- liftIO $ readFile minImpFilePath + -- If a module is imported open, then pattern synonyms can be + -- imported from it with the @PatternSynonym@ language extension. If, + -- however, a pattern synonym is imported explicitly, the extension is + -- required. So we switch on the @PatternSynonym@ extension for + -- parsing the minimal imports. + let dflags' = xopt_set dflags PatternSynonyms + -- Parse the minimal imports file -- gets the annnotations too - runParser dflags minImpFilePath minImpFileContents >>= \case + runParser dflags' minImpFilePath minImpFileContents >>= \case Left () -> liftIO $ - fatalErrorMsg dflags (text $ "smuggler: failed to parse minimal imports from " ++ minImpFilePath) + fatalErrorMsg + dflags + (text $ "smuggler: failed to parse minimal imports from " ++ minImpFilePath) Right (annsImpMod, L _ impMod) -> do -- The actual minimal imports themselves, as generated by GHC, -- with open imports processed as specified @@ -226,7 +242,9 @@ -- Clean up: delete the GHC-generated imports file liftIO $ removeFile minImpFilePath + where + -- Generates the things that would be exportabe if there were no -- explicit export header, so suitable for replacing one exportable :: RnM [AvailInfo]