Skip to content

Commit

Permalink
Improve AutoValueFinalMethods check
Browse files Browse the repository at this point in the history
- Add more specific diagnostic instead of listing all 3 methods
- Flag at first matched method instead of class
- add sanity test for @memoized abstract method

RELNOTES: Improve AutoValueFinalMethods check by adding more specific diagnostic at first matched method.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=218687201
  • Loading branch information
sumitbhagwani authored and cushon committed Nov 2, 2018
1 parent 63ab3bd commit 3ed318d
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 3 deletions.
Expand Up @@ -22,6 +22,7 @@
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.not;

import com.google.common.base.Joiner;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.VisitorState;
Expand All @@ -35,6 +36,8 @@
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.Tree;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import javax.lang.model.element.Modifier;

Expand Down Expand Up @@ -72,20 +75,32 @@ public Description matchClass(ClassTree tree, VisitorState state) {
return NO_MATCH;
}
SuggestedFix.Builder fix = SuggestedFix.builder();
List<String> matchedMethods = new ArrayList<>();
MethodTree firstMatchedMethod = null;
for (Tree memberTree : tree.getMembers()) {
if (!(memberTree instanceof MethodTree)) {
continue;
}
if (METHOD_MATCHER.matches((MethodTree) memberTree, state)) {
MethodTree method = (MethodTree) memberTree;
if (METHOD_MATCHER.matches(method, state)) {
Optional<SuggestedFix> optionalSuggestedFix =
SuggestedFixes.addModifiers(memberTree, state, Modifier.FINAL);
SuggestedFixes.addModifiers(method, state, Modifier.FINAL);
if (optionalSuggestedFix.isPresent()) {
matchedMethods.add(method.getName().toString());
fix.merge(optionalSuggestedFix.get());
if (firstMatchedMethod == null) {
firstMatchedMethod = method;
}
}
}
}
if (!fix.isEmpty()) {
return describeMatch(tree, fix.build());
String message =
String.format(
"Make %s final in AutoValue classes, "
+ "so it is clear to readers that AutoValue is not overriding them",
Joiner.on(", ").join(matchedMethods));
return buildDescription(firstMatchedMethod).setMessage(message).addFix(fix.build()).build();
}
return NO_MATCH;
}
Expand Down
Expand Up @@ -123,4 +123,44 @@ public void testNegativeCases() {
"}")
.doTest();
}

@Test
public void testAbstractMemoizedNegativeCase() {
compilationHelper
.addSourceLines(
"out/Test.java",
"import com.google.auto.value.AutoValue;",
"import com.google.auto.value.extension.memoized.Memoized;",
"@AutoValue",
"abstract class Test {",
" static Test create() {",
" return null;",
" }",
" @Override",
" @Memoized",
" public abstract int hashCode(); ",
"}")
.doTest();
}

@Test
public void testDiagnosticString() {
compilationHelper
.addSourceLines(
"out/Test.java",
"import com.google.auto.value.AutoValue;",
"import com.google.auto.value.extension.memoized.Memoized;",
"@AutoValue",
"abstract class Test {",
" static Test create() {",
" return null;",
" }",
" @Override",
" // BUG: Diagnostic contains: Make equals final in AutoValue classes",
" public boolean equals(Object obj) {",
" return true;",
" }",
"}")
.doTest();
}
}

0 comments on commit 3ed318d

Please sign in to comment.