1 Split -Wunused-imports
¶
The current iteration of -Wunused-imports
fulfills two arguably
different purposes - warn on “unused imports”, and warn on
“duplicate imports”, the latter of which has proven to be a heavy
burden on the ecosystem.
We’re suggesting that we split off the “duplicate imports” part from
-Wunused-imports
into another warning, called
-Wduplicate-imports
, and crucially, do not include
-Wduplicate-imports
into -Wall
.
1.1 Motivation¶
-Wunused-imports
has long been a thorn in the side of people who
want both: * to not have unused imports (in the sense described in the
proposed change specification
section) * to compile warning-free
across different versions of their dependencies
The most often case where this comes up is with Prelude
and other
widely used modules in base
. In particular, when Prelude
gets a
new reexport, which previously only lived in another module from
base
, users of -Wall
suddenly get new warnings, causing not only
noise, but crucially breakage for users of -Wall -Werror
.
This effectively reduces the usability of the -Wunused-imports
warning, because it forces library authors (who in general are doing
this work for free) who want to enforce code quality (removing actual
unused imports and potentially even unused dependencies) to also have to
do extra work, with questionable benefits.
This not only a theoretic concern:
Additional work was required in the implementation of the export ``liftA2` proposal <https://github.com/haskell/core-libraries-committee/issues/50>`__:
effort in
ghc
itself to maintain warning-freenessadditional non-trivial sections in the migration guide
two additional non-trivial(requiring
CPP
or turning off the warning entirely) merge requests, toCabal-syntax
andcontainers
respectively
The implementation of the AMP proposal required very similar additional work to the
liftA2
case above, seethe concerns raised in https://www.yesodweb.com/blog/2016/05/are-unused-import-warnings-harmful
the migration guide for ghc 7.10 - https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/7.10#ghc-says-the-import-of-is-redundant
This is listed as a “bad part” of Haskell - https://www.snoyman.com/blog/2020/11/haskell-bad-parts-2/
in particular, the article mentions the re-export of
(<>)
fromPrelude
as requiring the exact sameCPP
workarounds as already mentioned
In the sized integer types proposal, a critical counterpoint is introducing new emissions of
-Wunused-imports
The export ``foldl’` proposal <https://github.com/haskell/core-libraries-committee/issues/167>`__ seems like it would encounter the same implementation issues as the export
liftA2
proposal, and similar concerns to the sized integer types proposal
1.2 Proposed Change Specification¶
An “unused import” looks like this:
import X (f)
import Z
...neither f, nor anything from Z is used, hence they are both unused imports...
A “duplicate import” looks like this:
import X -- X exports f
import Y (f)
...f is used here...
Here, one of the imports is currently marked as unused. Note that X
and Y
could be the same module just as well.
The proposal is for -Wunused-imports
to be changed so that only the
“unused import” case emits a warning.
We also propose to introduce another warning, -Wduplicate-imports
,
which warns on only the “duplicate import” case.
Finally, since the majority of uses of -Wunused-imports
seem to come
from -Wall
uses, we propose to not include the new
-Wduplicate-imports
in -Wall
, so that library authors can
benefit without having to do yet more work.
The wideness of applicability of -Wduplicate-imports
can also be
debated, hence it is unclear if it should be in -Wall
in general.
For example, it makes some sense for app writers, but not so much for
library authors.
Expressing the proposed change via the current ghc implementation(source):
Current:
-Wunused-imports
reportswarnUnusedModules: import M, where nothing is used from M
warnUnusedImports: import M(f), where f is unused, and M doesn’t fall under warnUnusedModules
warnDuplicateImports: import M + import M(f), even when f is used complain about duplicate import of f
-Wall
includes-Wunused-imports
Proposed:
-Wunused-imports
reportswarnUnusedModules: import M, where nothing is used from M
warnUnusedImports: import M(f), where f is unused, and M doesn’t fall under warnUnusedModules
-Wduplicate-imports
warnDuplicateImports: import M + import M(f), even when f is used complain about duplicate import of f
-Wall
includes-Wunused-imports
, but not-Wduplicate-imports
1.3 Examples¶
1.3.1 Example 1¶
import Foo
import Foo (x)
bla = x
Current: * with -Wunused-imports
- warn that the Foo
import is
unused
Proposed: * with -Wunused-imports
- nothing * with
-Wduplicate-imports
- warn that the Foo
import is duplicate
1.3.2 Example 2¶
import Foo (x)
import Bar (x)
bla = x
Current: * with -Wunused-imports
- warn that the Bar
import is
unused
Proposed: * with -Wunused-imports
- nothing * with
-Wduplicate-imports
- warn that the Bar
import is duplicate
1.3.3 Example 3¶
import Foo
import Bar
bla = x
Current: * with -Wunused-imports
- warn that the Bar
import is
unused
Proposed: * with -Wunused-imports
- nothing * with
-Wduplicate-imports
- warn that the Bar
import is duplicate
1.3.4 Example 4¶
import Foo
bla = ()
Current: * with -Wunused-imports
- warn that the Foo
import is
unused
Proposed: * with -Wunused-imports
- warn that the Foo
import is
unused * with -Wduplicate-imports
- nothing
1.3.5 Example 5¶
import Foo (x)
bla = ()
Current: * with -Wunused-imports
- warn that the Foo
import is
unused
Proposed: * with -Wunused-imports
- warn that the Foo
import is
unused * with -Wduplicate-imports
- nothing
1.4 Effect and Interactions¶
Unsure what to fill in here, it seems that the Proposed Change Specification covers the effects of this change.
1.5 Costs and Drawbacks¶
The main cost is changing the behaviour of a warning without notice, even if we explicitly warn users that it has changed.
Is this acceptable? From initial feedback given in the proposal discussion, it seems that it is.
1.6 Alternatives¶
1.6.1 Relaxed redundant imports¶
We could also instead implement the spec that’s suggested in the “relaxed redundant imports” proposal, however We feel that that’s an unnecessary complication for several reasons:
It will take more time to discuss and implement.
More importantly, it will be more confusing to understand when a warning triggers for end users.
The spec is simpler, hence easier to maintain.
It is not necessary, as splitting up the existing
-Wunused-imports
and not including-Wduplicate-imports
in-Wall
achieves the same goal.
They also seem to be mostly orthogonal to me - if someone wants to have
duplicate import warnings as per the “relaxed redundant imports” spec,
then we could have another proposal after this one, potentially amending
the new -Wduplicate-imports
warning instead.
1.7 Unresolved Questions¶
1.7.1 Niche -Weverything
breakage¶
Almost directly quoting Adam Gundry here:
An obscure backwards compatibility point: with this proposal, compiling
a module with duplicate imports will fail under
-Werror -Weverything -Wno-unused-imports
, whereas previously it
would have succeeded (since -Wno-unused-imports
previously
suppressed both).
We could avoid this by making -Wunused-imports
into a group that
includes both -Wreally-unused-imports
(what this proposal currently
calls -Wunused-imports
, included in -Wall
) and
-Wduplicate-imports
(in -Weverything
). This would also mean that
users who explicitly ask for -Wunused-imports
continue to get both.
Is this worth it? I’m not sure. Adding a group feels a bit fiddly for a comparatively rare edge case.
A quick GitHub search for -Werror
-Weverything
and
-Wno-unused-imports
in the same filed turned up ~70 results, with
almost all of them being in editor plugins or the ghc user guide.
This is not conclusive or exhaustive, and it relies on the search correctly finding things, but it might be a good indication that this is indeed a niche case.
1.8 Implementation Plan¶
One of the proposal authors will implement this.