Skip to content

Commit

Permalink
Update NullnessPropagationTransfer to recognize that if equals() retu…
Browse files Browse the repository at this point in the history
…rns true, argument in not null

equals() implementation should return false on null arg :
https://docs.oracle.com/javase/9/docs/api/java/lang/Object.html#equals-java.lang.Object-

RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=196042167
  • Loading branch information
sumitbhagwani authored and ronshapiro committed May 11, 2018
1 parent 9b37d54 commit 5244e0a
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
Expand Up @@ -17,6 +17,7 @@
package com.google.errorprone.dataflow.nullnesspropagation; package com.google.errorprone.dataflow.nullnesspropagation;


import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NONNULL; import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NONNULL;
import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULL; import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULL;
import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULLABLE; import static com.google.errorprone.dataflow.nullnesspropagation.Nullness.NULLABLE;
Expand Down Expand Up @@ -54,6 +55,7 @@
import com.sun.tools.javac.code.Symbol.MethodSymbol; import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symbol.TypeSymbol; import com.sun.tools.javac.code.Symbol.TypeSymbol;
import com.sun.tools.javac.code.Symbol.VarSymbol; import com.sun.tools.javac.code.Symbol.VarSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Types; import com.sun.tools.javac.code.Types;
import com.sun.tools.javac.processing.JavacProcessingEnvironment; import com.sun.tools.javac.processing.JavacProcessingEnvironment;
Expand Down Expand Up @@ -497,7 +499,13 @@ Nullness visitMethodInvocation(
ClassAndMethod callee = tryGetMethodSymbol(node.getTree(), Types.instance(context)); ClassAndMethod callee = tryGetMethodSymbol(node.getTree(), Types.instance(context));
setReceiverNonnull(bothUpdates, node.getTarget().getReceiver(), callee); setReceiverNonnull(bothUpdates, node.getTarget().getReceiver(), callee);
setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee); setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee);
setConditionalArgumentNullness(thenUpdates, elseUpdates, node.getArguments(), callee); setConditionalArgumentNullness(
thenUpdates,
elseUpdates,
node.getArguments(),
callee,
Types.instance(context),
Symtab.instance(context));
return returnValueNullness(callee); return returnValueNullness(callee);
} }


Expand Down Expand Up @@ -745,7 +753,9 @@ private static void setConditionalArgumentNullness(
LocalVariableUpdates thenUpdates, LocalVariableUpdates thenUpdates,
LocalVariableUpdates elseUpdates, LocalVariableUpdates elseUpdates,
List<Node> arguments, List<Node> arguments,
ClassAndMethod callee) { ClassAndMethod callee,
Types types,
Symtab symtab) {
MemberName calleeName = callee.name(); MemberName calleeName = callee.name();
for (LocalVariableNode var : for (LocalVariableNode var :
variablesAtIndexes(NULL_IMPLIES_TRUE_PARAMETERS.get(calleeName), arguments)) { variablesAtIndexes(NULL_IMPLIES_TRUE_PARAMETERS.get(calleeName), arguments)) {
Expand All @@ -761,9 +771,32 @@ private static void setConditionalArgumentNullness(
thenUpdates.set(var, NULL); thenUpdates.set(var, NULL);
elseUpdates.set(var, NONNULL); elseUpdates.set(var, NONNULL);
} }
if (isEqualsMethod(calleeName, arguments, types, symtab)) {
LocalVariableNode var = variablesAtIndexes(ImmutableSet.of(0), arguments).get(0);
thenUpdates.set(var, NONNULL);
}
}

private static boolean isEqualsMethod(
MemberName calleeName, List<Node> arguments, Types types, Symtab symtab) {
// we don't care about class name -- we're matching against Object.equals(Object)
// this implies that non-overriding methods are assumed to be null-guaranteeing.
// Also see http://errorprone.info/bugpattern/NonOverridingEquals
if (!calleeName.member.equals("equals") || arguments.size() != 1) {
return false;
}
if (!(getOnlyElement(arguments).getTree() instanceof JCIdent)) {
return false;
}
Symbol sym = ((JCIdent) getOnlyElement(arguments).getTree()).sym;
if (sym == null || sym.type == null) {
return false;
}
return types.isSameType(sym.type, symtab.objectType)
&& (!variablesAtIndexes(ImmutableSet.of(0), arguments).isEmpty());
} }


private static Iterable<LocalVariableNode> variablesAtIndexes( private static List<LocalVariableNode> variablesAtIndexes(
Set<Integer> indexes, List<Node> arguments) { Set<Integer> indexes, List<Node> arguments) {
List<LocalVariableNode> result = new ArrayList<>(); List<LocalVariableNode> result = new ArrayList<>();
for (Integer i : indexes) { for (Integer i : indexes) {
Expand Down
Expand Up @@ -111,6 +111,19 @@ public void testRequiredNonNullParameters() throws Exception {
} }
} }


@Test
public void testEqualsParameters() throws Exception {
int found = 0;
for (Method method : loadClass("java.lang.Object").getMethods()) {
if (!method.getName().equals("equals")) {
continue;
}
found++;
assertThat(invokeWithSingleNullArgument(method, 0)).isEqualTo(Boolean.FALSE);
}
assertWithMessage("equals()").that(found).isGreaterThan(0);
}

private static Object invokeWithSingleNullArgument(Method method, int nullParam) private static Object invokeWithSingleNullArgument(Method method, int nullParam)
throws IllegalAccessException, InvocationTargetException { throws IllegalAccessException, InvocationTargetException {
Class<?>[] params = method.getParameterTypes(); Class<?>[] params = method.getParameterTypes();
Expand Down

0 comments on commit 5244e0a

Please sign in to comment.