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 fc710c5958..422d1295b0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -714,9 +714,29 @@ private static boolean isCatchVariable(VariableDeclarationNode node) { return variableElement != null && variableElement.getKind() == EXCEPTION_PARAMETER; } + /** + * In JSpecify mode, this method invokes {@link + * GenericsChecks#registerVarLocalDeclaration(VariableTree)} to register {@code var}-declared + * locals. This is required since sometimes during dataflow analysis, we require the full generic + * type of such a variable at a use, which may require running generic method inference on the + * right-hand side of the declaration. javac does not provide an efficient way to retrieve a local + * variable declaration given its {@code Symbol}. So, as an efficient alternative, we cache all + * such declarations when they are first visited by dataflow analysis, in case inference needs to + * be performed later at a use. + * + *

Note that the above assumes dataflow analysis will always see the declaration of a local + * before any use. This depends on the worklist ordering used by the dataflow solver, which is a + * fragile dependence. But, any alternative would require running a separate visitor over the + * whole method body (or a traversal of the full CFG) to find the declarations, which could be + * expensive. So, we go with this approach, and our tests should catch if there is some unexpected + * change in worklist orderings. + */ @Override public TransferResult visitVariableDeclaration( VariableDeclarationNode node, TransferInput input) { + if (config.isJSpecifyMode()) { + genericsChecks.registerVarLocalDeclaration(node.getTree()); + } ReadableUpdates updates = new ReadableUpdates(); if (isCatchVariable(node)) { updates.set(node, NONNULL); diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/RunOnceForwardAnalysisImpl.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/RunOnceForwardAnalysisImpl.java index 7cc46d2efb..176ead72e0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/RunOnceForwardAnalysisImpl.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/RunOnceForwardAnalysisImpl.java @@ -5,6 +5,8 @@ import org.checkerframework.nullaway.dataflow.analysis.ForwardTransferFunction; import org.checkerframework.nullaway.dataflow.analysis.Store; import org.checkerframework.nullaway.dataflow.cfg.ControlFlowGraph; +import org.checkerframework.nullaway.dataflow.cfg.node.Node; +import org.jspecify.annotations.Nullable; /** * A ForwardAnalysis implementation that overrides {@link #performAnalysis(ControlFlowGraph)} to @@ -32,4 +34,27 @@ public void performAnalysis(ControlFlowGraph cfg) { analysisPerformed = true; } } + + /** + * Override as a workaround for Checker Framework issue + * 7726. This version returns the current value for {@code n} even if we have a running + * analysis and {@code n} is not a (transitive) operand of the current node of the analysis. + * Otherwise, its implementation is identical to that of {@code + * org.checkerframework.nullaway.dataflow.analysis.AbstractAnalysis#getValue(Node).} + * + *

We should remove this method if / when CF issue 7726 is fixed in a suitable manner. + */ + @Override + public @Nullable V getValue(Node n) { + if (isRunning) { + if (currentNode == null + || currentNode == n + || (currentTree != null && currentTree == n.getTree())) { + return null; + } + assert !n.isLValue() : "Did not expect an lvalue, but got " + n; + } + return nodeValues.get(n); + } } diff --git a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java index 0931007d68..de4b8aadf4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java +++ b/nullaway/src/main/java/com/uber/nullaway/generics/GenericsChecks.java @@ -115,6 +115,12 @@ private static final class InferenceFailure implements MethodInferenceResult { */ private final Map inferredPolyExpressionTypes = new LinkedHashMap<>(); + /** Maps each {@code var}-declared local to its inferred NullAway type */ + private final Map inferredVarLocalTypes = new LinkedHashMap<>(); + + /** Maps each {@code var}-declared local to its declaration tree */ + private final Map varLocalDeclarations = new LinkedHashMap<>(); + public @Nullable Type getInferredPolyExpressionType(Tree tree) { Preconditions.checkArgument( tree instanceof LambdaExpressionTree || tree instanceof MemberReferenceTree, @@ -466,9 +472,27 @@ private void reportInvalidOverridingMethodParamTypeError( * Foo<@Nullable A>}). * * @param tree A tree for which we need the type with preserved annotations. + * @param state the visitor state * @return Type of the tree with preserved annotations. */ /* package-private */ @Nullable Type getTreeType(Tree tree, VisitorState state) { + return getTreeType(tree, state, false); + } + + /** + * This method returns the type of the given tree, including any type use annotations. + * + *

This method is required because in some cases, the type returned by {@link + * com.google.errorprone.util.ASTHelpers#getType(Tree)} fails to preserve type use annotations, + * e.g., when dealing with {@link com.sun.source.tree.NewClassTree} (e.g., {@code new + * Foo<@Nullable A>}). + * + * @param tree A tree for which we need the type with preserved annotations. + * @param state the visitor state + * @param calledFromDataflow true if the type is being computed as part of dataflow analysis + * @return Type of the tree with preserved annotations. + */ + private @Nullable Type getTreeType(Tree tree, VisitorState state, boolean calledFromDataflow) { if (tree instanceof ExpressionTree exprTree) { NullabilityUtil.ExprTreeAndState exprTreeAndState = NullabilityUtil.stripParensAndUpdateTreePath(exprTree, state); @@ -492,7 +516,8 @@ private void reportInvalidOverridingMethodParamTypeError( // For constructor calls using diamond operator, infer from assignment context. // TODO handle diamond constructor calls passed to generic methods // https://github.com/uber/NullAway/issues/1470 - Type fromAssignmentContext = getDiamondTypeFromContext(newClassTree, state); + Type fromAssignmentContext = + getDiamondTypeFromContext(newClassTree, state, calledFromDataflow); if (fromAssignmentContext != null) { return fromAssignmentContext; } @@ -533,6 +558,11 @@ private void reportInvalidOverridingMethodParamTypeError( return lambdaParameterType; } } + // If it's a local variable declared using `var`, get the inferred type + Type inferredVarLocalType = getInferredVarLocalType(symbol, state, calledFromDataflow); + if (inferredVarLocalType != null) { + return inferredVarLocalType; + } result = ASTHelpers.getType(tree); // type on the tree itself can be missing nested annotations in certain cases, so use the // type on the symbol instead. for type variables, we've found that the type on the symbol @@ -546,7 +576,8 @@ private void reportInvalidOverridingMethodParamTypeError( // the symbol for the assigned location instead, if available Symbol lhsSymbol = ASTHelpers.getSymbol(assignmentTree.getVariable()); if (lhsSymbol != null) { - result = lhsSymbol.type; + Type inferredVarLocalType = getInferredVarLocalType(lhsSymbol, state, calledFromDataflow); + result = inferredVarLocalType != null ? inferredVarLocalType : lhsSymbol.type; } else { result = ASTHelpers.getType(assignmentTree); } @@ -566,7 +597,7 @@ private void reportInvalidOverridingMethodParamTypeError( Type invokedMethodType = symbol.type; Type enclosingType = getEnclosingTypeForCallExpression( - symbol, invocationTree, state.getPath(), state, false); + symbol, invocationTree, state.getPath(), state, calledFromDataflow); if (enclosingType != null) { invokedMethodType = TypeSubstitutionUtils.memberType(state.getTypes(), enclosingType, symbol, config); @@ -609,9 +640,10 @@ private void reportInvalidOverridingMethodParamTypeError( * Gets the type of a constructor call using a diamond operator from its assignment context, if * available. */ - private @Nullable Type getDiamondTypeFromContext(NewClassTree tree, VisitorState state) { + private @Nullable Type getDiamondTypeFromContext( + NewClassTree tree, VisitorState state, boolean calledFromDataflow) { return getDiamondTypeFromParentContext( - tree, state, castToNonNull(state.getPath().getParentPath())); + tree, state, castToNonNull(state.getPath().getParentPath()), calledFromDataflow); } /** @@ -619,7 +651,7 @@ private void reportInvalidOverridingMethodParamTypeError( * parent context. */ private @Nullable Type getDiamondTypeFromParentContext( - NewClassTree tree, VisitorState state, TreePath parentPath) { + NewClassTree tree, VisitorState state, TreePath parentPath, boolean calledFromDataflow) { Tree parent = parentPath.getLeaf(); while (parent instanceof ParenthesizedTree) { parentPath = parentPath.getParentPath(); @@ -629,7 +661,7 @@ private void reportInvalidOverridingMethodParamTypeError( parent = parentPath.getLeaf(); } if (parent instanceof VariableTree || parent instanceof AssignmentTree) { - return getTreeType(parent, state.withPath(parentPath)); + return getTreeType(parent, state.withPath(parentPath), calledFromDataflow); } if (parent instanceof ReturnTree) { TreePath enclosingMethodOrLambda = @@ -658,7 +690,8 @@ private void reportInvalidOverridingMethodParamTypeError( } if (parent instanceof NewClassTree parentConstructorCall) { // get the type returned by the parent constructor call - Type parentClassType = getTreeType(parentConstructorCall, state.withPath(parentPath)); + Type parentClassType = + getTreeType(parentConstructorCall, state.withPath(parentPath), calledFromDataflow); if (parentClassType != null) { Symbol parentCtorSymbol = ASTHelpers.getSymbol(parentConstructorCall); // get the proper type for the constructor, as a member of the type returned by the @@ -796,18 +829,17 @@ public void checkTypeParameterNullnessForAssignability(Tree tree, VisitorState s && isAssignmentToField(tree)) { maybeStorePolyExpressionTypeFromTarget(rhsTree, lhsType); } + boolean varLocalDeclaration = + tree instanceof VariableTree varTree && isVarLocalVariableDeclaration(varTree); TreePath pathToRhs = new TreePath(state.getPath(), rhsTree); - Type rhsType = getTreeType(rhsTree, state.withPath(pathToRhs)); + Type rhsType = + varLocalDeclaration + ? getInferredTypeForVarLocalDeclaration( + (VariableTree) tree, rhsTree, pathToRhs, state, false) + : getTypeForRhsOfAssignment(rhsTree, pathToRhs, lhsType, assignedToLocal, state, false); if (rhsType != null) { - if (isGenericCallNeedingInference(rhsTree)) { - rhsType = - inferGenericMethodCallType( - state.withPath(pathToRhs), - (MethodInvocationTree) rhsTree, - pathToRhs, - lhsType, - assignedToLocal, - false); + if (varLocalDeclaration) { + lhsType = rhsType; } boolean isAssignmentValid = subtypeParameterNullability(lhsType, rhsType, state); if (!isAssignmentValid) { @@ -817,7 +849,104 @@ && isAssignmentToField(tree)) { } private static boolean isAssignmentToLocalVariable(Tree tree) { - return isAssignmentToKind(tree, ElementKind.LOCAL_VARIABLE); + return isAssignmentToKind(tree, ElementKind.LOCAL_VARIABLE) + || isAssignmentToKind(tree, ElementKind.RESOURCE_VARIABLE); + } + + private static boolean isVarLocalVariableDeclaration(VariableTree tree) { + return tree instanceof JCTree.JCVariableDecl variableDecl && variableDecl.declaredUsingVar(); + } + + /** + * Associates the {@code Symbol} for a {@code var}-declared local with the local's declaration. + * This is used in case we observe a use of the local before we have processed the declaration + * (e.g., due to dataflow analysis), and need to jump to the declaration to infer the variable's + * type. + */ + public void registerVarLocalDeclaration(VariableTree tree) { + if (!isVarLocalVariableDeclaration(tree)) { + return; + } + Symbol symbol = ASTHelpers.getSymbol(tree); + if (symbol != null + && (symbol.getKind().equals(ElementKind.LOCAL_VARIABLE) + || symbol.getKind().equals(ElementKind.RESOURCE_VARIABLE))) { + varLocalDeclarations.put(symbol, (JCTree.JCVariableDecl) tree); + } + } + + /** + * Gets the inferred type for a local variable declared with {@code var}. + * + * @param symbol symbol for the local + * @param state visitor state + * @param calledFromDataflow whether this method was called as part of dataflow analysis + * @return the inferred type, or {@code null} if the symbol is not for a var-declared local + */ + private @Nullable Type getInferredVarLocalType( + Symbol symbol, VisitorState state, boolean calledFromDataflow) { + Type cachedType = inferredVarLocalTypes.get(symbol); + if (cachedType != null) { + return cachedType; + } + VariableTree variableDecl = varLocalDeclarations.get(symbol); + if (variableDecl == null) { + return null; + } + ExpressionTree initializer = variableDecl.getInitializer(); + if (initializer == null) { + // this can happen for enhanced for loops + // TODO handle this properly; see https://github.com/uber/NullAway/issues/1581 + return typeOrNullIfRaw(symbol.type); + } + TreePath pathToInitializer = pathWithLeaf(state.getPath(), initializer); + return getInferredTypeForVarLocalDeclaration( + variableDecl, initializer, pathToInitializer, state, calledFromDataflow); + } + + private @Nullable Type getInferredTypeForVarLocalDeclaration( + VariableTree varTree, + ExpressionTree initializer, + TreePath pathToInitializer, + VisitorState state, + boolean calledFromDataflow) { + Type rhsType = + getTypeForRhsOfAssignment( + initializer, + pathToInitializer, + null, + isAssignmentToLocalVariable(varTree), + state, + calledFromDataflow); + // do _not_ cache the inferred type if called from dataflow, since it may rely on incomplete + // results from the dataflow analysis + if (rhsType != null && !calledFromDataflow) { + Symbol symbol = ASTHelpers.getSymbol(varTree); + if (symbol != null) { + inferredVarLocalTypes.put(symbol, rhsType); + } + } + return rhsType; + } + + private @Nullable Type getTypeForRhsOfAssignment( + ExpressionTree rhsTree, + TreePath pathToRhs, + @Nullable Type typeFromAssignmentContext, + boolean assignedToLocal, + VisitorState state, + boolean calledFromDataflow) { + if (isGenericCallNeedingInference(rhsTree)) { + return inferGenericMethodCallType( + state.withPath(pathToRhs), + (MethodInvocationTree) rhsTree, + pathToRhs, + typeFromAssignmentContext, + assignedToLocal, + calledFromDataflow); + } else { + return getTreeType(rhsTree, state.withPath(pathToRhs), calledFromDataflow); + } } private static boolean isAssignmentToField(Tree tree) { @@ -924,7 +1053,8 @@ private MethodInferenceResult runInferenceForCall( solver, methodSymbol, invocationTree, - allInvocations); + allInvocations, + calledFromDataflow); typeVarNullability = new HashMap<>(solver.solve()); // The solver only computes a solution for variables that appear in constraints. For // unconstrained variables, treat them as NONNULL, consistent with solver behavior for @@ -934,22 +1064,6 @@ private MethodInferenceResult runInferenceForCall( typeVarNullability.putIfAbsent(typeVar, ConstraintSolver.InferredNullability.NONNULL); } - // Store inferred types for lambda arguments - new InvocationArguments(invocationTree, methodSymbol.type.asMethodType()) - .forEach( - (argument, argPos, formalParamType, unused) -> { - if (argument instanceof LambdaExpressionTree - || argument instanceof MemberReferenceTree) { - Type polyExprTreeType = ASTHelpers.getType(argument); - if (polyExprTreeType != null) { - Type typeWithInferredNullability = - TypeSubstitutionUtils.updateTypeWithInferredNullability( - polyExprTreeType, formalParamType, typeVarNullability, state, config); - inferredPolyExpressionTypes.put(argument, typeWithInferredNullability); - } - } - }); - InferenceSuccess successResult = new InferenceSuccess(typeVarNullability); // don't cache result if we were called from dataflow, since the result may rely on dataflow // facts that do not reflect the fixed point @@ -957,6 +1071,21 @@ private MethodInferenceResult runInferenceForCall( for (MethodInvocationTree invTree : allInvocations) { inferredTypeVarNullabilityForGenericCalls.put(invTree, successResult); } + // Store inferred types for lambda or method reference arguments + new InvocationArguments(invocationTree, methodSymbol.type.asMethodType()) + .forEach( + (argument, argPos, formalParamType, unused) -> { + if (argument instanceof LambdaExpressionTree + || argument instanceof MemberReferenceTree) { + Type polyExprTreeType = ASTHelpers.getType(argument); + if (polyExprTreeType != null) { + Type typeWithInferredNullability = + TypeSubstitutionUtils.updateTypeWithInferredNullability( + polyExprTreeType, formalParamType, typeVarNullability, state, config); + inferredPolyExpressionTypes.put(argument, typeWithInferredNullability); + } + } + }); } return successResult; } catch (UnsatisfiableConstraintsException e) { @@ -1004,6 +1133,7 @@ private String inferenceFailureMessage(UnsatisfiableConstraintsException e) { * @param allInvocations a set of all method invocations that require inference, including nested * ones. This is an output parameter that gets mutated while generating the constraints to add * nested invocations. + * @param calledFromDataflow whether this method is being called from dataflow analysis * @throws UnsatisfiableConstraintsException if the constraints are determined to be unsatisfiable */ private void generateConstraintsForCall( @@ -1014,7 +1144,8 @@ private void generateConstraintsForCall( ConstraintSolver solver, Symbol.MethodSymbol methodSymbol, MethodInvocationTree methodInvocationTree, - Set allInvocations) + Set allInvocations, + boolean calledFromDataflow) throws UnsatisfiableConstraintsException { Type.MethodType methodType = handler.onOverrideMethodType(methodSymbol, methodSymbol.type.asMethodType(), state); @@ -1035,7 +1166,8 @@ private void generateConstraintsForCall( solver, allInvocations, argument, - formalParamType); + formalParamType, + calledFromDataflow); }); } @@ -1050,13 +1182,15 @@ private void generateConstraintsForCall( * nested invocations. * @param rhsExpr the right-hand side expression of the pseudo-assignment * @param lhsType the left-hand side type of the pseudo-assignment + * @param calledFromDataflow whether this method is being called from dataflow analysis */ private void generateConstraintsForPseudoAssignment( VisitorState state, ConstraintSolver solver, Set allInvocations, ExpressionTree rhsExpr, - Type lhsType) { + Type lhsType, + boolean calledFromDataflow) { NullabilityUtil.ExprTreeAndState exprTreeAndState = NullabilityUtil.stripParensAndUpdateTreePath(rhsExpr, state); rhsExpr = exprTreeAndState.expr(); @@ -1068,14 +1202,22 @@ private void generateConstraintsForPseudoAssignment( Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(invTree); allInvocations.add(invTree); generateConstraintsForCall( - state, state.getPath(), lhsType, false, solver, symbol, invTree, allInvocations); + state, + state.getPath(), + lhsType, + false, + solver, + symbol, + invTree, + allInvocations, + calledFromDataflow); } else if (rhsExpr instanceof LambdaExpressionTree lambda) { handleLambdaInGenericMethodInference( - state, state.getPath(), solver, allInvocations, lhsType, lambda); + state, state.getPath(), solver, allInvocations, lhsType, lambda, calledFromDataflow); } else if (rhsExpr instanceof MemberReferenceTree memberReferenceTree) { handleMethodRefInGenericMethodInference(state, solver, lhsType, memberReferenceTree); } else { // all other cases - Type argumentType = getTreeType(rhsExpr, state); + Type argumentType = getTreeType(rhsExpr, state, calledFromDataflow); if (argumentType == null) { // bail out of any checking involving raw types for now return; @@ -1098,6 +1240,7 @@ private void generateConstraintsForPseudoAssignment( * nested invocations. * @param lhsType the type to which the lambda is being assigned * @param lambda The lambda argument + * @param calledFromDataflow whether this method is being called from dataflow analysis */ private void handleLambdaInGenericMethodInference( VisitorState state, @@ -1105,7 +1248,8 @@ private void handleLambdaInGenericMethodInference( ConstraintSolver solver, Set allInvocations, Type lhsType, - LambdaExpressionTree lambda) { + LambdaExpressionTree lambda, + boolean calledFromDataflow) { Symbol.MethodSymbol fiMethod = NullabilityUtil.getFunctionalInterfaceMethod(lambda, state.getTypes()); @@ -1127,7 +1271,8 @@ private void handleLambdaInGenericMethodInference( solver, allInvocations, returnedExpression, - fiReturnType); + fiReturnType, + calledFromDataflow); } else if (body instanceof BlockTree) { // Case 2: Block body, e.g., () -> { return null; } TreePath bodyPath = new TreePath(lambdaPath, body); @@ -1137,7 +1282,12 @@ private void handleLambdaInGenericMethodInference( ExpressionTree returnExpr = castToNonNull(returnTree.getExpression()); TreePath returnExprPath = new TreePath(returnPath, returnExpr); generateConstraintsForPseudoAssignment( - state.withPath(returnExprPath), solver, allInvocations, returnExpr, fiReturnType); + state.withPath(returnExprPath), + solver, + allInvocations, + returnExpr, + fiReturnType, + calledFromDataflow); } } } @@ -1624,7 +1774,7 @@ public void compareGenericTypeParameterNullabilityForCall( if (currentPath != null && ASTHelpers.stripParentheses(currentPath.getLeaf()) == tree) { TreePath parentPath = currentPath.getParentPath(); if (parentPath != null) { - enclosingType = getDiamondTypeFromParentContext(newClassTree, state, parentPath); + enclosingType = getDiamondTypeFromParentContext(newClassTree, state, parentPath, false); } } } @@ -1988,7 +2138,7 @@ private Type substituteTypeArgsInGenericMethodType( if (result instanceof InferenceSuccess successResult) { methodTypeAtCallSite = restoreNestedNullabilityForTypeVarArguments( - invocationTree, methodType, methodTypeAtCallSite, state); + invocationTree, methodType, methodTypeAtCallSite, state, calledFromDataflow); return TypeSubstitutionUtils.updateMethodTypeWithInferredNullability( methodTypeAtCallSite, methodType, successResult.typeVarNullability, state, config); } else { @@ -2022,7 +2172,8 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( MethodInvocationTree invocationTree, Type.MethodType origMethodType, Type.MethodType methodTypeAtCallSite, - VisitorState state) { + VisitorState state, + boolean calledFromDataflow) { Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(invocationTree); if (methodSymbol.isVarArgs()) { // skip handling of varargs for now @@ -2057,7 +2208,10 @@ private Type.MethodType restoreNestedNullabilityForTypeVarArguments( } else { // need to compute the substitution ExpressionTree actualParam = actualParams.get(i); Type actualArgType = - getTreeType(actualParam, state.withPath(pathWithLeaf(pathToInvocation, actualParam))); + getTreeType( + actualParam, + state.withPath(pathWithLeaf(pathToInvocation, actualParam)), + calledFromDataflow); // only handle cases of non-raw actual parameter types that have the same base type as the // inferred parameter type at the call site if (actualArgType != null @@ -2136,7 +2290,7 @@ private InvocationAndContext getInvocationAndContextForInference( } if (parent instanceof AssignmentTree || parent instanceof VariableTree) { return getInvocationInferenceInfoForAssignment( - parent, invocation, state.withPath(parentPath)); + parent, invocation, state.withPath(parentPath), calledFromDataflow); } else if (parent instanceof ReturnTree) { // find the enclosing method and return its return type TreePath enclosingMethodOrLambda = @@ -2201,14 +2355,21 @@ private InvocationAndContext getInvocationAndContextForInference( } private InvocationAndContext getInvocationInferenceInfoForAssignment( - Tree assignment, MethodInvocationTree invocation, VisitorState state) { + Tree assignment, + MethodInvocationTree invocation, + VisitorState state, + boolean calledFromDataflow) { Preconditions.checkArgument( assignment instanceof AssignmentTree || assignment instanceof VariableTree); TreePath path = state.getPath(); if (path.getLeaf() != assignment) { state = state.withPath(pathWithLeaf(path, assignment)); } - Type treeType = getTreeType(assignment, state); + Type treeType = + assignment instanceof VariableTree variableTree + && isVarLocalVariableDeclaration(variableTree) + ? null // no info from assignment context if declared with `var` + : getTreeType(assignment, state, calledFromDataflow); return new InvocationAndContext(invocation, treeType, isAssignmentToLocalVariable(assignment)); } @@ -2324,14 +2485,14 @@ public Nullness getGenericParameterNullnessAtInvocation( false, calledFromDataflow); } else { - enclosingType = getTreeType(receiver, state.withPath(receiverPath)); + enclosingType = getTreeType(receiver, state.withPath(receiverPath), calledFromDataflow); } } } else { Verify.verify(tree instanceof NewClassTree); // for a constructor invocation, the type from the invocation itself is the "enclosing type" // for the purposes of determining type arguments - enclosingType = getTreeType(tree, state); + enclosingType = getTreeType(tree, state, calledFromDataflow); } return enclosingType; } @@ -2582,6 +2743,8 @@ public boolean passingLambdaOrMethodRefWithGenericReturnToUnmarkedCode( public void clearCache() { inferredTypeVarNullabilityForGenericCalls.clear(); inferredPolyExpressionTypes.clear(); + inferredVarLocalTypes.clear(); + varLocalDeclarations.clear(); } public boolean isNullableAnnotated(Type type) { diff --git a/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java b/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java new file mode 100644 index 0000000000..eb55d38c2c --- /dev/null +++ b/nullaway/src/test/java/com/uber/nullaway/jspecify/VarDeclaredLocalTests.java @@ -0,0 +1,246 @@ +package com.uber.nullaway.jspecify; + +import com.google.errorprone.CompilationTestHelper; +import com.uber.nullaway.NullAwayTestsBase; +import com.uber.nullaway.generics.JSpecifyJavacConfig; +import java.util.Arrays; +import org.junit.Test; + +public class VarDeclaredLocalTests extends NullAwayTestsBase { + + @Test + public void genericInferenceForVarLocal() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Foo { + T get(); + } + static Foo make(U u) { + throw new RuntimeException(); + } + void test() { + var foo1 = make(null); + // BUG: Diagnostic contains: dereferenced expression foo1.get() is @Nullable + foo1.get().hashCode(); + var foo2 = make(new Object()); + foo2.get().hashCode(); + var foo3 = make(null); + // BUG: Diagnostic contains: dereferenced expression foo3.get() is @Nullable + foo3.get().hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void varLocalReassigned() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Foo { + T get(); + } + static Foo make(U u) { + throw new RuntimeException(); + } + static Foo> makeNested(Foo f) { + throw new RuntimeException(); + } + void testPositive() { + var foo = make(new Object()); + // BUG: Diagnostic contains: inference failure + foo = make(null); + } + void testPositive2(Foo<@Nullable Object> f1, Foo f2) { + var foo = makeNested(f1); + // BUG: Diagnostic contains: inference failure + foo = makeNested(f2); + } + void testNegative() { + var foo = make(null); + // no warning here since NullAway infers the type argument to be @Nullable Object + // based on the constraint from the assignment context, and it's legal to pass + // new Object() as a parameter + foo = make(new Object()); + } + } + """) + .doTest(); + } + + @Test + public void varInTryWithResources() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + import java.util.*; + import java.util.stream.*; + @NullMarked + class Test { + void test(Iterator<@Nullable Object> iterator) { + // just testing that we don't crash here + try (var stream = StreamSupport.stream( + Spliterators.spliteratorUnknownSize(iterator, Spliterator.ORDERED), false)) { + } + } + } + """) + .doTest(); + } + + @Test + public void sameNameVarInLoop() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Foo { + T get(); + } + static Foo make(U u) { + throw new RuntimeException(); + } + void test() { + for (var foo1 = make(null); foo1 == null; ) { + // BUG: Diagnostic contains: dereferenced expression foo1.get() is @Nullable + foo1.get().hashCode(); + } + for (var foo1 = make(new Object()); foo1 == null; ) { + foo1.get().hashCode(); + } + for (var foo1 = make(null); foo1 == null; ) { + // BUG: Diagnostic contains: dereferenced expression foo1.get() is @Nullable + foo1.get().hashCode(); + } + } + }""") + .doTest(); + } + + @Test + public void varGenericInferenceFromDataflowInLoop() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Foo { + T get(); + } + static Foo make(U u) { + throw new RuntimeException(); + } + void test() { + String s = "hello"; + while (true) { + var foo = make(s); + // BUG: Diagnostic contains: dereferenced expression foo.get() is @Nullable + foo.get().hashCode(); + s = null; + } + } + }""") + .doTest(); + } + + @Test + public void genericInferenceForVarLocalWithDuplicateNameInAnonymousClass() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + @NullMarked + class Test { + interface Foo { + T get(); + } + interface Runner { + void run(); + } + static Foo make(U u) { + throw new RuntimeException(); + } + void test() { + var foo = make(new Object()); + new Runner() { + @Override + public void run() { + var foo = make(null); + // BUG: Diagnostic contains: dereferenced expression foo.get() is @Nullable + foo.get().hashCode(); + } + }; + foo.get().hashCode(); + } + } + """) + .doTest(); + } + + @Test + public void enhancedForLoop() { + makeHelper() + .addSourceLines( + "Test.java", + """ + package com.uber; + import org.jspecify.annotations.NullMarked; + import org.jspecify.annotations.Nullable; + import java.util.*; + @NullMarked + class Test { + interface Foo { + T get(); + } + void test(List> l) { + for (var foo : l) { + var x = foo.get(); + // TODO we should be reporting a warning here consistently + // See https://github.com/uber/NullAway/issues/1581 + // commented out since we only report a warning on JDK 27+ + // x.hashCode(); + } + } + } + """) + .doTest(); + } + + private CompilationTestHelper makeHelper() { + return makeTestHelperWithArgs( + JSpecifyJavacConfig.withJSpecifyModeArgs( + Arrays.asList( + "-XepOpt:NullAway:AnnotatedPackages=com.uber", + "-XepOpt:NullAway:WarnOnGenericInferenceFailure=true"))); + } +}