diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggester.java b/core/src/main/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggester.java index 4230cf94aed..404f4ec37c5 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggester.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggester.java @@ -25,6 +25,7 @@ import static com.google.errorprone.util.ASTHelpers.getReturnType; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.hasAnnotation; +import static com.google.errorprone.util.ASTHelpers.hasDirectAnnotationWithSimpleName; import static com.google.errorprone.util.ASTHelpers.isAbstract; import static com.google.errorprone.util.ASTHelpers.isSameType; import static com.google.errorprone.util.ASTHelpers.isSubtype; @@ -78,10 +79,9 @@ public final class CanIgnoreReturnValueSuggester extends BugChecker implements M private static final String AUTO_VALUE = "com.google.auto.value.AutoValue"; private static final String IMMUTABLE = "com.google.errorprone.annotations.Immutable"; - private static final String CIRV = "com.google.errorprone.annotations.CanIgnoreReturnValue"; + private static final String CIRV_SIMPLE_NAME = "CanIgnoreReturnValue"; private static final ImmutableSet EXEMPTING_METHOD_ANNOTATIONS = ImmutableSet.of( - CIRV, "com.google.errorprone.annotations.CheckReturnValue", "com.google.errorprone.refaster.annotation.AfterTemplate"); @@ -118,7 +118,12 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) { return Description.NO_MATCH; } - // If the method has an exempting annotation, then bail out. + // If the method is @CanIgnoreReturnValue (in any package), then bail out. + if (hasDirectAnnotationWithSimpleName(methodSymbol, CIRV_SIMPLE_NAME)) { + return Description.NO_MATCH; + } + + // If the method has another exempting annotation, then bail out. if (exemptingMethodAnnotations.stream() .anyMatch(annotation -> hasAnnotation(methodSymbol, annotation, state))) { return Description.NO_MATCH; @@ -187,7 +192,8 @@ private Description annotateWithCanIgnoreReturnValue(MethodTree methodTree, Visi } // now annotate it with @CanIgnoreReturnValue - fix.prefixWith(methodTree, "@" + qualifyType(state, fix, CIRV) + "\n"); + String cirv = "com.google.errorprone.annotations.CanIgnoreReturnValue"; + fix.prefixWith(methodTree, "@" + qualifyType(state, fix, cirv) + "\n"); return describeMatch(methodTree, fix.build()); } @@ -307,9 +313,8 @@ private boolean isIgnorableMethodCallOnSameInstance( || isIdentifier(receiver, "super")) { // If the method we're calling is @CIRV and the enclosing class could be represented by // the object being returned by the other method, then it's probable that the other - // method is likely to - // be an ignorable result. - return hasAnnotation(calledMethod, CIRV, state) + // method is likely to be an ignorable result. + return hasDirectAnnotationWithSimpleName(calledMethod, CIRV_SIMPLE_NAME) && isSubtype(enclosingClassType, methodReturnType, state) && isSubtype(enclosingClassType, getReturnType(mit), state); } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggesterTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggesterTest.java index 1c9a88c1b49..a5f1606036d 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggesterTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/checkreturnvalue/CanIgnoreReturnValueSuggesterTest.java @@ -418,6 +418,43 @@ public void simpleCaseAlreadyAnnotatedWithCirv() { .doTest(); } + @Test + public void alreadyAnnotatedWithProtobufCirv_b356526159() { + helper + .addInputLines( + "Client.java", + "package com.google.protobuf;", + "public final class Client {", + " private String name;", + " @CanIgnoreReturnValue", + " public Client setName(String name) {", + " this.name = name;", + " return this;", + " }", + "}") + .expectUnchanged() + .doTest(); + } + + @Test + public void alreadyAnnotatedWithArbitraryCirv_b356526159() { + helper + .addInputLines( + "Client.java", + "package com.google.frobber;", + "public final class Client {", + " @interface CanIgnoreReturnValue {}", + " private String name;", + " @CanIgnoreReturnValue", + " public Client setName(String name) {", + " this.name = name;", + " return this;", + " }", + "}") + .expectUnchanged() + .doTest(); + } + @Test public void simpleCaseAlreadyAnnotatedWithCrv() { helper