Skip to content

Commit

Permalink
Prepare for Nullable ASTHelpers.getSymbol (#733)
Browse files Browse the repository at this point in the history
This method will return `@Nullable` in the next Error Prone release: google/error-prone#3760 This change prepares for that by adding appropriate null checks.
  • Loading branch information
XN137 committed Mar 10, 2023
1 parent d83b7d0 commit 07a3847
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 13 deletions.
13 changes: 11 additions & 2 deletions nullaway/src/main/java/com/uber/nullaway/NullAway.java
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ private boolean withinAnnotatedCode(VisitorState state) {
}

private boolean checkMarkingForPath(VisitorState state) {
Symbol enclosingMarkableSymbol;
TreePath path = state.getPath();
Tree currentTree = path.getLeaf();
// Find the closest class or method symbol, since those are the only ones we have code
Expand All @@ -334,7 +333,10 @@ private boolean checkMarkingForPath(VisitorState state) {
}
currentTree = path.getLeaf();
}
enclosingMarkableSymbol = ASTHelpers.getSymbol(currentTree);
Symbol enclosingMarkableSymbol = ASTHelpers.getSymbol(currentTree);
if (enclosingMarkableSymbol == null) {
return false;
}
return !codeAnnotationInfo.isSymbolUnannotated(enclosingMarkableSymbol, config);
}

Expand Down Expand Up @@ -2277,6 +2279,13 @@ private boolean mayBeNullExpr(VisitorState state, ExpressionTree expr) {
return exprMayBeNull ? nullnessFromDataflow(state, expr) : false;
}
case METHOD_INVOCATION:
if (!(exprSymbol instanceof Symbol.MethodSymbol)) {
throw new IllegalStateException(
"unexpected symbol "
+ exprSymbol
+ " for method invocation "
+ state.getSourceForNode(expr));
}
// Special case: mayBeNullMethodCall runs handler.onOverrideMayBeNullExpr before dataflow.
return mayBeNullMethodCall(state, expr, (Symbol.MethodSymbol) exprSymbol);
case CONDITIONAL_EXPRESSION:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,16 +330,13 @@ private static boolean isDirectTypeUseAnnotation(Attribute.TypeCompound t) {
/**
* Check if a field might be null, based on the type.
*
* @param symbol symbol for field; must be non-null
* @param symbol symbol for field
* @param config NullAway config
* @return true if based on the type, package, and name of the field, the analysis should assume
* the field might be null; false otherwise
* @throws NullPointerException if {@code symbol} is null
*/
public static boolean mayBeNullFieldFromType(
Symbol symbol, Config config, CodeAnnotationInfo codeAnnotationInfo) {
Preconditions.checkNotNull(
symbol, "mayBeNullFieldFromType should never be called with a null symbol");
return !(symbol.getSimpleName().toString().equals("class")
|| symbol.isEnum()
|| codeAnnotationInfo.isSymbolUnannotated(symbol, config))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ && isBoxingMethod(ASTHelpers.getSymbol(methodInvocationTree))) {
case IDENTIFIER: // check for CONST
// Check for a constant field (static final)
Symbol symbol = ASTHelpers.getSymbol(tree);
if (symbol.getKind().equals(ElementKind.FIELD)) {
if (symbol instanceof Symbol.VarSymbol
&& symbol.getKind().equals(ElementKind.FIELD)) {
Symbol.VarSymbol varSymbol = (Symbol.VarSymbol) symbol;
// From docs: getConstantValue() returns the value of this variable if this is a
// static final field initialized to a compile-time constant. Returns null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ public TransferResult<Nullness, NullnessStore> visitVariableDeclaration(
public TransferResult<Nullness, NullnessStore> visitFieldAccess(
FieldAccessNode fieldAccessNode, TransferInput<Nullness, NullnessStore> input) {
ReadableUpdates updates = new ReadableUpdates();
Symbol symbol = ASTHelpers.getSymbol(fieldAccessNode.getTree());
Symbol symbol = Preconditions.checkNotNull(ASTHelpers.getSymbol(fieldAccessNode.getTree()));
setReceiverNonnull(updates, fieldAccessNode.getReceiver(), symbol);
Nullness nullness = NULLABLE;
boolean fieldMayBeNull;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@ public ClassAndMemberInfo(TreePath path) {

public ClassAndMemberInfo(Tree regionTree) {
// regionTree should either represent a field or a method
Symbol symbol = ASTHelpers.getSymbol(regionTree);
if (!(regionTree instanceof MethodTree
|| (regionTree instanceof VariableTree
&& ASTHelpers.getSymbol(regionTree).getKind().equals(ElementKind.FIELD)))) {
&& symbol != null
&& symbol.getKind().equals(ElementKind.FIELD)))) {
throw new RuntimeException(
"not expecting a region tree " + regionTree + " of type " + regionTree.getClass());
}
this.member = ASTHelpers.getSymbol(regionTree);
this.member = symbol;
this.clazz = ASTHelpers.enclosingClass(this.member);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,11 @@ public void onNonNullFieldAssignment(
if (pathToMethod == null || pathToMethod.getLeaf().getKind() != Tree.Kind.METHOD) {
return;
}
Symbol.MethodSymbol methodSymbol =
(Symbol.MethodSymbol) ASTHelpers.getSymbol(pathToMethod.getLeaf());
Symbol symbol = ASTHelpers.getSymbol(pathToMethod.getLeaf());
if (!(symbol instanceof Symbol.MethodSymbol)) {
return;
}
Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) symbol;
// Check if the method and the field are in the same class.
if (!field.enclClass().equals(methodSymbol.enclClass())) {
// We don't want m in A.m() to be a candidate initializer for B.f unless A == B
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ public NullnessHint onDataflowVisitMethodInvocation(
Node base = node.getTarget().getReceiver();
// Argument list and types should be already checked by grpcIsMetadataContainsKeyCall
Symbol keyArgSymbol = ASTHelpers.getSymbol(tree.getArguments().get(0));
if (getter != null && keyArgSymbol.getKind().equals(ElementKind.FIELD)) {
if (getter != null
&& keyArgSymbol instanceof Symbol.VarSymbol
&& keyArgSymbol.getKind().equals(ElementKind.FIELD)) {
Symbol.VarSymbol varSymbol = (Symbol.VarSymbol) keyArgSymbol;
String immutableFieldFQN =
varSymbol.enclClass().flatName().toString() + "." + varSymbol.flatName().toString();
Expand Down

0 comments on commit 07a3847

Please sign in to comment.