Some infelicities

[jrp2014]

This is just the plugin I was looking for, and I have been giving it a whirl.

I used ghc 8.8.3 (by adjusting the cabal bounds).

-  build-depends:       base ^>= 4.12
+  build-depends:       base ^>= 4.13
   ghc-options:         -Wall
                        -Wincomplete-uni-patterns
                        -Wincomplete-record-updates
@@ -62,8 +62,8 @@ library
   build-depends:       bytestring ^>= 0.10
                      , containers >= 0.5 && < 0.7
                      , filepath ^>= 1.4
-                     , ghc ^>= 8.6.0
-                     , ghc-exactprint ^>= 0.6
+                     , ghc ^>= 8.8.3
+                     , ghc-exactprint ^>= 0.6.2
                      , unordered-containers ^>= 0.2.7

This doesn't compile, so I also have to

diff --git a/src/Smuggler/Loc.hs b/src/Smuggler/Loc.hs
index 6580578..b2ae1f2 100644
--- a/src/Smuggler/Loc.hs
+++ b/src/Smuggler/Loc.hs
@@ -4,8 +4,7 @@ module Smuggler.Loc
        , unL
        ) where
 
-import SrcLoc (GenLocated (L), Located, SrcSpan (RealSrcSpan), srcSpanStartCol, srcSpanStartLine,
-               unLoc)
+import SrcLoc (GenLocated (L), Located, SrcSpan (RealSrcSpan), srcSpanStartCol, srcSpanStartLine)
 
 -- | Returns location in the way of @line:col@.
 showLoc :: SrcSpan -> String
@@ -18,4 +17,4 @@ showL name showA (L l nm) = "(" ++ showLoc l ++ name ++ showA nm ++ ")"
 
 -- | Shorter synonim for 'unLoc'.
 unL :: GenLocated l e -> e
-unL = unLoc
+unL (L _ e) = e

Perhaps the answer is to use ghc-lib.

The tests now don't pass

Linking /Users/xxx/Documents/Haskell/smuggler/dist-newstyle/build/x86_64-osx/ghc-8.8.3/smuggler-0.2.0.0/t/smuggler-test/build/smuggler-test/smuggler-test ...
Running 1 test suites...
Test suite smuggler-test: RUNNING...

Test test/Test/BoolPartlyUsedLast failed:
Expected:
- import Data.Bool ( not)
But got:


Test test/Test/TypeConstructorUsed failed:
Expected:
- import Data.Bool (Bool (True))
But got:


Test test/Test/TypeUsed failed:
Expected:
- import Data.Bool (Bool)
But got:


Test test/Test/WildcardUsed failed:
Expected:
- import Data.Bool (Bool (..))
But got:


Test test/Test/TypeConstructorUnused failed:
Expected:
- import Data.Bool (Bool ( True))
But got:


Test test/Test/TypeConstructorUnusedComma failed:
Expected:
- import Data.Bool (Bool (False))
But got:


Test test/Test/WildcardUnused failed:
Expected:
- import Data.Bool ( not)
But got:


Test test/Test/ConstructorsUnused failed:
Expected:
- import Data.Bool ( not)
But got:


Test test/Test/TypeUnused failed:
Expected:
- import Data.Bool ( not)
But got:

Cleaning tests
Some tests failed

Test suite smuggler-test: FAIL

I tried running the plugin on a large project with a lot of redundant imports, https://github.com/agda/agda. There were several issues:

• because smuggler reads files using ByteString it mangles Unicode characters in the code. Eg:

--- | Get the next version of the concrete name. For instance, @nextName "x" = "x₁"@.
+-- | Get the next version of the concrete name. For instance, @nextName "x" = "xâ<U+0082><U+0081>"@.

• a good number of redundant imports get zapped, but there is a fair bit of mangling (where two adjacent imports get joined into a single line, with the second import keyword missing). I am not sure how close the result is to the minimal imports that GHC sees with -ddump-minimal-imports
• some imports are redundant, but needed for backward compatibility (eg, around Monad.Fail and Data.Monoid and Data.Semigroup.

To make this app even more useful, it would be helpful to have it generate a minimal exports list (based on what is used elsewhere in the project)!

Thanks for all the hard work that has clearly gone into all this.

[chshersh]

Hi @jrp2014, thanks for using our package and providing your feedback! We appreciate you find it useful 😊 I'm going to provide comment for each of your concerns separately:

1. There's a separate issue Support GHC-8.8.1 #66 to support GHC-8.8.1. At this moment, smuggler implementation is based on Source Plugins and the ghc-exactprint Haskell library, which means that it can realistically support a single version of GHC. Thus using something like ghc-lib-parser won't be enough, since ghc-lib-parser is only for parsing. We also need something for Source Plugins API and exact printing API. Though, if you've already figured out the patch to support GHC-8.8, I'm happy to accept the PR 🙂

2. If there are some problems with handling Unicode, a separate issue can be created and fixed. I believe the fix won't be that difficult to implement.

3. a good number of redundant imports get zapped, but there is a fair bit of mangling (where two adjacent imports get joined into a single line, with the second import keyword missing)

   I'm not sure I understand what's going on here. Can you give the example of how smuggler worked and what do you expect?

4. some imports are redundant, but needed for backward compatibility

   I think it's possible to implement an option that allows you to specify imports you want to keep.

[jrp2014]

Hi, I've had a go at rewriting smuggler https://github.com/jrp2014/smuggler using the approach that I mentioned in another exchange, namely getting ghc to do the hard work and grafting back the result. It runs on ghcs 8.10.1, 8.8.3 and 8.6.5 (and perhaps earlier, but I haven't tested). The results for the tests are different for 8.6.5 because newer versions of ghc do not need to import Data.Bool. This version of smuggler can also write in explicit exports for you. There are some further thoughts in the README and TODO.

The limitations of the approach are that it doesn't preserve comments or extra newlines (eg, to separate project from library imports. It may be possible to try to do a bit better, but it will be hard to do so robustly.

Since it relies on ghc, I'm fairly confident that it largely works, but it needs more testing and I'd be grateful for any other thoughts you might have. The trickiest part for me was figuring out how best to use exactPrint and I suspect that some of the less used IE constructors are not handled (mainly because I haven't had time to find examples and discover what the correct annotations should be).

Anyway, thanks for providing the basic infrastructure to get me going.