From 4b80162537b7054c52ddba575f15aaea6f9c7d9e Mon Sep 17 00:00:00 2001 From: Manu Sridharan Date: Tue, 18 Jul 2023 19:39:08 -0700 Subject: [PATCH] Revert "Prepare for Nullable `ASTHelpers.getSymbol` (#733)" This reverts commit 07a384798331d40b90305e3c090a9254892ac009. --- .../src/main/java/com/uber/nullaway/NullAway.java | 13 ++----------- .../java/com/uber/nullaway/NullabilityUtil.java | 5 ++++- .../java/com/uber/nullaway/dataflow/AccessPath.java | 3 +-- .../dataflow/AccessPathNullnessPropagation.java | 2 +- .../fixserialization/out/ClassAndMemberInfo.java | 6 ++---- .../FieldInitializationSerializationHandler.java | 7 ++----- .../com/uber/nullaway/handlers/GrpcHandler.java | 4 +--- 7 files changed, 13 insertions(+), 27 deletions(-) diff --git a/nullaway/src/main/java/com/uber/nullaway/NullAway.java b/nullaway/src/main/java/com/uber/nullaway/NullAway.java index ee15f9b6f4..76bb0d3d61 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullAway.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullAway.java @@ -315,6 +315,7 @@ 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 @@ -333,10 +334,7 @@ private boolean checkMarkingForPath(VisitorState state) { } currentTree = path.getLeaf(); } - Symbol enclosingMarkableSymbol = ASTHelpers.getSymbol(currentTree); - if (enclosingMarkableSymbol == null) { - return false; - } + enclosingMarkableSymbol = ASTHelpers.getSymbol(currentTree); return !codeAnnotationInfo.isSymbolUnannotated(enclosingMarkableSymbol, config); } @@ -2279,13 +2277,6 @@ 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: diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index 54a4c37710..c5e8716e17 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -330,13 +330,16 @@ private static boolean isDirectTypeUseAnnotation(Attribute.TypeCompound t) { /** * Check if a field might be null, based on the type. * - * @param symbol symbol for field + * @param symbol symbol for field; must be non-null * @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)) diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index 4a90c1fbf5..f324a0befb 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -429,8 +429,7 @@ && isBoxingMethod(ASTHelpers.getSymbol(methodInvocationTree))) { case IDENTIFIER: // check for CONST // Check for a constant field (static final) Symbol symbol = ASTHelpers.getSymbol(tree); - if (symbol instanceof Symbol.VarSymbol - && symbol.getKind().equals(ElementKind.FIELD)) { + if (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 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 85fa9ce61c..abb1967c5a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPathNullnessPropagation.java @@ -718,7 +718,7 @@ public TransferResult visitVariableDeclaration( public TransferResult visitFieldAccess( FieldAccessNode fieldAccessNode, TransferInput input) { ReadableUpdates updates = new ReadableUpdates(); - Symbol symbol = Preconditions.checkNotNull(ASTHelpers.getSymbol(fieldAccessNode.getTree())); + Symbol symbol = ASTHelpers.getSymbol(fieldAccessNode.getTree()); setReceiverNonnull(updates, fieldAccessNode.getReceiver(), symbol); Nullness nullness = NULLABLE; boolean fieldMayBeNull; diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java index 258830dad7..5925fbc32a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java @@ -51,15 +51,13 @@ 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 - && symbol != null - && symbol.getKind().equals(ElementKind.FIELD)))) { + && ASTHelpers.getSymbol(regionTree).getKind().equals(ElementKind.FIELD)))) { throw new RuntimeException( "not expecting a region tree " + regionTree + " of type " + regionTree.getClass()); } - this.member = symbol; + this.member = ASTHelpers.getSymbol(regionTree); this.clazz = ASTHelpers.enclosingClass(this.member); } diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.java index be058c938d..66461bcd15 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/FieldInitializationSerializationHandler.java @@ -68,11 +68,8 @@ public void onNonNullFieldAssignment( if (pathToMethod == null || pathToMethod.getLeaf().getKind() != Tree.Kind.METHOD) { return; } - Symbol symbol = ASTHelpers.getSymbol(pathToMethod.getLeaf()); - if (!(symbol instanceof Symbol.MethodSymbol)) { - return; - } - Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) symbol; + Symbol.MethodSymbol methodSymbol = + (Symbol.MethodSymbol) ASTHelpers.getSymbol(pathToMethod.getLeaf()); // 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 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 0e27408718..93beff9624 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java @@ -94,9 +94,7 @@ 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 instanceof Symbol.VarSymbol - && keyArgSymbol.getKind().equals(ElementKind.FIELD)) { + if (getter != null && keyArgSymbol.getKind().equals(ElementKind.FIELD)) { Symbol.VarSymbol varSymbol = (Symbol.VarSymbol) keyArgSymbol; String immutableFieldFQN = varSymbol.enclClass().flatName().toString() + "." + varSymbol.flatName().toString();