Error Prone gives us a powerful tool to disallow certain patterns from entering our Java code. We must be careful how we use it so that we benefit users without creating busywork for them.
An Error Prone bug pattern (error) should have the following properties:
The bottom line is that, when a user sees one of our errors, she should think, “I’m glad Error Prone caught that for me.”
The PreconditionsCheckNotNull check flags uses of the Preconditions.checkNotNull method with a literal as the first argument. Since the first argument is the variable to be checked for non-nullity, passing a literal makes this a no-op. The problem is clear (“You switched the order of the arguments”), it is never what the programmer intended, it could cause a real problem (a NullPointerException in a later part of the program), and it occurs with reasonable frequency.
The ArrayEquals check flags comparisons between two arrays that use the Object.equals method. The intent in these cases is to compare the content of the arrays, but Object.equals compares for reference equality. The problem is clear once explained, in none of the cases did the programmer actually intend to compare for reference equality, it is a correctness problem since the arrays are not actually being compared, and it occurs occasionally.
Consider a check that flags uses of “new Integer(i)” and suggests using “Integer.valueOf(i)” instead. This improves performance and is correct for most cases. However, this bug is not a correctness problem, and in most cases the performance impact is unimportant. It also occurs frequently enough that an error message is likely to annoy users.
Consider a check that detects that an @Override annotation is used on all methods that override another method. This is part of our style guide but is not automatically enforced. However, this is not really a correctness problem, and my intuition is that it happens with pretty high frequency. Making this an error would be nitpicking.
NOTE: These criteria are still under discussion. Feel free to email error-prone-discuss@googlegroups.com if you have strong feelings on the topic.
Warnings can be style guide violations, potential serious bugs, or performance gotchas. There are several reasons to have a warning instead of a compiler error.
A warning should have the following properties:
The warning should be easy to understand and the fix should be clear. The problem should be obvious and actionable when pointed out.
The warning should have very few false positives. Developers should feel that we are pointing out an actual issue at least 90% of the time.
The warning should be for something that has the potential for significant impact. We want the warnings to be important enough so that when developers see them they take them seriously and often choose to fix them.
The warning should occur with a small but noticeable frequency. There is no point in detecting warnings that never actually occur, but if a warning occurs too frequently, it’s likely that it’s not causing any real problems. We don’t want to overwhelm people with too many warnings.
The bottom line is that, when a user sees one of our warnings, she should think, “I’m glad the compiler pointed that out.”
Error Prone checks run during compilation and rely primarily on fast, local AST and type analysis. As a result, checks are intentionally neither 100% sound nor 100% complete: they do not aim to detect every semantically equivalent form of a bug, nor do they guarantee a zero percent false positive rate on arbitrary syntax.
When triaging reports of false positives or false negatives in existing checks, we weigh:
Real-world likelihood. Does the code pattern occur in real, human-written (or standard tool-generated) Java code? For example, patterns that rely on redundant parentheses, immediately-invoked lambdas or anonymous classes, or indirection through constants where only a literal makes sense in practice are generally out of scope.
Check severity and purpose. False positives in Severity.ERROR checks on
idiomatic code are high priority because they break builds. Conversely,
false negatives in Severity.WARNING or Severity.SUGGESTION checks are
lower priority, as those checks are best-effort aids rather than strict
guarantees.
Complexity and performance. A check that handles 95% of real-world occurrences with straightforward AST matching is often preferable to a much more complex check that handles rare edge cases at the cost of maintainability, compile time, or new false positives.
We welcome bug reports grounded in real-world code. However, we receive a growing volume of automated or AI-assisted issue reports generated by mutating test cases into semantically equivalent but unidiomatic variants. Reports of false negatives on synthetic code patterns that are unlikely to occur in real code, especially for warning or suggestion checks, are unlikely to be prioritized and will generally be closed.