[FixSerialization] Update path serialization for reported errors and …

…fixes. (#714)

This PR updates serialization services to serialize the actual `Path` instead of `URI` for all reporting errors and fixes.

nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java

@@ -31,6 +31,7 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.Writer;
+import java.net.URI;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -164,4 +165,34 @@ private void appendToFile(String row, Path path) {
       throw new RuntimeException("Error happened for writing at file: " + path, e);
     }
   }
+
+  /**
+   * Converts the given uri to the real path. Note, in NullAway CI tests, source files exists in
+   * memory and there is no real path leading to those files. Instead, we just serialize the path
+   * from uri as the full paths are not checked in tests.
+   *
+   * @param uri Given uri.
+   * @return Real path for the give uri.
+   */
+  @Nullable
+  public static Path pathToSourceFileFromURI(@Nullable URI uri) {
+    if (uri == null) {
+      return null;
+    }
+    if ("jimfs".equals(uri.getScheme())) {
+      // In NullAway unit tests, files are stored in memory and have this scheme.
+      return Paths.get(uri);
+    }
+    if (!"file".equals(uri.getScheme())) {
+      return null;
+    }
+    Path path = Paths.get(uri);
+    try {
+      return path.toRealPath();
+    } catch (IOException e) {
+      // In this case, we still would like to continue the serialization instead of returning null
+      // and not serializing anything.
+      return path;
+    }
+  }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/adapters/SerializationV2Adapter.java

@@ -51,13 +51,13 @@ public String getErrorsOutputFileHeader() {
         "enc_class",
         "enc_member",
         "offset",
-        "uri",
+        "path",
         "target_kind",
         "target_class",
         "target_method",
         "target_param",
         "target_index",
-        "target_uri");
+        "target_path");
   }
 
   @Override
@@ -71,7 +71,7 @@ public String serializeError(ErrorInfo errorInfo) {
             : "null"),
         (errorInfo.getRegionMember() != null ? errorInfo.getRegionMember().toString() : "null"),
         String.valueOf(errorInfo.getOffset()),
-        errorInfo.getUri().getPath(),
+        errorInfo.getPath() != null ? errorInfo.getPath().toString() : "null",
         (errorInfo.getNonnullTarget() != null
             ? SymbolLocation.createLocationFromSymbol(errorInfo.getNonnullTarget())
                 .tabSeparatedToString()

nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java

@@ -27,7 +27,9 @@
 import com.google.common.base.Preconditions;
 import com.google.errorprone.util.ASTHelpers;
 import com.sun.tools.javac.code.Symbol;
+import com.uber.nullaway.fixserialization.Serializer;
 import java.net.URI;
+import java.nio.file.Path;
 import javax.annotation.Nullable;
 import javax.lang.model.element.ElementKind;
 
@@ -36,8 +38,8 @@ public abstract class AbstractSymbolLocation implements SymbolLocation {
 
   /** Element kind of the targeted symbol */
   protected final ElementKind type;
-  /** URI of the file containing the symbol, if available. */
-  @Nullable protected final URI uri;
+  /** Path of the file containing the symbol, if available. */
+  @Nullable protected final Path path;
   /** Enclosing class of the symbol. */
   protected final Symbol.ClassSymbol enclosingClass;
 
@@ -51,9 +53,15 @@ public AbstractSymbolLocation(ElementKind type, Symbol target) {
             + ".");
     this.type = type;
     this.enclosingClass = castToNonNull(ASTHelpers.enclosingClass(target));
-    this.uri =
+    // We currently serialize the URI for the classfile if the URI for the sourcefile is not
+    // available, but only if said URI corresponds to a "file:" or "jimfs:" scheme (i.e. not
+    // "jar:"). It's likely that this is no longer needed and should be removed as a follow up:
+    // https://github.com/uber/NullAway/issues/716 Leaving this workaround up temporarily for the
+    // sake of experiments with version `1.3.6-alpha-N` of the auto-annotator.
+    URI pathInURI =
         enclosingClass.sourcefile != null
             ? enclosingClass.sourcefile.toUri()
             : (enclosingClass.classfile != null ? enclosingClass.classfile.toUri() : null);
+    this.path = Serializer.pathToSourceFileFromURI(pathInURI);
   }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/location/FieldLocation.java

@@ -45,6 +45,6 @@ public String tabSeparatedToString() {
         "null",
         variableSymbol.toString(),
         "null",
-        uri != null ? uri.toASCIIString() : "null");
+        path != null ? path.toString() : "null");
   }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodLocation.java

@@ -45,6 +45,6 @@ public String tabSeparatedToString() {
         enclosingMethod.toString(),
         "null",
         "null",
-        uri != null ? uri.toASCIIString() : "null");
+        path != null ? path.toString() : "null");
   }
 }

nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java

@@ -66,6 +66,6 @@ public String tabSeparatedToString() {
         enclosingMethod.toString(),
         paramSymbol.toString(),
         String.valueOf(index),
-        uri != null ? uri.toASCIIString() : "null");
+        path != null ? path.toString() : "null");
   }
 }

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

@@ -30,7 +30,8 @@
 import com.sun.tools.javac.code.Symbol;
 import com.sun.tools.javac.util.JCDiagnostic;
 import com.uber.nullaway.ErrorMessage;
-import java.net.URI;
+import com.uber.nullaway.fixserialization.Serializer;
+import java.nio.file.Path;
 import javax.annotation.Nullable;
 
 /** Stores information regarding an error which will be reported by NullAway. */
@@ -53,8 +54,8 @@ public class ErrorInfo {
 
   /** Offset of program point where this error is reported. */
   private final int offset;
-  /** Uri to the containing source file where this error is reported. */
-  private final URI uri;
+  /** Path to the containing source file where this error is reported. */
+  @Nullable private final Path path;
 
   public ErrorInfo(
       TreePath path, Tree errorTree, ErrorMessage errorMessage, @Nullable Symbol nonnullTarget) {
@@ -67,7 +68,8 @@ public ErrorInfo(
     this.nonnullTarget = nonnullTarget;
     JCDiagnostic.DiagnosticPosition treePosition = (JCDiagnostic.DiagnosticPosition) errorTree;
     this.offset = treePosition.getStartPosition();
-    this.uri = path.getCompilationUnit().getSourceFile().toUri();
+    this.path =
+        Serializer.pathToSourceFileFromURI(path.getCompilationUnit().getSourceFile().toUri());
   }
 
   /**
@@ -121,12 +123,13 @@ public int getOffset() {
   }
 
   /**
-   * Returns URI to the containing source file where this error is reported.
+   * Returns Path to the containing source file where this error is reported.
    *
-   * @return URI to the containing source file where this error is reported.
+   * @return Path to the containing source file where this error is reported.
    */
-  public URI getUri() {
-    return uri;
+  @Nullable
+  public Path getPath() {
+    return path;
   }
 
   /** Finds the class and member of program point where the error is reported. */

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

@@ -79,12 +79,13 @@ public NullAwaySerializationTest() {
               "Needs exactly 10 values to create FixDisplay object but found: " + values.length);
           // Fixes are written in Temp Directory and is not known at compile time, therefore,
           // relative paths are getting compared.
-          FixDisplay display =
-              new FixDisplay(values[7], values[2], values[3], values[0], values[1], values[5]);
-          if (display.uri.contains("com/uber/")) {
-            display.uri = display.uri.substring(display.uri.indexOf("com/uber/"));
-          }
-          return display;
+          return new FixDisplay(
+              values[7],
+              values[2],
+              values[3],
+              values[0],
+              values[1],
+              SerializationTestHelper.getRelativePathFromUnitTestTempDirectory(values[5]));
         };
     this.errorDisplayFactory =
         values -> {
@@ -97,27 +98,27 @@ public NullAwaySerializationTest() {
               values[2],
               values[3],
               Integer.parseInt(values[4]),
-              values[5],
+              SerializationTestHelper.getRelativePathFromUnitTestTempDirectory(values[5]),
               values[6],
               values[7],
               values[8],
               values[9],
               values[10],
-              values[11]);
+              SerializationTestHelper.getRelativePathFromUnitTestTempDirectory(values[11]));
         };
     this.fieldInitDisplayFactory =
         values -> {
           Preconditions.checkArgument(
               values.length == 7,
               "Needs exactly 7 values to create FieldInitDisplay object but found: "
                   + values.length);
-          FieldInitDisplay display =
-              new FieldInitDisplay(
-                  values[6], values[2], values[3], values[0], values[1], values[5]);
-          if (display.uri.contains("com/uber/")) {
-            display.uri = display.uri.substring(display.uri.indexOf("com/uber/"));
-          }
-          return display;
+          return new FieldInitDisplay(
+              values[6],
+              values[2],
+              values[3],
+              values[0],
+              values[1],
+              SerializationTestHelper.getRelativePathFromUnitTestTempDirectory(values[5]));
         };
   }
 
@@ -1865,8 +1866,16 @@ public void errorSerializationVersion1() {
               "Needs exactly 10 values to create ErrorDisplay for version 1 object but found: "
                   + values.length);
           return new ErrorDisplayV1(
-              values[0], values[1], values[2], values[3], values[4], values[5], values[6],
-              values[7], values[8], values[9]);
+              values[0],
+              values[1],
+              values[2],
+              values[3],
+              values[4],
+              values[5],
+              values[6],
+              values[7],
+              values[8],
+              SerializationTestHelper.getRelativePathFromUnitTestTempDirectory(values[9]));
         };
     SerializationTestHelper<ErrorDisplayV1> tester = new SerializationTestHelper<>(root);
     tester

nullaway/src/test/java/com/uber/nullaway/tools/ErrorDisplay.java

@@ -39,7 +39,7 @@ public class ErrorDisplay implements Display {
   public final String method;
   public final String variable;
   public final String index;
-  public final String uri;
+  public final String nonElementPath;
 
   public ErrorDisplay(
       String type,
@@ -53,21 +53,19 @@ public ErrorDisplay(
       String method,
       String variable,
       String index,
-      String uri) {
+      String nonElementPath) {
     this.type = type;
     this.message = message;
     this.encMember = encMember;
     this.encClass = encClass;
     this.offset = offset;
-    // relative paths are getting compared.
-    this.path = path.contains("com/uber/") ? path.substring(path.indexOf("com/uber/")) : path;
+    this.path = path;
     this.kind = kind;
     this.clazz = clazz;
     this.method = method;
     this.variable = variable;
     this.index = index;
-    // relative paths are getting compared.
-    this.uri = uri.contains("com/uber/") ? uri.substring(uri.indexOf("com/uber/")) : uri;
+    this.nonElementPath = nonElementPath;
   }
 
   public ErrorDisplay(
@@ -94,19 +92,29 @@ public boolean equals(Object o) {
         && clazz.equals(that.clazz)
         && encClass.equals(that.encClass)
         && offset == that.offset
-        && path.equals(that.path)
+        && SerializationTestHelper.pathsAreEqual(path, that.path)
         && kind.equals(that.kind)
         && method.equals(that.method)
         && variable.equals(that.variable)
         && index.equals(that.index)
-        && uri.equals(that.uri);
+        && SerializationTestHelper.pathsAreEqual(nonElementPath, that.nonElementPath);
   }
 
   @Override
   public int hashCode() {
     return Objects.hash(
-        type, message, encMember, encClass, offset, path, kind, clazz, method, variable, index,
-        uri);
+        type,
+        message,
+        encMember,
+        encClass,
+        offset,
+        path,
+        kind,
+        clazz,
+        method,
+        variable,
+        index,
+        nonElementPath);
   }
 
   @Override
@@ -145,8 +153,8 @@ public String toString() {
         + "\n\tindex='"
         + index
         + '\''
-        + "\n\turi='"
-        + uri
+        + "\n\tnonElementPath='"
+        + nonElementPath
         + '\''
         + '}';
   }

nullaway/src/test/java/com/uber/nullaway/tools/FieldInitDisplay.java

@@ -34,16 +34,16 @@ public class FieldInitDisplay implements Display {
   public final String location;
   public final String className;
   public final String field;
-  public String uri;
+  public final String path;
 
   public FieldInitDisplay(
-      String field, String method, String param, String location, String className, String uri) {
+      String field, String method, String param, String location, String className, String path) {
     this.field = field;
     this.method = method;
     this.param = param;
     this.location = location;
     this.className = className;
-    this.uri = uri;
+    this.path = path;
   }
 
   @Override
@@ -60,12 +60,12 @@ public boolean equals(Object o) {
         && Objects.equals(location, that.location)
         && Objects.equals(className, that.className)
         && Objects.equals(field, that.field)
-        && Objects.equals(uri, that.uri);
+        && SerializationTestHelper.pathsAreEqual(path, that.path);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(method, param, location, className, field, uri);
+    return Objects.hash(method, param, location, className, field, path);
   }
 
   @Override
@@ -86,8 +86,8 @@ public String toString() {
         + ", \n\tclassName='"
         + className
         + '\''
-        + ", \n\turi='"
-        + uri
+        + ", \n\tpath='"
+        + path
         + '\''
         + "\n  }\n";
   }