Skip to content

[Use fewer imports]: combine field imports for same imported type, class, etc. #1133 #1654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Morabbin
Copy link

Summary

Combine multiple fields for any imported type, constructor, or class (this is called a "thing" in the GHC API).

Suppose a type, constructor, or class with fields will appear in at most one non-hiding import declaration (as guaranteed by simplify). The "use fewer imports" hint will combine multiple field imports into a single import list element.

Hiding imports are left untouched.

For example:

import A (Foo(A,B), Bar(C,D), Foo(E), Foo(C,D))
import A (Foo(A,F), Baz(D,A))

would first simplify to:

import A (Foo(A,B), Bar(C,D), Foo(E), Foo(C,D), Foo(A,F), Baz(D,A))

and then simplifyFields would yield:

import A (Foo(A,B,C,D,E,F), Bar(C,D), Baz(D,A))

To sort or not to sort?

Note that sorting occurs only when field imports are combined.

The import list itself preserves the order of the types, etc. in the original imports. It also leaves field imports unsorted if there's only one import list element for that type, class, or constructor. The latter is easy (just don't change it), but the former requires a kind of stable nub.

We could use a similar stable nub to preserve the field import order from the original too.

Is that worth doing?

Test Plan

Unit tests

Added several cases to the section in Hint.Import:

~/Work/hlint $ cabal run -- hlint --test
...
Testing (with refactoring)
Source annotations ....................
Input/outputs ..........................................................................................................................................................................
Hint names ........
Hint annotations ........

Tests passed (975)

Running this hlint on hlint src

~/Work/hlint $ cabal run hlint -- src --report
Writing report to report.html ...
No hints

…, class ndmitchell#1133

# Summary

Combine multiple fields for any imported type, constructor, or class (this is called a "thing" in the GHC API).

Suppose a type, constructor, or class with fields will appear in at most one non-hiding import declaration (as guaranteed by `simplify`). The "use fewer imports" hint will combine multiple field imports into a single import list element.

Hiding imports are left untouched.

For example:
```haskell
import A (Foo(A,B), Bar(C,D), Foo(E), Foo(C,D))
import A (Foo(A,F), Baz(D,A))
```
would first `simplify` to:
```haskell
import A (Foo(A,B), Bar(C,D), Foo(E), Foo(C,D), Foo(A,F), Baz(D,A))
```
and then `simplifyFields` would yield:
```haskell
import A (Foo(A,B,C,D,E,F), Bar(C,D), Baz(D,A))
```

# Test Plan

## Unit tests
Added several cases to the <TEST> section in Hint.Import:
```sh
~/Work/hlint $ cabal run -- hlint --test
...
Testing (with refactoring)
Source annotations ....................
Input/outputs ..........................................................................................................................................................................
Hint names ........
Hint annotations ........

Tests passed (975)
```

## Running this hlint on hlint src
```sh
~/Work/hlint $ cabal install exe:hlint --overwrite-policy=always
Resolving dependencies...
Build profile: -w ghc-9.12.2 -O1
In order, the following will be built (use -v for more details):
 - hlint-3.10 (lib) (requires build)
 - hlint-3.10 (exe:hlint) (requires build)
Starting     hlint-3.10 (lib)
...
Completed    hlint-3.10 (exe:hlint)
Symlinking 'hlint' to '/Users/aam/.cabal/bin/hlint'
~/Work/hlint $ cabal run hlint -- src --report
Writing report to report.html ...
No hints
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant