Review feedback

Author: niloc132

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

@@ -48,7 +48,7 @@
  * and maintain state such as module for the input and whether the input is an extern. Also
  * calculates provided and required types.
  */
-public class CompilerInput extends DependencyInfo.Base {
+public class CompilerInput implements DependencyInfo {
 
   private static final long serialVersionUID = 2L;
 
@@ -573,11 +573,13 @@ public String toString() {
 
   @Override
   public boolean isEs6Module() {
+    // Instead of doing a full parse to read all load flags, just ask the delegate, which at least has this much info
     return getDependencyInfo().isEs6Module();
   }
 
   @Override
   public boolean isGoogModule() {
+    // Instead of doing a full parse to read all load flags, just ask the delegate, which at least has this much info
     return getDependencyInfo().isGoogModule();
   }
 

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

@@ -41,7 +41,7 @@
  * A JavaScript chunk has a unique name, consists of a list of compiler inputs, and can depend on
  * other chunks.
  */
-public final class JSChunk extends DependencyInfo.Base implements Serializable {
+public final class JSChunk implements Serializable, DependencyInfo {
   // The name of the artificial chunk containing all strong sources when there is no chunk spec.
   // If there is a chunk spec, strong sources go in their respective chunks, and this chunk does
   // not exist.
@@ -59,7 +59,7 @@ public final class JSChunk extends DependencyInfo.Base implements Serializable {
   private final String name;
 
   /** Source code inputs */
-  // non-final for deserilaization
+  // non-final for deserialization
   // CompilerInputs must be explicitly added to the JSChunk again after deserialization
   // A map keyed by the {@code CompilerInput.getName()} to speed up getByName and removeByName.
   private transient Map<String, CompilerInput> inputs = new LinkedHashMap<>();
@@ -131,15 +131,6 @@ public ImmutableMap<String, String> getLoadFlags() {
     throw new UnsupportedOperationException();
   }
 
-  @Override
-  public boolean isModule() {
-    // NOTE: The meaning of "module" has changed over time.  A "JsChunk" is
-    // a collection of inputs that are loaded together. A "module" file,
-    // is a CommonJs module, ES6 module, goog.module or other file whose
-    // top level symbols are not in global scope.
-    throw new UnsupportedOperationException();
-  }
-
   /** Adds a source file input to this chunk. */
   public void add(SourceFile file) {
     add(new CompilerInput(file));

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

@@ -29,7 +29,7 @@
 import org.jspecify.nullness.Nullable;
 
 /** A DependencyInfo class that determines load flags by parsing the AST just-in-time. */
-public class LazyParsedDependencyInfo extends DependencyInfo.Base {
+public class LazyParsedDependencyInfo implements DependencyInfo {
 
   private final DependencyInfo delegate;
   private @Nullable JsAst ast;
@@ -45,13 +45,13 @@ public LazyParsedDependencyInfo(DependencyInfo delegate, JsAst ast, AbstractComp
 
   @Override
   public boolean isEs6Module() {
-    // Instead of doing a full parse to look this up, just ask the delegate, which at least has this much info
+    // Instead of doing a full parse to read all load flags, just ask the delegate, which at least has this much info
     return delegate.isEs6Module();
   }
 
   @Override
   public boolean isGoogModule() {
-    // Instead of doing a full parse to look this up, just ask the delegate, which at least has this much info
+    // Instead of doing a full parse to read all load flags, just ask the delegate, which at least has this much info
     return delegate.isGoogModule();
   }
 

src/com/google/javascript/jscomp/deps/DependencyInfo.java

@@ -129,20 +129,21 @@ abstract static class Builder {
   /** Gets the symbols required by this file. */
   ImmutableList<Require> getRequires();
 
-  ImmutableList<String> getRequiredSymbols();
+  default ImmutableList<String> getRequiredSymbols() {
+    return Require.asSymbolList(getRequires());
+  }
 
   /** Gets the symbols type-required by this file (i.e. for typechecking only). */
   ImmutableList<String> getTypeRequires();
 
   /** Gets the loading information for this file. */
   ImmutableMap<String, String> getLoadFlags();
 
-  /** Whether the symbol is provided by a module */
-  boolean isModule();
-
+  /** Whether the symbol is provided by an ES6 module */
   default boolean isEs6Module() {
     return "es6".equals(getLoadFlags().get("module"));
   }
+  /** Whether the symbol is provided by a goog module */
   default boolean isGoogModule() {
     return "goog".equals(getLoadFlags().get("module"));
   }
@@ -153,21 +154,6 @@ default boolean isGoogModule() {
   /** Whether the file has the '@nocompile' annotation. */
   boolean getHasNoCompileAnnotation();
 
-  /**
-   * Abstract base implementation that defines derived accessors such
-   * as {@link #isModule}.
-   */
-  abstract class Base implements DependencyInfo {
-    @Override public boolean isModule() {
-      return isGoogModule();
-    }
-
-    @Override
-    public ImmutableList<String> getRequiredSymbols() {
-      return Require.asSymbolList(getRequires());
-    }
-  }
-
   /** Utility methods. */
   class Util {
     private Util() {}

src/com/google/javascript/jscomp/deps/DepsGenerator.java

@@ -277,7 +277,6 @@ private void validateDependencies(Iterable<DependencyInfo> preparsedFileDependen
         } else if (provider == depInfo) {
           reportSameFile(namespace, depInfo);
         } else {
-          depInfo.isModule();
           boolean providerIsEs6Module = provider.isEs6Module();
 
           switch (require.getType()) {

src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java

@@ -27,7 +27,7 @@
  * A class to hold JS dependency information for a single .js file.
  */
 @Immutable @AutoValue @AutoValue.CopyAnnotations
-public abstract class SimpleDependencyInfo extends DependencyInfo.Base {
+public abstract class SimpleDependencyInfo implements DependencyInfo {
 
   public static Builder builder(String srcPathRelativeToClosure, String pathOfDefiningFile) {
     return new AutoValue_SimpleDependencyInfo.Builder()

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

@@ -67,7 +67,7 @@ public void testLoadFlagsParsesEs3() {
     // is lifted and we can depend on a newer Truth, these assertions should be
     // changed to assertThat(info.getLoadFlags()).containsExactly(...)
     assertThat(info.getLoadFlags()).containsExactly("foo", "bar");
-    assertThat(info.isModule()).isFalse();
+    assertThat(info.isGoogModule()).isFalse();
   }
 
   @Test
@@ -82,7 +82,7 @@ public void testLoadFlagsParsesEs5() {
     DependencyInfo info = new LazyParsedDependencyInfo(delegate, ast, compiler);
 
     assertThat(info.getLoadFlags()).containsExactly("module", "goog", "lang", "es5");
-    assertThat(info.isModule()).isTrue();
+    assertThat(info.isGoogModule()).isTrue();
   }
 
   @Test
@@ -97,7 +97,7 @@ public void testLoadFlagsParsesEs6Impl() {
     DependencyInfo info = new LazyParsedDependencyInfo(delegate, ast, compiler);
 
     assertThat(info.getLoadFlags()).containsExactly("foo", "bar", "lang", "es6");
-    assertThat(info.isModule()).isFalse();
+    assertThat(info.isGoogModule()).isFalse();
   }
 
   @Test
@@ -112,7 +112,7 @@ public void testLoadFlagsParsesEs6() {
     DependencyInfo info = new LazyParsedDependencyInfo(delegate, ast, compiler);
 
     assertThat(info.getLoadFlags()).containsExactly("foo", "bar", "lang", "es6");
-    assertThat(info.isModule()).isFalse();
+    assertThat(info.isGoogModule()).isFalse();
   }
 
   @Test