Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new MutableMethodReturnType error prone.
RELNOTES: Add new MutableMethodReturnType error prone. MOE_MIGRATED_REVID=162345145
- Loading branch information
Showing
7 changed files
with
673 additions
and
55 deletions.
There are no files selected for viewing
86 changes: 86 additions & 0 deletions
86
core/src/main/java/com/google/errorprone/bugpatterns/ImmutableCollections.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| /* | ||
| * Copyright 2017 Google Inc. All Rights Reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.errorprone.bugpatterns; | ||
|
|
||
| import com.google.common.collect.ImmutableBiMap; | ||
| import com.sun.tools.javac.code.Type; | ||
| import java.util.Optional; | ||
|
|
||
| /** Common utility functions for immutable collections. */ | ||
| final class ImmutableCollections { | ||
|
|
||
| private ImmutableCollections() {} | ||
|
|
||
| private static final ImmutableBiMap<String, String> MUTABLE_TO_IMMUTABLE_CLASS_NAME_MAP = | ||
| ImmutableBiMap.<String, String>builder() | ||
| .put( | ||
| com.google.common.collect.BiMap.class.getName(), | ||
| com.google.common.collect.ImmutableBiMap.class.getName()) | ||
| .put( | ||
| com.google.common.collect.ListMultimap.class.getName(), | ||
| com.google.common.collect.ImmutableListMultimap.class.getName()) | ||
| .put( | ||
| com.google.common.collect.Multimap.class.getName(), | ||
| com.google.common.collect.ImmutableMultimap.class.getName()) | ||
| .put( | ||
| com.google.common.collect.Multiset.class.getName(), | ||
| com.google.common.collect.ImmutableMultiset.class.getName()) | ||
| .put( | ||
| com.google.common.collect.RangeMap.class.getName(), | ||
| com.google.common.collect.ImmutableRangeMap.class.getName()) | ||
| .put( | ||
| com.google.common.collect.RangeSet.class.getName(), | ||
| com.google.common.collect.ImmutableRangeSet.class.getName()) | ||
| .put( | ||
| com.google.common.collect.SetMultimap.class.getName(), | ||
| com.google.common.collect.ImmutableSetMultimap.class.getName()) | ||
| .put( | ||
| com.google.common.collect.SortedMultiset.class.getName(), | ||
| com.google.common.collect.ImmutableSortedMultiset.class.getName()) | ||
| .put( | ||
| com.google.common.collect.Table.class.getName(), | ||
| com.google.common.collect.ImmutableTable.class.getName()) | ||
| .put( | ||
| java.util.Collection.class.getName(), | ||
| com.google.common.collect.ImmutableCollection.class.getName()) | ||
| .put( | ||
| java.util.List.class.getName(), | ||
| com.google.common.collect.ImmutableList.class.getName()) | ||
| .put( | ||
| java.util.Map.class.getName(), com.google.common.collect.ImmutableMap.class.getName()) | ||
| .put( | ||
| java.util.NavigableMap.class.getName(), | ||
| com.google.common.collect.ImmutableSortedMap.class.getName()) | ||
| .put( | ||
| java.util.NavigableSet.class.getName(), | ||
| com.google.common.collect.ImmutableSortedSet.class.getName()) | ||
| .put( | ||
| java.util.Set.class.getName(), com.google.common.collect.ImmutableSet.class.getName()) | ||
| .build(); | ||
|
|
||
| static boolean isImmutableType(Type type) { | ||
| return MUTABLE_TO_IMMUTABLE_CLASS_NAME_MAP.containsValue(getTypeQualifiedName(type)); | ||
| } | ||
|
|
||
| static Optional<String> mutableToImmutable(String fullyQualifiedClassName) { | ||
| return Optional.ofNullable(MUTABLE_TO_IMMUTABLE_CLASS_NAME_MAP.get(fullyQualifiedClassName)); | ||
| } | ||
|
|
||
| private static String getTypeQualifiedName(Type type) { | ||
| return type.tsym.getQualifiedName().toString(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
212 changes: 212 additions & 0 deletions
212
core/src/main/java/com/google/errorprone/bugpatterns/MutableMethodReturnType.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,212 @@ | ||
| /* | ||
| * Copyright 2017 Google Inc. All Rights Reserved. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.errorprone.bugpatterns; | ||
|
|
||
| import static com.google.common.base.Preconditions.checkState; | ||
| import static com.google.errorprone.BugPattern.Category.JDK; | ||
| import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; | ||
| import static com.google.errorprone.matchers.Matchers.anyOf; | ||
| import static com.google.errorprone.matchers.Matchers.hasAnnotation; | ||
|
|
||
| import com.google.common.collect.ImmutableList; | ||
| import com.google.common.collect.ImmutableSet; | ||
| import com.google.errorprone.BugPattern; | ||
| import com.google.errorprone.VisitorState; | ||
| import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; | ||
| import com.google.errorprone.fixes.SuggestedFix; | ||
| import com.google.errorprone.fixes.SuggestedFixes; | ||
| import com.google.errorprone.matchers.Description; | ||
| import com.google.errorprone.matchers.Matcher; | ||
| import com.google.errorprone.util.ASTHelpers; | ||
| import com.sun.source.tree.ClassTree; | ||
| import com.sun.source.tree.IdentifierTree; | ||
| import com.sun.source.tree.MethodTree; | ||
| import com.sun.source.tree.ParameterizedTypeTree; | ||
| import com.sun.source.tree.ReturnTree; | ||
| import com.sun.source.tree.Tree; | ||
| import com.sun.source.util.SimpleTreeVisitor; | ||
| import com.sun.source.util.TreeScanner; | ||
| import com.sun.tools.javac.code.Flags; | ||
| import com.sun.tools.javac.code.Symbol.MethodSymbol; | ||
| import com.sun.tools.javac.code.Type; | ||
| import com.sun.tools.javac.code.Type.ClassType; | ||
| import java.util.Optional; | ||
| import java.util.function.Predicate; | ||
| import javax.lang.model.element.Modifier; | ||
|
|
||
| /** @author dorir@google.com (Dori Reuveni) */ | ||
| @BugPattern( | ||
| name = "MutableMethodReturnType", | ||
| category = JDK, | ||
| summary = | ||
| "Method return type should use the immutable type (such as ImmutableList) instead of" | ||
| + " the general collection interface type (such as List)", | ||
| severity = WARNING | ||
| ) | ||
| public final class MutableMethodReturnType extends BugChecker implements MethodTreeMatcher { | ||
|
|
||
| private static final Matcher<MethodTree> ANNOTATED_WITH_PRODUCES_OR_PROVIDES = | ||
| anyOf(hasAnnotation("dagger.Provides"), hasAnnotation("dagger.producers.Produces")); | ||
|
|
||
| @Override | ||
| public Description matchMethod(MethodTree methodTree, VisitorState state) { | ||
| MethodSymbol methodSymbol = ASTHelpers.getSymbol(methodTree); | ||
|
|
||
| if (methodSymbol.isConstructor()) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| if (isMethodCanBeOverridden(methodSymbol, state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| if (ANNOTATED_WITH_PRODUCES_OR_PROVIDES.matches(methodTree, state)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| Type returnType = methodSymbol.getReturnType(); | ||
| if (ImmutableCollections.isImmutableType(returnType)) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| ImmutableSet<ClassType> returnStatementsTypes = getMethodReturnTypes(methodTree); | ||
| if (returnStatementsTypes.isEmpty()) { | ||
| return Description.NO_MATCH; | ||
| } | ||
| boolean alwaysReturnsImmutableType = | ||
| returnStatementsTypes.stream().allMatch(ImmutableCollections::isImmutableType); | ||
| if (!alwaysReturnsImmutableType) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| Optional<String> immutableReturnType = | ||
| ImmutableCollections.mutableToImmutable(getTypeQualifiedName(returnType)); | ||
| if (!immutableReturnType.isPresent()) { | ||
| immutableReturnType = | ||
| getCommonImmutableTypeForAllReturnStatementsTypes(returnStatementsTypes); | ||
| } | ||
| if (!immutableReturnType.isPresent()) { | ||
| return Description.NO_MATCH; | ||
| } | ||
|
|
||
| Type newReturnType = state.getTypeFromString(immutableReturnType.get()); | ||
| SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); | ||
| fixBuilder.replace( | ||
| getTypeTree(methodTree.getReturnType()), | ||
| SuggestedFixes.qualifyType(state, fixBuilder, newReturnType.asElement())); | ||
| SuggestedFix fix = fixBuilder.build(); | ||
|
|
||
| return describeMatch(methodTree.getReturnType(), fix); | ||
| } | ||
|
|
||
| private static boolean isMethodCanBeOverridden(MethodSymbol methodSymbol, VisitorState state) { | ||
| if (methodSymbol.isStatic() || methodSymbol.isPrivate() || isFinalMethod(methodSymbol)) { | ||
| return false; | ||
| } | ||
|
|
||
| ClassTree enclosingClassTree = ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class); | ||
| boolean isEnclosingClassFinal = isFinalClass(enclosingClassTree); | ||
| if (isEnclosingClassFinal) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private static boolean isFinalClass(ClassTree classTree) { | ||
| return classTree.getModifiers().getFlags().contains(Modifier.FINAL); | ||
| } | ||
|
|
||
| private static boolean isFinalMethod(MethodSymbol methodSymbol) { | ||
| return (methodSymbol.flags() & Flags.FINAL) == Flags.FINAL; | ||
| } | ||
|
|
||
| private static Optional<String> getCommonImmutableTypeForAllReturnStatementsTypes( | ||
| ImmutableSet<ClassType> returnStatementsTypes) { | ||
| checkState(!returnStatementsTypes.isEmpty()); | ||
|
|
||
| ClassType arbitraryClassType = returnStatementsTypes.asList().get(0); | ||
| ImmutableList<String> superTypes = getImmutableSuperTypesForClassType(arbitraryClassType); | ||
|
|
||
| return superTypes | ||
| .stream() | ||
| .filter(areAllReturnStatementsAssignable(returnStatementsTypes)) | ||
| .findFirst(); | ||
| } | ||
|
|
||
| private static Predicate<String> areAllReturnStatementsAssignable( | ||
| ImmutableSet<ClassType> returnStatementsTypes) { | ||
| return s -> | ||
| returnStatementsTypes | ||
| .stream() | ||
| .map(MutableMethodReturnType::getImmutableSuperTypesForClassType) | ||
| .allMatch(c -> c.contains(s)); | ||
| } | ||
|
|
||
| private static ImmutableList<String> getImmutableSuperTypesForClassType(ClassType classType) { | ||
| ImmutableList.Builder<String> immutableSuperTypes = ImmutableList.builder(); | ||
|
|
||
| ClassType superType = classType; | ||
| while (superType.supertype_field instanceof ClassType) { | ||
| if (ImmutableCollections.isImmutableType(superType)) { | ||
| immutableSuperTypes.add(getTypeQualifiedName(superType.asElement().type)); | ||
| } | ||
| superType = (ClassType) superType.supertype_field; | ||
| } | ||
|
|
||
| return immutableSuperTypes.build(); | ||
| } | ||
|
|
||
| private static String getTypeQualifiedName(Type type) { | ||
| return type.tsym.getQualifiedName().toString(); | ||
| } | ||
|
|
||
| private static ImmutableSet<ClassType> getMethodReturnTypes(MethodTree methodTree) { | ||
| ImmutableSet.Builder<ClassType> returnTypes = ImmutableSet.builder(); | ||
| methodTree.accept( | ||
| new TreeScanner<Void, Void>() { | ||
| @Override | ||
| public Void visitReturn(ReturnTree node, Void unused) { | ||
| Type type = ASTHelpers.getType(node.getExpression()); | ||
| if (type instanceof ClassType) { | ||
| returnTypes.add((ClassType) type); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| }, | ||
| null /* unused */); | ||
| return returnTypes.build(); | ||
| } | ||
|
|
||
| private static Tree getTypeTree(Tree tree) { | ||
| return tree.accept(GET_TYPE_TREE_VISITOR, null /* unused */); | ||
| } | ||
|
|
||
| private static final SimpleTreeVisitor<Tree, Void> GET_TYPE_TREE_VISITOR = | ||
| new SimpleTreeVisitor<Tree, Void>() { | ||
| @Override | ||
| public Tree visitIdentifier(IdentifierTree tree, Void unused) { | ||
| return tree; | ||
| } | ||
|
|
||
| @Override | ||
| public Tree visitParameterizedType(ParameterizedTypeTree tree, Void unused) { | ||
| return tree.getType(); | ||
| } | ||
| }; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.