diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMethodInfo.java similarity index 50% rename from nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java rename to nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMethodInfo.java index ebbd407c5d..2f671db65c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMethodInfo.java @@ -25,32 +25,32 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ClassTree; import com.sun.source.tree.MethodTree; -import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; import javax.annotation.Nullable; -/** 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 */ +/** Class and method corresponding to a program point at which an error was reported. */ +public class ClassAndMethodInfo { + /** Path to the program point of the reported error */ public final 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; + /** + * Finding values for these properties is costly and are not needed by default, hence, they are + * not {@code final} and are only initialized at request. + */ + @Nullable private MethodTree method; @Nullable private ClassTree clazz; - public ClassAndMemberInfo(TreePath path) { + public ClassAndMethodInfo(TreePath path) { this.path = path; } - /** Finds the class and member where the error / fix is reported according to {@code path}. */ + /** Finds the class and method where the error is reported according to {@code path}. */ public void findValues() { - 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). - enclosingMethod = + method = path.getLeaf() instanceof MethodTree ? (MethodTree) path.getLeaf() : ASTHelpers.findEnclosingNode(path, MethodTree.class); @@ -60,52 +60,30 @@ public void findValues() { path.getLeaf() instanceof ClassTree ? (ClassTree) path.getLeaf() : ASTHelpers.findEnclosingNode(path, ClassTree.class); - if (clazz != null) { + if (clazz != null && method != null) { + // It is possible that the computed method is not enclosed by the computed class, e.g., for + // the following case: + // class C { + // void foo() { + // class Local { + // Object f = null; // error + // } + // } + // } + // Here the above code will compute clazz to be Local and method as foo(). In such cases, set + // method to null, we always want the corresponding method to be nested in the corresponding + // class. Symbol.ClassSymbol classSymbol = ASTHelpers.getSymbol(clazz); - if (enclosingMethod != null) { - // It is possible that the computed method is not enclosed by the computed class, e.g., for - // the following case: - // class C { - // void foo() { - // class Local { - // Object f = null; // error - // } - // } - // } - // Here the above code will compute clazz to be Local and method as foo(). In such cases, - // 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)) { - enclosingMethod = null; - } - } - if (enclosingMethod != null) { - member = ASTHelpers.getSymbol(enclosingMethod); - } else { - // 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()) { - // Directly on a field declaration. - fieldSymbol = (Symbol.VarSymbol) sym; - } else { - // Can be enclosed by a field declaration tree. - VariableTree fieldDeclTree = ASTHelpers.findEnclosingNode(path, VariableTree.class); - if (fieldDeclTree != null) { - fieldSymbol = ASTHelpers.getSymbol(fieldDeclTree); - } - } - if (fieldSymbol != null && fieldSymbol.isEnclosedBy(classSymbol)) { - member = fieldSymbol; - } + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(method); + if (!methodSymbol.isEnclosedBy(classSymbol)) { + method = null; } } } @Nullable - public Symbol getMember() { - return member; + public MethodTree getMethod() { + return method; } @Nullable 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 7127b82834..07b00e38c4 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 @@ -36,7 +36,7 @@ public class ErrorInfo { private final ErrorMessage errorMessage; - private final ClassAndMemberInfo classAndMemberInfo; + private final ClassAndMethodInfo classAndMethodInfo; /** * if non-null, this error involved a pseudo-assignment of a @Nullable expression into a @NonNull @@ -60,7 +60,7 @@ public class ErrorInfo { '\r', 'r'); public ErrorInfo(TreePath path, ErrorMessage errorMessage, @Nullable Symbol nonnullTarget) { - this.classAndMemberInfo = new ClassAndMemberInfo(path); + this.classAndMethodInfo = new ClassAndMethodInfo(path); this.errorMessage = errorMessage; this.nonnullTarget = nonnullTarget; } @@ -97,20 +97,20 @@ public String tabSeparatedToString() { "\t", errorMessage.getMessageType().toString(), escapeSpecialCharacters(errorMessage.getMessage()), - (classAndMemberInfo.getClazz() != null - ? ASTHelpers.getSymbol(classAndMemberInfo.getClazz()).flatName() + (classAndMethodInfo.getClazz() != null + ? ASTHelpers.getSymbol(classAndMethodInfo.getClazz()).flatName() : "null"), - (classAndMemberInfo.getMember() != null - ? classAndMemberInfo.getMember().toString() + (classAndMethodInfo.getMethod() != null + ? ASTHelpers.getSymbol(classAndMethodInfo.getMethod()).toString() : "null"), (nonnullTarget != null ? SymbolLocation.createLocationFromSymbol(nonnullTarget).tabSeparatedToString() : EMPTY_NONNULL_TARGET_LOCATION_STRING)); } - /** Finds the class and member of program point where the error is reported. */ + /** Finds the class and method of program point where the error is reported. */ public void initEnclosing() { - classAndMemberInfo.findValues(); + classAndMethodInfo.findValues(); } /** @@ -125,7 +125,7 @@ public static String header() { "message_type", "message", "enc_class", - "enc_member", + "enc_method", "target_kind", "target_class", "target_method", 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..c00a0a751d 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 @@ -36,13 +36,13 @@ public class SuggestedNullableFixInfo { /** Error which will be resolved by this type change. */ private final ErrorMessage errorMessage; - private final ClassAndMemberInfo classAndMemberInfo; + private final ClassAndMethodInfo classAndMethodInfo; public SuggestedNullableFixInfo( TreePath path, SymbolLocation symbolLocation, ErrorMessage errorMessage) { + this.classAndMethodInfo = new ClassAndMethodInfo(path); this.symbolLocation = symbolLocation; this.errorMessage = errorMessage; - this.classAndMemberInfo = new ClassAndMemberInfo(path); } @Override @@ -76,17 +76,17 @@ public String tabSeparatedToString() { symbolLocation.tabSeparatedToString(), errorMessage.getMessageType().toString(), "nullable", - (classAndMemberInfo.getClazz() == null + (classAndMethodInfo.getClazz() == null ? "null" - : ASTHelpers.getSymbol(classAndMemberInfo.getClazz()).flatName()), - (classAndMemberInfo.getMember() == null + : ASTHelpers.getSymbol(classAndMethodInfo.getClazz()).flatName()), + (classAndMethodInfo.getMethod() == null ? "null" - : classAndMemberInfo.getMember().toString())); + : ASTHelpers.getSymbol(classAndMethodInfo.getMethod()).toString())); } - /** Finds the class and member of program point where triggered this type change. */ + /** Finds the class and method of program point where triggered this type change. */ public void initEnclosing() { - classAndMemberInfo.findValues(); + classAndMethodInfo.findValues(); } /** diff --git a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java b/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java index 2e49f878d9..019726c100 100644 --- a/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java +++ b/nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java @@ -1369,105 +1369,6 @@ public void errorSerializationTestLocalClassField() { .doTest(); } - @Test - public void errorSerializationTestEnclosedByFieldDeclaration() { - SerializationTestHelper tester = new SerializationTestHelper<>(root); - tester - .setArgs( - Arrays.asList( - "-d", - temporaryFolder.getRoot().getAbsolutePath(), - "-XepOpt:NullAway:AnnotatedPackages=com.uber", - "-XepOpt:NullAway:SerializeFixMetadata=true", - "-XepOpt:NullAway:FixSerializationConfigPath=" + configPath)) - .addSourceLines( - "com/uber/Main.java", - "package com.uber;", - "import javax.annotation.Nullable;", - "public class Main {", - " // BUG: Diagnostic contains: passing @Nullable parameter", - " Foo f = new Foo(null);", // Member should be "f" - " // BUG: Diagnostic contains: assigning @Nullable expression", - " Foo f1 = null;", // Member should be "f1" - " // BUG: Diagnostic contains: assigning @Nullable expression", - " static Foo f2 = null;", // Member should be "f2" - " static {", - " // BUG: Diagnostic contains: assigning @Nullable expression", - " f2 = null;", // Member should be "null" (not field or method) - " }", - " class Inner {", - " // BUG: Diagnostic contains: passing @Nullable parameter", - " Foo f = new Foo(null);", // Member should be "f" but class is Main$Inner - " }", - "}") - .addSourceLines( - "com/uber/Foo.java", - "package com.uber;", - "public class Foo {", - " public Foo(Object foo){", - " }", - "}") - .setExpectedOutputs( - new ErrorDisplay( - "PASS_NULLABLE", - "passing @Nullable parameter", - "com.uber.Main", - "f", - "PARAMETER", - "com.uber.Foo", - "Foo(java.lang.Object)", - "foo", - "0", - "com/uber/Foo.java"), - new ErrorDisplay( - "ASSIGN_FIELD_NULLABLE", - "assigning @Nullable expression to @NonNull field", - "com.uber.Main", - "f1", - "FIELD", - "com.uber.Main", - "null", - "f1", - "null", - "com/uber/Main.java"), - new ErrorDisplay( - "ASSIGN_FIELD_NULLABLE", - "assigning @Nullable expression to @NonNull field", - "com.uber.Main", - "f2", - "FIELD", - "com.uber.Main", - "null", - "f2", - "null", - "com/uber/Main.java"), - new ErrorDisplay( - "ASSIGN_FIELD_NULLABLE", - "assigning @Nullable expression to @NonNull field", - "com.uber.Main", - "null", - "FIELD", - "com.uber.Main", - "null", - "f2", - "null", - "com/uber/Main.java"), - new ErrorDisplay( - "PASS_NULLABLE", - "passing @Nullable parameter", - "com.uber.Main$Inner", - "f", - "PARAMETER", - "com.uber.Foo", - "Foo(java.lang.Object)", - "foo", - "0", - "com/uber/Foo.java")) - .setFactory(errorDisplayFactory) - .setOutputFileNameAndHeader(ERROR_FILE_NAME, ERROR_FILE_HEADER) - .doTest(); - } - @Test public void suggestNullableArgumentOnBytecode() { SerializationTestHelper tester = new SerializationTestHelper<>(root); diff --git a/nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java b/nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java index 106482336b..05317462e8 100644 --- a/nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java +++ b/nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java @@ -30,7 +30,7 @@ public class ErrorDisplay implements Display { public final String type; public final String message; - public final String encMember; + public final String encMethod; public final String encClass; public final String kind; public final String clazz; @@ -43,7 +43,7 @@ public ErrorDisplay( String type, String message, String encClass, - String encMember, + String encMethod, String kind, String clazz, String method, @@ -52,7 +52,7 @@ public ErrorDisplay( String uri) { this.type = type; this.message = message; - this.encMember = encMember; + this.encMethod = encMethod; this.encClass = encClass; this.kind = kind; this.clazz = clazz; @@ -63,8 +63,8 @@ public ErrorDisplay( this.uri = uri.contains("com/uber/") ? uri.substring(uri.indexOf("com/uber/")) : uri; } - public ErrorDisplay(String type, String message, String encClass, String encMember) { - this(type, message, encClass, encMember, "null", "null", "null", "null", "null", "null"); + public ErrorDisplay(String type, String message, String encClass, String encMethod) { + this(type, message, encClass, encMethod, "null", "null", "null", "null", "null", "null"); } @Override @@ -80,10 +80,10 @@ public boolean equals(Object o) { // To increase readability, a shorter version of the actual message might be present in the // expected output of tests. && (message.contains(that.message) || that.message.contains(message)) - && encMember.equals(that.encMember) - && clazz.equals(that.clazz) + && encMethod.equals(that.encMethod) && encClass.equals(that.encClass) && kind.equals(that.kind) + && clazz.equals(that.clazz) && method.equals(that.method) && variable.equals(that.variable) && index.equals(that.index) @@ -93,7 +93,7 @@ public boolean equals(Object o) { @Override public int hashCode() { return Objects.hash( - type, message, encMember, encClass, kind, clazz, method, variable, index, uri); + type, message, encMethod, encClass, kind, clazz, method, variable, index, uri); } @Override @@ -105,8 +105,8 @@ public String toString() { + "\n\tmessage='" + message + '\'' - + "\n\tencMember='" - + encMember + + "\n\tencMethod='" + + encMethod + '\'' + "\n\tencClass='" + encClass