Skip to content

Commit

Permalink
CanIgnoreReturnValueSuggester: Support additional exempting method an…
Browse files Browse the repository at this point in the history
…notations

While there, excempt `@AfterTemplate` methods with analysis.

Fixes #4009

COPYBARA_INTEGRATE_REVIEW=#4009 from PicnicSupermarket:rossendrijver/CIRV_Refaster fd25f02
PiperOrigin-RevId: 563515580
  • Loading branch information
rickie authored and Error Prone Team committed Sep 7, 2023
1 parent a046d80 commit 6ca3f0a
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import static com.google.errorprone.util.ASTHelpers.stripParentheses;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
Expand All @@ -59,6 +61,7 @@
import com.sun.tools.javac.code.Type;
import java.util.HashSet;
import java.util.Set;
import javax.inject.Inject;

/**
* Checker that recommends annotating a method with {@code @CanIgnoreReturnValue} if the method
Expand All @@ -74,21 +77,39 @@ 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 CRV = "com.google.errorprone.annotations.CheckReturnValue";
private static final String CIRV = "com.google.errorprone.annotations.CanIgnoreReturnValue";
private static final ImmutableSet<String> EXEMPTING_METHOD_ANNOTATIONS =
ImmutableSet.of(
CIRV,
"com.google.errorprone.annotations.CheckReturnValue",
"com.google.errorprone.refaster.annotation.AfterTemplate");

private static final Supplier<Type> PROTO_BUILDER =
VisitorState.memoize(s -> s.getTypeFromString("com.google.protobuf.MessageLite.Builder"));

private static final ImmutableSet<String> BANNED_METHOD_PREFIXES =
ImmutableSet.of("get", "is", "has", "new", "clone", "copy");

private final ImmutableSet<String> exemptingMethodAnnotations;

@Inject
public CanIgnoreReturnValueSuggester(ErrorProneFlags errorProneFlags) {
this.exemptingMethodAnnotations =
Sets.union(
EXEMPTING_METHOD_ANNOTATIONS,
errorProneFlags
.getSet("CanIgnoreReturnValue:ExemptingMethodAnnotations")
.orElse(ImmutableSet.of()))
.immutableCopy();
}

@Override
public Description matchMethod(MethodTree methodTree, VisitorState state) {
MethodSymbol methodSymbol = getSymbol(methodTree);

// if the method is already directly annotated w/ @CRV or @CIRV, bail out
if (hasAnnotation(methodSymbol, CRV, state) || hasAnnotation(methodSymbol, CIRV, state)) {
// If the method has an exempting annotation, then bail out.
if (exemptingMethodAnnotations.stream()
.anyMatch(annotation -> ASTHelpers.hasAnnotation(methodSymbol, annotation, state))) {
return Description.NO_MATCH;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,24 @@ public void constructor() {
.doTest();
}

@Test
public void refasterAfterTemplate() {
helper
.addInputLines(
"A.java",
"import com.google.errorprone.refaster.annotation.AfterTemplate;",
"class A {",
" static final class MethodLacksBeforeTemplateAnnotation {",
" @AfterTemplate",
" String after(String str) {",
" return str;",
" }",
" }",
"}")
.expectUnchanged()
.doTest();
}

@Test
public void sometimesThrows() {
helper
Expand Down Expand Up @@ -832,4 +850,26 @@ public void providesMethod_b267362954() {
.expectUnchanged()
.doTest();
}

@Test
public void exemptedByCustomAnnotation() {
helper
.addInputLines("Foo.java", "package example;", "@interface Foo {}")
.expectUnchanged()
.addInputLines(
"ExemptedByCustomAnnotation.java",
"package example;",
"public final class ExemptedByCustomAnnotation {",
" private String name;",
"",
" @Foo",
" public ExemptedByCustomAnnotation setName(String name) {",
" this.name = name;",
" return this;",
" }",
"}")
.expectUnchanged()
.setArgs("-XepOpt:CanIgnoreReturnValue:ExemptingMethodAnnotations=example.Foo")
.doTest();
}
}

0 comments on commit 6ca3f0a

Please sign in to comment.