Skip to content

Commit

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

While there, excempt `@AfterTemplate` methods with analysis.
  • Loading branch information
rickie committed Jun 7, 2023
1 parent 0a8926e commit eb0f56d
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import com.google.common.collect.ImmutableSet;
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 Down Expand Up @@ -76,19 +77,39 @@ public final class CanIgnoreReturnValueSuggester extends BugChecker implements M
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 String REFASTER_AFTER_TEMPLATE =
"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> additionalExemptingMethodAnnotations;

public CanIgnoreReturnValueSuggester(ErrorProneFlags errorProneFlags) {
this.additionalExemptingMethodAnnotations =
errorProneFlags
.getList("CanIgnoreReturnValue:ExemptingMethodAnnotations")
.map(ImmutableSet::copyOf)
.orElseGet(ImmutableSet::of);
}

@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 is already directly annotated w/ @CRV, @CIRV, or @AfterTemplate, bail out.
if (hasAnnotation(methodSymbol, CRV, state)
|| hasAnnotation(methodSymbol, CIRV, state)
|| hasAnnotation(methodSymbol, REFASTER_AFTER_TEMPLATE, state)) {
return Description.NO_MATCH;
}

// If the method is annotated with an annotation that is exempted, bail out.
if (additionalExemptingMethodAnnotations.stream()
.anyMatch(annotation -> ASTHelpers.hasAnnotation(methodTree, 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 eb0f56d

Please sign in to comment.