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

[Joachim Breitner]

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/