Split the single classpath (combining bootclasspath and regular classpath

together) into two separate classpaths. This is the preparation cl. In the
following cls, I will graduately make the dependency issues clear, i.e., wether
it is a problem in the bootclasspath or the regular classpath.

RELNOTES: n/a.
PiperOrigin-RevId: 192828237

4 files changed, 181 insertions, 85 deletions

diff --git a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassCache.java b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassCache.java
index b44c55dcf3..9f905259e3 100644
--- a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassCache.java
+++ b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassCache.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.importdeps;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -28,6 +29,7 @@ import com.google.devtools.build.importdeps.ClassInfo.MemberInfo;
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.PrintStream;
 import java.nio.file.Path;
 import java.util.HashMap;
 import java.util.Map;
@@ -40,69 +42,30 @@ import org.objectweb.asm.FieldVisitor;
 import org.objectweb.asm.MethodVisitor;
 import org.objectweb.asm.Opcodes;
 
-/** A cache that stores all the accessible classes. */
+/** A cache that stores all the accessible classes in a set of JARs. */
 public final class ClassCache implements Closeable {
 
-  private final ImmutableMap<String, LazyClassEntry> classIndex;
-  /**
-   * If the cache is open, then the {@code closer} is nonnull. After the cache is closed, the {@code
-   * closer} is set to {@literal null}.
-   */
-  @Nullable private Closer closer;
-
-  public ClassCache(Path... jars) throws IOException {
-    this(ImmutableList.copyOf(jars));
-  }
+  private final LazyClasspath lazyClasspath;
+  private boolean isClosed;
 
-  public ClassCache(ImmutableList<Path> jars) throws IOException {
-    closer = Closer.create();
-    this.classIndex = buildClassIndex(jars, closer);
+  public ClassCache(ImmutableList<Path> bootclasspath, ImmutableList<Path> regularClasspath)
+      throws IOException {
+    lazyClasspath = new LazyClasspath(bootclasspath, regularClasspath);
   }
 
   public AbstractClassEntryState getClassState(String internalName) {
-    ensureCacheIsOpen();
-    LazyClassEntry entry = classIndex.get(internalName);
+    checkState(!isClosed, "The cache has been closed.");
+    LazyClassEntry entry = lazyClasspath.getLazyEntry(internalName);
     if (entry == null) {
       return MissingState.singleton();
     }
-    return entry.getState(classIndex);
+    return entry.getState(lazyClasspath);
   }
 
   @Override
   public void close() throws IOException {
-    if (closer == null) {
-      return;
-    }
-    closer.close();
-    closer = null;
-  }
-
-  private static ImmutableMap<String, LazyClassEntry> buildClassIndex(
-      ImmutableList<Path> jars, Closer closer) throws IOException {
-    HashMap<String, LazyClassEntry> result = new HashMap<>();
-    for (Path jarPath : jars) {
-      try {
-        ZipFile zipFile = closer.register(new ZipFile(jarPath.toFile()));
-        zipFile
-            .stream()
-            .forEach(
-                entry -> {
-                  String name = entry.getName();
-                  if (!name.endsWith(".class")) {
-                    return; // Not a class file.
-                  }
-                  String internalName = name.substring(0, name.lastIndexOf('.'));
-                  result.computeIfAbsent(internalName, key -> new LazyClassEntry(key, zipFile));
-                });
-      } catch (Throwable e) {
-        throw new RuntimeException("Error in reading zip file " + jarPath, e);
-      }
-    }
-    return ImmutableMap.copyOf(result);
-  }
-
-  private void ensureCacheIsOpen() {
-    checkState(closer != null, "The cache should be open!");
+    lazyClasspath.close();
+    isClosed = true;
   }
 
   static class LazyClassEntry {
@@ -124,8 +87,8 @@ public final class ClassCache implements Closeable {
     }
 
     @Nullable
-    public AbstractClassEntryState getState(ImmutableMap<String, LazyClassEntry> classIndex) {
-      resolveIfNot(classIndex);
+    public AbstractClassEntryState getState(LazyClasspath classpath) {
+      resolveIfNot(classpath);
       checkState(
           state != null && !state.isMissingState(),
           "The state cannot be null or MISSING. %s",
@@ -141,16 +104,15 @@ public final class ClassCache implements Closeable {
           .toString();
     }
 
-    private void resolveIfNot(ImmutableMap<String, LazyClassEntry> classIndex) {
+    private void resolveIfNot(LazyClasspath lazyClasspath) {
       if (state != null) {
         return;
       }
-      resolveClassEntry(this, classIndex);
+      resolveClassEntry(this, lazyClasspath);
       checkNotNull(state, "After resolution, the state cannot be null");
     }
 
-    private static void resolveClassEntry(
-        LazyClassEntry classEntry, ImmutableMap<String, LazyClassEntry> classIndex) {
+    private static void resolveClassEntry(LazyClassEntry classEntry, LazyClasspath lazyClasspath) {
       if (classEntry.state != null) {
         // Already resolved. See if it is the existing state.
         return;
@@ -165,13 +127,13 @@ public final class ClassCache implements Closeable {
         ImmutableList<String> resolutionFailurePath = null;
         for (String superName :
             combineWithoutNull(classReader.getSuperName(), classReader.getInterfaces())) {
-          LazyClassEntry superClassEntry = classIndex.get(superName);
+          LazyClassEntry superClassEntry = lazyClasspath.getLazyEntry(superName);
 
           if (superClassEntry == null) {
             resolutionFailurePath = ImmutableList.of(superName);
             break;
           } else {
-            resolveClassEntry(superClassEntry, classIndex);
+            resolveClassEntry(superClassEntry, lazyClasspath);
             AbstractClassEntryState superState = superClassEntry.state;
             if (superState instanceof ExistingState) {
               // Do nothing. Good to proceed.
@@ -192,11 +154,12 @@ public final class ClassCache implements Closeable {
         classReader.accept(classInfoBuilder, ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES);
         if (resolutionFailurePath == null) {
           classEntry.state =
-              ExistingState.create(classInfoBuilder.build(classIndex, /*incomplete=*/ false));
+              ExistingState.create(classInfoBuilder.build(lazyClasspath, /*incomplete=*/ false));
         } else {
           classEntry.state =
               IncompleteState.create(
-                  classInfoBuilder.build(classIndex, /*incomplete=*/ true), resolutionFailurePath);
+                  classInfoBuilder.build(lazyClasspath, /*incomplete=*/ true),
+                  resolutionFailurePath);
         }
       } catch (IOException e) {
         throw new RuntimeException("Error when resolving class entry " + entryName);
@@ -204,10 +167,7 @@ public final class ClassCache implements Closeable {
         System.err.println(
             "A runtime exception occurred. The following is the content in the class index. "
                 + e.getMessage());
-        int counter = 0;
-        for (Map.Entry<String, LazyClassEntry> entry : classIndex.entrySet()) {
-          System.err.printf("%d  %s\n    %s\n\n", ++counter, entry.getKey(), entry.getValue());
-        }
+        lazyClasspath.printClasspath(System.err);
         throw e;
       }
     }
@@ -225,6 +185,96 @@ public final class ClassCache implements Closeable {
     return list.build();
   }
 
+  /** The classpath, emulating the behavior of the real classpath. */
+  @VisibleForTesting
+  static final class LazyClasspath implements Closeable {
+    private final ClassIndex bootclasspath;
+    private final ClassIndex regularClasspath;
+
+    public LazyClasspath(ImmutableList<Path> bootclasspath, ImmutableList<Path> regularClasspath)
+        throws IOException {
+      this.bootclasspath = new ClassIndex("boot classpath", bootclasspath);
+      this.regularClasspath = new ClassIndex("regular classpath", regularClasspath);
+    }
+
+    public LazyClassEntry getLazyEntry(String internalName) {
+      LazyClassEntry entry = bootclasspath.getClassEntry(internalName);
+      if (entry != null) {
+        return entry;
+      }
+      return regularClasspath.getClassEntry(internalName);
+    }
+
+    public void printClasspath(PrintStream stream) {
+      bootclasspath.printClasspath(stream);
+      regularClasspath.printClasspath(stream);
+    }
+
+    @Override
+    public void close() throws IOException {
+      bootclasspath.close();
+      regularClasspath.close();
+    }
+  }
+
+  /**
+   * Representation of a class path, composed of a list of JARs. It indexes all the class files with
+   * the class names.
+   */
+  private static final class ClassIndex implements Closeable {
+
+    private final String name;
+    private final ImmutableMap<String, LazyClassEntry> classIndex;
+    private final Closer closer;
+
+    public ClassIndex(String name, ImmutableList<Path> jarFiles) throws IOException {
+      this.name = name;
+      this.closer = Closer.create();
+      classIndex = buildClassIndex(jarFiles, closer);
+    }
+
+    @Override
+    public void close() throws IOException {
+      closer.close();
+    }
+
+    public LazyClassEntry getClassEntry(String internalName) {
+      return classIndex.get(internalName);
+    }
+
+    private void printClasspath(PrintStream stream) {
+      stream.println("Classpath: " + name);
+      int counter = 0;
+      for (Map.Entry<String, LazyClassEntry> entry : classIndex.entrySet()) {
+        stream.printf("%d  %s\n    %s\n\n", ++counter, entry.getKey(), entry.getValue());
+      }
+    }
+
+    private static ImmutableMap<String, LazyClassEntry> buildClassIndex(
+        ImmutableList<Path> jars, Closer closer) throws IOException {
+      HashMap<String, LazyClassEntry> result = new HashMap<>();
+      for (Path jarPath : jars) {
+        try {
+          ZipFile zipFile = closer.register(new ZipFile(jarPath.toFile()));
+          zipFile
+              .stream()
+              .forEach(
+                  entry -> {
+                    String name = entry.getName();
+                    if (!name.endsWith(".class")) {
+                      return; // Not a class file.
+                    }
+                    String internalName = name.substring(0, name.lastIndexOf('.'));
+                    result.computeIfAbsent(internalName, key -> new LazyClassEntry(key, zipFile));
+                  });
+        } catch (Throwable e) {
+          throw new RuntimeException("Error in reading zip file " + jarPath, e);
+        }
+      }
+      return ImmutableMap.copyOf(result);
+    }
+  }
+
   /** Builder to build a ClassInfo object from the class file. */
   private static class ClassInfoBuilder extends ClassVisitor {
 
@@ -263,14 +313,15 @@ public final class ClassCache implements Closeable {
       return null;
     }
 
-    public ClassInfo build(ImmutableMap<String, LazyClassEntry> classIndex, boolean incomplete) {
-      ImmutableList<ClassInfo> superClassInfos = superClasses
-          .stream()
-          .map(classIndex::get)
-          // nulls possible when building ClassInfo for an "incomplete" class
-          .filter(entry -> entry != null && entry.state != null)
-          .map(entry -> entry.state.classInfo().get())
-          .collect(ImmutableList.toImmutableList());
+    public ClassInfo build(LazyClasspath lazyClasspath, boolean incomplete) {
+      ImmutableList<ClassInfo> superClassInfos =
+          superClasses
+              .stream()
+              .map(lazyClasspath::getLazyEntry)
+              // nulls possible when building ClassInfo for an "incomplete" class
+              .filter(entry -> entry != null && entry.state != null)
+              .map(entry -> entry.state.classInfo().get())
+              .collect(ImmutableList.toImmutableList());
       checkState(
           incomplete || superClassInfos.size() == superClasses.size(),
           "Missing class info for some of %s's super types %s: %s",
diff --git a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ImportDepsChecker.java b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ImportDepsChecker.java
index 22e7a5b48d..433e0a825f 100644
--- a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ImportDepsChecker.java
+++ b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ImportDepsChecker.java
@@ -44,11 +44,8 @@ public final class ImportDepsChecker implements Closeable {
       throws IOException {
     this.classCache =
         new ClassCache(
-            ImmutableList.<Path>builder()
-                .addAll(bootclasspath)
-                .addAll(classpath)
-                .addAll(inputJars)
-                .build());
+            bootclasspath,
+            ImmutableList.<Path>builder().addAll(classpath).addAll(inputJars).build());
     this.resultCollector = new ResultCollector();
     this.inputJars = inputJars;
   }
diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassCacheTest.java b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassCacheTest.java
index 8049621449..a7cb666291 100644
--- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassCacheTest.java
+++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassCacheTest.java
@@ -19,7 +19,12 @@ import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.importdeps.AbstractClassEntryState.ExistingState;
+import com.google.devtools.build.importdeps.ClassCache.LazyClassEntry;
+import com.google.devtools.build.importdeps.ClassCache.LazyClasspath;
+import com.google.devtools.build.importdeps.ClassInfo.MemberInfo;
 import java.io.IOException;
+import java.util.Optional;
+import java.util.stream.Collectors;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -30,7 +35,8 @@ public class ClassCacheTest extends AbstractClassCacheTest {
 
   @Test
   public void testLibraryJar() throws Exception {
-    try (ClassCache cache = new ClassCache(bootclasspath, libraryJar)) {
+    try (ClassCache cache =
+        new ClassCache(ImmutableList.of(bootclasspath), ImmutableList.of(libraryJar))) {
       assertCache(
           cache,
           libraryJarPositives,
@@ -44,7 +50,9 @@ public class ClassCacheTest extends AbstractClassCacheTest {
   @Test
   public void testClientJarWithSuperClasses() throws IOException {
     try (ClassCache cache =
-        new ClassCache(bootclasspath, clientJar, libraryJar, libraryInterfaceJar)) {
+        new ClassCache(
+            ImmutableList.of(bootclasspath),
+            ImmutableList.of(clientJar, libraryJar, libraryInterfaceJar))) {
       assertCache(
           cache,
           clientJarPositives,
@@ -54,7 +62,8 @@ public class ClassCacheTest extends AbstractClassCacheTest {
 
   @Test
   public void testClientJarWithoutSuperClasses() throws IOException {
-    try (ClassCache cache = new ClassCache(bootclasspath, clientJar)) {
+    try (ClassCache cache =
+        new ClassCache(ImmutableList.of(bootclasspath), ImmutableList.of(clientJar))) {
       // Client should be incomplete, as its parent class and interfaces are not available on the
       // classpath. The following is the resolution path.
       {
@@ -76,7 +85,8 @@ public class ClassCacheTest extends AbstractClassCacheTest {
 
   @Test
   public void testLibraryException() throws IOException {
-    try (ClassCache cache = new ClassCache(bootclasspath, libraryExceptionJar)) {
+    try (ClassCache cache =
+        new ClassCache(ImmutableList.of(bootclasspath), ImmutableList.of(libraryExceptionJar))) {
       assertCache(
           cache,
           libraryExceptionJarPositives,
@@ -86,7 +96,8 @@ public class ClassCacheTest extends AbstractClassCacheTest {
 
   @Test
   public void testLibraryAnnotations() throws IOException {
-    try (ClassCache cache = new ClassCache(bootclasspath, libraryAnnotationsJar)) {
+    try (ClassCache cache =
+        new ClassCache(ImmutableList.of(bootclasspath), ImmutableList.of(libraryAnnotationsJar))) {
       assertCache(
           cache,
           libraryAnnotationsJarPositives,
@@ -96,7 +107,7 @@ public class ClassCacheTest extends AbstractClassCacheTest {
 
   @Test
   public void testCannotAccessClosedCache() throws IOException {
-    ClassCache cache = new ClassCache(ImmutableList.of());
+    ClassCache cache = new ClassCache(ImmutableList.of(), ImmutableList.of());
     cache.close();
     cache.close(); // Can close multiple times.
     assertThrows(IllegalStateException.class, () -> cache.getClassState("empty"));
@@ -110,7 +121,9 @@ public class ClassCacheTest extends AbstractClassCacheTest {
   public void testSuperNotExistThenSubclassNotExist() throws IOException {
     try (ClassCache cache =
         new ClassCache(
-            libraryJar, libraryJar, libraryAnnotationsJar, libraryInterfaceJar, clientJar)) {
+            ImmutableList.of(),
+            ImmutableList.of(
+                libraryJar, libraryJar, libraryAnnotationsJar, libraryInterfaceJar, clientJar))) {
       assertThat(
               cache
                   .getClassState("com/google/devtools/build/importdeps/testdata/Library$Class9")
@@ -124,6 +137,41 @@ public class ClassCacheTest extends AbstractClassCacheTest {
     }
   }
 
+  @Test
+  public void testLazyClasspathLoadsBootclasspathFirst() throws IOException {
+    {
+      LazyClasspath classpath =
+          new LazyClasspath(ImmutableList.of(libraryJar), ImmutableList.of(libraryWoMembersJar));
+      LazyClassEntry entry =
+          classpath.getLazyEntry("com/google/devtools/build/importdeps/testdata/Library$Class4");
+      AbstractClassEntryState state = entry.getState(classpath);
+      Optional<ClassInfo> classInfo = state.classInfo();
+      assertThat(classInfo.isPresent()).isTrue();
+      assertThat(classInfo.get().declaredMembers()).hasSize(2);
+      assertThat(
+              classInfo
+                  .get()
+                  .declaredMembers()
+                  .stream()
+                  .map(MemberInfo::memberName)
+                  .collect(Collectors.toSet()))
+          .containsExactly("<init>", "createClass5");
+    }
+    {
+      LazyClasspath classpath =
+          new LazyClasspath(ImmutableList.of(libraryWoMembersJar), ImmutableList.of(libraryJar));
+      LazyClassEntry entry =
+          classpath.getLazyEntry("com/google/devtools/build/importdeps/testdata/Library$Class4");
+      AbstractClassEntryState state = entry.getState(classpath);
+      Optional<ClassInfo> classInfo = state.classInfo();
+      assertThat(classInfo.isPresent()).isTrue();
+      // The empty class has a default constructor.
+      assertThat(classInfo.get().declaredMembers()).hasSize(1);
+      assertThat(classInfo.get().declaredMembers().iterator().next().memberName())
+          .isEqualTo("<init>");
+    }
+  }
+
   private void assertCache(
       ClassCache cache, ImmutableList<String> positives, ImmutableList<String> negatives) {
     for (String positive : positives) {
diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/DepsCheckerClassVisitorTest.java b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/DepsCheckerClassVisitorTest.java
index 277fd808a4..089e3c2e6d 100644
--- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/DepsCheckerClassVisitorTest.java
+++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/DepsCheckerClassVisitorTest.java
@@ -109,7 +109,7 @@ public class DepsCheckerClassVisitorTest extends AbstractClassCacheTest {
     ImmutableList<String> clientClasses =
         ImmutableList.of(PACKAGE_NAME + "Client", PACKAGE_NAME + "Client$NestedAnnotation");
     ResultCollector resultCollector = new ResultCollector();
-    try (ClassCache cache = new ClassCache(ImmutableList.copyOf(classpath));
+    try (ClassCache cache = new ClassCache(ImmutableList.copyOf(classpath), ImmutableList.of());
         ZipFile zipFile = new ZipFile(clientJar.toFile())) {
       assertThat(cache.getClassState("java/lang/invoke/LambdaMetafactory").isExistingState())
           .isTrue();