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 adf1380744..3df9070e59 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -248,6 +248,7 @@ public TransferResult visitCharacterLiteral( @Override public TransferResult visitStringLiteral( StringLiteralNode stringLiteralNode, TransferInput input) { + handler.ondataflowVisitStringLiteral(stringLiteralNode, input); return noStoreChanges(NONNULL, input); } @@ -897,7 +898,7 @@ public TransferResult visitMethodInvocation( state.getTypes(), state.context, apContext, - values(input), + input, thenUpdates, elseUpdates, 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 1298cc3a3f..4432416f18 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java @@ -35,11 +35,13 @@ import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import java.util.Objects; import java.util.Optional; import javax.annotation.Nullable; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; import org.checkerframework.nullaway.dataflow.cfg.node.Node; @@ -71,7 +73,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java index c2b650e827..fa9f61ad39 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/AssertionHandler.java @@ -28,9 +28,12 @@ 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.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; import org.checkerframework.nullaway.dataflow.cfg.node.Node; @@ -49,7 +52,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { 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 7bd6f65321..27c2aa73ca 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/BaseNoOpHandler.java @@ -36,15 +36,18 @@ import com.sun.tools.javac.util.Context; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.NullAway; +import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; import java.util.Optional; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST; import org.checkerframework.nullaway.dataflow.cfg.node.LocalVariableNode; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; +import org.checkerframework.nullaway.dataflow.cfg.node.StringLiteralNode; /** * Provides a default (No-Op) implementation of every method defined by the Handler interface. @@ -152,7 +155,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { @@ -193,4 +196,10 @@ public void onNonNullFieldAssignment( Symbol field, AccessPathNullnessAnalysis analysis, VisitorState state) { // NoOp } + + @Override + public void ondataflowVisitStringLiteral( + StringLiteralNode node, TransferInput input) { + // NoOp + } } 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 6d1aedaec1..b26bbe50d5 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/CompositeHandler.java @@ -37,15 +37,18 @@ import com.sun.tools.javac.util.Context; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.NullAway; +import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; import java.util.Optional; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST; import org.checkerframework.nullaway.dataflow.cfg.node.LocalVariableNode; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; +import org.checkerframework.nullaway.dataflow.cfg.node.StringLiteralNode; /** * Registry of all handlers registered on our analysis. @@ -183,7 +186,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { @@ -191,7 +194,7 @@ public NullnessHint onDataflowVisitMethodInvocation( for (Handler h : handlers) { NullnessHint n = h.onDataflowVisitMethodInvocation( - node, types, context, apContext, inputs, thenUpdates, elseUpdates, bothUpdates); + node, types, context, apContext, input, thenUpdates, elseUpdates, bothUpdates); nullnessHint = nullnessHint.merge(n); } return nullnessHint; @@ -251,4 +254,12 @@ public void onNonNullFieldAssignment( h.onNonNullFieldAssignment(field, analysis, state); } } + + @Override + public void ondataflowVisitStringLiteral( + StringLiteralNode node, TransferInput input) { + for (Handler h : handlers) { + h.ondataflowVisitStringLiteral(node, input); + } + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java index 581ff5159d..b0d0a27771 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java @@ -39,6 +39,7 @@ import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -46,6 +47,7 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.type.TypeKind; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; import org.checkerframework.nullaway.dataflow.cfg.node.Node; @@ -83,7 +85,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { 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 4af0d77394..4397eb6a0b 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handler.java @@ -43,9 +43,11 @@ import com.uber.nullaway.dataflow.NullnessStore; import java.util.List; import java.util.Optional; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.UnderlyingAST; import org.checkerframework.nullaway.dataflow.cfg.node.LocalVariableNode; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; +import org.checkerframework.nullaway.dataflow.cfg.node.StringLiteralNode; /** * The general interface representing a handler. @@ -224,7 +226,7 @@ NullnessStore.Builder onDataflowInitialStore( * @param types {@link Types} for the current compilation * @param apContext the current access path context information (see {@link * AccessPath.AccessPathContext}). - * @param inputs NullnessStore information known before the method invocation. + * @param input Nullness and NullnessStore information known before the method invocation. * @param thenUpdates NullnessStore updates to be added along the then path, handlers can add via * the set() method. * @param elseUpdates NullnessStore updates to be added along the else path, handlers can add via @@ -240,7 +242,7 @@ NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates); @@ -325,6 +327,9 @@ Optional onExpressionDereference( void onNonNullFieldAssignment( Symbol field, AccessPathNullnessAnalysis analysis, VisitorState state); + void ondataflowVisitStringLiteral( + StringLiteralNode node, TransferInput input); + /** * A three value enum for handlers implementing onDataflowVisitMethodInvocation to communicate * their knowledge of the method return nullability to the the rest of NullAway. diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java index 1630a50ac6..b1ff33e60e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java @@ -63,6 +63,7 @@ public static Handler buildDefault(Config config) { handlerListBuilder.add(new GrpcHandler()); handlerListBuilder.add(new RequiresNonNullHandler()); handlerListBuilder.add(new EnsuresNonNullHandler()); + handlerListBuilder.add(new PreconditionsHandler()); if (config.serializationIsActive() && config.getSerializationConfig().fieldInitInfoEnabled) { handlerListBuilder.add( new FieldInitializationSerializationHandler(config.getSerializationConfig())); 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 07c6509cd0..7128d9aada 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/InferredJARModelsHandler.java @@ -36,8 +36,10 @@ import com.sun.tools.javac.util.Context; import com.uber.nullaway.Config; import com.uber.nullaway.NullAway; +import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import com.uber.nullaway.jarinfer.JarInferStubxProvider; import java.io.DataInputStream; import java.io.IOException; @@ -51,6 +53,7 @@ import javax.annotation.Nullable; import javax.lang.model.element.Modifier; import javax.lang.model.type.TypeKind; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; /** This handler loads inferred nullability model from stubs for methods in unannotated packages. */ @@ -169,7 +172,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { 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 d09b2e9631..9cbe0ca63a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/LibraryModelsHandler.java @@ -43,16 +43,20 @@ import com.uber.nullaway.LibraryModels; import com.uber.nullaway.LibraryModels.MethodRef; import com.uber.nullaway.NullAway; +import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; 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.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; import org.checkerframework.nullaway.dataflow.cfg.node.Node; @@ -138,7 +142,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { @@ -160,7 +164,7 @@ public NullnessHint onDataflowVisitMethodInvocation( // arguments is null, then the return should be non-null. boolean anyNull = false; for (int idx : nullImpliesNullIndexes) { - if (!inputs.valueOfSubNode(node.getArgument(idx)).equals(NONNULL)) { + if (!Objects.equals(input.getValueOfSubNode(node.getArgument(idx)), NONNULL)) { anyNull = true; } } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java index c07c1da46f..53ed8b03bb 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/OptionalEmptinessHandler.java @@ -40,6 +40,7 @@ import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessAnalysis; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import java.lang.annotation.Annotation; import java.util.List; import java.util.Objects; @@ -54,6 +55,7 @@ import javax.lang.model.element.Name; import javax.lang.model.element.VariableElement; import javax.lang.model.type.TypeMirror; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; import org.checkerframework.nullaway.dataflow.cfg.node.Node; @@ -108,7 +110,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/PreconditionsHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/PreconditionsHandler.java new file mode 100644 index 0000000000..1ebfce3b06 --- /dev/null +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/PreconditionsHandler.java @@ -0,0 +1,94 @@ +package com.uber.nullaway.handlers; + +import com.google.common.base.Preconditions; +import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ClassTree; +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.uber.nullaway.NullAway; +import com.uber.nullaway.Nullness; +import com.uber.nullaway.dataflow.AccessPath; +import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; +import java.util.HashMap; +import java.util.Map; +import javax.annotation.Nullable; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; +import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; +import org.checkerframework.nullaway.dataflow.cfg.node.Node; +import org.checkerframework.nullaway.dataflow.cfg.node.StringLiteralNode; + +public class PreconditionsHandler extends BaseNoOpHandler { + // ToDo: Similar logic could be used to handle more general forms of Assertions in + // AssertionHandler + + private static final String PRECONDITIONS_CLASS_NAME = "com.google.common.base.Preconditions"; + private static final String CHECK_ARGUMENT_METHOD_NAME = "checkArgument"; + + @Nullable private Name preconditionsClass; + @Nullable private Name checkArgumentMethod; + + private Map argumentToInputNullnessStoreMap = + new HashMap(); + + @Override + public NullnessHint onDataflowVisitMethodInvocation( + MethodInvocationNode node, + Types types, + Context context, + AccessPath.AccessPathContext apContext, + TransferInput input, + AccessPathNullnessPropagation.Updates thenUpdates, + AccessPathNullnessPropagation.Updates elseUpdates, + AccessPathNullnessPropagation.Updates bothUpdates) { + Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree()); + if (preconditionsClass == null) { + preconditionsClass = callee.name.table.fromString(PRECONDITIONS_CLASS_NAME); + checkArgumentMethod = callee.name.table.fromString(CHECK_ARGUMENT_METHOD_NAME); + } + Preconditions.checkNotNull(checkArgumentMethod); + if (callee.enclClass().getQualifiedName().equals(preconditionsClass) + && callee.name.equals(checkArgumentMethod)) { + if (callee.getParameters().size() == 1) { + // Debug + System.err.println("Found Preconditions.checkArgument check at node: " + node.toString()); + NullnessStore inputThenStore = input.getThenStore(); + System.err.println("inputThenStore: " + inputThenStore.toString()); + // Update based on NullnessStore for first arg + for (AccessPath accessPath : inputThenStore.getAccessPathsWithValue(Nullness.NONNULL)) { + bothUpdates.set(accessPath, Nullness.NONNULL); + } + } else if (callee.getParameters().size() == 2) { + NullnessStore deferredFirstArgStore = + argumentToInputNullnessStoreMap.get(node.getArgument(1)); + if (deferredFirstArgStore == null) { + // Missing deferredFirstArgStore, Ignoring! + return NullnessHint.UNKNOWN; + } + System.err.println("Found deferredFirstArgStore: " + deferredFirstArgStore.toString()); + for (AccessPath accessPath : + deferredFirstArgStore.getAccessPathsWithValue(Nullness.NONNULL)) { + bothUpdates.set(accessPath, Nullness.NONNULL); + } + } + } + return NullnessHint.UNKNOWN; + } + + @Override + public void ondataflowVisitStringLiteral( + StringLiteralNode node, TransferInput input) { + argumentToInputNullnessStoreMap.put(node, input.getThenStore()); + } + + @Override + public void onMatchTopLevelClass( + NullAway analysis, ClassTree tree, VisitorState state, Symbol.ClassSymbol classSymbol) { + // Try to keep this map from growing without bound, by clearing it whenever we start processing + // a new top level class + argumentToInputNullnessStoreMap.clear(); + } +} 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 2f8522ab4f..c78684bce0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/RestrictiveAnnotationHandler.java @@ -37,9 +37,11 @@ import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import java.util.HashSet; import java.util.List; import javax.annotation.Nullable; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; public class RestrictiveAnnotationHandler extends BaseNoOpHandler { @@ -125,7 +127,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java index 5a9548a97c..ea69bcba1f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java @@ -40,8 +40,11 @@ import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import com.uber.nullaway.handlers.BaseNoOpHandler; +import java.util.Objects; import javax.annotation.Nullable; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; /** @@ -99,7 +102,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { @@ -135,7 +138,7 @@ public NullnessHint onDataflowVisitMethodInvocation( supported = false; break; } else if (valueConstraint.equals("!null") - && inputs.valueOfSubNode(node.getArgument(i)).equals(Nullness.NONNULL)) { + && Objects.equals(input.getValueOfSubNode(node.getArgument(i)), Nullness.NONNULL)) { // We already know this argument can't be null, so we can treat it as not part of the // clause // for the purpose of deciding the non-nullness of the other arguments. diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java index 122bde79c2..1ca529be19 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/contract/fieldcontract/EnsuresNonNullHandler.java @@ -39,6 +39,7 @@ import com.uber.nullaway.annotations.EnsuresNonNull; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; +import com.uber.nullaway.dataflow.NullnessStore; import com.uber.nullaway.handlers.AbstractFieldContractHandler; import com.uber.nullaway.handlers.contract.ContractUtils; import java.util.Collections; @@ -46,6 +47,7 @@ import java.util.Set; import java.util.stream.Collectors; import javax.lang.model.element.VariableElement; +import org.checkerframework.nullaway.dataflow.analysis.TransferInput; import org.checkerframework.nullaway.dataflow.cfg.node.MethodInvocationNode; /** @@ -179,7 +181,7 @@ public NullnessHint onDataflowVisitMethodInvocation( Types types, Context context, AccessPath.AccessPathContext apContext, - AccessPathNullnessPropagation.SubNodeValues inputs, + TransferInput input, AccessPathNullnessPropagation.Updates thenUpdates, AccessPathNullnessPropagation.Updates elseUpdates, AccessPathNullnessPropagation.Updates bothUpdates) { @@ -187,7 +189,7 @@ public NullnessHint onDataflowVisitMethodInvocation( // A synthetic node might be inserted by the Checker Framework during CFG construction, it is // safer to do a null check here. return super.onDataflowVisitMethodInvocation( - node, types, context, apContext, inputs, thenUpdates, elseUpdates, bothUpdates); + node, types, context, apContext, input, thenUpdates, elseUpdates, bothUpdates); } Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(node.getTree()); Preconditions.checkNotNull(methodSymbol); @@ -211,6 +213,6 @@ public NullnessHint onDataflowVisitMethodInvocation( } } return super.onDataflowVisitMethodInvocation( - node, types, context, apContext, inputs, thenUpdates, elseUpdates, bothUpdates); + node, types, context, apContext, input, thenUpdates, elseUpdates, bothUpdates); } } diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwayPreconditionTests.java b/nullaway/src/test/java/com/uber/nullaway/NullAwayPreconditionTests.java new file mode 100644 index 0000000000..d0fc59c542 --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwayPreconditionTests.java @@ -0,0 +1,134 @@ +package com.uber.nullaway; + +import java.util.Arrays; +import org.junit.Test; + +public class NullAwayPreconditionTests extends NullAwayTestsBase { + + @Test + public void checkNotNullTest() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Preconditions;", + "class Test {", + " private void foo(@Nullable Object a) {", + " Preconditions.checkNotNull(a);", + " a.toString();", + " }", + "}") + .doTest(); + } + + @Test + public void simpleCheckArgumentTest() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Preconditions;", + "class Test {", + " private void foo(@Nullable Object a) {", + " Preconditions.checkArgument(a != null);", + " a.toString();", + " }", + "}") + .doTest(); + } + + @Test + public void simpleCheckArgumentWithMessageTest() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Preconditions;", + "class Test {", + " private void foo(@Nullable Object a) {", + " Preconditions.checkArgument(a != null, \"a ought to be non-null\");", + " a.toString();", + " }", + "}") + .doTest(); + } + + @Test + public void compoundCheckArgumentTest() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Preconditions;", + "class Test {", + " private void foo(@Nullable Object a) {", + " Preconditions.checkArgument(a != null && !a.equals(this));", + " a.toString();", + " }", + "}") + .doTest(); + } + + @Test + public void compoundCheckArgumentLastTest() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Preconditions;", + "class Test {", + " private void foo(@Nullable Object a) {", + " Preconditions.checkArgument(this.hashCode() != 5 && a != null);", + " a.toString();", + " }", + "}") + .doTest(); + } + + @Test + public void nestedCallCheckArgumentTest() { + makeTestHelperWithArgs( + Arrays.asList( + "-d", + temporaryFolder.getRoot().getAbsolutePath(), + "-XepOpt:NullAway:AnnotatedPackages=com.uber")) + .addSourceLines( + "Test.java", + "package com.uber;", + "import javax.annotation.Nullable;", + "import com.google.common.base.Preconditions;", + "import com.google.common.base.Strings;", + "class Test {", + " private void foo(@Nullable String a) {", + " Preconditions.checkArgument(!Strings.isNullOrEmpty(a));", + " a.toString();", + " }", + "}") + .doTest(); + } +}