Error Prone integration: default warnings activation

[davido]

There should be a way to activate all error prone default warnings. End users can manually enable warnings with -Xep:SomeCheck:WARNING per check, but this is way too intrusive, to mess around with settings on check base. Instead, we should benefit from error prone project decision on which warnings check should be enabled and not move that decision to the end users.

FWIW: In Buck Error Prone integration just works: [1] and produces the default warnings in build.log file: [2].

• [1] https://gerrit-review.googlesource.com/92919
• [2] http://paste.openstack.org/show/592272

[katre]

Since I see you've sent a patch to review, I'm assigning this to you. :)

[damienmg]

/cc @cushon

[davido]

/cc @hanwen

After my naive attempt to fix it in: [1] was rejected, any code pointers, how could i offer new CLI option, say --error_prone_warning_checks and propagate this value to the right place, to instantiate the verbose error prone checker instance, a là:

/**
   * Constructs an {@link ErrorPronePlugin} instance with the set of checks that are enabled as
   * errors and warnings in open-source Error Prone.
   */
  public ErrorPronePlugin() {
    boolean activateWarnings = CliUtils.getErrorProneWarningChecks();
    this.scannerSupplier = activateWarnings
        ? BuiltInCheckerSuppliers.defaultChecks();
        : BuiltInCheckerSuppliers.errorChecks(); 
  }

So my question in the code above, how can I get CliUtils.getErrorProneWarningChecks(); at this location?

• [1] https://bazel-review.googlesource.com/7891

[kevingessner]

Any updates on this issue? This was quite a surprising thing to find while switching my builds to bazel. I'm using a massive .bazelrc to enable all the default warnings, which works but is not ideal.

[davido]

The problem with the handcrafted .bazelrc approach, is that you would need to update it on every Error Prone release. That sucks, obviously. The workaround for now is apply my patch: [1].

• [1] https://bazel-review.googlesource.com/7891

[kevingessner]

Thanks for pointing out the patch. I'd rather maintain a large bazelrc (which I can check in to git) and risk falling slightly behind Error Prone, than worry about maintaining a fork of bazel. Hopeful that patch can make it into a release soon.

[cushon]

This seems like an Error Prone feature request, not a Bazel feature request. The Bazel Error Prone integration can already be configured using javacopts, so I'd prefer to keep using that approach instead of adding top-level Bazel configuration. It seems like the underlying problem here is that there's no easy way to enable all of the default warning severity checks?

The reason they aren't on by default in Bazel is that we haven't found build output to be a great place to surface warnings. You tend to get a lot of log spam for warnings in code that isn't 'interesting', e.g. google/error-prone#664. --output_filter helps but it isn't perfect. We integrate most Error Prone warnings into code review instead.

Anyway, this seems like a reasonable piece of configuration to provide. If you agree that an Error Prone flag would be sufficient, can you file a FR for that?

[davido]

Done: google/error-prone#667.

[davido]

I would rather keep this issue and the provided diff on bazel-review open as workaround for this missing feature until the new EP version with this feature included is released.

[hlopko]

Friendly ping, should we ping the error prone issue? David can you do that?

[cushon]

The Error Prone feature request is under consideration. I don't think there's anything that needs to be done on the Bazel side here.

[cgrushko]

@greggdonovan (or his team :) ) came up with a nice way to enable the checks for the main repository only (therefore not triggering on external repos).

bazelbuild/BUILD_file_generator@2503109

[greggdonovan]

All credit goes to @kevingessner :)

[kevingessner]

😃 That bit of prelude magic is a workaround for #3427, so if someone would like to fix that...