diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,33 +5,47 @@ `smuggler2` uses [PVP Versioning][1]. The change log is available [on GitHub][2]. -## [0.3.6.1]: -- 19 June 2020 +## [0.3.6.2]: -- 10 January 2021 + +- print out timings / memory usage, when `-v2` or greater is set +- test harness tweaks +- remove Windows workarounds for versions prior to ghc 8.10.3. THey don't work, + and are not needed for 8.10.3 + +## [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 +## [0.3.5.2]: -- 15 June 2020 + - tweaks to allow `smuggler2` to run under Windows -## [0.3.5.1]: -- 14 June 2020 +## [0.3.5.1]: -- 14 June 2020 + - fix bug that left some files with open imports unprocessed - add `LeaveOpenImports` and `MakeOpenImports` options -## [0.3.4.2]: -- 10 June 2020 +## [0.3.4.2]: -- 10 June 2020 + - Add test golden files to the distribution -## [0.3.4.1]: -- 9 June 2020 +## [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 +## [0.3.3.2]: -- 8 June 2020 + - pattern synonyms and type families are handled - ghc-smuggler2 wrapper for ghc -## [0.3.2.2]: -- 31 May 2020 +## [0.3.2.2]: -- 31 May 2020 + - Documentation only -## [0.3.2.1]: -- 30 May 2020 -- Provisional version for Hackage +## [0.3.2.1]: -- 30 May 2020 +- Provisional version for Hackage [1]: https://pvp.haskell.org [2]: https://github.com/jrp2014/smuggler2/releases diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -1,28 +1,24 @@ # For convenience # -.PHONY: build install test clean accept doc hlint ghcid upload upload-docs weed +.PHONY: build install test clean accept doc hlint ghcid upload upload-docs weed stan all: build test doc build: + cabal outdated # Creates a package environment file needed to get the tests to run in some # 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 outdated - cabal build all:libs - cabal build all:exes - cabal build all:tests - - #cabal build # --write-ghc-environment-files=always + cabal build all --write-ghc-environment-files=always debug: cabal build -fdebug install: cabal install --lib smuggler2 - cabal install exe:ghc-smuggler2 --overwrite-policy=always + #cabal install exe:ghc-smuggler2 --overwrite-policy=always test: build git diff --check @@ -31,6 +27,8 @@ clean: cabal clean cabal v1-clean + test/clean.sh + rm -rf .hie accept: cabal run smuggler2-test -- --accept --delete-output=onpass @@ -51,6 +49,9 @@ hlint: hlint src test/Test.hs + +stan: + stan ghcid: ghcid diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -66,13 +66,13 @@ ### Alternatively, using a local version If you have installed `smuggler2` from a local copy of this repository, you may -need to add `-package-env default -package smuggler2` to your `ghc-options` if +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 repostory also has a very simple `ghc` wrapper `ghc-smuggler2` in the `app` +The repository 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: @@ -87,8 +87,6 @@ $ cabal build -w ghc-smuggler2 ``` -Smuggler2 tries not to change files when there is no work to do. - You can just run `ghcid` as usual: ```bash @@ -122,7 +120,7 @@ - `LeaveOpenImports` and `MakeOpenImports` take a comma-separated list of module names. The specified modules are to be left open if they were open in the sourcee (in the case of `LeaveOpenImports`) and made open even if they were - not originall (in the case of `MakeOpenImports`). For example, you could add + not originally (in the case of `MakeOpenImports`). For example, you could add ```bash -fplugin-opt=Smuggler2.Plugin:LeaveOpenImports:Relude,RIO,Prelude,Some.Module @@ -149,47 +147,17 @@ ## 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 the -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: - -```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 removing redundant imports.) It has advantages and disadvantages. The - advantage is that a minimal set of imports is generated in a reproducable + advantage is that a minimal set of imports is generated in a reproducible 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 + it. This can be a useful check and better documents your modules. The 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 +* 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 @@ -201,36 +169,66 @@ option to remove them anyway, at the risk of producing code that fails to compile. -- CPP files will not be processed correctly: the imports will be generated for +* 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. 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 + solves this problem by generating all possible versions of the module (exponential in the number of `#if` directives), operating on each version individually, and splicing results back into the original file. A tour de force! -- `smuggler2` depends on the current `ghc` compiler and `base` library to check - whether an import is redundant. Different versions of the compiler may, of - course, need different slightly imports, typically from `base`. The - [base library changelog](https://hackage.haskell.org/package/base/changelog) - provides some details of what was made available when. +* 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 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: -- Multiple separate import lines referring to the same library are not + ```bash + $ cabal build --dependencies-only + $ cabal clean + $ cabal -w ghc-smuggler2 + ``` + +* 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. + +* Multiple separate import lines referring to the same library are not consolidated -- Literate Haskell `.lhs` files will procssed into ordinary haskell files wth a +* Literate Haskell `.lhs` files will processed into ordinary haskell files wth a `-lhs` suffix. -* `hiding` clauses are not be analysed. So hiding things that are not used will - not be spotted. In fact, hiding imports will be discarded. +* `hiding` imports are not needed and replaced by explicit ones. -* The plugin does not seem to run reliably on Windows. This is probably more of +`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 + +- `smuggler2` depends on the current `ghc` compiler and `base` library to check + whether an import is redundant. Different versions of the compiler may, of + course, need different slightly imports, typically from `base`. The + [base library changelog](https://hackage.haskell.org/package/base/changelog) + provides some details of what was made available when. + +- The plugin does not run reliably on Windows with versions of `ghc` prior to + 8.10.3. 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 @@ -245,6 +243,8 @@ on `ghc-8.6.5` because it seems to need to import `Data.Bool` whereas later versions of GHC don't. The results compile on `ghc-8.6.5` and later anyway, but the imports are not as minimal for later versions as they could be. + `ghc-exactprint 0.6.3.1` adds extra '\r` inside comments under Windows, so the + tests fail. - `cabal >= 3.0` (ideally `3.2`) - The Windows version of the plugin is a bit flaky because of apparent compiler bugs. @@ -256,13 +256,18 @@ ```shell $ cabal update -$ cabal build +$ cabal build --write-ghc-environment-files=always ``` +Writing the ghc environment file allows tests to be run from within the +repository using `ghc -fplugin=Smuggler2.Plugin` without needing to use +`cabal exec -- ghc -fplugin=Smugler2.Plugin` or a `-package smuggler2` flag. +`cabal clean` to get rid of it, to avoid surprises when you are done. + To build with debugging: ```shell -$ cabal build -fdebug +$ cabal build -fdebug --write-ghc-environment-files=always ``` Curently this just adds an `-fdump-minimal-imports` parameter to GHC @@ -273,13 +278,13 @@ There is a `tasty-golden`-based test suite that can be run by ```shell -$ cabal test smuggler-test --enable-tests +$ cabal test smuggler2-test --enable-tests ``` Further help can be found by ```shell -$ cabal run smuggler-test -- --help +$ cabal run smuggler2-test -- --help ``` (note the extra `--`) @@ -295,11 +300,6 @@ It is sometimes necessary to run `cabal clean` before running tests to ensure that old build artefacts do not lead to misleading results. -`smuggler-test` uses `cabal exec ghc` internally to run a test. The `cabal` -command that is to be used to do that can be set using the `CABAL` environment -variable. This may be helpful for certain workflows where `cabal` is not in the -current path, or you want to add extra flags to the `cabal` command. - Importing a test module from another test module in the same directory is likely to lead to race conditions as 'Tasty' runs tests in parallel and so will try to generate the same `smuggler2` output both when the imported module is being @@ -316,7 +316,7 @@ library is not widely used, at least in publicly available code, so the use here can, no doubt, be optimised. -The library is needed because the annotated AST that GHC generates does not have +The library is needed because the annotated AST that GHC alone generates does not have enough information to reconstitute the original source. Some parts of the renamed syntax tree (for example, imports) are not found in the typechecked one. `ghc-exactprint` provides parsers that preserve this information, which is @@ -330,12 +330,13 @@ > These functions are > [said to be under heavy development](https://hackage.haskell.org/package/ghc-exactprint-0.6.3/docs/Language-Haskell-GHC-ExactPrint-Transform.html). -> It is not entirely obvious how they are intended to be used or composed. The +> The > approach provided by [`retrie`](https://hackage.haskell.org/package/retrie) > wraps an AST and `Anns` into a single type that seems to make AST > transformations easier to compose and reduces the risk of the `Anns` and AST > getting out of sync as it is being transformed, something with which the type -> system doesn't help you since the `Anns` are stored as a `Map`. +> system doesn't help you since the `Anns` are stored as a `Map`. (That approach +> is not used by `smuggler2`.) ### Imports @@ -343,7 +344,9 @@ - parses the original file - dumps the minimal exports that GHC generates and parses them back in (to pick - up the annotations needed for printing) + up the annotations needed for printing). The ghc version of + `getMinimalImports` does not handle pattern and type imports correctly. + `smuggler2` uses a fixed version of that function. - drops implicit imports (such as Prelude) and, optionally, imports that are for instances only - replaces the original imports with minimal ones @@ -361,7 +364,8 @@ ## Other projects - Smuggler2 was is a rewrite of - [`smuggler`](https://hackage.haskell.org/package/smuggler) + [`smuggler`](https://hackage.haskell.org/package/smuggler) that rewrites + rather than just pruning existing imports - `retrie` a [code modding tool](https://hackage.haskell.org/package/retrie) that works with GHC 8.10.1 - `refact-global-hse` an ambitious diff --git a/TODO.md b/TODO.md --- a/TODO.md +++ b/TODO.md @@ -1,7 +1,5 @@ # Things to be done -- [ ] Does hiding work? No, as ghc doesn't check them - - [X] Does CPP work? Yes, but the mark is dropped, so put output into a new file. Similarly for literate Haskell. @@ -33,14 +31,17 @@ - [ ] 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 +- [ ] Figure out why github workflow uses ghc 8.10.1 when it should be running 8.8.3 - [X] Add options for handling `NoImplictPrelude` and keeping it pristine? -- [ ] Add option to import using widcards only +- [ ] Add option to import using wildcards only -- [ ] Add the `PatternSynonym` language extension when it is required. +- [ ] Rexporting of patterns? -- [ ] Rexporting of patterns +- [X] Initial language pragmas are not retained if there is no `module ... + where` -- [ ] Hidings should be retained +- [X] `ghc-exactprint` adds `\r` into comments at eol. Woraround? + +- [ ] check `getMinimalImports` for corner cases 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.6.1 +version: 0.3.6.2 synopsis: GHC Source Plugin that helps to minimise imports and generate explicit exports @@ -22,11 +22,14 @@ category: Development, Refactoring, Compiler Plugin build-type: Simple extra-doc-files: - README.md CHANGELOG.md + README.md TODO.md extra-source-files: + .hlint.yaml + Makefile + Setup.hs test/tests/**/*.hs test/tests/**/*.MinimiseImportsNoExportProcessingPreludeDataVersion-golden test/tests/**/*.MinimiseImportsReplaceExports-golden @@ -34,15 +37,16 @@ test/tests/**/*.NoImportProcessingNoExportProcessing-golden test/tests/**/*.NoImportProcessingReplaceExports-golden test/tests/**/*.PreserveInstanceImportsNoExportProcessingPrelude-golden - Makefile - Setup.hs weeder.dhall - .hlint.yaml -- wildcards are allowed only in the basename --test/tests/*.*-golden -tested-with: GHC ==8.6.5 || ==8.8.3 || ==8.10.1 +-- Plugins support on Windows versions of ghc before 8.10.3 is broken +-- ghc 8.6.5 produces different results from later versions because it +-- uses a different version of base +tested-with: + GHC ==8.6.5 || ==8.8.3 || ==8.10.1 || ==8.10.2 || ==8.10.3 flag debug description: Enable debugging support @@ -69,7 +73,7 @@ -Wredundant-constraints -fhide-source-paths -fobject-code if impl(ghc >=8.10.0) - ghc-options: -Wunused-packages -fwrite-ide-info + ghc-options: -Wunused-packages -fwrite-ide-info -hiedir=.hie if flag(debug) ghc-options: -ddump-minimal-imports @@ -83,6 +87,7 @@ FlexibleInstances InstanceSigs LambdaCase + MonoLocalBinds MultiParamTypeClasses TupleSections @@ -100,18 +105,15 @@ other-modules: Paths_smuggler2 autogen-modules: Paths_smuggler2 build-depends: - , containers ^>=0.6.0 - , directory ^>=1.3.3 - , filepath ^>=1.4.2 + , containers ^>=0.6 + , directory ^>=1.3 + , filepath ^>=1.4 , ghc >=8.6.5 && <8.11 , ghc-boot >=8.6.5 && <8.11 , ghc-exactprint ^>=0.6.3 , split ^>=0.2.3 , syb ^>=0.7.1 - if flag(debug) - build-depends: text - common executable-options ghc-options: -rtsopts @@ -121,16 +123,8 @@ 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 - +-- The golden results are for ghc 8.8 and later. ghc 8.6.5 produces different +-- results because it has a different version of base test-suite smuggler2-test import: common-options import: executable-options diff --git a/src/Smuggler2/Imports.hs b/src/Smuggler2/Imports.hs --- a/src/Smuggler2/Imports.hs +++ b/src/Smuggler2/Imports.hs @@ -19,7 +19,7 @@ LImportDecl ) import HscTypes -- earlier versions of Ghc don't have ModIface_ import LoadIface ( loadSrcInterface ) -import Name ( HasOccName(..), isDataOcc, isSymOcc, isTcOcc ) +import Name ( HasOccName(..), isDataOcc, isTcOcc ) import Outputable ( Outputable(ppr), text, (<+>) ) #if MIN_VERSION_GLASGOW_HASKELL(8,8,0,0) import RdrName ( gresToAvailInfo ) @@ -141,6 +141,10 @@ all_non_overloaded = not . any flIsOverloaded + +-- The following were originally one function. Splitting it out this way allows +-- different variants to be used in different contexts above. This seems to +-- work, but needs review. Eg, do we need to use an isSymOcc in some cases? to_ie_post_rn_name :: Located name -> LIEWrappedName name to_ie_post_rn_name (L l n) = L l (IEName (L l n)) diff --git a/src/Smuggler2/Options.hs b/src/Smuggler2/Options.hs --- a/src/Smuggler2/Options.hs +++ b/src/Smuggler2/Options.hs @@ -1,4 +1,5 @@ {-# LANGUAGE DeriveAnyClass #-} +{-# LANGUAGE StrictData #-} -- | -- Description: handling of command line options module Smuggler2.Options diff --git a/src/Smuggler2/Parser.hs b/src/Smuggler2/Parser.hs --- a/src/Smuggler2/Parser.hs +++ b/src/Smuggler2/Parser.hs @@ -25,12 +25,12 @@ -- (which should never happen). We need to use -- 'Language.Haskell.GHC.ExactPrint.parseModuleFromStringInternal' -- because 'Language.Haskell.GHC.ExactPrintparseModuleFromString' --- doesn't pick up the correct 'DynFlags' in -- some cases. +-- doesn't pick up the correct 'DynFlags' in some cases. runParser :: DynFlags -> FilePath -> String -> RnM (Either () (Anns, ParsedSource)) runParser dflags fileName fileContents = do - -- Withoout the following, comments are stripped (see #10942) - -- It would be more efficient, but less visible to apply this tweak at the + -- Without the following, comments are stripped (see #10942) + -- It would be more efficient, but less visible, to apply this tweak at the -- outset, in the main plugin function, but keep it here for visibility -- See also https://gitlab.haskell.org/ghc/ghc/-/wikis/api-annotations, which -- notes that the flags are returned as annotations by the diff --git a/src/Smuggler2/Plugin.hs b/src/Smuggler2/Plugin.hs --- a/src/Smuggler2/Plugin.hs +++ b/src/Smuggler2/Plugin.hs @@ -1,3 +1,4 @@ +{-# LANGUAGE CPP #-} {-# LANGUAGE LambdaCase #-} -- | @@ -16,11 +17,10 @@ import DynFlags ( DynFlags (dumpDir), HasDynFlags (getDynFlags), - setUnsafeGlobalDynFlags, xopt, xopt_set, ) -import ErrUtils (compilationProgressMsg, fatalErrorMsg, warningMsg) +import ErrUtils (compilationProgressMsg, fatalErrorMsg, warningMsg, withTiming) import GHC ( GenLocated (L), GhcPs, @@ -33,6 +33,7 @@ ModSummary (ms_hspp_buf, ms_mod), Module (moduleName), ParsedSource, + hsmodName, ml_hs_file, moduleNameString, ms_location, @@ -72,7 +73,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 @@ -98,10 +99,9 @@ return tcEnv | otherwise = do dflags <- getDynFlags - liftIO $ setUnsafeGlobalDynFlags dflags -- this seems to be needed for Windows only liftIO $ compilationProgressMsg dflags ("smuggler2 " ++ showVersion version) - -- Get imports usage + -- Get imports usage uses <- readMutVar $ tcg_used_gres tcEnv let usage = findImportUsage imports uses @@ -157,9 +157,17 @@ -- Run smuggling only for its side effects; don't change the tcEnv we -- were givem. - tcEnv <$ smuggling dflags minImpFilePath + tcEnv + <$ withTiming +#if MIN_VERSION_GLASGOW_HASKELL(8,10,1,0) + dflags +#else + getDynFlags +#endif + (text "smuggler2") + (const ()) + (smuggling dflags minImpFilePath) where - -- The original imports imports :: [LImportDecl GhcRn] imports = tcg_rn_imports tcEnv @@ -231,20 +239,18 @@ ++ wasLhs -- Print the result - let newContent = exactPrint astHsMod' annsHsMod' + let newContent = crnlTonl $ exactPrint astHsMod' annsHsMod' let newModulePath = modulePath -<.> ext liftIO $ writeFile newModulePath newContent liftIO $ compilationProgressMsg dflags - ( "smuggler2: output written to " ++ newModulePath ) + ("smuggler2: output written to " ++ newModulePath) -- 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] @@ -255,29 +261,6 @@ exports <- exports_from_avail Nothing rdr_env importAvails this_mod return (snd exports) - -- Replace a target module's imports - -- See - replaceImports :: - Monad m => - -- | annotations for the replacement imports - Anns -> - -- | the replacement imports - [LImportDecl GhcPs] -> - -- | target module - ParsedSource -> - TransformT m ParsedSource - replaceImports anns minImports t@(L l m) = - case importAction options of - NoImportProcessing -> return t - _ -> do - -- This does all the work - -- retrie has a neat `insertImports' function that also - -- deduplicates - imps <- graftT anns minImports - -- nudge down the imports list onto a new line - unless (null imps) $ setEntryDPT (head imps) (DP (2, 0)) - return $ L l m {hsmodImports = imps} - -- Add explict exports to the target module addExplicitExports :: Monad m => @@ -290,12 +273,15 @@ case exportAction options of NoExportProcessing -> return t AddExplicitExports -> - -- Only add explicit exports if there are none. - -- Seems to work even if there is no explict module declaration - -- presumably because the annotations that we generate are just - -- unused by exactPrint - if isNothing currentExplicitExports then result else return t - ReplaceExports -> result + -- Only add explicit exports if there are none and there is a + -- @module X where ...@ + if isNothing currentExplicitExports && isJust (hsmodName hsMod) + then result + else return t + ReplaceExports -> + if isJust (hsmodName hsMod) + then result + else return t where currentExplicitExports :: Maybe (Located [LIE GhcPs]) currentExplicitExports = hsmodExports hsMod @@ -315,6 +301,29 @@ -- annotations in the current transformation return $ L astLoc hsMod {hsmodExports = Just lExportsList} + -- Replace a target module's imports + -- See + replaceImports :: + Monad m => + -- | annotations for the replacement imports + Anns -> + -- | the replacement imports + [LImportDecl GhcPs] -> + -- | target module + ParsedSource -> + TransformT m ParsedSource + replaceImports anns minImports t@(L l m) = + case importAction options of + NoImportProcessing -> return t + _ -> do + -- This does all the work + -- retrie has a neat `insertImports' function that also + -- deduplicates + imps <- graftT anns minImports + -- nudge down the imports list onto a new line + unless (null imps) $ setEntryDPT (head imps) (DP (2, 0)) + return $ L l m {hsmodImports = imps} + -- 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, @@ -348,19 +357,20 @@ where notImplicit :: ImportDecl pass -> Bool notImplicit = not . ideclImplicit - -- + notInstancesOnly :: ImportDecl pass -> Bool notInstancesOnly i = case ideclHiding i of Just (False, L _ []) -> False _ -> True - -- + keepInstanceOnlyImports :: Bool keepInstanceOnlyImports = importAction options /= MinimiseImports + -- Ignore explicit instance only imports, unless the 'MinimiseImports' -- option is specified letThrough :: LImportDecl pass -> Bool letThrough (L _ i) = notImplicit i && (keepInstanceOnlyImports || notInstancesOnly i) - -- + leaveOpen :: LImportDecl pass -> LImportDecl pass leaveOpen (L l decl) = L l $ case ideclHiding decl of Just (False, L _ _) -- ie, not hiding @@ -392,3 +402,12 @@ -- strBufToStr :: StringBuffer -> String strBufToStr sb@(StringBuffer _ len _) = lexemeToString sb len + + -- This is a workaround until 'ghc-exactprint' stops inserting (preserving) + -- extra @\r@ at the end of comment lines. + crnlTonl :: String -> String + crnlTonl ('\r' : '\n' : rest) = '\n' : crnlTonl rest + crnlTonl "\r" = "" -- Haddock comments from source files with dos line endings end with a CR + crnlTonl (c : rest) = c : crnlTonl rest + crnlTonl "" = "" + diff --git a/test/Test.hs b/test/Test.hs --- a/test/Test.hs +++ b/test/Test.hs @@ -1,27 +1,34 @@ {-# LANGUAGE OverloadedStrings #-} -module Main where +module Main ( + testDir, + optionsList, + mkExt, + testOptions, + goldenTests, + findByExtension', + main, + compile ) where -import Control.Monad (forM) -import Data.List (intercalate, sort) -import Data.Maybe (fromMaybe) -import qualified Data.Set as Set (fromList, member) -import GHC (mkModuleName, moduleNameString) -import GHC.Paths (ghc) -import Smuggler2.Options (ExportAction (..), ImportAction (..), Options (..)) -import System.Directory (doesDirectoryExist, getDirectoryContents) -import System.Environment (lookupEnv) -import System.FilePath (takeBaseName, takeExtension, (-<.>), ()) -import System.Process.Typed - ( ProcessConfig, - proc, - runProcess_, - setEnvInherit, - setWorkingDirInherit, - ) -import Test.Tasty (TestTree, defaultMain, testGroup) -import Test.Tasty.Golden (goldenVsFileDiff, writeBinaryFile) +import Control.Monad ( forM ) +import Data.List ( intercalate, sort ) +import Data.Maybe ( fromMaybe ) +import qualified Data.Set as Set ( fromList, member ) +import GHC ( mkModuleName, moduleNameString ) +import GHC.Paths ( ghc ) +import Smuggler2.Options + ( ExportAction(..), ImportAction(..), Options(..) ) +import System.Directory + ( doesDirectoryExist, getDirectoryContents ) +import System.Exit ( ExitCode(ExitFailure, ExitSuccess) ) +import System.FilePath + ( (-<.>), (), takeBaseName, takeExtension ) +import System.Process.Typed ( proc, runProcess ) +import Test.Tasty ( TestTree, defaultMain, testGroup ) +import Test.Tasty.Golden ( goldenVsFileDiff, writeBinaryFile ) + + -- | Where the tests are, relative to the project level cabal file testDir :: FilePath testDir = "test" "tests" @@ -70,9 +77,18 @@ [ goldenVsFileDiff (takeBaseName testFile) -- test name ( \ref new -> -- how to display diffs - -- The -G. is needed because cabal sdist changes the golden file - -- permissions and so all the tests fail. - ["git", "diff", "-G.", "--ignore-cr-at-eol", "--no-index", ref, new] + -- The -G. is needed because cabal sdist changes the golden file + -- permissions and so all the tests fail. + [ "git", + "diff", + "-G.", + "--ignore-cr-at-eol", + "--ws-error-highlight=all", + "--no-index", + "--exit-code", + ref, + new + ] ) (testFile -<.> testName ++ "-golden") -- golden file outputFilename @@ -87,9 +103,10 @@ ] where testName = fromMaybe "NoNewExtension" (newExtension opts) + -- | A version of 'Test.Tasty.Golden.findByExtension' that does not look into -- subdirectories. This allows tests to be run on a module that imports other --- modules without risking the race condition where smuggler is being run +-- modules without risking the race condition where smuggler2 is being run -- on the imported module directly, and also when the importing module is being -- tested. ('Tasty' runs test in parallel and the same output files are -- produced in both direct and imported cases.) Putting the imported module in @@ -110,9 +127,7 @@ let entries = filter (not . (`elem` [".", ".."])) allEntries fmap concat $ forM entries $ \e -> do let path = dir ++ "/" ++ e - isDir <- - doesDirectoryExist - path + isDir <- doesDirectoryExist path return $ if isDir then [] -- don't recurse @@ -122,32 +137,20 @@ main :: IO () main = defaultMain =<< testOptions optionsList --- | Use `cabal exec` to run the compilation, so that the smuggler plugin is --- picked up from the local database. GHC alone would use the global one. +-- | Run a compilation. Assumes that @smuggler2@ has been built with +-- @--write-ghc-environment-files=always@ so that it is picked up from +-- the local database. compile :: FilePath -> Options -> IO () compile testcase opts = do - cabalPath <- lookupEnv "CABAL" -- find, eg, @/opt/ghc/bin/cabal@ or @cabal -vnormal+nowrap@ - let cabalCmd = words $ fromMaybe "cabal" cabalPath -- default to @cabal@ if @CABAL@ is not set - let cabalConfig = - setWorkingDirInherit . setEnvInherit $ - proc - (head cabalCmd) - (tail cabalCmd ++ cabalArgs) :: - ProcessConfig () () () - runProcess_ cabalConfig + r <- runProcess (proc ghc ghcArgs) + return $ case r of + ExitSuccess -> () + ExitFailure c -> + error $ "Failed to compile " ++ testcase ++ ". Exit code " ++ show c where - cabalArgs :: [String] - cabalArgs = - -- - not sure why it is necessary to mention the smuggler2 package explicitly, - -- but it appears to be hidden otherwise. - -- - This also puts the .imports files that smuggler2 generates somewhere they - -- can easily be found - [ "--with-compiler=" ++ ghc, - "exec", - ghc, - "--", - "-package smuggler2", - "-v0", + ghcArgs :: [String] + ghcArgs = + [ "-v0", "-dumpdir=" ++ testDir, "-fno-code", "-i" ++ testDir, @@ -162,9 +165,10 @@ mo = intercalate "," (moduleNameString <$> makeOpenImports opts) in -- The extension should have been set by 'optionsList' -- The rest of the list are the other arguments for the test - ( fromMaybe "missng" ne : [show ia, show ea] - ++ ["LeaveOpenImports:" ++ lo | not (null lo)] - ++ ["MakeOpenImports:" ++ mo | not (null mo)] + ( fromMaybe "missng" ne : + [show ia, show ea] + ++ ["LeaveOpenImports:" ++ lo | not (null lo)] + ++ ["MakeOpenImports:" ++ mo | not (null mo)] ) ) ++ [testcase] diff --git a/test/tests/Bare.NoImportProcessingNoExportProcessing-golden b/test/tests/Bare.NoImportProcessingNoExportProcessing-golden --- a/test/tests/Bare.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/Bare.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/BareHello.NoImportProcessingNoExportProcessing-golden b/test/tests/BareHello.NoImportProcessingNoExportProcessing-golden --- a/test/tests/BareHello.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/BareHello.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/BoolBoolUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/BoolBoolUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/BoolBoolUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/BoolBoolUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/BoolBoolUsed.NoImportProcessingNoExportProcessing-golden b/test/tests/BoolBoolUsed.NoImportProcessingNoExportProcessing-golden --- a/test/tests/BoolBoolUsed.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/BoolBoolUsed.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/BoolPartlyUsedFirst.NoImportProcessingNoExportProcessing-golden b/test/tests/BoolPartlyUsedFirst.NoImportProcessingNoExportProcessing-golden --- a/test/tests/BoolPartlyUsedFirst.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/BoolPartlyUsedFirst.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/BoolPartlyUsedLast.NoImportProcessingNoExportProcessing-golden b/test/tests/BoolPartlyUsedLast.NoImportProcessingNoExportProcessing-golden --- a/test/tests/BoolPartlyUsedLast.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/BoolPartlyUsedLast.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/CPP.MinimiseImportsNoExportProcessingPreludeDataVersion-golden b/test/tests/CPP.MinimiseImportsNoExportProcessingPreludeDataVersion-golden --- a/test/tests/CPP.MinimiseImportsNoExportProcessingPreludeDataVersion-golden +++ b/test/tests/CPP.MinimiseImportsNoExportProcessingPreludeDataVersion-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/CPP.MinimiseImportsReplaceExports-golden b/test/tests/CPP.MinimiseImportsReplaceExports-golden --- a/test/tests/CPP.MinimiseImportsReplaceExports-golden +++ b/test/tests/CPP.MinimiseImportsReplaceExports-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/CPP.NoImportProcessingAddExplicitExports-golden b/test/tests/CPP.NoImportProcessingAddExplicitExports-golden --- a/test/tests/CPP.NoImportProcessingAddExplicitExports-golden +++ b/test/tests/CPP.NoImportProcessingAddExplicitExports-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/CPP.NoImportProcessingNoExportProcessing-golden b/test/tests/CPP.NoImportProcessingNoExportProcessing-golden --- a/test/tests/CPP.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/CPP.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/CPP.NoImportProcessingReplaceExports-golden b/test/tests/CPP.NoImportProcessingReplaceExports-golden --- a/test/tests/CPP.NoImportProcessingReplaceExports-golden +++ b/test/tests/CPP.NoImportProcessingReplaceExports-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/CPP.PreserveInstanceImportsNoExportProcessingPrelude-golden b/test/tests/CPP.PreserveInstanceImportsNoExportProcessingPrelude-golden --- a/test/tests/CPP.PreserveInstanceImportsNoExportProcessingPrelude-golden +++ b/test/tests/CPP.PreserveInstanceImportsNoExportProcessingPrelude-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/Class.NoImportProcessingNoExportProcessing-golden b/test/tests/Class.NoImportProcessingNoExportProcessing-golden --- a/test/tests/Class.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/Class.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ConstructorsUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/ConstructorsUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ConstructorsUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ConstructorsUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/Dummy.NoImportProcessingNoExportProcessing-golden b/test/tests/Dummy.NoImportProcessingNoExportProcessing-golden --- a/test/tests/Dummy.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/Dummy.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/Duplicated.NoImportProcessingNoExportProcessing-golden b/test/tests/Duplicated.NoImportProcessingNoExportProcessing-golden --- a/test/tests/Duplicated.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/Duplicated.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ExportExplicit.NoImportProcessingNoExportProcessing-golden b/test/tests/ExportExplicit.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ExportExplicit.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ExportExplicit.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ExportImplicit.NoImportProcessingNoExportProcessing-golden b/test/tests/ExportImplicit.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ExportImplicit.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ExportImplicit.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ExportPattern2.NoImportProcessingNoExportProcessing-golden b/test/tests/ExportPattern2.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ExportPattern2.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ExportPattern2.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ImplicitImportUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/ImplicitImportUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ImplicitImportUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ImplicitImportUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ImplicitImportUsed.NoImportProcessingNoExportProcessing-golden b/test/tests/ImplicitImportUsed.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ImplicitImportUsed.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ImplicitImportUsed.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ImportEmpty.NoImportProcessingNoExportProcessing-golden b/test/tests/ImportEmpty.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ImportEmpty.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ImportEmpty.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/ImportPattern.NoImportProcessingNoExportProcessing-golden b/test/tests/ImportPattern.NoImportProcessingNoExportProcessing-golden --- a/test/tests/ImportPattern.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/ImportPattern.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/MoreExports.NoImportProcessingNoExportProcessing-golden b/test/tests/MoreExports.NoImportProcessingNoExportProcessing-golden --- a/test/tests/MoreExports.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/MoreExports.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/MultilineUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/MultilineUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/MultilineUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/MultilineUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/NoImplicitPrelude.NoImportProcessingNoExportProcessing-golden b/test/tests/NoImplicitPrelude.NoImportProcessingNoExportProcessing-golden --- a/test/tests/NoImplicitPrelude.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/NoImplicitPrelude.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/Operator.NoImportProcessingNoExportProcessing-golden b/test/tests/Operator.NoImportProcessingNoExportProcessing-golden --- a/test/tests/Operator.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/Operator.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/Pattern.NoImportProcessingNoExportProcessing-golden b/test/tests/Pattern.NoImportProcessingNoExportProcessing-golden --- a/test/tests/Pattern.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/Pattern.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/QualUnqual.NoImportProcessingNoExportProcessing-golden b/test/tests/QualUnqual.NoImportProcessingNoExportProcessing-golden --- a/test/tests/QualUnqual.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/QualUnqual.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/TypeConstructorUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/TypeConstructorUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/TypeConstructorUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/TypeConstructorUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/TypeConstructorUnusedComma.NoImportProcessingNoExportProcessing-golden b/test/tests/TypeConstructorUnusedComma.NoImportProcessingNoExportProcessing-golden --- a/test/tests/TypeConstructorUnusedComma.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/TypeConstructorUnusedComma.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/TypeConstructorUsed.NoImportProcessingNoExportProcessing-golden b/test/tests/TypeConstructorUsed.NoImportProcessingNoExportProcessing-golden --- a/test/tests/TypeConstructorUsed.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/TypeConstructorUsed.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/TypeFam.NoImportProcessingNoExportProcessing-golden b/test/tests/TypeFam.NoImportProcessingNoExportProcessing-golden --- a/test/tests/TypeFam.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/TypeFam.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/TypeUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/TypeUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/TypeUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/TypeUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/TypeUsed.NoImportProcessingNoExportProcessing-golden b/test/tests/TypeUsed.NoImportProcessingNoExportProcessing-golden --- a/test/tests/TypeUsed.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/TypeUsed.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/WildcardUnused.NoImportProcessingNoExportProcessing-golden b/test/tests/WildcardUnused.NoImportProcessingNoExportProcessing-golden --- a/test/tests/WildcardUnused.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/WildcardUnused.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched diff --git a/test/tests/WildcardUsed.NoImportProcessingNoExportProcessing-golden b/test/tests/WildcardUsed.NoImportProcessingNoExportProcessing-golden --- a/test/tests/WildcardUsed.NoImportProcessingNoExportProcessing-golden +++ b/test/tests/WildcardUsed.NoImportProcessingNoExportProcessing-golden @@ -1,1 +1,1 @@ -Source file was not touched +Source file was not touched