Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support @SuppressWarnings("all") #4076

Conversation

Stephan202
Copy link
Contributor

Checks that are suppressed using @SuppressWarnings("CheckName") are
now also suppressed by @SuppressWarnings("all"), as emitted by
libraries such as Lombok and Immutables.

Checks that are unsuppressible, or can only be suppressed by a custom
annotation are not suppressed by @SuppressWarnings("all").

While there: improve the deprecated BugChecker#isSuppressed(Tree) and
BugChecker#isSuppressed(Symbol) methods to respect
BugChecker#supportsSuppressWarnings().

Resolves #4065.

Checks that are suppressed using `@SuppressWarnings("CheckName")` are
now also suppressed by `@SuppressWarnings("all")`, as emitted by
libraries such as Lombok and Immutables.

Checks that are unsuppressible, or can only be suppressed by a custom
annotation are _not_ suppressed by `@SuppressWarnings("all")`.

While there: improve the deprecated `BugChecker#isSuppressed(Tree)` and
`BugChecker#isSuppressed(Symbol)` methods to respect
`BugChecker#supportsSuppressWarnings()`.
Copy link
Contributor Author

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some context. Naming is hard, so let me know if e.g. the BugChecker implementations should have a different name.

Comment on lines 272 to 279
private boolean isSuppressed(SuppressWarnings suppression) {
return suppression != null
&& !Collections.disjoint(Arrays.asList(suppression.value()), allNames());
if (suppression == null || !supportsSuppressWarnings()) {
return false;
}

List<String> suppressions = Arrays.asList(suppression.value());
return suppressions.contains("all") || !Collections.disjoint(suppressions, allNames());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is effectively deprecated, since it's only called by deprecated code; the first test below covers it.

Comment on lines +35 to +63
@Test
public void isSuppressed_withoutVisitorState() {
CompilationTestHelper.newInstance(LegacySuppressionCheck.class, getClass())
.addSourceLines(
"A.java",
"class A {",
" void m() {",
" // BUG: Diagnostic contains: []",
" int unsuppressed;",
" // BUG: Diagnostic contains: []",
" @SuppressWarnings(\"foo\") int unrelatedSuppression;",
" // BUG: Diagnostic contains: [Suppressible]",
" @SuppressWarnings(\"Suppressible\") int suppressed;",
" // BUG: Diagnostic contains: [Suppressible]",
" @SuppressWarnings(\"Alternative\") int suppressedWithAlternativeName;",
" // BUG: Diagnostic contains: [Suppressible]",
" @SuppressWarnings(\"all\") int allSuppressed;",
" // BUG: Diagnostic contains: [Suppressible]",
" @SuppressWarnings({\"foo\", \"Suppressible\"}) int alsoSuppressed;",
" // BUG: Diagnostic contains: [Suppressible]",
" @SuppressWarnings({\"all\", \"foo\"}) int redundantlySuppressed;",
" // BUG: Diagnostic contains: [Suppressible]",
" @SuppressWarnings({\"all\", \"OnlySuppressedInsideDeprecatedCode\"}) int ineffectiveSuppression;",
" // BUG: Diagnostic contains: []",
" @Deprecated int unuspportedSuppression;",
" }",
"}")
.doTest();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final test case here shows why the code under test is deprecated: the suppressionAnnotations = Deprecated.class directive isn't respected.

Copy link
Collaborator

@cushon cushon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

copybara-service bot pushed a commit that referenced this pull request Sep 14, 2023
Checks that are suppressed using `@SuppressWarnings("CheckName")` are
now also suppressed by `@SuppressWarnings("all")`, as emitted by
libraries such as Lombok and Immutables.

Checks that are unsuppressible, or can only be suppressed by a custom
annotation are _not_ suppressed by `@SuppressWarnings("all")`.

While there: improve the deprecated `BugChecker#isSuppressed(Tree)` and
`BugChecker#isSuppressed(Symbol)` methods to respect
`BugChecker#supportsSuppressWarnings()`.

Resolves #4065.

Fixes #4076

FUTURE_COPYBARA_INTEGRATE_REVIEW=#4076 from PicnicSupermarket:sschroevers/suppress-warnings-all 1b8f663
PiperOrigin-RevId: 565426144
@Stephan202 Stephan202 deleted the sschroevers/suppress-warnings-all branch September 14, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MissingOverride: ignore generated method
2 participants