Skip to content

Commit

Permalink
MutableConstantField: replace declaration type regardless of the orig…
Browse files Browse the repository at this point in the history
…inal declaration type.

RELNOTES: Suggest a fix on *any* constant which is initialized with an immutable collection and doesn't use the immutable collection in its declaration type. So "static final Iterable<String> FOO = ImmutableList.of();" will become "static final ImmutableList<String> FOO = ImmutableList.of();".

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=151030307
  • Loading branch information
dorireuv authored and eaftan committed Mar 29, 2017
1 parent 8df4bc9 commit 4557663
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 36 deletions.
Expand Up @@ -19,7 +19,7 @@
import static com.google.errorprone.BugPattern.Category.JDK; import static com.google.errorprone.BugPattern.Category.JDK;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;


import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState; import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
Expand Down Expand Up @@ -48,54 +48,49 @@
) )
public final class MutableConstantField extends BugChecker implements VariableTreeMatcher { public final class MutableConstantField extends BugChecker implements VariableTreeMatcher {


private static final ImmutableMap<String, String> MUTABLE_TO_IMMUTABLE_CLASS_NAME_MAP = private static final ImmutableSet<String> IMMUTABLE_CLASS_NAMES =
ImmutableMap.<String, String>builder() ImmutableSet.of(
.put("com.google.common.collect.BiMap", "com.google.common.collect.ImmutableBiMap") "com.google.common.collect.ImmutableBiMap",
.put( "com.google.common.collect.ImmutableList",
"com.google.common.collect.ListMultimap", "com.google.common.collect.ImmutableListMultimap",
"com.google.common.collect.ImmutableListMultimap") "com.google.common.collect.ImmutableMap",
.put("com.google.common.collect.RangeMap", "com.google.common.collect.ImmutableRangeMap") "com.google.common.collect.ImmutableMultimap",
.put("com.google.common.collect.RangeSet", "com.google.common.collect.ImmutableRangeSet") "com.google.common.collect.ImmutableMultiset",
.put( "com.google.common.collect.ImmutableRangeMap",
"com.google.common.collect.SetMultimap", "com.google.common.collect.ImmutableRangeSet",
"com.google.common.collect.ImmutableSetMultimap") "com.google.common.collect.ImmutableSet",
.put( "com.google.common.collect.ImmutableSetMultimap",
"com.google.common.collect.SortedMultiset", "com.google.common.collect.ImmutableSortedMap",
"com.google.common.collect.ImmutableSortedMultiset") "com.google.common.collect.ImmutableSortedMultiset",
.put("com.google.common.collect.Table", "com.google.common.collect.ImmutableTable") "com.google.common.collect.ImmutableSortedSet",
.put("java.util.List", "com.google.common.collect.ImmutableList") "com.google.common.collect.ImmutableTable");
.put("java.util.Map", "com.google.common.collect.ImmutableMap")
.put("java.util.Multimap", "com.google.common.collect.ImmutableMultimap")
.put("java.util.Multiset", "com.google.common.collect.ImmutableMultiset")
.put("java.util.NavigableMap", "com.google.common.collect.ImmutableSortedMap")
.put("java.util.NavigableSet", "com.google.common.collect.ImmutableSortedSet")
.put("java.util.Set", "com.google.common.collect.ImmutableSet")
.build();


@Override @Override
public Description matchVariable(VariableTree tree, VisitorState state) { public Description matchVariable(VariableTree tree, VisitorState state) {
if (!isConstantField(ASTHelpers.getSymbol(tree))) { if (!isConstantField(ASTHelpers.getSymbol(tree))) {
return Description.NO_MATCH; return Description.NO_MATCH;
} }


Tree lhsTree = tree.getType(); Tree rhsTree = tree.getInitializer();
Symbol lhsSymbol = ASTHelpers.getSymbol(lhsTree); Type rhsType = ASTHelpers.getType(rhsTree);
if (lhsSymbol == null) { if (rhsType == null) {
return Description.NO_MATCH; return Description.NO_MATCH;
} }
String lhsTypeQualifiedName = lhsSymbol.getQualifiedName().toString(); String rhsTypeQualifiedName = rhsType.tsym.getQualifiedName().toString();
if (!MUTABLE_TO_IMMUTABLE_CLASS_NAME_MAP.containsKey(lhsTypeQualifiedName)) { if (!IMMUTABLE_CLASS_NAMES.contains(rhsTypeQualifiedName)) {
return Description.NO_MATCH; return Description.NO_MATCH;
} }


String immutableClassName = MUTABLE_TO_IMMUTABLE_CLASS_NAME_MAP.get(lhsTypeQualifiedName); Tree lhsTree = tree.getType();
Type immutableType = state.getTypeFromString(immutableClassName); Type lhsType = ASTHelpers.getType(lhsTree);
Tree rhsTree = tree.getInitializer(); if (lhsType == null) {
Type rhsType = ASTHelpers.getType(rhsTree); return Description.NO_MATCH;
if (!ASTHelpers.isSameType(rhsType, immutableType, state)) { }
if (ASTHelpers.isSameType(lhsType, rhsType, state)) {
return Description.NO_MATCH; return Description.NO_MATCH;
} }


Type immutableType = state.getTypeFromString(rhsTypeQualifiedName);
SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
fixBuilder.replace( fixBuilder.replace(
getTypeTree(lhsTree), getTypeTree(lhsTree),
Expand Down
Expand Up @@ -27,6 +27,20 @@ public class MutableConstantFieldTest {
private final CompilationTestHelper testHelper = private final CompilationTestHelper testHelper =
CompilationTestHelper.newInstance(MutableConstantField.class, getClass()); CompilationTestHelper.newInstance(MutableConstantField.class, getClass());


@Test
public void staticFinalIterableInitializedInDeclarationWithImmutableSetOf_suggestsFix() {
testHelper
.addSourceLines(
"Test.java",
"import com.google.common.collect.ImmutableSet;",
"class Test {",
" // BUG: Diagnostic contains: static final ImmutableSet<String> COLORS =",
" static final Iterable<String> COLORS =",
" ImmutableSet.of(\"Red\", \"Green\", \"Blue\");",
"}")
.doTest();
}

@Test @Test
public void staticFinalSetInitializedInDeclarationWithImmutableSetOf_suggestsFix() { public void staticFinalSetInitializedInDeclarationWithImmutableSetOf_suggestsFix() {
testHelper testHelper
Expand Down Expand Up @@ -103,6 +117,19 @@ public void staticFinalMapInitializedInDeclarationWithImmutableMapOf_suggestsFix
.doTest(); .doTest();
} }


@Test
public void staticFinalImmutableSetInitializedInDeclarationWithImmutableSet_doesNotSuggestFix() {
testHelper
.addSourceLines(
"Test.java",
"import com.google.common.collect.ImmutableSet;",
"class Test {",
" static final ImmutableSet<String> COLORS =",
" ImmutableSet.of(\"Red\", \"Green\", \"Blue\");",
"}")
.doTest();
}

@Test @Test
public void staticFinalSetInitializedInStaticBlock_doesNotSuggestFix() { public void staticFinalSetInitializedInStaticBlock_doesNotSuggestFix() {
testHelper testHelper
Expand Down Expand Up @@ -163,9 +190,10 @@ public void mutable_doesNotSuggestFix() {
testHelper testHelper
.addSourceLines( .addSourceLines(
"Test.java", "Test.java",
"import com.google.common.collect.ImmutableSet;", "import java.util.ArrayList;",
"import java.util.List;",
"class Test {", "class Test {",
" static final ImmutableSet.Builder<String> MUTABLE = ImmutableSet.builder();", " static final List<String> MUTABLE = new ArrayList<>();",
"}") "}")
.doTest(); .doTest();
} }
Expand Down

0 comments on commit 4557663

Please sign in to comment.