From 9c8bd33b792c15872bed24c5f0f374c88d7c45dd Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Sun, 2 Dec 2018 08:11:41 -0800 Subject: [PATCH 1/4] WIP --- .../java/com/uber/nullaway/LibraryModels.java | 122 +++--- .../main/java/com/uber/nullaway/NullAway.java | 2 +- .../AccessPathNullnessPropagation.java | 6 +- .../handlers/ApacheThriftIsSetHandler.java | 2 + .../nullaway/handlers/BaseNoOpHandler.java | 6 +- .../nullaway/handlers/CompositeHandler.java | 10 +- .../nullaway/handlers/ContractHandler.java | 2 + .../com/uber/nullaway/handlers/Handler.java | 6 +- .../handlers/InferredJARModelsHandler.java | 2 + .../handlers/LibraryModelsHandler.java | 392 +++++++++++++----- .../RestrictiveAnnotationHandler.java | 6 +- 11 files changed, 374 insertions(+), 182 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index ecbdf97423..7bc95c80d4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -22,13 +22,11 @@ package com.uber.nullaway; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableCollection; +import static com.sun.tools.javac.code.TypeTag.FORALL; + import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; -import com.google.errorprone.util.ASTHelpers; import com.sun.tools.javac.code.Symbol; -import com.sun.tools.javac.code.Types; import java.util.Objects; import javax.annotation.Nullable; @@ -94,92 +92,80 @@ public interface LibraryModels { */ final class MethodRef { - public final String clazz; - public final String methodSig; - - private MethodRef(String clazz, String methodSig) { - this.clazz = clazz; - this.methodSig = methodSig; + public final String enclosingClass; + public final String methodName; + public final String methodArgs; + @Nullable public final String genericArgs; + + private MethodRef( + String enclosingClass, String methodName, String methodArgs, @Nullable String genericArgs) { + this.enclosingClass = enclosingClass; + this.methodName = methodName; + this.methodArgs = methodArgs; + this.genericArgs = genericArgs; } /** - * @param clazz containing class - * @param methodSig method signature in the appropriate format (see class docs) + * @param enclosingClass containing class + * @param methodArgs method signature in the appropriate format (see class docs) * @return corresponding {@link MethodRef} */ - public static MethodRef methodRef(Class clazz, String methodSig) { - return methodRef(clazz.getName(), methodSig); + public static MethodRef methodRef(String enclosingClass, String methodName, String methodArgs) { + return new MethodRef(enclosingClass, methodName, methodArgs, null); } - /** - * @param clazz containing class - * @param methodSig method signature in the appropriate format (see class docs) - * @return corresponding {@link MethodRef} - */ - public static MethodRef methodRef(String clazz, String methodSig) { - Preconditions.checkArgument( - isValidMethodSig(methodSig), methodSig + " is not a valid method signature"); - return new MethodRef(clazz, methodSig); - } - - private static boolean isValidMethodSig(String methodSig) { - // some basic checking to make sure it's not just a method name - return methodSig.contains("(") && methodSig.contains(")"); + public static MethodRef methodRef( + String enclosingClass, String methodName, String methodArgs, @Nullable String genericArgs) { + return new MethodRef(enclosingClass, methodName, methodArgs, genericArgs); } public static MethodRef fromSymbol(Symbol.MethodSymbol symbol) { - return methodRef(symbol.owner.getQualifiedName().toString(), symbol.toString()); + String methodStr = symbol.toString(); + int openParenInd = methodStr.indexOf('('); + String genericArgs = null; + if (symbol.type != null) { + if (symbol.type.hasTag(FORALL)) genericArgs = "<" + symbol.type.getTypeArguments() + ">"; + } + + return methodRef( + symbol.owner.getQualifiedName().toString(), + symbol.name.toString(), + methodStr.substring(openParenInd), + genericArgs); } @Override - public boolean equals(Object obj) { - if (obj instanceof MethodRef) { - MethodRef other = (MethodRef) obj; - return clazz.equals(other.clazz) && methodSig.equals(other.methodSig); + public boolean equals(Object o) { + if (this == o) { + return true; } - return false; + if (o == null || getClass() != o.getClass()) { + return false; + } + MethodRef methodRef = (MethodRef) o; + return Objects.equals(enclosingClass, methodRef.enclosingClass) + && Objects.equals(methodName, methodRef.methodName) + && Objects.equals(methodArgs, methodRef.methodArgs); } @Override public int hashCode() { - return Objects.hash(clazz, methodSig); + return Objects.hash(enclosingClass, methodName, methodArgs); } @Override public String toString() { - return "MethodRef{" + "clazz='" + clazz + '\'' + ", methodSig='" + methodSig + '\'' + '}'; - } - } - - /** utility methods for dealing with library models */ - final class LibraryModelUtil { - - private LibraryModelUtil() {} - - public static boolean hasNullableReturn( - LibraryModels models, Symbol.MethodSymbol symbol, Types types) { - // need to check if symbol is in the model or if it overrides a method in the model - return methodInSet(symbol, types, models.nullableReturns()) != null; - } - - public static boolean hasNonNullReturn( - LibraryModels models, Symbol.MethodSymbol symbol, Types types) { - // need to check if symbol is in the model or if it overrides a method in the model - return methodInSet(symbol, types, models.nonNullReturns()) != null; - } - - @Nullable - private static Symbol.MethodSymbol methodInSet( - Symbol.MethodSymbol symbol, Types types, ImmutableCollection memberNames) { - if (memberNames.contains(MethodRef.fromSymbol(symbol))) { - return symbol; - } - for (Symbol.MethodSymbol superSymbol : ASTHelpers.findSuperMethods(symbol, types)) { - if (memberNames.contains(MethodRef.fromSymbol(superSymbol))) { - return superSymbol; - } - } - return null; + return "MethodRef{" + + "enclosingClass='" + + enclosingClass + + '\'' + + ", methodName='" + + methodName + + '\'' + + ", methodArgs='" + + methodArgs + + '\'' + + '}'; } } } diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index e988c063fe..71e25c4661 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -536,7 +536,7 @@ private Description checkParamOverriding( if (NullabilityUtil.isUnannotated(overriddenMethod, config)) { nullableParamsOfOverriden = handler.onUnannotatedInvocationGetExplicitlyNullablePositions( - overriddenMethod, ImmutableSet.of()); + state.context, overriddenMethod, ImmutableSet.of()); } else { ImmutableSet.Builder builder = ImmutableSet.builder(); for (int i = startParam; i < superParamSymbols.size(); i++) { diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java index 200cb4a2d0..77b24ea2aa 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -208,7 +208,7 @@ private NullnessStore lambdaInitialStore( NullnessStore environmentMapping = Objects.requireNonNull( environmentNullness.getEnvironmentMapping(underlyingAST.getLambdaTree()), - "no environment stored for lambda " + underlyingAST.getLambdaTree()); + "no environment stored for lambda"); NullnessStore.Builder result = environmentMapping.toBuilder(); LambdaExpressionTree code = underlyingAST.getLambdaTree(); // need to check annotation for i'th parameter of functional interface declaration @@ -217,7 +217,7 @@ private NullnessStore lambdaInitialStore( fiMethodSymbol.getParameters(); ImmutableSet nullableParamsFromHandler = handler.onUnannotatedInvocationGetExplicitlyNullablePositions( - fiMethodSymbol, ImmutableSet.of()); + context, fiMethodSymbol, ImmutableSet.of()); for (int i = 0; i < parameters.size(); i++) { LocalVariableNode param = parameters.get(i); @@ -858,7 +858,7 @@ public TransferResult visitMethodInvocation( node, callee, node.getArguments(), types, values(input), thenUpdates, bothUpdates); NullnessHint nullnessHint = handler.onDataflowVisitMethodInvocation( - node, types, values(input), thenUpdates, elseUpdates, bothUpdates); + node, types, context, values(input), thenUpdates, elseUpdates, bothUpdates); Nullness nullness = returnValueNullness(node, input, nullnessHint); if (booleanReturnType(node)) { ResultingStore thenStore = updateStore(input.getThenStore(), thenUpdates, bothUpdates); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java index 211ad49407..ea44f219d4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java @@ -28,6 +28,7 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; @@ -64,6 +65,7 @@ public void onMatchTopLevelClass( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java index dfda4cd7df..0c8756024a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java @@ -33,6 +33,7 @@ import com.sun.source.tree.ReturnTree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.NullAway; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import com.uber.nullaway.dataflow.NullnessStore; @@ -101,7 +102,9 @@ public void onMatchReturn(NullAway analysis, ReturnTree tree, VisitorState state @Override public ImmutableSet onUnannotatedInvocationGetExplicitlyNullablePositions( - Symbol.MethodSymbol methodSymbol, ImmutableSet explicitlyNullablePositions) { + Context context, + Symbol.MethodSymbol methodSymbol, + ImmutableSet explicitlyNullablePositions) { // NoOp return explicitlyNullablePositions; } @@ -143,6 +146,7 @@ public NullnessStore.Builder onDataflowInitialStore( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java index d60160e550..cc5f41e316 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java @@ -34,6 +34,7 @@ import com.sun.source.tree.ReturnTree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.NullAway; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import com.uber.nullaway.dataflow.NullnessStore; @@ -115,11 +116,13 @@ public void onMatchReturn(NullAway analysis, ReturnTree tree, VisitorState state @Override public ImmutableSet onUnannotatedInvocationGetExplicitlyNullablePositions( - Symbol.MethodSymbol methodSymbol, ImmutableSet explicitlyNullablePositions) { + Context context, + Symbol.MethodSymbol methodSymbol, + ImmutableSet explicitlyNullablePositions) { for (Handler h : handlers) { explicitlyNullablePositions = h.onUnannotatedInvocationGetExplicitlyNullablePositions( - methodSymbol, explicitlyNullablePositions); + context, methodSymbol, explicitlyNullablePositions); } return explicitlyNullablePositions; } @@ -174,6 +177,7 @@ public NullnessStore.Builder onDataflowInitialStore( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, @@ -182,7 +186,7 @@ public NullnessHint onDataflowVisitMethodInvocation( for (Handler h : handlers) { NullnessHint n = h.onDataflowVisitMethodInvocation( - node, types, inputs, thenUpdates, elseUpdates, bothUpdates); + node, types, context, inputs, thenUpdates, elseUpdates, bothUpdates); nullnessHint = nullnessHint.merge(n); } return nullnessHint; diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java index f9c6b13c5b..527f78a002 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/ContractHandler.java @@ -29,6 +29,7 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; @@ -90,6 +91,7 @@ public void onMatchTopLevelClass( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java index 5dc2501876..823e67936d 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java @@ -33,6 +33,7 @@ import com.sun.source.tree.ReturnTree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.NullAway; import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; @@ -143,7 +144,9 @@ void onMatchMethodReference( * @return Updated parameter nullability computed by this handler. */ ImmutableSet onUnannotatedInvocationGetExplicitlyNullablePositions( - Symbol.MethodSymbol methodSymbol, ImmutableSet explicitlyNullablePositions); + Context context, + Symbol.MethodSymbol methodSymbol, + ImmutableSet explicitlyNullablePositions); /** * Called when NullAway encounters an unannotated method and asks if return value is explicitly @@ -229,6 +232,7 @@ NullnessStore.Builder onDataflowInitialStore( NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java index 44f1087a26..43af8d3e12 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -33,6 +33,7 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.Config; import com.uber.nullaway.NullAway; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; @@ -163,6 +164,7 @@ public ImmutableSet onUnannotatedInvocationGetNonNullPositions( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index d0af1cb907..cb4edc4981 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -26,7 +26,6 @@ import static com.uber.nullaway.Nullness.NONNULL; import com.google.common.base.Preconditions; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Sets; @@ -36,16 +35,22 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.Name; +import com.sun.tools.javac.util.Names; import com.uber.nullaway.LibraryModels; +import com.uber.nullaway.LibraryModels.MethodRef; import com.uber.nullaway.NullAway; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.ServiceLoader; import java.util.Set; +import java.util.function.Function; +import javax.annotation.Nullable; import org.checkerframework.dataflow.cfg.node.MethodInvocationNode; import org.checkerframework.dataflow.cfg.node.Node; @@ -60,6 +65,8 @@ public class LibraryModelsHandler extends BaseNoOpHandler { private final LibraryModels libraryModels; + @Nullable private OptimizedLibraryModels optLibraryModels; + public LibraryModelsHandler() { super(); libraryModels = loadLibraryModels(); @@ -73,19 +80,18 @@ public ImmutableSet onUnannotatedInvocationGetNonNullPositions( List actualParams, ImmutableSet nonNullPositions) { return Sets.union( - nonNullPositions, - libraryModels.nonNullParameters().get(LibraryModels.MethodRef.fromSymbol(methodSymbol))) + nonNullPositions, getOptLibraryModels(state.context).nonNullParameters(methodSymbol)) .immutableCopy(); } @Override public ImmutableSet onUnannotatedInvocationGetExplicitlyNullablePositions( - Symbol.MethodSymbol methodSymbol, ImmutableSet explicitlyNullablePositions) { + Context context, + Symbol.MethodSymbol methodSymbol, + ImmutableSet explicitlyNullablePositions) { return Sets.union( explicitlyNullablePositions, - libraryModels - .explicitlyNullableParameters() - .get(LibraryModels.MethodRef.fromSymbol(methodSymbol))) + getOptLibraryModels(context).explicitlyNullableParameters(methodSymbol)) .immutableCopy(); } @@ -93,13 +99,14 @@ public ImmutableSet onUnannotatedInvocationGetExplicitlyNullablePositio public boolean onOverrideMayBeNullExpr( NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) { if (expr.getKind() == Tree.Kind.METHOD_INVOCATION - && LibraryModels.LibraryModelUtil.hasNullableReturn( - libraryModels, (Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) { + && getOptLibraryModels(state.context) + .hasNullableReturn( + (Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) { return analysis.nullnessFromDataflow(state, expr) || exprMayBeNull; } if (expr.getKind() == Tree.Kind.METHOD_INVOCATION - && LibraryModels.LibraryModelUtil.hasNonNullReturn( - libraryModels, (Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) { + && getOptLibraryModels(state.context) + .hasNonNullReturn((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) { return false; } return exprMayBeNull; @@ -109,17 +116,18 @@ public boolean onOverrideMayBeNullExpr( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree()); Preconditions.checkNotNull(callee); - setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee); - setConditionalArgumentNullness(thenUpdates, elseUpdates, node.getArguments(), callee); - if (LibraryModels.LibraryModelUtil.hasNonNullReturn(libraryModels, callee, types)) { + setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee, context); + setConditionalArgumentNullness(thenUpdates, elseUpdates, node.getArguments(), callee, context); + if (getOptLibraryModels(context).hasNonNullReturn(callee, types)) { return NullnessHint.FORCE_NONNULL; - } else if (LibraryModels.LibraryModelUtil.hasNullableReturn(libraryModels, callee, types)) { + } else if (getOptLibraryModels(context).hasNullableReturn(callee, types)) { return NullnessHint.HINT_NULLABLE; } else { return NullnessHint.UNKNOWN; @@ -130,9 +138,10 @@ private void setConditionalArgumentNullness( AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, List arguments, - Symbol.MethodSymbol callee) { + Symbol.MethodSymbol callee, + Context context) { Set nullImpliesTrueParameters = - libraryModels.nullImpliesTrueParameters().get(LibraryModels.MethodRef.fromSymbol(callee)); + getOptLibraryModels(context).nullImpliesTrueParameters(callee); for (AccessPath accessPath : accessPathsAtIndexes(nullImpliesTrueParameters, arguments)) { elseUpdates.set(accessPath, NONNULL); } @@ -154,12 +163,20 @@ private static Iterable accessPathsAtIndexes( return result; } + private OptimizedLibraryModels getOptLibraryModels(Context context) { + if (optLibraryModels == null) { + optLibraryModels = new OptimizedLibraryModels(libraryModels, context); + } + return optLibraryModels; + } + private void setUnconditionalArgumentNullness( AccessPathNullnessPropagation.Updates bothUpdates, List arguments, - Symbol.MethodSymbol callee) { + Symbol.MethodSymbol callee, + Context context) { Set requiredNonNullParameters = - libraryModels.failIfNullParameters().get(LibraryModels.MethodRef.fromSymbol(callee)); + getOptLibraryModels(context).failIfNullParameters(callee); for (AccessPath accessPath : accessPathsAtIndexes(requiredNonNullParameters, arguments)) { bothUpdates.set(accessPath, NONNULL); } @@ -177,23 +194,28 @@ private static class DefaultLibraryModels implements LibraryModels { private static final ImmutableSetMultimap FAIL_IF_NULL_PARAMETERS = new ImmutableSetMultimap.Builder() - .put(methodRef(Preconditions.class, "checkNotNull(T)"), 0) - .put(methodRef("java.util.Objects", "requireNonNull(T)"), 0) - .put(methodRef("org.junit.Assert", "assertNotNull(java.lang.Object)"), 0) + .put(methodRef("com.google.common.base.Preconditions", "checkNotNull", "(T)", ""), 0) + .put(methodRef("java.util.Objects", "requireNonNull", "(T)", ""), 0) + .put(methodRef("org.junit.Assert", "assertNotNull", "(java.lang.Object)"), 0) .put( - methodRef("org.junit.Assert", "assertNotNull(java.lang.String,java.lang.Object)"), + methodRef( + "org.junit.Assert", "assertNotNull", "(java.lang.String,java.lang.Object)"), 1) .put( - methodRef("org.junit.jupiter.api.Assertions", "assertNotNull(java.lang.Object)"), 0) + methodRef( + "org.junit.jupiter.api.Assertions", "assertNotNull", "(java.lang.Object)"), + 0) .put( methodRef( "org.junit.jupiter.api.Assertions", - "assertNotNull(java.lang.Object,java.lang.String)"), + "assertNotNull", + "(java.lang.Object,java.lang.String)"), 1) .put( methodRef( "org.junit.jupiter.api.Assertions", - "assertNotNull(java.lang.Object,java.util.function.Supplier)"), + "assertNotNull", + "(java.lang.Object,java.util.function.Supplier)"), 1) .build(); @@ -202,7 +224,8 @@ private static class DefaultLibraryModels implements LibraryModels { .put( methodRef( "android.view.GestureDetector.OnGestureListener", - "onScroll(android.view.MotionEvent,android.view.MotionEvent,float,float)"), + "onScroll", + "(android.view.MotionEvent,android.view.MotionEvent,float,float)"), 0) .build(); @@ -211,159 +234,188 @@ private static class DefaultLibraryModels implements LibraryModels { .put( methodRef( "com.android.sdklib.build.ApkBuilder", - "ApkBuilder(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), + "ApkBuilder", + "(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), 0) .put( methodRef( "com.android.sdklib.build.ApkBuilder", - "ApkBuilder(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), + "ApkBuilder", + "(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), 1) - .put(methodRef("com.google.common.collect.ImmutableList.Builder", "add(E)"), 0) + .put(methodRef("com.google.common.collect.ImmutableList.Builder", "add", "(E)"), 0) .put( methodRef( "com.google.common.collect.ImmutableList.Builder", - "addAll(java.lang.Iterable)"), + "addAll", + "(java.lang.Iterable)"), 0) - .put(methodRef("com.google.common.collect.ImmutableSet.Builder", "add(E)"), 0) + .put(methodRef("com.google.common.collect.ImmutableSet.Builder", "add", "(E)"), 0) .put( methodRef( "com.google.common.collect.ImmutableSet.Builder", - "addAll(java.lang.Iterable)"), + "addAll", + "(java.lang.Iterable)"), 0) - .put(methodRef("com.google.common.collect.ImmutableSortedSet.Builder", "add(E)"), 0) + .put(methodRef("com.google.common.collect.ImmutableSortedSet.Builder", "add", "(E)"), 0) .put( methodRef( "com.google.common.collect.ImmutableSortedSet.Builder", - "addAll(java.lang.Iterable)"), + "addAll", + "(java.lang.Iterable)"), 0) .put( methodRef( "com.google.common.collect.Iterables", - "getFirst(java.lang.Iterable,T)"), + "getFirst", + "(java.lang.Iterable,T)", + ""), 0) .put( methodRef( "com.google.common.util.concurrent.SettableFuture", - "setException(java.lang.Throwable)"), + "setException", + "(java.lang.Throwable)"), 0) - .put(methodRef("java.io.File", "File(java.lang.String)"), 0) - .put(methodRef("java.lang.Class", "getResource(java.lang.String)"), 0) - .put(methodRef("java.lang.Class", "isAssignableFrom(java.lang.Class)"), 0) - .put(methodRef("java.lang.System", "getProperty(java.lang.String)"), 0) + .put(methodRef("java.io.File", "", "(java.lang.String)"), 0) + .put(methodRef("java.lang.Class", "getResource", "(java.lang.String)"), 0) + .put(methodRef("java.lang.Class", "isAssignableFrom", "(java.lang.Class)"), 0) + .put(methodRef("java.lang.System", "getProperty", "(java.lang.String)"), 0) .put( methodRef( - "java.net.URLClassLoader", "newInstance(java.net.URL[],java.lang.ClassLoader)"), + "java.net.URLClassLoader", + "newInstance", + "(java.net.URL[],java.lang.ClassLoader)"), 0) .put( methodRef( - "javax.lang.model.element.Element", "getAnnotation(java.lang.Class)"), + "javax.lang.model.element.Element", + "getAnnotation", + "(java.lang" + ".Class)", + ""), 0) .put( methodRef( - "javax.lang.model.util.Elements", "getPackageElement(java.lang.CharSequence)"), + "javax.lang.model.util.Elements", + "getPackageElement", + "(java.lang.CharSequence)"), 0) .put( methodRef( - "javax.lang.model.util.Elements", "getTypeElement(java.lang.CharSequence)"), + "javax.lang.model.util.Elements", "getTypeElement", "(java.lang.CharSequence)"), 0) .put( methodRef( "javax.lang.model.util.Elements", - "getDocComment(javax.lang.model.element.Element)"), + "getDocComment", + "(javax.lang.model.element.Element)"), 0) - .put(methodRef("java.util.Deque", "addFirst(E)"), 0) - .put(methodRef("java.util.Deque", "addLast(E)"), 0) - .put(methodRef("java.util.Deque", "offerFirst(E)"), 0) - .put(methodRef("java.util.Deque", "offerLast(E)"), 0) - .put(methodRef("java.util.Deque", "add(E)"), 0) - .put(methodRef("java.util.Deque", "offer(E)"), 0) - .put(methodRef("java.util.Deque", "push(E)"), 0) - .put(methodRef("java.util.Collection", "toArray(T[])"), 0) - .put(methodRef("java.util.ArrayDeque", "addFirst(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "addLast(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "offerFirst(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "offerLast(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "add(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "offer(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "push(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "toArray(T[])"), 0) + .put(methodRef("java.util.Deque", "addFirst", "(E)"), 0) + .put(methodRef("java.util.Deque", "addLast", "(E)"), 0) + .put(methodRef("java.util.Deque", "offerFirst", "(E)"), 0) + .put(methodRef("java.util.Deque", "offerLast", "(E)"), 0) + .put(methodRef("java.util.Deque", "add", "(E)"), 0) + .put(methodRef("java.util.Deque", "offer", "(E)"), 0) + .put(methodRef("java.util.Deque", "push", "(E)"), 0) + .put(methodRef("java.util.Collection", "toArray", "(T[])", ""), 0) + .put(methodRef("java.util.ArrayDeque", "addFirst", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "addLast", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "offerFirst", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "offerLast", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "add", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "offer", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "push", "(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "toArray", "(T[])", ""), 0) .build(); private static final ImmutableSetMultimap NULL_IMPLIES_TRUE_PARAMETERS = new ImmutableSetMultimap.Builder() - .put(methodRef(Strings.class, "isNullOrEmpty(java.lang.String)"), 0) - .put(methodRef(Objects.class, "isNull(java.lang.Object)"), 0) - .put(methodRef("android.text.TextUtils", "isEmpty(java.lang.CharSequence)"), 0) - .put(methodRef("org.apache.commons.lang.StringUtils", "isEmpty(java.lang.String)"), 0) + .put( + methodRef("com.google.common.base.Strings", "isNullOrEmpty", "(java.lang.String)"), + 0) + .put(methodRef("java.util.Objects", "isNull", "(java.lang.Object)"), 0) + .put(methodRef("android.text.TextUtils", "isEmpty", "(java.lang.CharSequence)"), 0) + .put( + methodRef("org.apache.commons.lang.StringUtils", "isEmpty", "(java.lang.String)"), + 0) .put( methodRef( - "org.apache.commons.lang3.StringUtils", "isEmpty(java.lang.CharSequence)"), + "org.apache.commons.lang3.StringUtils", "isEmpty", "(java.lang.CharSequence)"), 0) .build(); private static final ImmutableSet NULLABLE_RETURNS = new ImmutableSet.Builder() - .add(methodRef("java.lang.ref.Reference", "get()")) - .add(methodRef("java.lang.ref.PhantomReference", "get()")) - .add(methodRef("java.lang.ref.SoftReference", "get()")) - .add(methodRef("java.lang.ref.WeakReference", "get()")) - .add(methodRef("java.util.concurrent.atomic.AtomicReference", "get()")) - .add(methodRef("java.util.Map", "get(java.lang.Object)")) - .add(methodRef("javax.lang.model.element.Element", "getEnclosingElement()")) - .add(methodRef("javax.lang.model.element.ExecutableElement", "getDefaultValue()")) - .add(methodRef("javax.lang.model.element.PackageElement", "getEnclosingElement()")) - .add(methodRef("javax.lang.model.element.VariableElement", "getConstantValue()")) - .add(methodRef("javax.lang.model.type.WildcardType", "getSuperBound()")) - .add(methodRef("android.app.ActivityManager", "getRunningAppProcesses()")) - .add(methodRef("android.view.View", "getHandler()")) - .add(methodRef("java.lang.Throwable", "getMessage()")) - .add(methodRef("android.webkit.WebView", "getUrl()")) + .add(methodRef("java.lang.ref.Reference", "get", "()")) + .add(methodRef("java.lang.ref.PhantomReference", "get", "()")) + .add(methodRef("java.lang.ref.SoftReference", "get", "()")) + .add(methodRef("java.lang.ref.WeakReference", "get", "()")) + .add(methodRef("java.util.concurrent.atomic.AtomicReference", "get", "()")) + .add(methodRef("java.util.Map", "get", "(java.lang.Object)")) + .add(methodRef("javax.lang.model.element.Element", "getEnclosingElement", "()")) + .add(methodRef("javax.lang.model.element.ExecutableElement", "getDefaultValue", "()")) + .add(methodRef("javax.lang.model.element.PackageElement", "getEnclosingElement", "()")) + .add(methodRef("javax.lang.model.element.VariableElement", "getConstantValue", "()")) + .add(methodRef("javax.lang.model.type.WildcardType", "getSuperBound", "()")) + .add(methodRef("android.app.ActivityManager", "getRunningAppProcesses", "()")) + .add(methodRef("android.view.View", "getHandler", "()")) + .add(methodRef("java.lang.Throwable", "getMessage", "()")) + .add(methodRef("android.webkit.WebView", "getUrl", "()")) .build(); private static final ImmutableSet NONNULL_RETURNS = new ImmutableSet.Builder() - .add(methodRef("com.google.gson", "fromJson(String,Class)")) - .add(methodRef("android.app.Activity", "findViewById(int)")) - .add(methodRef("android.view.View", "findViewById(int)")) - .add(methodRef("android.view.View", "getResources()")) - .add(methodRef("android.view.ViewGroup", "getChildAt(int)")) + .add(methodRef("com.google.gson", "fromJson", "(String,Class)", "")) + .add(methodRef("android.app.Activity", "findViewById", "(int)", "")) + .add(methodRef("android.view.View", "findViewById", "(int)", "")) + .add(methodRef("android.view.View", "getResources", "()")) + .add(methodRef("android.view.ViewGroup", "getChildAt", "(int)")) .add( methodRef( "android.content.res.Resources", - "getDrawable(int,android.content.res.Resources.Theme)")) - .add(methodRef("android.support.v4.app.Fragment", "getActivity()")) - .add(methodRef("androidx.fragment.app.Fragment", "getActivity()")) - .add(methodRef("android.support.v4.app.Fragment", "getArguments()")) - .add(methodRef("androidx.fragment.app.Fragment", "getArguments()")) - .add(methodRef("android.support.v4.app.Fragment", "getContext()")) - .add(methodRef("androidx.fragment.app.Fragment", "getContext()")) + "getDrawable", + "(int,android.content.res.Resources.Theme)")) + .add(methodRef("android.support.v4.app.Fragment", "getActivity", "()")) + .add(methodRef("androidx.fragment.app.Fragment", "getActivity", "()")) + .add(methodRef("android.support.v4.app.Fragment", "getArguments", "()")) + .add(methodRef("androidx.fragment.app.Fragment", "getArguments", "()")) + .add(methodRef("android.support.v4.app.Fragment", "getContext", "()")) + .add(methodRef("androidx.fragment.app.Fragment", "getContext", "()")) .add( methodRef( "android.support.v4.app.Fragment", - "onCreateView(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) + "onCreateView", + "(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) .add( methodRef( "androidx.fragment.app.Fragment", - "onCreateView(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) + "onCreateView", + "(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) .add( methodRef( "android.support.v4.content.ContextCompat", - "getDrawable(android.content.Context,int)")) + "getDrawable", + "(android.content.Context,int)")) .add( methodRef( "androidx.core.content.ContextCompat", - "getDrawable(android.content.Context,int)")) - .add(methodRef("android.support.v7.app.AppCompatDialog", "findViewById(int)")) - .add(methodRef("androidx.appcompat.app.AppCompatDialog", "findViewById(int)")) + "getDrawable", + "(android.content.Context,int)")) + .add( + methodRef("android.support.v7.app.AppCompatDialog", "findViewById", "(int)", "")) + .add( + methodRef("androidx.appcompat.app.AppCompatDialog", "findViewById", "(int)", "")) .add( methodRef( "android.support.v7.content.res.AppCompatResources", - "getDrawable(android.content.Context,int)")) + "getDrawable", + "(android.content.Context,int)")) .add( methodRef( "androidx.appcompat.content.res.AppCompatResources", - "getDrawable(android.content.Context,int)")) - .add(methodRef("android.support.design.widget.TextInputLayout", "getEditText()")) + "getDrawable", + "(android.content.Context,int)")) + .add(methodRef("android.support.design.widget.TextInputLayout", "getEditText", "()")) .build(); @Override @@ -482,4 +534,136 @@ public ImmutableSet nonNullReturns() { return nonNullReturns; } } + + /** + * A view of library models optimized to make lookup of {@link + * com.sun.tools.javac.code.Symbol.MethodSymbol}s fast + */ + private static class OptimizedLibraryModels { + + /** + * Mapping from {@link MethodRef} to some state, where lookups first check for a matching method + * name as an optimization. The {@link Name} data structure is used to avoid unnecessary String + * conversions when looking up {@link com.sun.tools.javac.code.Symbol.MethodSymbol}s. + * + * @param + */ + private static class NameIndexedMap { + + private final Map> state; + + NameIndexedMap(Map> state) { + this.state = state; + } + + @Nullable + public T get(Symbol.MethodSymbol symbol) { + Map methodRefTMap = state.get(symbol.name); + if (methodRefTMap == null) { + return null; + } + MethodRef ref = MethodRef.fromSymbol(symbol); + return methodRefTMap.get(ref); + } + + public boolean nameNotPresent(Symbol.MethodSymbol symbol) { + return state.get(symbol.name) == null; + } + } + + private final NameIndexedMap> failIfNullParams; + private final NameIndexedMap> explicitlyNullableParams; + private final NameIndexedMap> nonNullParams; + private final NameIndexedMap> nullImpliesTrueParams; + private final NameIndexedMap nullableRet; + private final NameIndexedMap nonNullRet; + + public OptimizedLibraryModels(LibraryModels models, Context context) { + Names names = Names.instance(context); + failIfNullParams = makeOptimizedIntSetLookup(names, models.failIfNullParameters()); + explicitlyNullableParams = + makeOptimizedIntSetLookup(names, models.explicitlyNullableParameters()); + nonNullParams = makeOptimizedIntSetLookup(names, models.nonNullParameters()); + nullImpliesTrueParams = makeOptimizedIntSetLookup(names, models.nullImpliesTrueParameters()); + nullableRet = makeOptimizedBoolLookup(names, models.nullableReturns()); + nonNullRet = makeOptimizedBoolLookup(names, models.nonNullReturns()); + } + + public boolean hasNonNullReturn(Symbol.MethodSymbol symbol, Types types) { + return lookupHandlingOverrides(symbol, types, nonNullRet) != null; + } + + public boolean hasNullableReturn(Symbol.MethodSymbol symbol, Types types) { + return lookupHandlingOverrides(symbol, types, nullableRet) != null; + } + + ImmutableSet failIfNullParameters(Symbol.MethodSymbol symbol) { + return lookupImmutableSet(symbol, failIfNullParams); + } + + ImmutableSet explicitlyNullableParameters(Symbol.MethodSymbol symbol) { + return lookupImmutableSet(symbol, explicitlyNullableParams); + } + + ImmutableSet nonNullParameters(Symbol.MethodSymbol symbol) { + return lookupImmutableSet(symbol, nonNullParams); + } + + ImmutableSet nullImpliesTrueParameters(Symbol.MethodSymbol symbol) { + return lookupImmutableSet(symbol, nullImpliesTrueParams); + } + + private ImmutableSet lookupImmutableSet( + Symbol.MethodSymbol symbol, NameIndexedMap> lookup) { + ImmutableSet result = lookup.get(symbol); + return (result == null) ? ImmutableSet.of() : result; + } + + private NameIndexedMap> makeOptimizedIntSetLookup( + Names names, ImmutableSetMultimap ref2Ints) { + return makeOptimizedLookup(names, ref2Ints.keySet(), ref2Ints::get); + } + + private NameIndexedMap makeOptimizedBoolLookup( + Names names, ImmutableSet refs) { + return makeOptimizedLookup(names, refs, (ref) -> true); + } + + private NameIndexedMap makeOptimizedLookup( + Names names, Set refs, Function fun) { + Map> finalThing = new LinkedHashMap<>(); + for (MethodRef ref : refs) { + Name methodName = names.fromString(ref.methodName); + Map forName = finalThing.get(methodName); + if (forName == null) { + forName = new LinkedHashMap<>(); + finalThing.put(methodName, forName); + } + forName.put(ref, fun.apply(ref)); + } + return new NameIndexedMap<>(finalThing); + } + + /** + * checks if symbol is present in the NameIndexedMap or if it overrides some method in the + * NameIndexedMap + */ + @Nullable + private static Symbol.MethodSymbol lookupHandlingOverrides( + Symbol.MethodSymbol symbol, Types types, NameIndexedMap optLookup) { + if (optLookup.nameNotPresent(symbol)) { + // no model matching the method name, so we don't need to check for overridden methods + return null; + } + if (optLookup.get(symbol) != null) { + return symbol; + } + for (Symbol.MethodSymbol superSymbol : ASTHelpers.findSuperMethods(symbol, types)) { + if (optLookup.get(superSymbol) != null) { + return superSymbol; + } + } + return null; + } + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java index daf4ba6327..0783ee66db 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java @@ -30,6 +30,7 @@ import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Types; +import com.sun.tools.javac.util.Context; import com.uber.nullaway.Config; import com.uber.nullaway.NullAway; import com.uber.nullaway.NullabilityUtil; @@ -86,7 +87,9 @@ public boolean onOverrideMayBeNullExpr( @Override public ImmutableSet onUnannotatedInvocationGetExplicitlyNullablePositions( - Symbol.MethodSymbol methodSymbol, ImmutableSet explicitlyNullablePositions) { + Context context, + Symbol.MethodSymbol methodSymbol, + ImmutableSet explicitlyNullablePositions) { HashSet positions = new HashSet(); positions.addAll(explicitlyNullablePositions); for (int i = 0; i < methodSymbol.getParameters().size(); ++i) { @@ -107,6 +110,7 @@ public boolean onUnannotatedInvocationGetExplicitlyNonNullReturn( public NullnessHint onDataflowVisitMethodInvocation( MethodInvocationNode node, Types types, + Context context, AccessPathNullnessPropagation.SubNodeValues inputs, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, From 05b2707cd010308943062948dd439324989f4929 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 4 Dec 2018 12:55:51 -0800 Subject: [PATCH 2/4] better variable names --- .../nullaway/handlers/LibraryModelsHandler.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index cb4edc4981..981bf28cd8 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -630,18 +630,18 @@ private NameIndexedMap makeOptimizedBoolLookup( } private NameIndexedMap makeOptimizedLookup( - Names names, Set refs, Function fun) { - Map> finalThing = new LinkedHashMap<>(); + Names names, Set refs, Function getValForRef) { + Map> nameMapping = new LinkedHashMap<>(); for (MethodRef ref : refs) { Name methodName = names.fromString(ref.methodName); - Map forName = finalThing.get(methodName); - if (forName == null) { - forName = new LinkedHashMap<>(); - finalThing.put(methodName, forName); + Map mapForName = nameMapping.get(methodName); + if (mapForName == null) { + mapForName = new LinkedHashMap<>(); + nameMapping.put(methodName, mapForName); } - forName.put(ref, fun.apply(ref)); + mapForName.put(ref, getValForRef.apply(ref)); } - return new NameIndexedMap<>(finalThing); + return new NameIndexedMap<>(nameMapping); } /** From fa7cde7e2f9ed2bdf774e73de098d1a52bb1c5d6 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 4 Dec 2018 14:32:20 -0800 Subject: [PATCH 3/4] undo API-breaking change --- .../java/com/uber/nullaway/LibraryModels.java | 36 +++- .../handlers/LibraryModelsHandler.java | 203 ++++++++---------- 2 files changed, 113 insertions(+), 126 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index 7bc95c80d4..f95f925a3e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -28,6 +28,8 @@ import com.google.common.collect.ImmutableSetMultimap; import com.sun.tools.javac.code.Symbol; import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.Nullable; /** Provides models for library routines for the null checker. */ @@ -105,19 +107,39 @@ private MethodRef( this.genericArgs = genericArgs; } + private static final Pattern METHOD_SIG_PATTERN = Pattern.compile("^(<.*>)?(\\w+)(\\(.*\\))$"); + /** * @param enclosingClass containing class * @param methodArgs method signature in the appropriate format (see class docs) * @return corresponding {@link MethodRef} */ - public static MethodRef methodRef(String enclosingClass, String methodName, String methodArgs) { - return new MethodRef(enclosingClass, methodName, methodArgs, null); + public static MethodRef methodRef(String enclosingClass, String methodSignature) { + Matcher matcher = METHOD_SIG_PATTERN.matcher(methodSignature); + if (matcher.find()) { + String genericArgs = matcher.group(1); + String methodName = matcher.group(2); + String methodArgs = matcher.group(3); + if (methodName.equals(enclosingClass.substring(enclosingClass.lastIndexOf('.') + 1))) { + // constructor + methodName = ""; + } + return new MethodRef(enclosingClass, methodName, methodArgs, genericArgs); + } else { + throw new IllegalArgumentException("malformed method signature " + methodSignature); + } } - public static MethodRef methodRef( - String enclosingClass, String methodName, String methodArgs, @Nullable String genericArgs) { - return new MethodRef(enclosingClass, methodName, methodArgs, genericArgs); - } + // public static MethodRef methodRef(String enclosingClass, String methodName, String + // methodArgs) { + // return new MethodRef(enclosingClass, methodName, methodArgs, null); + // } + // + // public static MethodRef methodRef( + // String enclosingClass, String methodName, String methodArgs, @Nullable String + // genericArgs) { + // return new MethodRef(enclosingClass, methodName, methodArgs, genericArgs); + // } public static MethodRef fromSymbol(Symbol.MethodSymbol symbol) { String methodStr = symbol.toString(); @@ -127,7 +149,7 @@ public static MethodRef fromSymbol(Symbol.MethodSymbol symbol) { if (symbol.type.hasTag(FORALL)) genericArgs = "<" + symbol.type.getTypeArguments() + ">"; } - return methodRef( + return new MethodRef( symbol.owner.getQualifiedName().toString(), symbol.name.toString(), methodStr.substring(openParenInd), diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java index 981bf28cd8..40d8ef7ece 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -194,28 +194,23 @@ private static class DefaultLibraryModels implements LibraryModels { private static final ImmutableSetMultimap FAIL_IF_NULL_PARAMETERS = new ImmutableSetMultimap.Builder() - .put(methodRef("com.google.common.base.Preconditions", "checkNotNull", "(T)", ""), 0) - .put(methodRef("java.util.Objects", "requireNonNull", "(T)", ""), 0) - .put(methodRef("org.junit.Assert", "assertNotNull", "(java.lang.Object)"), 0) + .put(methodRef("com.google.common.base.Preconditions", "checkNotNull(T)"), 0) + .put(methodRef("java.util.Objects", "requireNonNull(T)"), 0) + .put(methodRef("org.junit.Assert", "assertNotNull(java.lang.Object)"), 0) .put( - methodRef( - "org.junit.Assert", "assertNotNull", "(java.lang.String,java.lang.Object)"), + methodRef("org.junit.Assert", "assertNotNull(java.lang.String,java.lang.Object)"), 1) .put( - methodRef( - "org.junit.jupiter.api.Assertions", "assertNotNull", "(java.lang.Object)"), - 0) + methodRef("org.junit.jupiter.api.Assertions", "assertNotNull(java.lang.Object)"), 0) .put( methodRef( "org.junit.jupiter.api.Assertions", - "assertNotNull", - "(java.lang.Object,java.lang.String)"), + "assertNotNull(java.lang.Object,java.lang.String)"), 1) .put( methodRef( "org.junit.jupiter.api.Assertions", - "assertNotNull", - "(java.lang.Object,java.util.function.Supplier)"), + "assertNotNull(java.lang.Object,java.util.function.Supplier)"), 1) .build(); @@ -224,8 +219,7 @@ private static class DefaultLibraryModels implements LibraryModels { .put( methodRef( "android.view.GestureDetector.OnGestureListener", - "onScroll", - "(android.view.MotionEvent,android.view.MotionEvent,float,float)"), + "onScroll(android.view.MotionEvent,android.view.MotionEvent,float,float)"), 0) .build(); @@ -234,188 +228,159 @@ private static class DefaultLibraryModels implements LibraryModels { .put( methodRef( "com.android.sdklib.build.ApkBuilder", - "ApkBuilder", - "(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), + "ApkBuilder(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), 0) .put( methodRef( "com.android.sdklib.build.ApkBuilder", - "ApkBuilder", - "(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), + "ApkBuilder(java.io.File,java.io.File,java.io.File,java.lang.String,java.io.PrintStream)"), 1) - .put(methodRef("com.google.common.collect.ImmutableList.Builder", "add", "(E)"), 0) + .put(methodRef("com.google.common.collect.ImmutableList.Builder", "add(E)"), 0) .put( methodRef( "com.google.common.collect.ImmutableList.Builder", - "addAll", - "(java.lang.Iterable)"), + "addAll(java.lang.Iterable)"), 0) - .put(methodRef("com.google.common.collect.ImmutableSet.Builder", "add", "(E)"), 0) + .put(methodRef("com.google.common.collect.ImmutableSet.Builder", "add(E)"), 0) .put( methodRef( "com.google.common.collect.ImmutableSet.Builder", - "addAll", - "(java.lang.Iterable)"), + "addAll(java.lang.Iterable)"), 0) - .put(methodRef("com.google.common.collect.ImmutableSortedSet.Builder", "add", "(E)"), 0) + .put(methodRef("com.google.common.collect.ImmutableSortedSet.Builder", "add(E)"), 0) .put( methodRef( "com.google.common.collect.ImmutableSortedSet.Builder", - "addAll", - "(java.lang.Iterable)"), + "addAll(java.lang.Iterable)"), 0) .put( methodRef( "com.google.common.collect.Iterables", - "getFirst", - "(java.lang.Iterable,T)", - ""), + "getFirst(java.lang.Iterable,T)"), 0) .put( methodRef( "com.google.common.util.concurrent.SettableFuture", - "setException", - "(java.lang.Throwable)"), + "setException(java.lang.Throwable)"), 0) - .put(methodRef("java.io.File", "", "(java.lang.String)"), 0) - .put(methodRef("java.lang.Class", "getResource", "(java.lang.String)"), 0) - .put(methodRef("java.lang.Class", "isAssignableFrom", "(java.lang.Class)"), 0) - .put(methodRef("java.lang.System", "getProperty", "(java.lang.String)"), 0) + .put(methodRef("java.io.File", "File(java.lang.String)"), 0) + .put(methodRef("java.lang.Class", "getResource(java.lang.String)"), 0) + .put(methodRef("java.lang.Class", "isAssignableFrom(java.lang.Class)"), 0) + .put(methodRef("java.lang.System", "getProperty(java.lang.String)"), 0) .put( methodRef( - "java.net.URLClassLoader", - "newInstance", - "(java.net.URL[],java.lang.ClassLoader)"), + "java.net.URLClassLoader", "newInstance(java.net.URL[],java.lang.ClassLoader)"), 0) .put( methodRef( - "javax.lang.model.element.Element", - "getAnnotation", - "(java.lang" + ".Class)", - ""), + "javax.lang.model.element.Element", "getAnnotation(java.lang.Class)"), 0) .put( methodRef( - "javax.lang.model.util.Elements", - "getPackageElement", - "(java.lang.CharSequence)"), + "javax.lang.model.util.Elements", "getPackageElement(java.lang.CharSequence)"), 0) .put( methodRef( - "javax.lang.model.util.Elements", "getTypeElement", "(java.lang.CharSequence)"), + "javax.lang.model.util.Elements", "getTypeElement(java.lang.CharSequence)"), 0) .put( methodRef( "javax.lang.model.util.Elements", - "getDocComment", - "(javax.lang.model.element.Element)"), + "getDocComment(javax.lang.model.element.Element)"), 0) - .put(methodRef("java.util.Deque", "addFirst", "(E)"), 0) - .put(methodRef("java.util.Deque", "addLast", "(E)"), 0) - .put(methodRef("java.util.Deque", "offerFirst", "(E)"), 0) - .put(methodRef("java.util.Deque", "offerLast", "(E)"), 0) - .put(methodRef("java.util.Deque", "add", "(E)"), 0) - .put(methodRef("java.util.Deque", "offer", "(E)"), 0) - .put(methodRef("java.util.Deque", "push", "(E)"), 0) - .put(methodRef("java.util.Collection", "toArray", "(T[])", ""), 0) - .put(methodRef("java.util.ArrayDeque", "addFirst", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "addLast", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "offerFirst", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "offerLast", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "add", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "offer", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "push", "(E)"), 0) - .put(methodRef("java.util.ArrayDeque", "toArray", "(T[])", ""), 0) + .put(methodRef("java.util.Deque", "addFirst(E)"), 0) + .put(methodRef("java.util.Deque", "addLast(E)"), 0) + .put(methodRef("java.util.Deque", "offerFirst(E)"), 0) + .put(methodRef("java.util.Deque", "offerLast(E)"), 0) + .put(methodRef("java.util.Deque", "add(E)"), 0) + .put(methodRef("java.util.Deque", "offer(E)"), 0) + .put(methodRef("java.util.Deque", "push(E)"), 0) + .put(methodRef("java.util.Collection", "toArray(T[])"), 0) + .put(methodRef("java.util.ArrayDeque", "addFirst(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "addLast(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "offerFirst(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "offerLast(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "add(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "offer(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "push(E)"), 0) + .put(methodRef("java.util.ArrayDeque", "toArray(T[])"), 0) .build(); private static final ImmutableSetMultimap NULL_IMPLIES_TRUE_PARAMETERS = new ImmutableSetMultimap.Builder() - .put( - methodRef("com.google.common.base.Strings", "isNullOrEmpty", "(java.lang.String)"), - 0) - .put(methodRef("java.util.Objects", "isNull", "(java.lang.Object)"), 0) - .put(methodRef("android.text.TextUtils", "isEmpty", "(java.lang.CharSequence)"), 0) - .put( - methodRef("org.apache.commons.lang.StringUtils", "isEmpty", "(java.lang.String)"), - 0) + .put(methodRef("com.google.common.base.Strings", "isNullOrEmpty(java.lang.String)"), 0) + .put(methodRef("java.util.Objects", "isNull(java.lang.Object)"), 0) + .put(methodRef("android.text.TextUtils", "isEmpty(java.lang.CharSequence)"), 0) + .put(methodRef("org.apache.commons.lang.StringUtils", "isEmpty(java.lang.String)"), 0) .put( methodRef( - "org.apache.commons.lang3.StringUtils", "isEmpty", "(java.lang.CharSequence)"), + "org.apache.commons.lang3.StringUtils", "isEmpty(java.lang.CharSequence)"), 0) .build(); private static final ImmutableSet NULLABLE_RETURNS = new ImmutableSet.Builder() - .add(methodRef("java.lang.ref.Reference", "get", "()")) - .add(methodRef("java.lang.ref.PhantomReference", "get", "()")) - .add(methodRef("java.lang.ref.SoftReference", "get", "()")) - .add(methodRef("java.lang.ref.WeakReference", "get", "()")) - .add(methodRef("java.util.concurrent.atomic.AtomicReference", "get", "()")) - .add(methodRef("java.util.Map", "get", "(java.lang.Object)")) - .add(methodRef("javax.lang.model.element.Element", "getEnclosingElement", "()")) - .add(methodRef("javax.lang.model.element.ExecutableElement", "getDefaultValue", "()")) - .add(methodRef("javax.lang.model.element.PackageElement", "getEnclosingElement", "()")) - .add(methodRef("javax.lang.model.element.VariableElement", "getConstantValue", "()")) - .add(methodRef("javax.lang.model.type.WildcardType", "getSuperBound", "()")) - .add(methodRef("android.app.ActivityManager", "getRunningAppProcesses", "()")) - .add(methodRef("android.view.View", "getHandler", "()")) - .add(methodRef("java.lang.Throwable", "getMessage", "()")) - .add(methodRef("android.webkit.WebView", "getUrl", "()")) + .add(methodRef("java.lang.ref.Reference", "get()")) + .add(methodRef("java.lang.ref.PhantomReference", "get()")) + .add(methodRef("java.lang.ref.SoftReference", "get()")) + .add(methodRef("java.lang.ref.WeakReference", "get()")) + .add(methodRef("java.util.concurrent.atomic.AtomicReference", "get()")) + .add(methodRef("java.util.Map", "get(java.lang.Object)")) + .add(methodRef("javax.lang.model.element.Element", "getEnclosingElement()")) + .add(methodRef("javax.lang.model.element.ExecutableElement", "getDefaultValue()")) + .add(methodRef("javax.lang.model.element.PackageElement", "getEnclosingElement()")) + .add(methodRef("javax.lang.model.element.VariableElement", "getConstantValue()")) + .add(methodRef("javax.lang.model.type.WildcardType", "getSuperBound()")) + .add(methodRef("android.app.ActivityManager", "getRunningAppProcesses()")) + .add(methodRef("android.view.View", "getHandler()")) + .add(methodRef("java.lang.Throwable", "getMessage()")) + .add(methodRef("android.webkit.WebView", "getUrl()")) .build(); private static final ImmutableSet NONNULL_RETURNS = new ImmutableSet.Builder() - .add(methodRef("com.google.gson", "fromJson", "(String,Class)", "")) - .add(methodRef("android.app.Activity", "findViewById", "(int)", "")) - .add(methodRef("android.view.View", "findViewById", "(int)", "")) - .add(methodRef("android.view.View", "getResources", "()")) - .add(methodRef("android.view.ViewGroup", "getChildAt", "(int)")) + .add(methodRef("com.google.gson", "fromJson(String,Class)")) + .add(methodRef("android.app.Activity", "findViewById(int)")) + .add(methodRef("android.view.View", "findViewById(int)")) + .add(methodRef("android.view.View", "getResources()")) + .add(methodRef("android.view.ViewGroup", "getChildAt(int)")) .add( methodRef( "android.content.res.Resources", - "getDrawable", - "(int,android.content.res.Resources.Theme)")) - .add(methodRef("android.support.v4.app.Fragment", "getActivity", "()")) - .add(methodRef("androidx.fragment.app.Fragment", "getActivity", "()")) - .add(methodRef("android.support.v4.app.Fragment", "getArguments", "()")) - .add(methodRef("androidx.fragment.app.Fragment", "getArguments", "()")) - .add(methodRef("android.support.v4.app.Fragment", "getContext", "()")) - .add(methodRef("androidx.fragment.app.Fragment", "getContext", "()")) + "getDrawable(int,android.content.res.Resources.Theme)")) + .add(methodRef("android.support.v4.app.Fragment", "getActivity()")) + .add(methodRef("androidx.fragment.app.Fragment", "getActivity()")) + .add(methodRef("android.support.v4.app.Fragment", "getArguments()")) + .add(methodRef("androidx.fragment.app.Fragment", "getArguments()")) + .add(methodRef("android.support.v4.app.Fragment", "getContext()")) + .add(methodRef("androidx.fragment.app.Fragment", "getContext()")) .add( methodRef( "android.support.v4.app.Fragment", - "onCreateView", - "(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) + "onCreateView(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) .add( methodRef( "androidx.fragment.app.Fragment", - "onCreateView", - "(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) + "onCreateView(android.view.LayoutInflater,android.view.ViewGroup,android.os.Bundle)")) .add( methodRef( "android.support.v4.content.ContextCompat", - "getDrawable", - "(android.content.Context,int)")) + "getDrawable(android.content.Context,int)")) .add( methodRef( "androidx.core.content.ContextCompat", - "getDrawable", - "(android.content.Context,int)")) - .add( - methodRef("android.support.v7.app.AppCompatDialog", "findViewById", "(int)", "")) - .add( - methodRef("androidx.appcompat.app.AppCompatDialog", "findViewById", "(int)", "")) + "getDrawable(android.content.Context,int)")) + .add(methodRef("android.support.v7.app.AppCompatDialog", "findViewById(int)")) + .add(methodRef("androidx.appcompat.app.AppCompatDialog", "findViewById(int)")) .add( methodRef( "android.support.v7.content.res.AppCompatResources", - "getDrawable", - "(android.content.Context,int)")) + "getDrawable(android.content.Context,int)")) .add( methodRef( "androidx.appcompat.content.res.AppCompatResources", - "getDrawable", - "(android.content.Context,int)")) - .add(methodRef("android.support.design.widget.TextInputLayout", "getEditText", "()")) + "getDrawable(android.content.Context,int)")) + .add(methodRef("android.support.design.widget.TextInputLayout", "getEditText()")) .build(); @Override From e5cd47ab18d470724a6baa894ae6c68682397840 Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 4 Dec 2018 15:22:27 -0800 Subject: [PATCH 4/4] tweaks --- .../java/com/uber/nullaway/LibraryModels.java | 56 +++++-------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index f95f925a3e..5f1b97613b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -22,15 +22,12 @@ package com.uber.nullaway; -import static com.sun.tools.javac.code.TypeTag.FORALL; - import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.sun.tools.javac.code.Symbol; import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.annotation.Nullable; /** Provides models for library routines for the null checker. */ public interface LibraryModels { @@ -95,65 +92,46 @@ public interface LibraryModels { final class MethodRef { public final String enclosingClass; + /** + * we store the method name separately to enable fast comparison with MethodSymbols. See {@link + * com.uber.nullaway.handlers.LibraryModelsHandler.OptimizedLibraryModels} + */ public final String methodName; - public final String methodArgs; - @Nullable public final String genericArgs; - private MethodRef( - String enclosingClass, String methodName, String methodArgs, @Nullable String genericArgs) { + public final String fullMethodSig; + + private MethodRef(String enclosingClass, String methodName, String fullMethodSig) { this.enclosingClass = enclosingClass; this.methodName = methodName; - this.methodArgs = methodArgs; - this.genericArgs = genericArgs; + this.fullMethodSig = fullMethodSig; } private static final Pattern METHOD_SIG_PATTERN = Pattern.compile("^(<.*>)?(\\w+)(\\(.*\\))$"); /** * @param enclosingClass containing class - * @param methodArgs method signature in the appropriate format (see class docs) + * @param methodSignature method signature in the appropriate format (see class docs) * @return corresponding {@link MethodRef} */ public static MethodRef methodRef(String enclosingClass, String methodSignature) { Matcher matcher = METHOD_SIG_PATTERN.matcher(methodSignature); if (matcher.find()) { - String genericArgs = matcher.group(1); String methodName = matcher.group(2); - String methodArgs = matcher.group(3); if (methodName.equals(enclosingClass.substring(enclosingClass.lastIndexOf('.') + 1))) { // constructor methodName = ""; } - return new MethodRef(enclosingClass, methodName, methodArgs, genericArgs); + return new MethodRef(enclosingClass, methodName, methodSignature); } else { throw new IllegalArgumentException("malformed method signature " + methodSignature); } } - // public static MethodRef methodRef(String enclosingClass, String methodName, String - // methodArgs) { - // return new MethodRef(enclosingClass, methodName, methodArgs, null); - // } - // - // public static MethodRef methodRef( - // String enclosingClass, String methodName, String methodArgs, @Nullable String - // genericArgs) { - // return new MethodRef(enclosingClass, methodName, methodArgs, genericArgs); - // } - public static MethodRef fromSymbol(Symbol.MethodSymbol symbol) { String methodStr = symbol.toString(); - int openParenInd = methodStr.indexOf('('); - String genericArgs = null; - if (symbol.type != null) { - if (symbol.type.hasTag(FORALL)) genericArgs = "<" + symbol.type.getTypeArguments() + ">"; - } return new MethodRef( - symbol.owner.getQualifiedName().toString(), - symbol.name.toString(), - methodStr.substring(openParenInd), - genericArgs); + symbol.owner.getQualifiedName().toString(), symbol.name.toString(), methodStr); } @Override @@ -166,13 +144,12 @@ public boolean equals(Object o) { } MethodRef methodRef = (MethodRef) o; return Objects.equals(enclosingClass, methodRef.enclosingClass) - && Objects.equals(methodName, methodRef.methodName) - && Objects.equals(methodArgs, methodRef.methodArgs); + && Objects.equals(fullMethodSig, methodRef.fullMethodSig); } @Override public int hashCode() { - return Objects.hash(enclosingClass, methodName, methodArgs); + return Objects.hash(enclosingClass, fullMethodSig); } @Override @@ -181,11 +158,8 @@ public String toString() { + "enclosingClass='" + enclosingClass + '\'' - + ", methodName='" - + methodName - + '\'' - + ", methodArgs='" - + methodArgs + + ", fullMethodSig='" + + fullMethodSig + '\'' + '}'; }