[ghc-steering-committee] Split -Wunused-imports (recommend accept)
Chris Dornan
chris at chrisdornan.com
Wed Jun 28 15:20:04 UTC 2023
Joachim and everyone,
It looks like we are accepting this proposal -- I have changed the label to accepted on the repo. If anybody objects please speak up!
Chris
> On 28 Jun 2023, at 08:06, Adam Gundry <adam at well-typed.com> wrote:
>
> 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
>
> _______________________________________________
> ghc-steering-committee mailing list
> ghc-steering-committee at haskell.org
> https://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-steering-committee
More information about the ghc-steering-committee
mailing list