Delete SourceFile.fromGenerated and migrate existing usages

SourceFile now sets the 'original path' of a file on disk as the actual name of the SourceFile. This allows migrating uses of SourceFile.fromGenerated that were only providing a normalized name for a file on disk.

PiperOrigin-RevId: 368076603

BUILD.bazel

@@ -201,6 +201,7 @@ gen_java_tests(
         "@com_google_guava_failureaccess//jar",
         "@com_google_guava_guava//jar",
         "@com_google_guava_guava_testlib//jar",
+        "@com_google_jimfs_jimfs",
         "@com_google_protobuf//:protobuf_java",
         "@com_google_re2j_re2j",
         "@com_google_truth_extensions_truth_liteproto_extension",

WORKSPACE.bazel

@@ -75,6 +75,15 @@ maven_import(
     licenses = ["notice"],
 )
 
+maven_import(
+    # https://github.com/google/jimfs
+    group_id = "com.google.jimfs",
+    artifact_id = "jimfs",
+    version = "1.2",
+    sha256 = "de16d5c8489729a8512f1a02fbd81f58f89249b72066987da4cc5c87ecb9f72d",
+    licenses = ["notice"],
+)
+
 git_repository(
     name = "protobuf_proto_rules",
     commit = "218ffa7dfa5408492dc86c01ee637614f8695c45",

src/com/google/javascript/jscomp/SourceFile.java

@@ -18,6 +18,7 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.lang.Math.max;
 import static java.lang.Math.min;
@@ -87,23 +88,14 @@ public interface Generator {
 
   private SourceKind kind;
 
-  /**
-   * The fileName may not always identify the original file.
-   *
-   * <p>For example, supersourced Java inputs, or Java inputs that come from Jar files. This is an
-   * optional field that the creator of an AST or SourceFile can set. It could be a path to the
-   * original file, or in case this SourceFile came from a Jar, it could be the path to the Jar.
-   */
-  private final String originalPath;
-
   private final CodeLoader loader;
 
   // Source Line Information
   private transient int[] lineOffsets = null;
 
   private transient volatile String code = null;
 
-  private SourceFile(CodeLoader loader, String fileName, String originalPath, SourceKind kind) {
+  private SourceFile(CodeLoader loader, String fileName, SourceKind kind) {
     if (isNullOrEmpty(fileName)) {
       throw new IllegalArgumentException("a source must have a name");
     }
@@ -114,7 +106,6 @@ private SourceFile(CodeLoader loader, String fileName, String originalPath, Sour
 
     this.loader = loader;
     this.fileName = fileName;
-    this.originalPath = originalPath;
     this.kind = kind;
   }
 
@@ -216,8 +207,10 @@ private void setCodeAndDoBookkeeping(String sourceCode) {
     }
   }
 
+  /** @deprecated alias of {@link #getName()}. Use that instead */
+  @Deprecated
   public String getOriginalPath() {
-    return originalPath != null ? originalPath : fileName;
+    return this.getName();
   }
 
   /**
@@ -234,7 +227,11 @@ boolean hasSourceInMemory() {
     return code != null;
   }
 
-  /** Returns a unique name for the source file. */
+  /**
+   * Returns a unique name for the source file.
+   *
+   * <p>This name is not required to be an actual file path on disk.
+   */
   @Override
   public String getName() {
     return fileName;
@@ -544,11 +541,6 @@ public static SourceFile fromReader(String fileName, Reader r)
     return builder().buildFromReader(fileName, r);
   }
 
-  public static SourceFile fromGenerator(String fileName,
-      Generator generator) {
-    return builder().buildFromGenerator(fileName, generator);
-  }
-
   /** Create a new builder for source files. */
   public static Builder builder() {
     return new Builder();
@@ -579,6 +571,18 @@ public Builder withCharset(Charset charset) {
       return this;
     }
 
+    /**
+     * Sets a name for this source file that does not need to correspond to a path on disk.
+     *
+     * <p>Allow passing a reasonable human-readable name in cases like for zip files and for
+     * generated files with unstable artifact prefixes.
+     *
+     * <p>The name must still be unique.
+     *
+     * <p>Required for `buildFromZipEntry`. Optional for `buildFromFile` and `buildFromPath`.
+     * Forbidden for `buildFromInputStream`, `buildFromReader`, and `buildFromCode`, as those
+     * methods already take a source name that does not need to correspond to anything on disk.
+     */
     public Builder withOriginalPath(String originalPath) {
       this.originalPath = originalPath;
       return this;
@@ -597,19 +601,23 @@ public SourceFile buildFromPath(Path path) {
         return fromZipEntry(path.toString(), charset, kind);
       }
       return new SourceFile(
-          new CodeLoader.OnDisk(path, charset), path.toString(), originalPath, kind);
+          new CodeLoader.OnDisk(path, charset),
+          originalPath != null ? originalPath : path.toString(),
+          kind);
     }
 
     @GwtIncompatible("java.io.File")
     public SourceFile buildFromZipEntry(ZipEntryReader zipEntryReader) {
       checkNotNull(zipEntryReader);
       checkNotNull(charset);
-      return new SourceFile(
-          new CodeLoader.AtZip(zipEntryReader, charset), originalPath, originalPath, kind);
+      return new SourceFile(new CodeLoader.AtZip(zipEntryReader, charset), originalPath, kind);
     }
 
     public SourceFile buildFromCode(String fileName, String code) {
-      return new SourceFile(new CodeLoader.Preloaded(code), fileName, originalPath, kind);
+      checkState(
+          originalPath == null,
+          "originalPath argument is ignored in favor of fileName for buildFromCode");
+      return new SourceFile(new CodeLoader.Preloaded(code), fileName, kind);
     }
 
     @GwtIncompatible("java.io.InputStream")
@@ -621,10 +629,6 @@ public SourceFile buildFromInputStream(String fileName, InputStream s) throws IO
     public SourceFile buildFromReader(String fileName, Reader r) throws IOException {
       return buildFromCode(fileName, CharStreams.toString(r));
     }
-
-    public SourceFile buildFromGenerator(String fileName, Generator generator) {
-      return new SourceFile(new CodeLoader.Generated(generator), fileName, originalPath, kind);
-    }
   }
 
   //////////////////////////////////////////////////////////////////////////////
@@ -649,8 +653,6 @@ Reader openUncachedReader() throws IOException {
       return null;
     }
 
-    void restoreFrom(CodeLoader other) {}
-
     static final class Preloaded extends CodeLoader {
       private static final long serialVersionUID = 2L;
       private final String preloadedCode;
@@ -666,29 +668,6 @@ String loadUncachedCode() {
       }
     }
 
-    static final class Generated extends CodeLoader {
-      // Avoid serializing generator and remove the burden to make classes that implement
-      // Generator serializable. There should be no need to obtain generated source in the
-      // second stage of compilation. Making the generator transient relies on not clearing the
-      // code cache for these classes up serialization which might be quite wasteful.
-      private transient Generator generator;
-
-      Generated(Generator generator) {
-        super();
-        this.generator = generator;
-      }
-
-      @Override
-      String loadUncachedCode() throws IOException {
-        return generator.getCode();
-      }
-
-      @Override
-      void restoreFrom(CodeLoader other) {
-        this.generator = ((Generated) other).generator;
-      }
-    }
-
     @GwtIncompatible("com.google.common.io.CharStreams")
     static final class OnDisk extends CodeLoader {
       private static final long serialVersionUID = 1L;
@@ -763,9 +742,9 @@ private Charset getCharset() {
   }
 
   public void restoreFrom(SourceFile other) {
+    // TODO(b/181147184): determine if this method is necessary after moving to TypedAST
     this.code = other.code;
     this.lineOffsets = other.lineOffsets;
-    this.loader.restoreFrom(other.loader);
   }
 
   @GwtIncompatible("ObjectOutputStream")

test/com/google/javascript/jscomp/RecoverableJsAstTest.java

@@ -18,13 +18,19 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.jimfs.Jimfs;
 import com.google.common.truth.Correspondence;
-import com.google.javascript.jscomp.SourceFile.Generator;
 import com.google.javascript.rhino.Node;
+import java.io.IOException;
+import java.nio.file.FileSystem;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.Objects;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -35,10 +41,18 @@
  */
 @RunWith(JUnit4.class)
 public class RecoverableJsAstTest {
-  private String srcCode = "";
+  private Path tempFile;
+  private FileSystem fs;
+
+  @Before
+  public void setup() throws IOException {
+    FileSystem fs = Jimfs.newFileSystem();
+    this.tempFile = fs.getPath("/test.js");
+    Files.createFile(this.tempFile);
+  }
 
   @Test
-  public void testSimple() {
+  public void testSimple() throws IOException {
     setSourceCode("var a;");
     SourceAst realAst = createRealAst();
 
@@ -67,7 +81,7 @@ public void testSimple() {
   }
 
   @Test
-  public void testWarningReplay() {
+  public void testWarningReplay() throws IOException {
     setSourceCode("var f() = a;");
     SourceAst realAst = createRealAst();
 
@@ -91,21 +105,14 @@ private RecoverableJsAst makeDefensiveCopy(SourceAst ast) {
     return new RecoverableJsAst(ast, true);
   }
 
-  private String getSourceCode() {
-    return srcCode;
-  }
-
-  private void setSourceCode(String code) {
-    srcCode = code;
+  private void setSourceCode(String code) throws IOException {
+    Files.write(tempFile, code.getBytes(UTF_8));
   }
 
   private SourceAst createRealAst() {
-    SourceFile file = SourceFile.fromGenerator("tests.js", new Generator() {
-      @Override
-      public String getCode() {
-        return getSourceCode();
-      }
-    });
+    // use SourceFile.fromPath instead of a preloaded SourceFile.fromCode so that test cases may
+    // modify the contents of the SourceFile via changing the underlying file.
+    SourceFile file = SourceFile.fromPath(tempFile, UTF_8);
     return new JsAst(file);
   }
 

test/com/google/javascript/jscomp/SourceFileTest.java

@@ -288,37 +288,29 @@ public void testDiskFile() throws IOException {
     assertThat(actualContent).isEqualTo(expectedContent);
   }
 
-  private static class CodeGeneratorHelper implements SourceFile.Generator {
-    int reads = 0;
-
-    @Override
-    public String getCode() {
-      reads++;
-      return "var a;\n";
-    }
+  @Test
+  public void testDiskFileWithOriginalPath() throws IOException {
+    String expectedContent = "var c;";
 
-    public int numberOfReads() {
-      return reads;
-    }
-  }
+    Path tempFile = folder.newFile("test.js").toPath();
+    MoreFiles.asCharSink(tempFile, UTF_8).write(expectedContent);
 
-  @Test
-  public void testGeneratedFile() throws IOException {
-    String expectedContent = "var a;";
-    CodeGeneratorHelper myGenerator = new CodeGeneratorHelper();
-    SourceFile newFile = SourceFile.fromGenerator("file.js", myGenerator);
+    SourceFile newFile =
+        SourceFile.builder()
+            .withOriginalPath("original_test.js")
+            .buildFromFile(tempFile.toString());
     String actualContent;
 
     actualContent = newFile.getLine(1);
     assertThat(actualContent).isEqualTo(expectedContent);
-    assertThat(myGenerator.numberOfReads()).isEqualTo(1);
 
     newFile.clearCachedSource();
-    assertThat(newFile.hasSourceInMemory()).isFalse();
 
+    assertThat(newFile.hasSourceInMemory()).isFalse();
     actualContent = newFile.getLine(1);
     assertThat(actualContent).isEqualTo(expectedContent);
-    assertThat(myGenerator.numberOfReads()).isEqualTo(2);
+
+    assertThat(newFile.getName()).isEqualTo("original_test.js");
   }
 
   @Test