Are unused import warnings harmful?

May 18, 2016

GravatarBy Michael Snoyman

Which of the following snippets of code is better?

#if MIN_VERSION_base(4,8,0)
import Control.Applicative ((<*))
#else
import Control.Applicative ((<*), pure)
#endif

Versus:

import Control.Applicative ((<*), pure)

If you are working on a project that supports multiple GHC versions, enable extra warnings via -Wall, and actually like to get your code to compile without any warnings, you'll probably say that the former is better. I'm going to claim, however, that any sane human being knows intuitively that the latter is the better version of the code, for multiple reasons:

  • It doesn't require a language extension to be enabled
  • It's much shorter without losing any useful information to the reader
  • It's more robust to future changes: if you need to add an import, you don't have to remember to update two places

However, if you look through my code bases, and the code bases of many other open source Haskell authors, you'll find the former examples regularly. I'm beginning to come to the conclusion that we've been attacking this problem the wrong way, and what we should be doing is:

  • Turning on -Wall in our code
  • Either modify -Wall in GHC to not warn about unused imports, or explicitly disable unused import warnings via -fno-warn-unused-imports
  • As many of us already do, religiously use Travis CI to check multiple GHC versions to avoid accidental regressions
  • In our Travis builds, start turning on -Werror

Maintaining complex CPP in our imports is sometimes a necessary evil, such as when APIs change. But when we are simply doing it to work around changes in what Prelude or other modules export, it's an unnecessary evil. This is similar to the change to GHC a few years back which allowed hiding (isNotExported) to not generate a warning: it made it much easier to deal with the now-no-longer-present Prelude.catch function.

While it's true that removing unused imports is a nice thing to do to our codebases from time to time, their presence does not actually indicate any potential issues with our code. My concern with the presence of these warnings is that they will lead to one of two situations:

  • We simply accept that our libraries generate warnings when compiled, which ends up hiding actionable warnings via a terrible signal-to-noise ratio
  • In an effort to clean up all warnings, we end up creating hideous messes like those above, or breaking backwards compatibility with old versions of dependencies

I haven't actually started making these modifications to my libraries, as I'm not yet fully convinced that this is a good idea. There are also other points in this design space, like explicitly marking some imports as redundant, though that would require some deeper changes to GHC and wouldn't be usable until we drop support for all current GHC versions.

Comments

comments powered by Disqus

Archives