Skip to content

Commit

Permalink
Revert "[Refactor] Pass resolved Symbols into Handler methods (uber#729
Browse files Browse the repository at this point in the history
…)"

This reverts commit f743be3.
  • Loading branch information
msridhar committed Jul 18, 2023
1 parent f05ed44 commit 417df02
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 77 deletions.
8 changes: 4 additions & 4 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Original file line number Diff line number Diff line change
Expand Up @@ -2271,7 +2271,7 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
return mayBeNullFieldAccess(state, expr, exprSymbol);
} else {
// Check handler.onOverrideMayBeNullExpr before dataflow.
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, true);
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, state, true);
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
}
case METHOD_INVOCATION:
Expand All @@ -2290,7 +2290,7 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
"whoops, better handle " + expr.getKind() + " " + state.getSourceForNode(expr));
}
}
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, exprMayBeNull);
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, state, exprMayBeNull);
return exprMayBeNull;
}

Expand All @@ -2303,7 +2303,7 @@ private boolean mayBeNullMethodCall(
if (!Nullness.hasNullableAnnotation(exprSymbol, config)) {
exprMayBeNull = false;
}
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, exprMayBeNull);
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, state, exprMayBeNull);
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
}

Expand All @@ -2327,7 +2327,7 @@ private boolean mayBeNullFieldAccess(VisitorState state, ExpressionTree expr, Sy
if (!NullabilityUtil.mayBeNullFieldFromType(exprSymbol, config, codeAnnotationInfo)) {
exprMayBeNull = false;
}
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, exprSymbol, state, exprMayBeNull);
exprMayBeNull = handler.onOverrideMayBeNullExpr(this, expr, state, exprMayBeNull);
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -917,16 +917,15 @@ public TransferResult<Nullness, NullnessStore> visitMethodInvocation(
ReadableUpdates elseUpdates = new ReadableUpdates();
ReadableUpdates bothUpdates = new ReadableUpdates();
Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree());
Preconditions.checkNotNull(
callee); // this could be null before https://github.com/google/error-prone/pull/2902
Preconditions.checkNotNull(callee);
setReceiverNonnull(bothUpdates, node.getTarget().getReceiver(), callee);
setNullnessForMapCalls(
node, callee, node.getArguments(), values(input), thenUpdates, bothUpdates);
NullnessHint nullnessHint =
handler.onDataflowVisitMethodInvocation(
node, callee, state, apContext, values(input), thenUpdates, elseUpdates, bothUpdates);
node, state, apContext, values(input), thenUpdates, elseUpdates, bothUpdates);
Nullness nullness = returnValueNullness(node, input, nullnessHint);
if (booleanReturnType(callee)) {
if (booleanReturnType(node)) {
ResultingStore thenStore = updateStore(input.getThenStore(), thenUpdates, bothUpdates);
ResultingStore elseStore = updateStore(input.getElseStore(), elseUpdates, bothUpdates);
return conditionalResult(
Expand Down Expand Up @@ -985,8 +984,9 @@ private void setNullnessForMapCalls(
}
}

private static boolean booleanReturnType(Symbol.MethodSymbol methodSymbol) {
return methodSymbol.getReturnType().getTag() == TypeTag.BOOLEAN;
private boolean booleanReturnType(MethodInvocationNode node) {
Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(node.getTree());
return methodSymbol != null && methodSymbol.getReturnType().getTag() == TypeTag.BOOLEAN;
}

Nullness returnValueNullness(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.errorprone.VisitorState;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
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.Type;
Expand Down Expand Up @@ -66,13 +67,13 @@ public void onMatchTopLevelClass(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(node.getTree());
if (thriftIsSetCall(symbol, state.getTypes())) {
String methodName = symbol.getSimpleName().toString();
// remove "isSet"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ public class AssertionHandler extends BaseNoOpHandler {
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol callee,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree());
if (callee == null) {
return NullnessHint.UNKNOWN;
}

if (!methodNameUtil.isUtilInitialized()) {
methodNameUtil.initializeMethodNames(callee.name.table);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,7 @@ public Nullness[] onOverrideMethodInvocationParametersNullability(

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
// NoOp
return exprMayBeNull;
}
Expand All @@ -150,7 +146,6 @@ public NullnessStore.Builder onDataflowInitialStore(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,9 @@ public Nullness[] onOverrideMethodInvocationParametersNullability(

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
for (Handler h : handlers) {
exprMayBeNull = h.onOverrideMayBeNullExpr(analysis, expr, exprSymbol, state, exprMayBeNull);
exprMayBeNull = h.onOverrideMayBeNullExpr(analysis, expr, state, exprMayBeNull);
}
return exprMayBeNull;
}
Expand All @@ -177,7 +173,6 @@ public NullnessStore.Builder onDataflowInitialStore(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
Expand All @@ -188,7 +183,7 @@ public NullnessHint onDataflowVisitMethodInvocation(
for (Handler h : handlers) {
NullnessHint n =
h.onDataflowVisitMethodInvocation(
node, symbol, state, apContext, inputs, thenUpdates, elseUpdates, bothUpdates);
node, state, apContext, inputs, thenUpdates, elseUpdates, bothUpdates);
nullnessHint = nullnessHint.merge(n);
}
return nullnessHint;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ public void onMatchTopLevelClass(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
MethodInvocationTree tree = castToNonNull(node.getTree());
Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(tree);
Types types = state.getTypes();
if (grpcIsMetadataContainsKeyCall(symbol, types)) {
// On seeing o.containsKey(k), set AP for o.get(k) to @NonNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,13 @@ void onMatchMethodReference(
*
* @param analysis A reference to the running NullAway analysis.
* @param expr The expression in question.
* @param exprSymbol The symbol of the expression, might be null
* @param state The current visitor state.
* @param exprMayBeNull Whether or not the expression may be null according to the base analysis
* or upstream handlers.
* @return Whether or not the expression may be null, as updated by this handler.
*/
boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull);
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull);

/**
* Called to potentially override the nullability of an annotated or unannotated method's return,
Expand Down Expand Up @@ -222,7 +217,6 @@ NullnessStore.Builder onDataflowInitialStore(
* Called when the Dataflow analysis visits each method invocation.
*
* @param node The AST node for the method callsite.
* @param symbol The symbol of the called method
* @param state The current visitor state.
* @param apContext the current access path context information (see {@link
* AccessPath.AccessPathContext}).
Expand All @@ -239,7 +233,6 @@ NullnessStore.Builder onDataflowInitialStore(
*/
NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@

import com.google.common.base.Preconditions;
import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
Expand Down Expand Up @@ -176,29 +178,24 @@ public Nullness[] onOverrideMethodInvocationParametersNullability(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
if (isReturnAnnotatedNullable(symbol)) {
if (isReturnAnnotatedNullable(ASTHelpers.getSymbol(node.getTree()))) {
return NullnessHint.HINT_NULLABLE;
}
return NullnessHint.UNKNOWN;
}

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
&& exprSymbol instanceof Symbol.MethodSymbol) {
return exprMayBeNull || isReturnAnnotatedNullable((Symbol.MethodSymbol) exprSymbol);
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)) {
return exprMayBeNull
|| isReturnAnnotatedNullable(ASTHelpers.getSymbol((MethodInvocationTree) expr));
}
return exprMayBeNull;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,10 @@ public Nullness onOverrideMethodInvocationReturnNullability(

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
&& exprSymbol instanceof Symbol.MethodSymbol) {
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
if (expr.getKind() == Tree.Kind.METHOD_INVOCATION) {
OptimizedLibraryModels optLibraryModels = getOptLibraryModels(state.context);
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) exprSymbol;
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) ASTHelpers.getSymbol(expr);
// When looking up library models of annotated code, we match the exact method signature only;
// overriding methods in subclasses must be explicitly given their own library model.
// When dealing with unannotated code, we default to generality: a model applies to a method
Expand Down Expand Up @@ -188,13 +183,14 @@ private CodeAnnotationInfo getCodeAnnotationInfo(Context context) {
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol callee,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree());
Preconditions.checkNotNull(callee);
boolean isMethodAnnotated =
!getCodeAnnotationInfo(state.context).isSymbolUnannotated(callee, this.config);
setUnconditionalArgumentNullness(bothUpdates, node.getArguments(), callee, state, apContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,9 @@ public class OptionalEmptinessHandler extends BaseNoOpHandler {

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
if (expr.getKind().equals(Tree.Kind.METHOD_INVOCATION)
&& exprSymbol instanceof Symbol.MethodSymbol
&& optionalIsGetCall((Symbol.MethodSymbol) exprSymbol, state.getTypes())) {
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
if (expr.getKind() == Tree.Kind.METHOD_INVOCATION
&& optionalIsGetCall((Symbol.MethodSymbol) ASTHelpers.getSymbol(expr), state.getTypes())) {
return true;
}
return exprMayBeNull;
Expand All @@ -114,13 +109,14 @@ public void onMatchTopLevelClass(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol symbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
Symbol.MethodSymbol symbol = ASTHelpers.getSymbol(node.getTree());

Types types = state.getTypes();
if (optionalIsPresentCall(symbol, types)) {
updateNonNullAPsForOptionalContent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
package com.uber.nullaway.handlers;

import com.google.errorprone.VisitorState;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol;
Expand Down Expand Up @@ -71,16 +72,11 @@ private boolean isSymbolRestrictivelyNullable(Symbol symbol, Context context) {

@Override
public boolean onOverrideMayBeNullExpr(
NullAway analysis,
ExpressionTree expr,
@Nullable Symbol exprSymbol,
VisitorState state,
boolean exprMayBeNull) {
NullAway analysis, ExpressionTree expr, VisitorState state, boolean exprMayBeNull) {
Tree.Kind exprKind = expr.getKind();
if (exprSymbol != null
&& (exprKind.equals(Tree.Kind.METHOD_INVOCATION)
|| exprKind.equals(Tree.Kind.IDENTIFIER))) {
return exprMayBeNull || isSymbolRestrictivelyNullable(exprSymbol, state.context);
if (exprKind.equals(Tree.Kind.METHOD_INVOCATION) || exprKind.equals(Tree.Kind.IDENTIFIER)) {
Symbol symbol = ASTHelpers.getSymbol(expr);
return exprMayBeNull || isSymbolRestrictivelyNullable(symbol, state.context);
}
return exprMayBeNull;
}
Expand Down Expand Up @@ -152,13 +148,13 @@ public Nullness onOverrideMethodInvocationReturnNullability(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol methodSymbol,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(node.getTree());
return isSymbolRestrictivelyNullable(methodSymbol, state.context)
? NullnessHint.HINT_NULLABLE
: NullnessHint.UNKNOWN;
Expand All @@ -173,6 +169,7 @@ public NullnessHint onDataflowVisitFieldAccess(
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates updates) {
;
return isSymbolRestrictivelyNullable(symbol, context)
? NullnessHint.HINT_NULLABLE
: NullnessHint.UNKNOWN;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,15 @@ public MethodInvocationNode onCFGBuildPhase1AfterVisitMethodInvocation(
@Override
public NullnessHint onDataflowVisitMethodInvocation(
MethodInvocationNode node,
Symbol.MethodSymbol callee,
VisitorState state,
AccessPath.AccessPathContext apContext,
AccessPathNullnessPropagation.SubNodeValues inputs,
AccessPathNullnessPropagation.Updates thenUpdates,
AccessPathNullnessPropagation.Updates elseUpdates,
AccessPathNullnessPropagation.Updates bothUpdates) {
Preconditions.checkNotNull(analysis);
Symbol.MethodSymbol callee = ASTHelpers.getSymbol(node.getTree());
Preconditions.checkNotNull(callee);
MethodInvocationTree tree = castToNonNull(node.getTree());
for (String clause : ContractUtils.getContractClauses(callee, config)) {

Expand Down
Loading

0 comments on commit 417df02

Please sign in to comment.