diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index ecbdf97423..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 com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableCollection; 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; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** Provides models for library routines for the null checker. */ public interface LibraryModels { @@ -94,92 +91,77 @@ 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; /** - * @param clazz containing class - * @param methodSig method signature in the appropriate format (see class docs) - * @return corresponding {@link MethodRef} + * we store the method name separately to enable fast comparison with MethodSymbols. See {@link + * com.uber.nullaway.handlers.LibraryModelsHandler.OptimizedLibraryModels} */ - public static MethodRef methodRef(Class clazz, String methodSig) { - return methodRef(clazz.getName(), methodSig); + public final String methodName; + + public final String fullMethodSig; + + private MethodRef(String enclosingClass, String methodName, String fullMethodSig) { + this.enclosingClass = enclosingClass; + this.methodName = methodName; + this.fullMethodSig = fullMethodSig; } + private static final Pattern METHOD_SIG_PATTERN = Pattern.compile("^(<.*>)?(\\w+)(\\(.*\\))$"); + /** - * @param clazz containing class - * @param methodSig method signature in the appropriate format (see class docs) + * @param enclosingClass containing class + * @param methodSignature 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 methodSignature) { + Matcher matcher = METHOD_SIG_PATTERN.matcher(methodSignature); + if (matcher.find()) { + String methodName = matcher.group(2); + if (methodName.equals(enclosingClass.substring(enclosingClass.lastIndexOf('.') + 1))) { + // constructor + methodName = ""; + } + return new MethodRef(enclosingClass, methodName, methodSignature); + } else { + throw new IllegalArgumentException("malformed method signature " + methodSignature); + } } public static MethodRef fromSymbol(Symbol.MethodSymbol symbol) { - return methodRef(symbol.owner.getQualifiedName().toString(), symbol.toString()); + String methodStr = symbol.toString(); + + return new MethodRef( + symbol.owner.getQualifiedName().toString(), symbol.name.toString(), methodStr); } @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; + } + if (o == null || getClass() != o.getClass()) { + return false; } - return false; + MethodRef methodRef = (MethodRef) o; + return Objects.equals(enclosingClass, methodRef.enclosingClass) + && Objects.equals(fullMethodSig, methodRef.fullMethodSig); } @Override public int hashCode() { - return Objects.hash(clazz, methodSig); + return Objects.hash(enclosingClass, fullMethodSig); } @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 + + '\'' + + ", fullMethodSig='" + + fullMethodSig + + '\'' + + '}'; } } } 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..40d8ef7ece 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,7 +194,7 @@ 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("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( @@ -291,8 +308,8 @@ private static class DefaultLibraryModels implements LibraryModels { 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("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( @@ -482,4 +499,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 getValForRef) { + Map> nameMapping = new LinkedHashMap<>(); + for (MethodRef ref : refs) { + Name methodName = names.fromString(ref.methodName); + Map mapForName = nameMapping.get(methodName); + if (mapForName == null) { + mapForName = new LinkedHashMap<>(); + nameMapping.put(methodName, mapForName); + } + mapForName.put(ref, getValForRef.apply(ref)); + } + return new NameIndexedMap<>(nameMapping); + } + + /** + * 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,