search

commercialhaskell / stack

The Haskell Tool Stack
http://haskellstack.org
BSD 3-Clause "New" or "Revised" License
3.98k stars 845 forks source link

Cache warnings and remind about them even when modules are not recompiled #3340

Open bucklereed opened 7 years ago

bucklereed commented 7 years ago

Suppose you're hacking away happily on a library with two independent modules, A.hs and B.hs. Both modules are compiling and passing tests, but they're also spewing warnings that you want to fix before you commit. Here's what the stack build output might look like, once you fix the critical bugs:

$ stack build
Preprocessing library for example-0.1.0.0..
Building library for example-0.1.0.0..
[1 of 2] Compiling A ( src/A.hs, .stack-work/dist/i386-linux-nopie/Cabal-2.0.0.2/build/A.o )

/home/user/src/example/src/A.hs:2:1: warning: [-Wunused-imports (in -Wextra)]
    The import of ‘mempty’ from module ‘Data.Monoid’ is redundant
  |
2 | import Data.Monoid (Monoid, mempty)

[2 of 2] Compiling B ( src/B.hs, .stack-work/dist/i386-linux-nopie/Cabal-2.0.0.2/build/B.o )

/home/user/src/example/src/B.hs:2:1: warning: [-Wunused-imports (in -Wextra)]
    The import of ‘mempty’ from module ‘Data.Monoid’ is redundant
  |
2 | import Data.Monoid (Monoid, mempty)

Registering library for example-0.1.0.0..

That's pretty great: you've got a list of things to fix, handed to you by GHC, and it comes with file names and line/column numbers that emacs can parse, which you can then use to jump straight to the warning's nexus.

So, more hacking. To fix these devilish warnings, it's taken several iterations and you've lost track of how many there were and where -- once GHC stops complaining, you're good. Arbitrarily, you started with A.hs, and, since the modules are independent, you haven't touched B.hs at all. Now stack build tells you this:

$ stack build
Preprocessing library for example-0.1.0.0..
Building library for example-0.1.0.0..
[1 of 2] Compiling A ( src/A.hs, .stack-work/dist/i386-linux-nopie/Cabal-2.0.0.2/build/A.o )
Registering library for example-0.1.0.0..

Looks good, all green, let's commit. And... whoops. The warnings from B.hs haven't been fixed at all, and you're not going to notice until either someone else, possibly a build server, points it out to you, or you do something else to B.hs.

What are the options here?

  1. -fforce-recomp: slow if you do it on every build, and requires discipline to do before every commit (and it's still slow once you move away from trivial examples).

  2. -Werror: an awfully big stick. Most of the time, warnings aren't fundamental problems -- the hard part is getting the code to compile at all and the tests to pass. Liberal use of -Werror can get really, really annoying.

Or: 3. stack could be clever and keep track of warnings, and remind you about them when modules haven't been compiled.

A sketch of what that might look like:

$ stack build
Preprocessing library for example-0.1.0.0..
Building library for example-0.1.0.0..
[1 of 2] Compiling A ( src/A.hs, .stack-work/dist/i386-linux-nopie/Cabal-2.0.0.2/build/A.o )
Registering library for example-0.1.0.0..
Warnings: 1 warning in modules that were not recompiled. Re-run with --show-previous-warnings to display them.

With appropriate highlighting and colour, this would be pretty unmissable. stack build should exit with status 0 by default in this case, but it would be nice to have a --exit-failure-on-warnings flag, and a corresponding option in stack.yaml.

As the message suggests, --show-previous-warnings would do something like this:

$ stack build --show-previous-warnings
Preprocessing library for example-0.1.0.0..
Building library for example-0.1.0.0..
[1 of 2] Compiling A ( src/A.hs, .stack-work/dist/i386-linux-nopie/Cabal-2.0.0.2/build/A.o )
Registering library for example-0.1.0.0..
Warnings: 1 warning in modules that were not recompiled:

/home/user/src/example/src/B.hs:2:1: warning: [-Wunused-imports (in -Wextra)]
    The import of ‘mempty’ from module ‘Data.Monoid’ is redundant
  |
2 | import Data.Monoid (Monoid, mempty)

Editors running stack build would also use this flag so that they can offer jump-to-error/warning.

Implementation thoughts: GHC 8.2 offers -ddump-json that would be very handy, but Stack would need to be able to parse older GHCs' output anyway, so it's (sadly) not useful yet.

Stack.Build.Execute already does some warning-filtering. It doesn't look too hard to extract and extend it to capturing location information and the warning message itself. The filtering could happen in mungeBuildOutput, whose return type could turn into ConduitM Text (Either CompilerWarning Text) m ().

The one bit I'm not quite sure about is how to get GHC to explicitly tell us that it's not recompiling an unchanged module. I see that Stack does its own dirty check, but there's the awkward case of what to do when a warning-generating file is deleted and nothing else is touched, I think?

mgsloan commented 7 years ago

On one hand I'm wary of doing much fancy parsing of the output. On the other hand, this can probably be done in a way that works well for 99.9% of cases. So yeah, I think a PR for this would likely be accepted.