Sort imports

[lspitzner]

See #155

This implements the basic functionality, including some trickery to handle comments sensibly.

Terminology:

import Foo ( MyData(MyExposedConstructor), myFunction )
                    \----constructor----/
             \----------item------------/  \--item--/
\----------------------import-------------------------/

import A                       \ import block
-- comment on why we import B  |
import qualified B             |
import C                       /

-- comment in the middle       | independent comment (not connected to import D)

import D ( a                   \ import block
         , b                   |
         , b                   |
         )                     | 
import E                       /

-- that is, blocks are series of import statements that don't contain any empty lines.

-- Merging (for example on constructors):
import A ( MyType(Foo), MyType(Bar) )
-- -->
import A ( MyType (Foo, Bar) )

-- question left for the reader: How can these be merged?
import qualified A (foo, bar)
import           A hiding (bar, baz)

Implemented features

• [x] Sort import statements
• [x] Sort and uniq items
• [x] Sort and uniq constructors
• [x] Merge constructors
• [x] Respect blocks of imports, only sort inside blocks
• [x] Sorting imports moves comments connected to those imports around, but keeps all comments
• [x] Retain newlines between comments and keep independent comments unmodified

Issues that must be fixed before merging

• [x] I messed up and CI says the code does not compile. Whoops.
• [x] Fix existing tests
• [x] Fix compat with different ghc versions
• [ ] Needs some new tests
• [ ] Should be tested on some larger existing code-bases

all other are non-blockers

Features yet to be implemented

• [ ] Merge imports

• [ ] Sort exports - basics would work already, even module exports work already or are trivial, but haddock headers and sections are really annoying to get right in general.

• [ ] Paragraph-fill version of item lists

  import Foo
    ( longNameOne, longNameTwo, longNameThree, longNameFour, longNameFive
    , longNameSix, longNameSeven, longNameEight, longNameNine, longNameTen
    , longNameEleven, longNameTwelve, longNameThirteen, longNameFourteen
    , longNameFifteen
    )

  Issues to be fixed, low priority

• [ ] Comments on items and on constructors get lost when merging happens

[expipiplus1]

@lspitzner I've got a large codebase which I've been deferring neatening imports on until brittany had support! I'll give this branch a test run this weekend.

[expipiplus1]

@lspitzner works very well! https://github.com/expipiplus1/vulkan/commit/dcca2969c301c0a062c489150983a5f767f336f8

I like how it preserves groups of imports and doesn't barf on {-# SOURCE #-} imports.

If possible: when you live code next could it be friendly to Singapore time?

[tfausak]

I don't have a public codebase to point to, but this worked great on our ~80 KLOC project :+1:

[expipiplus1]

@lspitzner This is a great improvement already, would it be possible to get this merged and move the exising TODO items to issues?

[lspitzner]

sorry I am so unresponsive atm. I have been / am still struggling to move all my maintenance and potentially CI over to nix. It has been a bit frustrating but I think it will be worth it in the long run.

There are two minor fixups I wanted to add on this branch, both simple "if this special case occurs, just fall back on not modifying the thing in question". These should indeed make this a useful addition without any downsides even for users that might have curious comments on their imports. Any other improvements can indeed be moved to separate issues.

So I am not taking a summer break, but the priorities will be nix > ghc810 > sorting imports.

[expipiplus1]

@lspitzner Is there any specific help you could use on the nix side of things? FWIW this very simple github action (using cachix for caching) has been working well for me: https://github.com/expipiplus1/vulkan/blob/master/.github/workflows/ci.yml#L170-L186

[expipiplus1]

I've been using this for quite some time, and think it's certainly stable enough to be merged as is, perhaps conditional on a config setting as it's considered incomplete.

[tfausak]

Closing this in favor of #330.