diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java b/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java index 1c5b0b28ff..d35424b9c6 100755 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorBuilder.java @@ -139,7 +139,18 @@ public Description createErrorDescription( if (nonNullTarget != null) { SerializationService.serializeFixSuggestion(config, state, nonNullTarget, errorMessage); } - SerializationService.serializeReportingError(config, state, nonNullTarget, errorMessage); + // For the case of initializer errors, the leaf of state.getPath() may not be the field / + // method on which the error is being reported (since we do a class-wide analysis to find such + // errors). In such cases, the suggestTree is the appropriate field / method tree, so use + // that as the errorTree for serialization. + Tree errorTree = + (suggestTree != null + && (errorMessage.messageType.equals(FIELD_NO_INIT) + || errorMessage.messageType.equals(METHOD_NO_INIT))) + ? suggestTree + : state.getPath().getLeaf(); + SerializationService.serializeReportingError( + config, state, errorTree, nonNullTarget, errorMessage); } // #letbuildersbuild diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java index d832fbfa90..0074c1c237 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/SerializationService.java @@ -25,6 +25,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.errorprone.VisitorState; +import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; import com.sun.source.util.Trees; import com.sun.tools.javac.code.Symbol; @@ -114,14 +115,19 @@ public static void serializeFixSuggestion( * * @param config NullAway config. * @param state Visitor state. + * @param errorTree Tree of the element involved in the reporting error. * @param errorMessage Error caused by the target. */ public static void serializeReportingError( - Config config, VisitorState state, @Nullable Symbol target, ErrorMessage errorMessage) { + Config config, + VisitorState state, + Tree errorTree, + @Nullable Symbol target, + ErrorMessage errorMessage) { Serializer serializer = config.getSerializationConfig().getSerializer(); Preconditions.checkNotNull( serializer, "Serializer shouldn't be null at this point, error in configuration setting!"); - serializer.serializeErrorInfo(new ErrorInfo(state.getPath(), errorMessage, target)); + serializer.serializeErrorInfo(new ErrorInfo(state.getPath(), errorTree, errorMessage, target)); } /** 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 b9a44bae75..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 @@ -22,31 +22,51 @@ package com.uber.nullaway.fixserialization.out; +import com.google.common.base.Preconditions; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ClassTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.Tree; import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import javax.annotation.Nullable; +import javax.lang.model.element.ElementKind; /** Class and member corresponding to a program point at which an error / fix was reported. */ public class ClassAndMemberInfo { /** Path to the program point of the reported error / fix */ - public final TreePath path; + @Nullable public TreePath path; // Finding values for these properties is costly and are not needed by default, hence, they are // not final and are only initialized at request. @Nullable private Symbol member; - @Nullable private ClassTree clazz; + @Nullable private Symbol.ClassSymbol clazz; public ClassAndMemberInfo(TreePath path) { + Preconditions.checkNotNull(path); this.path = path; } + public ClassAndMemberInfo(Tree regionTree) { + // regionTree should either represent a field or a method + if (!(regionTree instanceof MethodTree + || (regionTree instanceof VariableTree + && ASTHelpers.getSymbol(regionTree).getKind().equals(ElementKind.FIELD)))) { + throw new RuntimeException( + "not expecting a region tree " + regionTree + " of type " + regionTree.getClass()); + } + this.member = ASTHelpers.getSymbol(regionTree); + this.clazz = ASTHelpers.enclosingClass(this.member); + } + /** Finds the class and member where the error / fix is reported according to {@code path}. */ public void findValues() { + if (this.member != null || path == null) { + // Values are already computed. + return; + } MethodTree enclosingMethod; // If the error is reported on a method, that method itself is the relevant program point. // Otherwise, use the enclosing method (if present). @@ -56,12 +76,12 @@ public void findValues() { : ASTHelpers.findEnclosingNode(path, MethodTree.class); // If the error is reported on a class, that class itself is the relevant program point. // Otherwise, use the enclosing class. - clazz = + ClassTree classTree = path.getLeaf() instanceof ClassTree ? (ClassTree) path.getLeaf() : ASTHelpers.findEnclosingNode(path, ClassTree.class); - if (clazz != null) { - Symbol.ClassSymbol classSymbol = ASTHelpers.getSymbol(clazz); + if (classTree != null) { + clazz = ASTHelpers.getSymbol(classTree); if (enclosingMethod != null) { // It is possible that the computed method is not enclosed by the computed class, e.g., for // the following case: @@ -76,7 +96,7 @@ public void findValues() { // set method to null, we always want the corresponding method to be nested in the // corresponding class. Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(enclosingMethod); - if (!methodSymbol.isEnclosedBy(classSymbol)) { + if (!methodSymbol.isEnclosedBy(clazz)) { enclosingMethod = null; } } @@ -86,7 +106,7 @@ public void findValues() { // Node is not enclosed by any method, can be a field declaration or enclosed by it. Symbol sym = ASTHelpers.getSymbol(path.getLeaf()); Symbol.VarSymbol fieldSymbol = null; - if (sym != null && sym.getKind().isField() && sym.isEnclosedBy(classSymbol)) { + if (sym != null && sym.getKind().isField() && sym.isEnclosedBy(clazz)) { // Directly on a field declaration. fieldSymbol = (Symbol.VarSymbol) sym; } else { @@ -96,7 +116,7 @@ public void findValues() { fieldSymbol = ASTHelpers.getSymbol(fieldDeclTree); } } - if (fieldSymbol != null && fieldSymbol.isEnclosedBy(classSymbol)) { + if (fieldSymbol != null && fieldSymbol.isEnclosedBy(clazz)) { member = fieldSymbol; } } @@ -109,7 +129,7 @@ public Symbol getMember() { } @Nullable - public ClassTree getClazz() { + public Symbol.ClassSymbol getClazz() { return clazz; } } diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java index 73fb35fb84..a0ca6ee703 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java @@ -22,7 +22,10 @@ package com.uber.nullaway.fixserialization.out; -import com.google.errorprone.util.ASTHelpers; +import static com.uber.nullaway.ErrorMessage.MessageTypes.FIELD_NO_INIT; +import static com.uber.nullaway.ErrorMessage.MessageTypes.METHOD_NO_INIT; + +import com.sun.source.tree.Tree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.util.JCDiagnostic; @@ -53,11 +56,16 @@ public class ErrorInfo { /** Uri to the containing source file where this error is reported. */ private final URI uri; - public ErrorInfo(TreePath path, ErrorMessage errorMessage, @Nullable Symbol nonnullTarget) { - this.classAndMemberInfo = new ClassAndMemberInfo(path); + public ErrorInfo( + TreePath path, Tree errorTree, ErrorMessage errorMessage, @Nullable Symbol nonnullTarget) { + this.classAndMemberInfo = + (errorMessage.getMessageType().equals(FIELD_NO_INIT) + || errorMessage.getMessageType().equals(METHOD_NO_INIT)) + ? new ClassAndMemberInfo(errorTree) + : new ClassAndMemberInfo(path); this.errorMessage = errorMessage; this.nonnullTarget = nonnullTarget; - JCDiagnostic.DiagnosticPosition treePosition = (JCDiagnostic.DiagnosticPosition) path.getLeaf(); + JCDiagnostic.DiagnosticPosition treePosition = (JCDiagnostic.DiagnosticPosition) errorTree; this.offset = treePosition.getStartPosition(); this.uri = path.getCompilationUnit().getSourceFile().toUri(); } @@ -88,9 +96,7 @@ public Symbol getRegionMember() { */ @Nullable public Symbol getRegionClass() { - return classAndMemberInfo.getClazz() == null - ? null - : ASTHelpers.getSymbol(classAndMemberInfo.getClazz()); + return classAndMemberInfo.getClazz(); } /** diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java index 8232ced29b..284433980e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java @@ -22,7 +22,6 @@ package com.uber.nullaway.fixserialization.out; -import com.google.errorprone.util.ASTHelpers; import com.sun.source.util.TreePath; import com.uber.nullaway.ErrorMessage; import com.uber.nullaway.fixserialization.location.SymbolLocation; @@ -76,9 +75,7 @@ public String tabSeparatedToString() { symbolLocation.tabSeparatedToString(), errorMessage.getMessageType().toString(), "nullable", - (classAndMemberInfo.getClazz() == null - ? "null" - : ASTHelpers.getSymbol(classAndMemberInfo.getClazz()).flatName()), + (classAndMemberInfo.getClazz() == null ? "null" : classAndMemberInfo.getClazz().flatName()), (classAndMemberInfo.getMember() == null ? "null" : classAndMemberInfo.getMember().toString())); diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java index a5d15c0db1..6631b22a75 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java @@ -678,8 +678,8 @@ public void errorSerializationTest() { "METHOD_NO_INIT", "initializer method does not guarantee @NonNull field foo", "com.uber.Super", - "null", - 52, + "Super(boolean)", + 180, "com/uber/Super.java"), new ErrorDisplay( "ASSIGN_FIELD_NULLABLE", @@ -1370,8 +1370,8 @@ public void errorSerializationTestAnonymousClassField() { "FIELD_NO_INIT", "@NonNull field Main$1.bar not initialized", "com.uber.Main$1", - "null", - 119, + "bar", + 206, "com/uber/Main.java", "FIELD", "com.uber.Main$1", @@ -1412,8 +1412,8 @@ public void errorSerializationTestLocalClassField() { "FIELD_NO_INIT", "@NonNull field Main$1Foo.bar not initialized", "com.uber.Main$1Foo", - "null", - 99, + "bar", + 199, "com/uber/Main.java", "FIELD", "com.uber.Main$1Foo", @@ -1923,7 +1923,7 @@ public void errorSerializationVersion1() { "METHOD_NO_INIT", "initializer method does not guarantee @NonNull field foo", "com.uber.Super", - "null"), + "Super(boolean)"), new ErrorDisplayV1( "ASSIGN_FIELD_NULLABLE", "assigning @Nullable expression to @NonNull field",