Skip to content

Commit

Permalink
Flag @OtherNullnessAnnotation void as we already do @Nullable void.
Browse files Browse the repository at this point in the history
We got this wrong with `@CheckForNull` in one file in Guava (unknown commit).

PiperOrigin-RevId: 654797514
  • Loading branch information
cpovirk authored and Error Prone Team committed Jul 22, 2024
1 parent 94f5eb0 commit dd94f4a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.dataflow.nullnesspropagation.NullnessAnnotations.annotationsRelevantToNullness;
import static com.google.errorprone.matchers.Description.NO_MATCH;

import com.google.errorprone.BugPattern;
Expand All @@ -26,15 +27,14 @@
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import javax.lang.model.type.TypeKind;

/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
@BugPattern(
summary =
"void-returning methods should not be annotated with @Nullable,"
"void-returning methods should not be annotated with nullness annotations,"
+ " since they cannot return null",
severity = WARNING,
tags = StandardTags.STYLE)
Expand All @@ -46,11 +46,12 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
if (sym.getReturnType().getKind() != TypeKind.VOID) {
return NO_MATCH;
}
AnnotationTree annotation =
ASTHelpers.getAnnotationWithSimpleName(tree.getModifiers().getAnnotations(), "Nullable");
if (annotation == null) {
var relevantAnnos = annotationsRelevantToNullness(tree.getModifiers().getAnnotations());
if (relevantAnnos.isEmpty()) {
return NO_MATCH;
}
return describeMatch(annotation, SuggestedFix.delete(annotation));
var fix = SuggestedFix.builder();
relevantAnnos.forEach(fix::delete);
return describeMatch(relevantAnnos.get(0), fix.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,24 @@ public void positive() {
"import javax.annotation.Nullable;",
"class Test {",
" // BUG: Diagnostic contains:",
" // void-returning methods should not be annotated with @Nullable",
" @Nullable void f() {}",
"}")
.doTest();
}

@Test
public void positiveCheckForNull() {
compilationHelper
.addSourceLines(
"Test.java",
"import javax.annotation.CheckForNull;",
"class Test {",
" // BUG: Diagnostic contains:",
" @CheckForNull void f() {}",
"}")
.doTest();
}

@Test
public void negativeNotAnnotated() {
compilationHelper
Expand Down

0 comments on commit dd94f4a

Please sign in to comment.