[ghc-steering-committee] Split -Wunused-imports (recommend accept)

[Chris Dornan]

Just for the record, I am also open to refinement of the import-warning toolbox in future but I think this proposal is the best next step.

Given how straightforward the proposition is, how about we set a week for objections to be raised and move to accept the proposal if none are forthcoming (by Saturday, 2023-06-24).

Chris

> On 16 Jun 2023, at 13:33, Joachim Breitner <mail at joachim-breitner.de> wrote:
> 
> Hi,
> 
> this proposal highlights a fundamental tension with compiler warnings.
> 
> If you assume a never changing background of dependencies, then the
> current behavior isn’t too bad: GHC correctly tell you that you can
> remove a line of code, and that is a good thing. I don’t think that
> this is directly what is bothering developers.
> 
> But dependencies do change, and exports get moved around, and code that
> wasn’t warning before suddenly warns. And that is what’s annoying!
> 
> So there is a tension between “given the current assumptions, your code
> can be improved” vs. “assumptions change”.
> 
> I agree that at least in this case (and probably in general) we want
> the -Wall warnings to be reasonably insensitive to dependency changes,
> so that good code stays “good”. Of course this is not a hard rule –
> think of deprecation warnings, but a decent guideline.
> 
> Therefore in favor.
> 
> 
> The proposal mentions as alternative “Relaxed redundant imports”, which
> refines the warnings a bit more. In that alternative, an _duplicate
> explicit import_ is cause for a warning; in Chris’ example that would
> be
> 
>   import X (f) -- exports some function @f@
>   import y (f) -- exports the same function @f@
>   g = f
> 
> Under the current proposal, this would no longer warn with -Wall, but I
> think it’s silly to not warn here, as even with changing dependencies,
> this is a code smell. I sympathize with the author’s wish to not make
> this more complicated than necessary to achieve the goal (warnings
> about duplicate _implicit_ imports causing headcaches), but would like
> to keep the door open for someone who wants to implement the refined
> version in the future. That would probably entail a
>  -Wduplicate-explicit-import
> flag which can then live in -Wall, but is still compatible with this
> proposal, so all izz well.
> 
> 
> 
> Adam points out (as quoted in the proposal) that users of
>   -Weverything -Wno-unused-imports
> might see new breakage. I don’t think someone using -Weverything can
> expect any kind of stability, else we paint ourselves into a corner (or
> start introducing -Wreally-everything, -Wreally-really-everything, …)
> 
> 
> Cheers,
> Joachim
> 
> 
> 
> 
> Am Donnerstag, dem 15.06.2023 um 16:50 +0100 schrieb Chris Dornan:
>> Dear Committee,
>> 
>> Georgi Lyubenov has a simple proposal to improve the quality of life for many package maintainers that I recommend that we accept.
>> 
>> ## Split -Wunused-imports
>> 
>> Rendered proposal: <https://github.com/googleson78/ghc-proposals/blob/split-unused-imports/proposals/0000-split-wunused-imports.md>
>> Discussion of proposal: <https://github.com/ghc-proposals/ghc-proposals/pull/586>
>> 
>> ### Summary
>> 
>> (Straight from the proposal.)
>> 
>> We propose to split the -Wunused-imports warning into two parts:
>> 
>> - warn on actual unused imports
>> - warn on duplicate imports
>> 
>> while also not including the second in -Wall. This will greatly reduce "churn" for library authors.
>> 
>> 
>> ## In more Detail
>> 
>> As we know, many Haskell developers, including significant commercial projects, incorporate -Wall
>> -Werror into their workflow at some stage. To this end it is important that we minimise 'false
>> negatives', whereby the compiler emits 'frivolous' warnings.
>> 
>> This proposal identifies an important class of -Wall warnings that many package maintainers
>> appear to regard quite bothersome, namely when the compiler warns that import statement is 
>> superflous even when it is importing an entity that is being used by the module. The issue is
>> the way `-Wunused-imports` works, which will issue a warning for the following program.
>> 
>> ```haskell
>> import X -- exports some function @f@
>> import y -- exports the same function @f@
>> 
>> g = f
>> ```
>> 
>> The compiler will warn that the second Y import is redundant.
>> 
>> Many developers do not want to be warned about this. As far as they are concerned each import is
>> yielding enties that are in use and the fact that one of them can be removed is just not 
>> interesting.
>> 
>> In contrast, developers that use -Wall do want to be warned about the following.
>> 
>> ```haskell
>> import X -- exports some function @f@
>> import Z -- does not export f
>> 
>> g = f
>> ```
>> 
>> Here our module has a completely false dependency on Z which should be cleaned up at the point that
>> warnings are cleaned up.
>> 
>> This proposal proposes modifying -Wunused-imports so that it will warn about the second example but
>> stay silent for the first, and adds a -Wduplicate-imports that will issue a warning for the the
>> first example. In effect the enabling of both warnings in the new proposal will restore the current
>> -Wunused-imports behaviour. -Wall should *not* enable -Wduplicate-imports but is avilable to those
>> developers that rely on the current behaviour of -Wunused-imports (which some clearly do).
>> 
>> 
>> ## Recommendation
>> 
>> I recommend that we accept this proposal.
>> 
>> _______________________________________________
>> ghc-steering-committee mailing list
>> ghc-steering-committee at haskell.org
>> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
> 
> -- 
> Joachim Breitner
>  mail at joachim-breitner.de
>  http://www.joachim-breitner.de/
> 
> _______________________________________________
> ghc-steering-committee mailing list
> ghc-steering-committee at haskell.org
> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee