Error-prone: Collecting ideas for a new set of configuration options

[nick-someone]

A number of recent github issues (#647, #599, #512, #488) are focused around customizing and/or configuring Error Prone's behavior.

Historically, Error Prone has offered a (very) limited amount of configuration:

• Individual check severity level could be changed between OFF, WARNING, and ERROR
• Warnings could be disabled in generated code (-XepDisableWarningsInGeneratedCode)

In the past year, we've added a few more configuration flags

• -XepAllDisabledChecksAsWarnings - Turns on all of the optional checks as warnings for those wanting to see everything
• -XepAllErrorsAsWarnings - Demote all error-level checks to warning-level (this allows you to see all of the error prone errors at once as opposed to build, fix, build, fix, etc.)
• -XepDisableAllChecks - Completely disable all of the checks, allowing you to then turn on individual checks one at a time (used in a chain like -XepDisableAllChecks -Xep:EqualsIncompatibleType:ERROR)
• -XepPatchLocation and XepPatchChecks - used to control the patching feature.

We have a new flag that's only available in the newest snapshots

• -XepOpt:[Namespace:]FlagName[=Value] - Pass any number of arbitrary flags to individual bug checkers or collections thereof (individual checkers can be customized

We've generally responded to requests for configuration changes by adding new flags, and dealing with the combinations of these flags has been a bit arbitrary.

We'd like to solicit feedback from the community to help us build a newer configuration model:

• What parts of Error Prone's behavior are you customizing now? Links to existing configurations on github are helpful.
• If you are able to successfully configure Error Prone's behavior to your liking, do you feel that the configuration flags make it straightforward to do so?
• If you are not able to successfully configure Error Prone's behavior to your liking, what portions of Error Prone would you want to configure?
• In addition, I'll comment on this issue with a few things we can configure now, and a few of the proposed changes made in other issues. Feel free to :+1: these features, propose new ones, etc.

[nick-someone]

Customizing the severity level of the builtin checks, applied globally to the compilation:
-Xep:CheckName:ERROR

[nick-someone]

Disabling warning-level checks in code marked with the @Generated annotation:
-XepDisableWarningsInGeneratedCode

[nick-someone]

Forgoing the default set of built-in checks, and instead supplying a user-defined list of checks to activate:
-XepDisableAllChecks -Xep:Check1:ERROR -Xep:Check2:ERROR

[nick-someone]

Automatically enabling every check built-in to Error Prone, even those we aren't comfortable enabling by default due to a high false positive rate, or that can crash on some forms of code.

-XepAllDisabledChecksAsWarnings

[nick-someone]

Demote all error-level checks to warning-level (this allows you to see all of the error prone errors at once as opposed to build, fix, build, fix, etc.)

-XepAllErrorsAsWarnings

[nick-someone]

Passing in configuration data to individual checks, customizing their behavior above and beyond the severity of those checks:

-XepOpt:[Namespace:]FlagName[=Value]

[nick-someone]

488 - Allow severity/enable/disablement of checks by some nominal "category" as opposed to just by name (all of the Guice-related checks, for example). @Stephan202

[nick-someone]

599/#511 - Tell error prone to stop all analysis on files matching a specific regular expression or other predicate. @kageiit, @vanniktech, @msridhar

[Stephan202]

486 - Allow checks with the "suggestion" severity to be raised to "warning".

[Stephan202]

(Separate comment, to allow voting on the one above.)

Thanks for opening this discussion! You asked for Github links; our code is closed source but I can share our config here. We've configured Error Prone inside a Maven profile which is enabled by default. Relevant section of the configuration:

<compilerArgs combine.children="append">
    <arg>-Werror</arg>
    <!-- We want to enable almost all error-prone
    bug pattern checkers, so we enable all and then
    selectively deactivate some. -->
    <arg>-XepAllDisabledChecksAsWarnings</arg>
    <!-- The JAXB-generated classes exhibit some
    error-prone bug patterns. Nothing we can do
    about that, so we simply tell error-prone not
    to warn about generated code. -->
    <arg>-XepDisableWarningsInGeneratedCode</arg>
    <!-- See https://github.com/google/error-prone/issues/491. -->
    <arg>-Xep:ArgumentParameterMismatch:OFF</arg>
    <!-- See https://github.com/google/error-prone/issues/490. -->
    <arg>-Xep:ArgumentParameterSwap:OFF</arg>
    <!-- Disabled for now, but TBD. -->
    <arg>-Xep:MethodCanBeStatic:OFF</arg>
    <!-- Disabled for now, but TBD. -->
    <arg>-Xep:PrivateConstructorForUtilityClass:OFF</arg>
    <!-- Deals with an Android-specific limitation
    not applicable to us. See also
    https://github.com/google/error-prone/issues/488. -->
    <arg>-Xep:StaticOrDefaultInterfaceMethod:OFF</arg>
    <!-- Interesting idea, but way too much work for now. -->
    <arg>-Xep:Var:OFF</arg>
    <!-- The following bug patterns by default have
    "SUGGESTION" severity. We raise them to the
    "WARNING" level. See also
    https://github.com/google/error-prone/issues/486. -->
    <arg>-Xep:ConstantField:WARN</arg>
    <arg>-Xep:EmptySetMultibindingContributions:WARN</arg>
    <arg>-Xep:MixedArrayDimensions:WARN</arg>
    <arg>-Xep:MultipleTopLevelClasses:WARN</arg>
    <arg>-Xep:MultiVariableDeclaration:WARN</arg>
    <arg>-Xep:PackageLocation:WARN</arg>
    <arg>-Xep:PrivateConstructorForNoninstantiableModuleTest:WARN</arg>
    <arg>-Xep:RemoveUnusedImports:WARN</arg>
    <arg>-Xep:ThrowsUncheckedException:WARN</arg>
    <arg>-Xep:UnnecessaryStaticImport:WARN</arg>
    <arg>-Xep:UseBinds:WARN</arg>
    <arg>-Xep:WildcardImport:WARN</arg>
</compilerArgs>

If I could have one wish granted on this topic, then it'd be for the suggestions in #486 to be implemented. That way I won't have to maintain the list of checks you see above ^. (Each time I upgrade Error Prone I run a script over its source code to collect the set of rules with a suggestion-level severity. The ones I like are promoted to WARN. That's how I noticed #472. I'd prefer to have all new checks enabled by default, then disable the ones that won't fly.)

NB: We also have another Maven profile using which we can apply Refaster rewrite rules to our code base. Relevant snippet:

<compilerArgs combine.children="append">
    <arg>-XepPatchChecks:${errorprone.patchchecks}</arg>
    <arg>-XepPatchLocation:IN_PLACE</arg>
</compilerArgs>

[eaftan]

667 - Enable all default warning level checks. (We disable them in Bazel since we inside Google we surface those at code review time) @davido

[davido]

(We disable them in Bazel since we inside Google we surface those at code review time)

;-)

Edwin from gerrit team was fixing sporadically some error prone warnings, and I failed to see any rules of why he was suddenly fixing warning foo, bar but only in file baz. It turned out, that some specific tool chain at Google exposes those warnings for the files included in review during import into Google's own VCS.

The good news is, you don't need to wait until #667 is fixed to see the whole truth, (by that I mean all default EP warnings). For meantime, there is a (way too intrusive) workaround for Bazel.

Example: With this Bazel patch applied I was able to generate all default EP warnings report for Gerrit Code Review in this issue.

Needless to say, it would be great to have a way to do it without patching Bazel.

[hkdennis2k]

Should there a way to read the flags from one configuration file rather than a long command line list?

There are command line maximum length limit on some platforms, yes, I am talking Windows.

[eaftan]

@hkdennis2k, yes, a configuration file is definitely in scope for this.

Please vote for this comment if you want to specify configuration via a file rather than command-line flags.

[epmjohnston]

572 - whitelisting for @DoNotMock

[m-titov]

Please just fix https://github.com/google/error-prone/issues/472 . It should be easy fix.

[reinkrul]

I would really like to be able tell ErrorProne to write its scanner results to a file like JUnit does. This would enable integration with build tooling like Bamboo.

[msridhar]

Not eager to add yet another request here, but... 😄

It'd be cool if beyond generated code, there were a way to disable all warning-level checks on all code, i.e., -XepDisableWarnings. For us, this would be useful since we often compile with -Werror, in order to turn certain built-in javac warnings into errors. But with -Werror, we are forced to individually disable warning-level Error Prone checks that we don't want to run. As we update EP versions, we lose the careful work that the team has done to decide and adjust default error levels. If -XepDisableWarnings were available we would definitely use it.

[vovkab]

Add a way to control warnings & errors for generated code, ideally to have it separate from main source code.

My use case is:
I would like to enforce missing override check, so I would set MissingOverride:ERROR, but I don't care about this check in generated code, because it is not important. Right now there is no way to make it work.

[vorburger]

https://github.com/google/error-prone/issues/808 DisableErrorsInGeneratedCode

[codylerum]

How about FailOnWarnings

[Stephan202]

@codylerum you can have the compiler fail on warnings by passing -Werror. This is a standard javac flag, and it also works with warnings emitted by Error Prone.

[codylerum]

@Stephan202 Thanks that did the trick.

To others wanting to enable this using maven just add <failOnWarning>true</failOnWarning> to the maven-compiler-plugin

[hakanai]

I would really like a way to treat a warning as an error if the number of occurrences of the warning exceeds a configured value.

This would allow gradually fixing warnings while people may be attempting to add new instances of the same warning. Some types of warning are so common throughout our codebase that if they were fixed in a single code review, the review would be so large that nobody would read it thoroughly.

[davido]

We ended up doing this in Gerrit Code Review:

$ cat .bazelrc 
build --java_toolchain //tools:error_prone_warnings_toolchain

And conducting this file here, with the list of warnings to be treated as errors:

https://github.com/GerritCodeReview/gerrit/blob/master/tools/BUILD#L30-L96