google / google/error-prone

`SuppressWarningsWithoutExplanation` Inconsistency Between Documentation and Implementation

Open
#5,218 5 comments 1 reaction 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

There is an inconsistency between the documentation and the actual implementation of the `SuppressWarningsWithoutExplanation` bug checker in Error Prone.

### Description

1. The main documentation at https://errorprone.info/bugpattern/SuppressWarningsWithoutExplanation states the following:

- In the "**Introduction / One line summary**" section: "Use of `@SuppressWarnings` should be accompanied by a comment describing why the warning is safe to ignore."
- This implies **ALL** `@SuppressWarnings`
- In the "**The problem**" section: "Suppressions for `unchecked` or `rawtypes` warnings should have an accompanying comment..."
- This implies this check is only applicable to `@SuppressWarnings` for `unchecked` and `rawtypes`

2. The actual code implementation at [SuppressWarningsWithoutExplanation.java](https://github.com/google/error-prone/blob/master/core/src/main/java/com/google/errorprone/bugpatterns/SuppressWarningsWithoutExplanation.java):
- Code comment states: "The Google style guide mandates this for _all_ suppressions; this is only matching on `{@code deprecation}` as a trial."
- Implementation only checks for "deprecation" suppressions as shown in:
```Java
private static final Matcher SUPPRESS_WARNINGS =
allOf(
isSameType(SuppressWarnings.class),
hasArgumentWithValue("value", stringLiteral("deprecation")));
```

### Expected Behavior

Based on the Google style guide referenced in the code (which mandates explanatory comments for all suppressions) and the general documentation, this check should apply to ALL `@SuppressWarnings` annotations, not just those with "deprecation" value.
- Note: I cannot find a reference to this rule in the current published Java style guide at https://google.github.io/styleguide/javaguide.html

### Suggested Fix
1. Update the implementation to check all `@SuppressWarnings` annotations
2. Or update the implementation to check only `@SuppressWarnings` for `unchecked` or `rawtypes`
3. Or update the documentation to clearly state that this check currently only applies to "deprecation" suppressions

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.