diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ `smuggler2` uses [PVP Versioning][1]. The change log is available [on GitHub][2]. +## [0.3.4.1]: -- 9 June 2020 +- Place output from CPP and literate Haskell sources into new files + ## [0.3.3.2]: -- 8 June 2020 - pattern synonyms and type families are handled - ghc-smuggler2 wrapper for ghc diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -27,6 +27,8 @@ doc: cabal haddock + cabal v2-haddock --haddock-for-hackage --enable-doc +# cabal upload -d --publish dist-newstyle/smuggler2-0.3.x.y-docs.tar.gz hlint: hlint src test/Test.hs diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ common smuggler-options if flag(smuggler2) ghc-options: -fplugin=Smuggler2.Plugin - build-depends: smuggler2 >= 0.3 + build-depends: smuggler2 >= 0.3 && < 0.4 ``` and then `import: smuggler-options` in the appropriate `library` or `executable` @@ -76,12 +76,14 @@ ```bash $ cabal build -with-compiler=ghc-smuggler2 ``` + or just + ```bash $ cabal build -w ghc-smuggler2 ``` -### Options +## Options `Smuggler2` has several (case-insensitive) options, which can be set by adding `-fplugin-opt=Smuggler2.Plugin:` flags to your `ghc-options` @@ -123,16 +125,15 @@ $ ghcid --command='cabal repl' ``` - ## Caveats -- 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 - dependencies for your project 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. +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 +dependencies for your project 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. -`Smugggler2` is robust -- it can chew through the +`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 @@ -141,16 +142,20 @@ - 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 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): - `Smuggler2` rewrites the existing imports, rather than attempting to prune them. (This is a more aggressive approach than `smuggler` which focuses on - removing redundant imports.) It has advantages and disadvantages. The - advantage is that a minimal set of imports is generated in a reproducable - format. The disdvantage is that imports may be reordered, comments and - blankdropped, external imports mixed with external, etc. + removing redundant imports.) It has advantages and disadvantages. + The advantage is that a minimal set of imports is generated in a reproducable + format. So you can just import a library without specifying any specific + imports and `Smuggler2` will add an explict list of things that are + used from it. This can be a useful check and better document your modules. + The disdvantage is that imports may be reordered, comments and + blank lines dropped, external imports mixed with external, etc. - 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 @@ -167,7 +172,8 @@ - CPP files will not be processed correctly: the imports will be generated for current CPP settings and any CPP annotations in the import block will be discarded. This may be a particular problem if you are writing code for - several generations of `ghc` and `base` for example. + several generations of `ghc` and `base` for example. Nevetheless, `Smuggler2` + will generate a new CPP preprocessed output file with a `-cpp` suffix. [retrie](https://github.com/facebookincubator/retrie/blob/master/Retrie/CPP.hs) solves this problem generating all possible versions of the module (exponential in the number of `#if` directives), operating on each version @@ -183,15 +189,16 @@ - Multiple separate import lines referring to the same library are not consolidated -- Literate Haskell `lhs` files are not supported +- Literate Haskell `.lhs` files will procssed into ordinary haskell files a + `-lhs` suffix. -- `hiding` clauses may not be properly analysed. So hiding things that are not +* `hiding` clauses may not be properly analysed. So hiding things that are not used may not be spotted. -- The test suite does not seem to run reliably on Windows. This is probably more +* The test suite 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, +* Currently `cabal` does not have a particular way of specifying plugins. (See, eg, https://gitlab.haskell.org/ghc/ghc/issues/11244 and https://github.com/haskell/cabal/issues/2965) which would allow cleaner separation of user code and plugin-code diff --git a/TODO.md b/TODO.md --- a/TODO.md +++ b/TODO.md @@ -1,12 +1,13 @@ # Things to be done -- [ ] does hiding work? +- [ ] Does hiding work? No, as ghc doesn't check them -- [ ] does CPP work? (should do) +- [X] Does CPP work? Yes, but the mark is dropped, so put output into a new + file. Similarly for literate Haskell. -- [ ] Script to generate new test (from template?) +- [x] Script to generate new test (from template?) Not worth the effort -- [ ] get tasty-golden to delete successful results and update tests where +- [X] get tasty-golden to delete successful results and update tests where smuggler makes no changes - [ ] refactor the much more sophisticated https://github.com/ddssff/refact-global-hse @@ -17,16 +18,19 @@ it creates race conditions because both cases will generate the same minimum imprts dump file -- [ ] Do a better job of preserving imported pattern annotation. +- [X] Do a better job of preserving imported pattern annotation. -- [ ] Check that type imports / exports work, Type Operators, +- [X] Check that type imports / exports work, Type Operators, - [X] Running test suite on both an imported module and the module that imports it creates race conditions because both cases will generate the same minimum imports dump file -- [ ] User ghc environment files instead of `cabal exec` to launch tests +- [x] User ghc environment files instead of `cabal exec` to launch tests. But + these are invisible and so likely to trip you up - [ ] Use NamedFieldPuns / RecordWildCards for Options - [ ] Check that an export does not need to be qualified + +- [ ]- Figure out fhy github workflow uses ghc 8.10.1 when it should be running 8.8.3 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.3.2 +version: 0.3.4.1 synopsis: GHC Source Plugin that helps to minimise imports and generate explicit exports @@ -109,6 +109,7 @@ , directory ^>=1.3.3 , filepath ^>=1.4.2 , ghc >=8.6.5 && <8.11 + , ghc-boot >=8.6.5 && <8.11 , ghc-exactprint ^>=0.6.3 , syb ^>=0.7.1 diff --git a/src/Smuggler2/Anns.hs b/src/Smuggler2/Anns.hs --- a/src/Smuggler2/Anns.hs +++ b/src/Smuggler2/Anns.hs @@ -15,8 +15,12 @@ import Data.Maybe (fromMaybe) import GHC (AnnKeywordId (AnnCloseP, AnnOpenP)) import GhcPlugins (GenLocated (L), Located) -import Language.Haskell.GHC.ExactPrint (Annotation (annEntryDelta, annPriorComments, annsDP), - TransformT, modifyAnnsT, uniqueSrcSpanT) +import Language.Haskell.GHC.ExactPrint + ( Annotation (annEntryDelta, annPriorComments, annsDP), + TransformT, + modifyAnnsT, + uniqueSrcSpanT, + ) import Language.Haskell.GHC.ExactPrint.Types (DeltaPos (..), KeywordId (G), annNone, mkAnnKey) -- Inspired by retrie diff --git a/src/Smuggler2/Exports.hs b/src/Smuggler2/Exports.hs --- a/src/Smuggler2/Exports.hs +++ b/src/Smuggler2/Exports.hs @@ -1,8 +1,9 @@ -- | -- Description: Utility functions for generating an export list ast and --- associated Anns. It's a bit fiddlier than it could be because ghc's --- functions for producing exportable things generates @AvailInfo@ from which we --- need to reconstitue @IEWrappedName@ and then @IE@ +-- associated Anns. It's a bit fiddlier than it could be because ghc's +-- functions for producing exportable things generates @AvailInfo@ from which we +-- need to reconstitue @IEWrappedName@ and then @IE@ + module Smuggler2.Exports ( mkExportAnnT ) diff --git a/src/Smuggler2/Imports.hs b/src/Smuggler2/Imports.hs --- a/src/Smuggler2/Imports.hs +++ b/src/Smuggler2/Imports.hs @@ -1,7 +1,8 @@ {-# LANGUAGE CPP #-} --- | Description: A replacement for 'RnNames.getMinimalImports' that attempts --- to handle patterns and types, which is not done correctly in GHC 8.10.1 and --- earlier. +-- | +-- Description: A replacement for 'RnNames.getMinimalImports' that attempts +-- to handle patterns and types, which is not done correctly in GHC 8.10.1 and +-- earlier. module Smuggler2.Imports (getMinimalImports) where import Avail ( AvailInfo(..) ) @@ -20,7 +21,9 @@ import LoadIface ( loadSrcInterface ) import Name ( HasOccName(..), isDataOcc, isSymOcc, isTcOcc ) import Outputable ( Outputable(ppr), text, (<+>) ) +#if MIN_VERSION_GLASGOW_HASKELL(8,8,0,0) import RdrName ( gresToAvailInfo ) +#endif import RnNames ( ImportDeclUsage ) import SrcLoc ( GenLocated(L), Located, noLoc ) import TcRnMonad ( RnM ) diff --git a/src/Smuggler2/Options.hs b/src/Smuggler2/Options.hs --- a/src/Smuggler2/Options.hs +++ b/src/Smuggler2/Options.hs @@ -1,7 +1,6 @@ -{- | - Description: handling of command line options +-- | +-- Description: handling of command line options - -} module Smuggler2.Options ( Options (..), parseCommandLineOptions, diff --git a/src/Smuggler2/Parser.hs b/src/Smuggler2/Parser.hs --- a/src/Smuggler2/Parser.hs +++ b/src/Smuggler2/Parser.hs @@ -1,7 +1,6 @@ -{-| - Description: provides a wrapper around the 'ghc-exactprint' parser - -} {-# LANGUAGE CPP #-} +-- | +-- Description: provides a wrapper around the 'ghc-exactprint' parser module Smuggler2.Parser ( runParser, @@ -14,7 +13,7 @@ import Language.Haskell.GHC.ExactPrint (Anns) import Language.Haskell.GHC.ExactPrint.Parsers (parseModuleFromStringInternal) #if MIN_VERSION_GLASGOW_HASKELL(8,10,1,0) -import ErrUtils (fatalErrorMsg, printBagOfErrors) +import ErrUtils (fatalErrorMsg, printBagOfErrors ) import Outputable (text) #else import ErrUtils (fatalErrorMsg) diff --git a/src/Smuggler2/Plugin.hs b/src/Smuggler2/Plugin.hs --- a/src/Smuggler2/Plugin.hs +++ b/src/Smuggler2/Plugin.hs @@ -9,9 +9,11 @@ import Avail ( AvailInfo, Avails ) import Control.Monad ( unless ) +import Data.Bool ( bool ) import Data.Maybe ( fromMaybe, isNothing ) import Data.Version ( showVersion ) -import DynFlags ( DynFlags(dumpDir), HasDynFlags(getDynFlags) ) +import DynFlags + ( DynFlags(dumpDir), HasDynFlags(getDynFlags), xopt ) import ErrUtils ( compilationProgressMsg, fatalErrorMsg ) import GHC ( GenLocated(L), @@ -21,11 +23,14 @@ LIE, LImportDecl, Located, - ModSummary(ms_hspp_buf, ms_hspp_file, ms_mod), + ms_location, + ml_hs_file, + ModSummary(ms_hspp_buf, ms_mod), Module(moduleName), ParsedSource, moduleNameString, unLoc ) +import GHC.LanguageExtensions ( Extension(Cpp) ) import GHC.IO.Encoding ( setLocaleEncoding, utf8 ) import IOEnv ( MonadIO(liftIO), readMutVar ) import Language.Haskell.GHC.ExactPrint @@ -45,11 +50,10 @@ Plugin(pluginRecompile, typeCheckResultAction), defaultPlugin, purePlugin ) -import RnNames - ( ImportDeclUsage, findImportUsage ) -import Smuggler2.Anns -import Smuggler2.Imports -import Smuggler2.Exports +import RnNames ( ImportDeclUsage, findImportUsage ) +import Smuggler2.Anns ( mkLoc, mkParenT ) +import Smuggler2.Imports ( getMinimalImports ) +import Smuggler2.Exports ( mkExportAnnT ) import Smuggler2.Options ( ExportAction(AddExplicitExports, NoExportProcessing, ReplaceExports), @@ -59,7 +63,7 @@ import Smuggler2.Parser ( runParser ) import StringBuffer ( StringBuffer(StringBuffer), lexemeToString ) import System.Directory ( removeFile ) -import System.FilePath ( (-<.>), () ) +import System.FilePath ( (-<.>), (), isExtensionOf, takeExtension ) import System.IO ( IOMode(WriteMode), withFile ) import TcRnExports ( exports_from_avail ) import TcRnTypes @@ -92,19 +96,20 @@ -- This ensures that the source file is not touched if there are no unused -- imports, or exports already exist and we are not replacing them let noUnusedImports = all (\(_decl, used, unused) -> not (null used) && null unused) usage + let hasExplicitExports = case tcg_rn_exports tcEnv of - Nothing -> False -- There is not even a module header + Nothing -> False -- There is not even a module header (Just []) -> False - (Just _) -> True + (Just _) -> True -- ... so short circuit if: -- - we are skipping import processing or there are no unused imports, and -- - we are skipping export processing or there are explict exports and we are not replacing them -- (This is not a complete check; ideally, that the new imp/exports are -- different from the existing ones, etc) - if (importAction options == NoImportProcessing || noUnusedImports) && - ( exportAction options == NoExportProcessing - || (hasExplicitExports && exportAction options /= ReplaceExports) - ) + if (importAction options == NoImportProcessing || noUnusedImports) + && ( exportAction options == NoExportProcessing + || (hasExplicitExports && exportAction options /= ReplaceExports) + ) then return tcEnv else do dflags <- getDynFlags @@ -123,14 +128,14 @@ -- Run smuggling only for its side effects tcEnv <$ smuggling dflags minImpFilePath - where - -- Does all the work smuggling :: DynFlags -> FilePath -> RnM () smuggling dflags minImpFilePath = do - - let modulePath = ms_hspp_file modSummary + -- The preprocessed source + let modulePath = case ml_hs_file $ ms_location modSummary of + Nothing -> error "smuggler2: missing source file location" + Just loc -> loc -- Read files as UTF-8 strings (GHC accepts only ASCII or UTF-8) liftIO $ setLocaleEncoding utf8 @@ -138,7 +143,7 @@ -- Get the pre-processed module source code let modFileContents = case ms_hspp_buf modSummary of -- Not clear under what circumstances this could happen - Nothing -> error $ "smuggler: missing source file: " ++ modulePath + Nothing -> error $ "smuggler2: missing source file: " ++ modulePath Just contents -> strBufToStr contents -- Parse the whole module @@ -154,7 +159,6 @@ liftIO $ 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 let minImports = hsmodImports impMod @@ -170,19 +174,27 @@ replaceImports annsImpMod minImports astHsMod >>= addExplicitExports exports + -- Generate new file extension + -- Was the CPP used? + let usedCpp = bool "" "-cpp" (xopt Cpp dflags) + -- Was the source literate Haskell + let wasLhs = bool "" "-lhs" (isExtensionOf "lhs" modulePath) + -- prefix any user-supplied options + let ext = + fromMaybe (takeExtension modulePath) (newExtension options) + ++ usedCpp + ++ wasLhs + -- Print the result let newContent = exactPrint astHsMod' annsHsMod' - liftIO $ case newExtension options of - Nothing -> writeFile modulePath newContent - Just ext -> writeFile (modulePath -<.> ext) newContent + liftIO $ writeFile (modulePath -<.> ext) newContent - -- Clean up: delete the GHC-generated imports file + -- Clean up: delete the GHC-generated imports file liftIO $ removeFile minImpFilePath - where -- Generates the things that would be exportabe if there were no - -- explict export header, so suitable for replacing one + -- explicit export header, so suitable for replacing one exportable :: RnM [AvailInfo] exportable = do let rdr_env = tcg_rdr_env tcEnv @@ -191,7 +203,7 @@ exports <- exports_from_avail Nothing rdr_env imports this_mod return (snd exports) - -- | Replace a target module's imports + -- Replace a target module's imports -- See replaceImports :: Monad m => @@ -214,7 +226,7 @@ unless (null imps) $ setEntryDPT (head imps) (DP (2, 0)) return $ L l m {hsmodImports = imps} - -- | Add explict exports to the target module + -- Add explict exports to the target module addExplicitExports :: Monad m => -- | The list of exports to be added @@ -235,14 +247,13 @@ where currentExplicitExports :: Maybe (Located [LIE GhcPs]) currentExplicitExports = hsmodExports hsMod - -- This does all the export replacement work result :: Monad m => TransformT m ParsedSource result | null exports = return t -- there is nothing exportable | otherwise = do -- Generate the exports list - exportsList <- mapM mkExportAnnT exports + exportsList <- mapM mkExportAnnT exports -- add commas in between and parens around mapM_ addTrailingCommaT (init exportsList) lExportsList <- mkLoc exportsList >>= mkParenT unLoc @@ -251,7 +262,7 @@ -- annotations in the current transformation return $ L astLoc hsMod {hsmodExports = Just lExportsList} - -- | This version of the GHC function ignores implicit imports, as they + -- This version of the GHC function ignores implicit imports, as they -- cannot be parsed back in. (There is an extraneous (implicit)) -- It also provides for leaving out instance-only imports (eg, Data.List() ) printMinimalImports' :: DynFlags -> FilePath -> [ImportDeclUsage] -> RnM () @@ -273,15 +284,12 @@ where notImplicit :: ImportDecl pass -> Bool notImplicit = not . ideclImplicit - notInstancesOnly :: ImportDecl pass -> Bool notInstancesOnly i = case ideclHiding i of Just (False, L _ []) -> False - _ -> True - + _ -> True keepInstanceOnlyImports :: Bool keepInstanceOnlyImports = importAction options /= MinimiseImports - letThrough :: ImportDecl pass -> Bool letThrough i = notImplicit i && (keepInstanceOnlyImports || notInstancesOnly i) diff --git a/test/tests/CPP.hs b/test/tests/CPP.hs new file mode 100644 --- /dev/null +++ b/test/tests/CPP.hs @@ -0,0 +1,23 @@ +{-# LANGUAGE CPP #-} +module CPP where +#if !(MIN_VERSION_base(4,8,0)) +import Data.Monoid +#endif +#if !(MIN_VERSION_base(4,11,0)) +import Data.Semigroup +#endif + +#if !(MIN_VERSION_base(4,5,0)) && !(MIN_VERSION_base(4,9,0)) + +infixr 6 <> + +-- | An infix synonym for 'mappend'. +-- +-- /Since: 4.5.0.0/ +(<>) :: Monoid m => m -> m -> m +(<>) = mappend +{-# INLINE (<>) #-} + +#endif + +str = "abc" <> "def"