Add ErrorProne plugin

[lefou]

This adds a new ErrorProne plugin to the contrib section of Mill.

It provides the ErrorProneModule trait, which can be mixed into JavaModules. Derived modules like ScalaModule are also supported, as long as they support the javacOptions task to configure the Java compiler.

We use the Error Prone project as a compiler plugin. Although this is a simple javac compiler plugin, it's usage isn't trivial. Fetching the correct classpath and supplying options can be tricky, since they need to be provided in a special form. Also using with JVMs starting from version 16 requires special options handling due to the new more restrictive module classpath. This plugin is handling all of the known issues and make its use easy for the user.

Additional configuration options are supported via the errorProneOptions target.

Here is a usage example:

package build
import mill._, javalib._
import mill.contrib.errorprone._

import $ivy.`com.lihaoyi::mill-contrib-errorprone:`

object `package` extends RootModule with JavaModule with ErrorProneModule {
  def errorProneOptions = Seq("-XepAllErrorsAsWarnings")
}

Tasks:

• Implement ErrorProneModule and tests
• Support extra options
• Manually test ErrorProneModule in private projects
• Finish plugin readme
• Add example and include in documentation
• Document current quirks (Java 17 module classpath, test sub-module behavior)
• (Open follow-up issues)

Fix #3447

[lefou]

I discovered a downside of our current inner test sub-module configuration. Since the inner JavaTests trait automatically initializes it's compiler setup from its outer module (e.g. def javacOptions = T{ outer.javacOptions() }), compiler plugins like error-prone are automatically applied to all test sources. It's rather hard to later disable it in the actual modules, esp. if other traits contribute to the configuration too. But disabling it in tests is most likely what users want, since test typically use bad code practices for some reasons, e.g. ignoring return values, self-comparisions, static initializations.

Therefore, we should find a way to better handle it. We could make inheriting optional/configurable. Or provide a dedicated testJavacOptions to contain options to forward to tests.

I opened a discussion for it

• Rethinking settings propagation to inner test modules #3463

[lefou]

This is ready for review. I can't get the assertions in the example project right. @lihaoyi Any idea?

[lihaoyi]

I think this looks good.

One question on -XepAllErrorsAsWarnings: does it cause compilation to fail? From the name I would have expected it to log the warnings without failing the compile, but your example test indicates the compilation indeed fails

[lefou]

I think this looks good.

One question on -XepAllErrorsAsWarnings: does it cause compilation to fail?

No, it succeeds. It would fail without this options, so it's a nice demontration that options work as expected.

From the name I would have expected it to log the warnings without failing the compile, but your example test indicates the compilation indeed fails

I can't get the example to work. Have a closer look at the enclosing comment. Either I don't expect any output (which make the example usage missing the point) or I don't use that option, making the example less useful.

[lihaoyi]

got it. Lemme give a shot at fixing the example and see if I can get it working

[lihaoyi]

Pushed a fix for the example. Turns out /src/ShortSet.java should have been /src/example/ShortSet.java haha.

The error messages for example tests do suck though. I've lived with them for a while now, but really should do something to make them easier to read

[lefou]

Pushed a fix for the example. Turns out /src/ShortSet.java should have been /src/example/ShortSet.java haha.

Thanks. So simple yet hard to catch.

[lihaoyi]

@lefou I think this looks good to me. If you're happy with it, i can merge it and pay out the bounty using the same details I used before