Skip to content

Commit

Permalink
Replace Type#equals with TypeSymbol comparisons.
Browse files Browse the repository at this point in the history
RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=224534420
  • Loading branch information
graememorgan authored and ronshapiro committed Dec 18, 2018
1 parent 1b86aa1 commit e1c9315
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 37 deletions.
Expand Up @@ -543,6 +543,7 @@ public static ClassTree findClass(ClassSymbol symbol, VisitorState state) {
@Nullable
public static MethodSymbol findSuperMethodInType(
MethodSymbol methodSymbol, Type superType, Types types) {
// TODO(ghm): Using a comparison of tsym here appears to be a behaviour change.
if (methodSymbol.isStatic() || superType.equals(methodSymbol.owner.type)) {
return null;
}
Expand Down
Expand Up @@ -82,9 +82,10 @@ public class BadComparable extends BugChecker implements TypeCastTreeMatcher {
* when they're not the same, in which case we prefer the type of the expression. This ensures
* that a byte/short subtracted from another byte/short isn't regarded as an int.
*/
private static Type getTypeOfSubtract(BinaryTree expression) {
private static Type getTypeOfSubtract(BinaryTree expression, VisitorState state) {
Type expressionType = ASTHelpers.getType(expression.getLeftOperand());
if (!expressionType.equals(ASTHelpers.getType(expression.getRightOperand()))) {
if (!ASTHelpers.isSameType(
expressionType, ASTHelpers.getType(expression.getRightOperand()), state)) {
return ASTHelpers.getType(expression);
}
return expressionType;
Expand All @@ -110,7 +111,7 @@ private boolean matches(TypeCastTree tree, VisitorState state) {

// Ensure the expression type is wider and signed (ie a long) than the cast type ignoring
// boxing.
Type expressionType = getTypeOfSubtract((BinaryTree) expression);
Type expressionType = getTypeOfSubtract((BinaryTree) expression, state);
TypeTag expressionTypeTag = state.getTypes().unboxedTypeOrType(expressionType).getTag();
return (expressionTypeTag == TypeTag.LONG);
}
Expand Down
Expand Up @@ -27,8 +27,6 @@

import com.google.auto.value.AutoValue;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
Expand All @@ -48,9 +46,10 @@
import com.sun.tools.javac.code.TypeTag;
import com.sun.tools.javac.code.Types;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Comparator;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import javax.annotation.Nullable;

/** @author avenet@google.com (Arnaud J. Venet) */
Expand Down Expand Up @@ -131,11 +130,10 @@ public Description matchMethodInvocation(

public static TypeCompatibilityReport compatibilityOfTypes(
Type receiverType, Type argumentType, VisitorState state) {
return compatibilityOfTypes(
receiverType, argumentType, new HashSet<>(), new HashSet<>(), state);
return compatibilityOfTypes(receiverType, argumentType, typeSet(state), typeSet(state), state);
}

public static TypeCompatibilityReport compatibilityOfTypes(
private static TypeCompatibilityReport compatibilityOfTypes(
Type receiverType,
Type argumentType,
Set<Type> previousReceiverTypes,
Expand Down Expand Up @@ -263,17 +261,20 @@ private static TypeCompatibilityReport matchesSubtypeAndIsGenericMismatch(
.filter(
tp ->
!(previousReceiverTypes.contains(tp.receiver)
|| tp.receiver.equals(receiverType)
|| ASTHelpers.isSameType(tp.receiver, receiverType, state)
|| previousArgumentTypes.contains(tp.argument)
|| tp.argument.equals(argumentType)))
|| ASTHelpers.isSameType(tp.argument, argumentType, state)))
.map(
types ->
compatibilityOfTypes(
types.receiver,
types.argument,
Sets.union(previousReceiverTypes, ImmutableSet.of(receiverType)),
Sets.union(previousArgumentTypes, ImmutableSet.of(argumentType)),
state))
types -> {
Set<Type> nextReceiverTypes = typeSet(state);
nextReceiverTypes.addAll(previousReceiverTypes);
nextReceiverTypes.add(receiverType);
Set<Type> nextArgumentTypes = typeSet(state);
nextArgumentTypes.addAll(previousArgumentTypes);
nextArgumentTypes.add(argumentType);
return compatibilityOfTypes(
types.receiver, types.argument, nextReceiverTypes, nextArgumentTypes, state);
})
.filter(tcr -> !tcr.compatible())
.findFirst()
.orElse(TypeCompatibilityReport.createCompatibleReport());
Expand Down Expand Up @@ -346,11 +347,11 @@ static TypeCompatibilityReport incompatible(Type lhs, Type rhs) {
}
}

public static class TypeStringPair {
private static class TypeStringPair {
private String receiverTypeString;
private String argumentTypeString;

public TypeStringPair(Type receiverType, Type argumentType) {
private TypeStringPair(Type receiverType, Type argumentType) {
receiverTypeString = Signatures.prettyType(receiverType);
argumentTypeString = Signatures.prettyType(argumentType);
if (argumentTypeString.equals(receiverTypeString)) {
Expand All @@ -359,12 +360,33 @@ public TypeStringPair(Type receiverType, Type argumentType) {
}
}

public String getReceiverTypeString() {
private String getReceiverTypeString() {
return receiverTypeString;
}

public String getArgumentTypeString() {
private String getArgumentTypeString() {
return argumentTypeString;
}
}

private static TreeSet<Type> typeSet(VisitorState state) {
return new TreeSet<>(
new Comparator<Type>() {
@Override
public int compare(Type t1, Type t2) {
return state.getTypes().isSameType(t1, t2) ? 0 : t1.toString().compareTo(t2.toString());
}

@Override
public int hashCode() {
return super.hashCode();
}

/** Indicates whether some other object is "equal to" this comparator. */
@Override
public boolean equals(Object obj) {
return obj == this;
}
});
}
}
Expand Up @@ -94,7 +94,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState

for (Symbol overriddenMethod : overriddenMethods) {
Type declaringClass = overriddenMethod.outermostClass().asType();
if (!declaringClass.equals(currentClass)) {
if (!ASTHelpers.isSameType(declaringClass, currentClass, state)) {
String customMessage =
MESSAGE_BASE
+ "must not be invoked directly "
Expand Down Expand Up @@ -194,8 +194,9 @@ private List<MethodSymbol> getOverriddenMethods(VisitorState state, MethodSymbol
// package-private visibility, but interface methods have implicit public visibility.
Type currType = state.getTypes().supertype(method.owner.type);
while (currType != null
&& !currType.equals(state.getSymtab().objectType)
&& !currType.equals(Type.noType)) {
&& currType.tsym != null
&& !currType.tsym.equals(state.getSymtab().objectType.tsym)
&& !ASTHelpers.isSameType(currType, Type.noType, state)) {
Symbol sym = currType.tsym.members().findFirst(method.name);
if (sym instanceof MethodSymbol) {
list.add((MethodSymbol) sym);
Expand Down
Expand Up @@ -16,11 +16,12 @@

package com.google.errorprone.bugpatterns;

import static com.google.common.collect.Iterables.getLast;
import static com.google.errorprone.BugPattern.Category.TRUTH;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.util.ASTHelpers.getSymbol;

import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
Expand All @@ -29,9 +30,8 @@
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.tree.JCTree.JCExpression;
import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
import java.util.List;

/**
Expand Down Expand Up @@ -64,16 +64,16 @@ public Description matchMethodInvocation(
if (methodInvocationTree.getArguments().size() % 2 == 0) {
return Description.NO_MATCH;
}
JCMethodInvocation methodInvocation = (JCMethodInvocation) methodInvocationTree;
List<JCExpression> arguments = methodInvocation.getArguments();
List<? extends ExpressionTree> arguments = methodInvocationTree.getArguments();

Type typeVargs = methodInvocation.varargsElement;
if (typeVargs == null) {
MethodSymbol methodSymbol = getSymbol(methodInvocationTree);
if (methodSymbol == null || !methodSymbol.isVarArgs()) {
return Description.NO_MATCH;
}
Type typeVarargsArr = state.arrayTypeForType(typeVargs);
Type lastArgType = ASTHelpers.getType(Iterables.getLast(arguments));
if (typeVarargsArr.equals(lastArgType)) {
Type varArgsArrayType = getLast(methodSymbol.params()).type;
Type lastArgType = ASTHelpers.getType(getLast(arguments));
if (arguments.size() == methodSymbol.params().size()
&& ASTHelpers.isSameType(varArgsArrayType, lastArgType, state)) {
return Description.NO_MATCH;
}
return describeMatch(methodInvocationTree);
Expand Down
Expand Up @@ -334,7 +334,7 @@ private Description checkSubtype(ClassTree tree, VisitorState state) {
*/
private Type immutableSupertype(Symbol sym, VisitorState state) {
for (Type superType : state.getTypes().closure(sym.type)) {
if (superType.equals(sym.type)) {
if (superType.tsym.equals(sym.type.tsym)) {
continue;
}
// Don't use getImmutableAnnotation here: subtypes of trusted types are
Expand Down
Expand Up @@ -243,7 +243,7 @@ private Matcher<ExpressionTree> expressionHasReceiverAndType(
},
(ExpressionTree t, VisitorState state) -> {
Type type = ASTHelpers.getReceiverType(t);
return state.getTypeFromString(expectedType).equals(type);
return ASTHelpers.isSameType(state.getTypeFromString(expectedType), type, state);
});
}

Expand Down

0 comments on commit e1c9315

Please sign in to comment.