Improving the compilation error reporting of sbt

#1

Hi all!

My name is Martin Duhem, and I’m a software engineer working at the Scala Center since March 2017. These last months, I’ve been working on Scala Native, and I am starting to get involved (again) in sbt’s development. My next step will be focusing on usability improvements for sbt.

The first thing that I want to improve is the reporting of compilation errors inside sbt. Currently, I feel like the interesting bits of information coming from the errors are not sufficiently highlighted, and that a lot of time is wasted seeking those bits in the verbose error messages.

This is what error messages currently look like:

> compile
[info] Compiling 2 Scala sources to /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/target/scala-2.10/sbt-0.13/classes...
[warn] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/src/main/scala/sbt/errorssummary/ConciseReporter.scala:22: @deprecated now takes two arguments; see the scaladoc.
[warn]   @deprecated def oldStuff: Int = 2
[warn]    ^
[error] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/src/main/scala/sbt/errorssummary/ConciseReporter.scala:32: type mismatch;
[error]  found   : Unit
[error]  required: Int
[error]   override def bar(): Int = println("Hello")
[error]                                    ^
[error] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/src/main/scala/sbt/errorssummary/Plugin.scala:19: not found: value oops
[error]       oops
[error]       ^
[warn] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/src/main/scala/sbt/errorssummary/Plugin.scala:11: implicit conversion method any2Unit should be enabled
[warn] by making the implicit value scala.language.implicitConversions visible.
[warn] This can be achieved by adding the import clause 'import scala.language.implicitConversions'
[warn] or by setting the compiler option -language:implicitConversions.
[warn] See the Scala docs for value scala.language.implicitConversions for a discussion
[warn] why the feature should be explicitly enabled.
[warn]   implicit def any2Unit(any: Any): Unit = ()
[warn]                ^
[warn] two warnings found
[error] two errors found
[error] (compile:compileIncremental) Compilation failed
[error] Total time: 0 s, completed Jun 23, 2017 2:21:15 PM

I find that this is suboptimal for several reasons:

  • To me, the two most important things are the file name and the line number. They should be highlighted.
  • It’s hard to tell where a message starts and where it ends.
  • Errors are not grouped by file. That means that I may need to switch back and forth between files until I fixed all the errors.
  • I don’t need to see the absolute path, only enough to uniquely identify the file from the root of my project.
  • I consider the offset within the line to be of little use to the human reader (it’s not shown here, because this is Scala 2.10, but it is shown on 2.12)
  • Displaying one error requires easily 5-6 lines. This means a lot of scrolling to get information about all the errors.
  • The summary does not bring much to the table.
  • When doing refactoring, you get hundreds of lines of error messages, and no way to quickly see all the things that are wrong in a single file.

To address those points, I wrote an sbt plugin that replaces sbt’s default error reporter by something more colourful and concise. The plugin can be found on Github: https://github.com/Duhemm/sbt-errors-summary.

Given that people who saw it generally liked it, I’m proposing to use it as the default for sbt 1.0. This is how the same errors are displayed using this plugin:

> compile
[info] Compiling 2 Scala sources to /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/target/scala-2.10/sbt-0.13/classes...
[warn]  [0] /scala/sbt/errorssummary/ConciseReporter.scala:22: @deprecated now takes two arguments; see the scaladoc.
[warn]        @deprecated def oldStuff: Int = 2
[warn]         ^
[error] [1] /scala/sbt/errorssummary/ConciseReporter.scala:32: type mismatch;
[error]      found   : Unit
[error]      required: Int
[error]       override def bar(): Int = println("Hello")
[error]                                        ^
[error] [2] /scala/sbt/errorssummary/Plugin.scala:19: not found: value oops
[error]           oops
[error]           ^
[warn]  [3] /scala/sbt/errorssummary/Plugin.scala:11: implicit conversion method any2Unit should be enabled
[warn]      by making the implicit value scala.language.implicitConversions visible.
[warn]      This can be achieved by adding the import clause 'import scala.language.implicitConversions'
[warn]      or by setting the compiler option -language:implicitConversions.
[warn]      See the Scala docs for value scala.language.implicitConversions for a discussion
[warn]      why the feature should be explicitly enabled.
[warn]        implicit def any2Unit(any: Any): Unit = ()
[warn]                     ^
[error] /scala/sbt/errorssummary/Plugin.scala: 11 [3], 19 [2]
[error] /scala/sbt/errorssummary/ConciseReporter.scala: 22 [0], 32 [1]
[error] (compile:compileIncremental) Compilation failed
[error] Total time: 1 s, completed Jun 23, 2017 2:58:50 PM

This plugin addresses my pain points about sbt’s error reporting. It shows me a nice, colourful summary telling me immediately that a given file contains a warning or an error at a given line. If I need, I can quickly jump to the corresponding error message, thanks to the identifier in [square brackets]. I can completely ignore the top lines if I don’t need them. It saves a lot of screen space. I have short, readable paths.

Of course, this plugin is not optimal and would probably not make everyone happy, but I would like to hear your opinion about how errors should be reported in sbt, and how you would improve this plugin or the current situation. I would also like to improve the out of the box experience of sbt, which is why I would like to see this plugin, or anything else that would emerge from this discussion, become the default error reporter of sbt.

Thank you for your input!

11 Likes
#2

I don’t have anything deep to say, but I like the fact that errors are easier to visually parse, and I really like the summary at the end. +1 from me.

#3

if you change the format it may have an impact on the Emacs workflow in sbt-mode which has regular expressions to match the file and line number for jump to error support in compile-mode (if you added the column number too, that’d be great!). So please let us know if the format changes and help out with the update.

#4

In particular, please keep full paths if the envvar INSIDE_EMACS is defined.

#5

For me one of the biggest pains is when mistake in one line causes tens of errors. Then I need to scroll and scroll to find the first error in output. Once it is fixed all the others are gone as well.

I’m not sure if and how it can be fixed, just want to say that in at least 50% of cases I care only about the first error.

2 Likes
#6

There’s one thing that’s sort of annoying in the sbt error messages API. If given a Position, sbt presents that position on some fixed format. In Dotty, we want to do our own positional reporting, and separate error messages visually. To do this, we’re just not passing sbt a position.

It would be great if we could give sbt a position, but tell it not to render its location. Or, conversely, allow the API users to specify the format. This would greatly help out tools based on sbt, like scastie, in giving positional feedback while we’re free to customize the reporting.

1 Like
#7

Apart from making errors easier to parse, which would be great, an option to suppress errors complaining about Any or Nothing when there are other errors present would help me a lot, since often those are just follow up errors making it harder to find the real ones.

1 Like
#8

Second Felix. A choice to opt into a structured data format would be great for tool integration and outsourcing better presentation of error messages to the community.

3 Likes
#9

Agreed. I actually care less about the colors than I do the spacing – it’s much easier to tell where one error ends and the next begins because of the way the indentation works. I suspect that would produce a small but measureable improvement in debugging efficiency.

It does seem to be a worthwhile enhancement, so +1 from me, too…

#10

Hum … maybe a stupid question but … Shouldn’t we improve this directly in scalac rather than sbt? That way, other uses of scalac (e.g., though other build tools) will also benefit.

#11

I am very much in favor of this proposal. I’ve been using this plugin personally for more than two months and I really like it. I’ve noticed it’s increased my productivity.

With the new logging API that I’m working on for Zinc (progress is happening here), this will be possible. The only thing you need to do is to provide your own implementation of Problem in the bridge. We can go over it when it’s merged into the new Zinc 1.0 API.

The new logging API adds support for a FilteredReporter that allows Zinc users to do this :slight_smile: In sbt, you can modify the compiler reporter by overriding the compilerReporter key in Defaults.scala the same way sbt-errors-summary (@Duhemm’s plugin) does it. You can provide your way to do so from other build tools too.

Hum … maybe a stupid question but … Shouldn’t we improve this directly in scalac rather than sbt? That way, other uses of scalac (e.g., though other build tools) will also benefit.

Yes, I agree that error reporting could be greatly improved in upstream Scalac. But since these changes require lots of consensus and iterations, I think it’s a good idea to get them into sbt first, analyze users’ reaction, and based on that propose it to merge them upstream.

Also note that these changes only modify the way errors are syntactically displayed, but don’t change the error messages per se. Whomever gets to work in a new Scalac error reporting, should also improve the format every compiler message is presented to the user in a similar way as Rust (the pioneer) or Dotty do.

1 Like
#12

That rationale does not make any sense to me. A very large portion of scalac users use sbt. So as far as disruption is concerned, making the changes in sbt is basically as bad as directly in scalac. In terms of benefit, working directly in scalac brings improvements to everyone as opposed to most. IMO the cost/benefit ratio is in favor of scalac.

1 Like
#13

That rationale does not make any sense to me.

I believe that merging this into sbt will be way easier than merging it in scalac. In general, the review process in scalac is longer and requires more consensus. Does this make sense?

2 Likes
#14

I think it’s great that you’re working on improving the usability of Sbt! But to be honest, error messages are probably the last in most people’s list of Sbt pain points. :slight_smile:

I agree with @sjrd that scalac is the best place to make this change, though don’t underestimate the number of tools out there that parse error messages! Practically all IDEs do it.

Regarding the implementation itself:

  • I would start error numbering with 1, in the same way that source line numbers start at 1
  • the summary at the end is cryptic. I have no idea what the 4 numbers mean, so please add a word (error, warning, etc).

I’m not sure how color highlighting is implemented, but please make sure that it works well under CI (meaning, disabled).

Looking forward to more UX improvements!

1 Like
#15

Any effort to make compiler errors easier to read (and scan quickly) will be greatly appreciated. Watching sbt ~compile output is like 70% of my every day.

One improvement that could be made to the above would be highlighting the filename and line numbers inline. Here’s what you get with grep -n and I find it very helpful:

1 Like
#16

To me there are three most important things: absolute file name, line number and column number.

Unfortunately column number is missing in current sbt. I’ve tried to fix that in https://github.com/sbt/sbt/pull/2785, but there were concerns that it will break tooling of people so it was decided to include column information from sbt 1.0 onwards (https://github.com/sbt/zinc/pull/183).

I would prefer to see output which would look like this:

> compile
[info] Compiling 2 Scala sources to /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/target/scala-2.10/sbt-0.13/classes...
[warn]  [0] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/scala/sbt/errorssummary/ConciseReporter.scala:22:5:
[warn]        @deprecated now takes two arguments; see the scaladoc.
[warn]        @deprecated def oldStuff: Int = 2
[warn]         ^
[error] [1]  /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/scala/sbt/errorssummary/ConciseReporter.scala:32:15:
[error]      type mismatch;
[error]      found   : Unit
[error]      required: Int
[error]       override def bar(): Int = println("Hello")
[error]                                        ^
[error] [2] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/scala/sbt/errorssummary/Plugin.scala:19:7: 
[error]      not found: value oops
[error]           oops
[error]           ^
[warn]  [3] /Users/martin/Documents/Projects.nosync/Duhemm/sbt-errors-summary/scala/sbt/errorssummary/Plugin.scala:11:9:
[warn]      implicit conversion method any2Unit should be enabled
[warn]      by making the implicit value scala.language.implicitConversions visible.
[warn]      This can be achieved by adding the import clause 'import scala.language.implicitConversions'
[warn]      or by setting the compiler option -language:implicitConversions.
[warn]      See the Scala docs for value scala.language.implicitConversions for a discussion
[warn]      why the feature should be explicitly enabled.
[warn]        implicit def any2Unit(any: Any): Unit = ()
[warn]                     ^
[error] /scala/sbt/errorssummary/Plugin.scala: 11 [3], 19 [2]
[error] /scala/sbt/errorssummary/ConciseReporter.scala: 22 [0], 32 [1]
[error] (compile:compileIncremental) Compilation failed
[error] Total time: 1 s, completed Jun 23, 2017 2:58:50 PM

Notice that error message starts on new line just under exhaustive information about location of the error (column information are just guesses).

2 Likes
#17

My suggestion regarding the message’s wording: Use active voice wherever possible.

Generally, this makes error messages easier to understand and also shorter.

So instead of

This can be achieved by …

use

You can do this by …

or something equivalently active and short.

1 Like
#18

The way you are reporting the errors is based on:

  1. Where
  2. Why
  3. What

I would if possible have the following order:

  1. What
  2. Where
  3. Why

Just my preference. I want to know what the error is, then where it is, and finally what it is.

1 Like
#19

Hi @Duhemm,

I think this is an interesting area to work on improving, but I echo @sjrd’s thoughts: after iterating and experimenting externally, the upstream target is the Scala compiler, not the build tool nor the incremental compiler/compiler bridge.

For existing releases of Scala (and sbt or zinc) your plugin can continue to provide the functionality.

By the way, have you tried implementing it as a compiler plugin, rather than an sbt plugin?

#20

For what it’s worth, I disagree with @sjrd and @dwijnand.

Focusing on sbt means getting improvements (and iterating on them) into peoples’ hands faster. Something so user-facing like this should be easier to iterate on; this isn’t so in scalac.

So I still think that sbt is likely the right place for this.

2 Likes