[ghc-steering-committee] Split -Wunused-imports (recommend accept)
Adam Gundry
adam at well-typed.com
Wed Jun 28 07:06:38 UTC 2023
I'm content to accept this proposal. Like Joachim, my sense is that
"relaxed redundant imports" would be worth exploring further; moreover
the proposal's definitions of "unused" and "duplicate" are essentially
specified only by GHC's implementation. But this proposal is a
beneficial change and we can always further refine the warning flags in
the future.
Cheers,
Adam
On 27/06/2023 13:39, Arnaud Spiwack wrote:
> The problem statement, as I understand it:
>
> I'm writing a library, with dependency L-1 from where I import X and
> Y(f). A new version L-2 is released, where X exports (f) as well.
> My CI is set to reject all warnings, and start complaining on L-2.
> Silencing the warning on L-2 requires some rather gratuitous contortion
> (import X hiding (f)) just to be able to compile with L-1! (even if we
> don't require L-1 to compile without warning).
>
> I'm not really fond of the idea of removing this type of duplicate
> import warning from -wall, to be honest. But I do agree that we
> shouldn't have to contort the code that way if the libraries are
> otherwise completely compatible. The fact that it's easy, by merely
> silencing a warning on imports to break compatibility with a previous
> version is not something to be proud of.
>
> Therefore *I'm in favour* of this proposal (albeit reluctantly).
>
> Oleg Grengrus's comment on the discussion thread makes me think that
> maybe there should be two different recommended sets of warnings: one
> for libraries (where compatibility with several versions of a dependency
> is desirable), and one for applications (where you are typically bound
> to a single version of your dependencies). Though it's probably too much
> overhead for a single warning difference between the two.
>
> On Fri, 16 Jun 2023 at 16:05, Chris Dornan <chris at chrisdornan.com
> <mailto:chris at chrisdornan.com>> wrote:
>
> 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 <mailto: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 <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
> <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.
--
Adam Gundry, Haskell Consultant
Well-Typed LLP, https://www.well-typed.com/
Registered in England & Wales, OC335890
27 Old Gloucester Street, London WC1N 3AX, England
More information about the ghc-steering-committee
mailing list