Fix #657 by splitting field initialization region into smaller regions

nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMethodInfo.java → nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ClassAndMemberInfo.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 method corresponding to a program point at which an error was reported. */
-public class ClassAndMethodInfo {
-  /** Path to the program point of the reported error */
+/** 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;
 
-  /**
-   * 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;
+  // 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;
 
-  public ClassAndMethodInfo(TreePath path) {
+  public ClassAndMemberInfo(TreePath path) {
     this.path = path;
   }
 
-  /** Finds the class and method where the error is reported according to {@code path}. */
+  /** Finds the class and member where the error / fix 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).
-    method =
+    enclosingMethod =
         path.getLeaf() instanceof MethodTree
             ? (MethodTree) path.getLeaf()
             : ASTHelpers.findEnclosingNode(path, MethodTree.class);
@@ -60,30 +60,52 @@ public void findValues() {
         path.getLeaf() instanceof ClassTree
             ? (ClassTree) path.getLeaf()
             : ASTHelpers.findEnclosingNode(path, ClassTree.class);
-    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.
+    if (clazz != null) {
       Symbol.ClassSymbol classSymbol = ASTHelpers.getSymbol(clazz);
-      Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(method);
-      if (!methodSymbol.isEnclosedBy(classSymbol)) {
-        method = null;
+      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;
+        }
       }
     }
   }
 
   @Nullable
-  public MethodTree getMethod() {
-    return method;
+  public Symbol getMember() {
+    return member;
   }
 
   @Nullable

nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java

@@ -36,7 +36,7 @@
 public class ErrorInfo {
 
   private final ErrorMessage errorMessage;
-  private final ClassAndMethodInfo classAndMethodInfo;
+  private final ClassAndMemberInfo classAndMemberInfo;
 
   /**
    * 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.classAndMethodInfo = new ClassAndMethodInfo(path);
+    this.classAndMemberInfo = new ClassAndMemberInfo(path);
     this.errorMessage = errorMessage;
     this.nonnullTarget = nonnullTarget;
   }
@@ -97,20 +97,20 @@ public String tabSeparatedToString() {
         "\t",
         errorMessage.getMessageType().toString(),
         escapeSpecialCharacters(errorMessage.getMessage()),
-        (classAndMethodInfo.getClazz() != null
-            ? ASTHelpers.getSymbol(classAndMethodInfo.getClazz()).flatName()
+        (classAndMemberInfo.getClazz() != null
+            ? ASTHelpers.getSymbol(classAndMemberInfo.getClazz()).flatName()
             : "null"),
-        (classAndMethodInfo.getMethod() != null
-            ? ASTHelpers.getSymbol(classAndMethodInfo.getMethod()).toString()
+        (classAndMemberInfo.getMember() != null
+            ? classAndMemberInfo.getMember().toString()
             : "null"),
         (nonnullTarget != null
             ? SymbolLocation.createLocationFromSymbol(nonnullTarget).tabSeparatedToString()
             : EMPTY_NONNULL_TARGET_LOCATION_STRING));
   }
 
-  /** Finds the class and method of program point where the error is reported. */
+  /** Finds the class and member of program point where the error is reported. */
   public void initEnclosing() {
-    classAndMethodInfo.findValues();
+    classAndMemberInfo.findValues();
   }
 
   /**
@@ -125,7 +125,7 @@ public static String header() {
         "message_type",
         "message",
         "enc_class",
-        "enc_method",
+        "enc_member",
         "target_kind",
         "target_class",
         "target_method",

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 ClassAndMethodInfo classAndMethodInfo;
+  private final ClassAndMemberInfo classAndMemberInfo;
 
   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",
-        (classAndMethodInfo.getClazz() == null
+        (classAndMemberInfo.getClazz() == null
             ? "null"
-            : ASTHelpers.getSymbol(classAndMethodInfo.getClazz()).flatName()),
-        (classAndMethodInfo.getMethod() == null
+            : ASTHelpers.getSymbol(classAndMemberInfo.getClazz()).flatName()),
+        (classAndMemberInfo.getMember() == null
             ? "null"
-            : ASTHelpers.getSymbol(classAndMethodInfo.getMethod()).toString()));
+            : classAndMemberInfo.getMember().toString()));
   }
 
-  /** Finds the class and method of program point where triggered this type change. */
+  /** Finds the class and member of program point where triggered this type change. */
   public void initEnclosing() {
-    classAndMethodInfo.findValues();
+    classAndMemberInfo.findValues();
   }
 
   /**

nullaway/src/test/java/com/uber/nullaway/NullAwaySerializationTest.java

@@ -1369,6 +1369,105 @@ public void errorSerializationTestLocalClassField() {
         .doTest();
   }
 
+  @Test
+  public void errorSerializationTestEnclosedByFieldDeclaration() {
+    SerializationTestHelper<ErrorDisplay> 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<FixDisplay> tester = new SerializationTestHelper<>(root);

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 encMethod;
+  public final String encMember;
   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 encMethod,
+      String encMember,
       String kind,
       String clazz,
       String method,
@@ -52,7 +52,7 @@ public ErrorDisplay(
       String uri) {
     this.type = type;
     this.message = message;
-    this.encMethod = encMethod;
+    this.encMember = encMember;
     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 encMethod) {
-    this(type, message, encClass, encMethod, "null", "null", "null", "null", "null", "null");
+  public ErrorDisplay(String type, String message, String encClass, String encMember) {
+    this(type, message, encClass, encMember, "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))
-        && encMethod.equals(that.encMethod)
+        && encMember.equals(that.encMember)
+        && clazz.equals(that.clazz)
         && 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, encMethod, encClass, kind, clazz, method, variable, index, uri);
+        type, message, encMember, encClass, kind, clazz, method, variable, index, uri);
   }
 
   @Override
@@ -105,8 +105,8 @@ public String toString() {
         + "\n\tmessage='"
         + message
         + '\''
-        + "\n\tencMethod='"
-        + encMethod
+        + "\n\tencMember='"
+        + encMember
         + '\''
         + "\n\tencClass='"
         + encClass