diff options
author | kmb <kmb@google.com> | 2018-05-07 15:28:53 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-05-07 15:30:23 -0700 |
commit | ee3cc87ca8382c9eda37012f7fc470326aad1521 (patch) | |
tree | 4b1f279d1383cc178675013405b3c89a35cdf049 | |
parent | deda170f1d31bdb996055e2e9eaa0dfbadc0df3d (diff) |
Flag indirect dependencies in ImportDepsChecker tool
PiperOrigin-RevId: 195732395
15 files changed, 275 insertions, 37 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 623863af8b..232e69b936 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 @@ -13,11 +13,13 @@ // limitations under the License. package com.google.devtools.build.importdeps; +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 com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -35,6 +37,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Objects; +import java.util.function.Predicate; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import javax.annotation.Nullable; @@ -52,10 +55,11 @@ public final class ClassCache implements Closeable { public ClassCache( ImmutableList<Path> bootclasspath, + ImmutableList<Path> directClasspath, ImmutableList<Path> regularClasspath, ImmutableList<Path> inputJars) throws IOException { - lazyClasspath = new LazyClasspath(bootclasspath, regularClasspath, inputJars); + lazyClasspath = new LazyClasspath(bootclasspath, directClasspath, regularClasspath, inputJars); } public AbstractClassEntryState getClassState(String internalName) { @@ -81,16 +85,18 @@ public final class ClassCache implements Closeable { private final String internalName; private final ZipFile zipFile; private final Path jarPath; + private final boolean direct; /** * The state of this class entry. If {@literal null}, then this class has not been resolved yet. */ @Nullable private AbstractClassEntryState state = null; - private LazyClassEntry(String internalName, ZipFile zipFile, Path jarPath) { + private LazyClassEntry(String internalName, ZipFile zipFile, Path jarPath, boolean direct) { this.internalName = internalName; this.zipFile = zipFile; this.jarPath = jarPath; + this.direct = direct; } ZipFile getZipFile() { @@ -165,7 +171,8 @@ public final class ClassCache implements Closeable { } } } - ClassInfoBuilder classInfoBuilder = new ClassInfoBuilder(); + ClassInfoBuilder classInfoBuilder = + new ClassInfoBuilder().setJarPath(classEntry.jarPath).setDirect(classEntry.direct); classReader.accept(classInfoBuilder, ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES); if (resolutionFailurePath == null) { classEntry.state = @@ -211,14 +218,28 @@ public final class ClassCache implements Closeable { public LazyClasspath( ImmutableList<Path> bootclasspath, + ImmutableList<Path> directClasspath, ImmutableList<Path> regularClasspath, ImmutableList<Path> inputJars) throws IOException { - this.bootclasspath = new ClassIndex("boot classpath", bootclasspath); - this.regularClasspath = new ClassIndex("regular classpath", regularClasspath); - this.inputJars = new ClassIndex("input jars", inputJars); + checkArgument( + regularClasspath.containsAll(directClasspath), + "Some direct deps %s missing from transitive deps %s", + directClasspath, + regularClasspath); + this.bootclasspath = new ClassIndex("boot classpath", bootclasspath, Predicates.alwaysTrue()); + this.inputJars = new ClassIndex("input jars", inputJars, Predicates.alwaysTrue()); + this.regularClasspath = + new ClassIndex( + "regular classpath", + regularClasspath, + jar -> + bootclasspath.contains(jar) + || inputJars.contains(jar) + || directClasspath.contains(jar)); + // Reflect runtime resolution order, with input before classpath similar to javac this.orderedClasspath = - ImmutableList.of(this.bootclasspath, this.regularClasspath, this.inputJars); + ImmutableList.of(this.bootclasspath, this.inputJars, this.regularClasspath); this.orderedClasspath.forEach(closer::register); } @@ -255,10 +276,11 @@ public final class ClassCache implements Closeable { private final ImmutableMap<String, LazyClassEntry> classIndex; private final Closer closer; - public ClassIndex(String name, ImmutableList<Path> jarFiles) throws IOException { + public ClassIndex(String name, ImmutableList<Path> jarFiles, Predicate<Path> isDirect) + throws IOException { this.name = name; this.closer = Closer.create(); - classIndex = buildClassIndex(jarFiles, closer); + classIndex = buildClassIndex(jarFiles, closer, isDirect); } @Override @@ -290,9 +312,10 @@ public final class ClassCache implements Closeable { } private static ImmutableMap<String, LazyClassEntry> buildClassIndex( - ImmutableList<Path> jars, Closer closer) throws IOException { + ImmutableList<Path> jars, Closer closer, Predicate<Path> isDirect) throws IOException { HashMap<String, LazyClassEntry> result = new HashMap<>(); for (Path jarPath : jars) { + boolean jarIsDirect = isDirect.test(jarPath); try { ZipFile zipFile = closer.register(new ZipFile(jarPath.toFile())); zipFile @@ -305,7 +328,8 @@ public final class ClassCache implements Closeable { } String internalName = name.substring(0, name.lastIndexOf('.')); result.computeIfAbsent( - internalName, key -> new LazyClassEntry(key, zipFile, jarPath)); + internalName, + key -> new LazyClassEntry(key, zipFile, jarPath, jarIsDirect)); }); } catch (Throwable e) { throw new RuntimeException("Error in reading zip file " + jarPath, e); @@ -321,6 +345,8 @@ public final class ClassCache implements Closeable { private String internalName; private final ImmutableSet.Builder<MemberInfo> members = ImmutableSet.builder(); private ImmutableList<String> superClasses; + private Path jarPath; + private boolean directDep; public ClassInfoBuilder() { super(Opcodes.ASM6); @@ -353,6 +379,16 @@ public final class ClassCache implements Closeable { return null; } + public ClassInfoBuilder setJarPath(Path jarPath) { + this.jarPath = jarPath; + return this; + } + + public ClassInfoBuilder setDirect(boolean direct) { + this.directDep = direct; + return this; + } + public ClassInfo build(LazyClasspath lazyClasspath, boolean incomplete) { ImmutableList<ClassInfo> superClassInfos = superClasses @@ -368,7 +404,12 @@ public final class ClassCache implements Closeable { internalName, superClasses, superClassInfos); - return ClassInfo.create(checkNotNull(internalName), superClassInfos, members.build()); + return ClassInfo.create( + checkNotNull(internalName), + checkNotNull(jarPath), + directDep, + superClassInfos, + members.build()); } } } diff --git a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassInfo.java b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassInfo.java index e017e69252..0a1e921ad0 100644 --- a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassInfo.java +++ b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ClassInfo.java @@ -20,6 +20,7 @@ import com.google.auto.value.extension.memoized.Memoized; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import java.nio.file.Path; /** * Representation of a class. It maintains the internal name, declared members, as well as the super @@ -30,13 +31,19 @@ public abstract class ClassInfo { public static ClassInfo create( String internalName, + Path jarPath, + boolean directDep, ImmutableList<ClassInfo> superClasses, ImmutableSet<MemberInfo> declaredMembers) { - return new AutoValue_ClassInfo(internalName, superClasses, declaredMembers); + return new AutoValue_ClassInfo(internalName, jarPath, directDep, superClasses, declaredMembers); } public abstract String internalName(); + public abstract Path jarPath(); + + public abstract boolean directDep(); + /** * Returns all the available super classes. There may be more super classes (super class or * interfaces), but those do not exist on the classpath. diff --git a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/DepsCheckerClassVisitor.java b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/DepsCheckerClassVisitor.java index 2e04864aea..b57c273e10 100644 --- a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/DepsCheckerClassVisitor.java +++ b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/DepsCheckerClassVisitor.java @@ -172,13 +172,19 @@ public class DepsCheckerClassVisitor extends ClassVisitor { AbstractClassEntryState state = classCache.getClassState(internalName); if (state.isMissingState()) { resultCollector.addMissingOrIncompleteClass(internalName, state); - } else if (state.isIncompleteState()) { - String missingAncestor = state.asIncompleteState().getMissingAncestor(); - AbstractClassEntryState ancestorState = classCache.getClassState(missingAncestor); - checkState( - ancestorState.isMissingState(), "The ancestor should be missing. %s", ancestorState); - resultCollector.addMissingOrIncompleteClass(missingAncestor, ancestorState); - resultCollector.addMissingOrIncompleteClass(internalName, state); + } else { + if (state.isIncompleteState()) { + String missingAncestor = state.asIncompleteState().getMissingAncestor(); + AbstractClassEntryState ancestorState = classCache.getClassState(missingAncestor); + checkState( + ancestorState.isMissingState(), "The ancestor should be missing. %s", ancestorState); + resultCollector.addMissingOrIncompleteClass(missingAncestor, ancestorState); + resultCollector.addMissingOrIncompleteClass(internalName, state); + } + ClassInfo info = state.classInfo().get(); + if (!info.directDep()) { + resultCollector.addIndirectDep(info.jarPath()); + } } return state; } 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 21089667c3..9276d92eb6 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 @@ -25,9 +25,13 @@ import java.io.Closeable; import java.io.IOError; import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.nio.file.Path; +import java.util.Objects; +import java.util.jar.JarFile; import java.util.stream.Collectors; import java.util.zip.ZipFile; +import javax.annotation.Nullable; import org.objectweb.asm.ClassReader; /** @@ -42,10 +46,11 @@ public final class ImportDepsChecker implements Closeable { public ImportDepsChecker( ImmutableList<Path> bootclasspath, + ImmutableList<Path> directClasspath, ImmutableList<Path> classpath, ImmutableList<Path> inputJars) throws IOException { - this.classCache = new ClassCache(bootclasspath, classpath, inputJars); + this.classCache = new ClassCache(bootclasspath, directClasspath, classpath, inputJars); this.resultCollector = new ResultCollector(); this.inputJars = inputJars; } @@ -90,6 +95,7 @@ public final class ImportDepsChecker implements Closeable { public Dependencies emitJdepsProto(String ruleLabel) { Dependencies.Builder builder = Dependencies.newBuilder(); ImmutableList<Path> paths = classCache.collectUsedJarsInRegularClasspath(); + // TODO(b/77723273): Consider "implicit" for Jars only needed to resolve supertypes paths.forEach( path -> builder.addDependency( @@ -99,7 +105,7 @@ public final class ImportDepsChecker implements Closeable { private static final String INDENT = " "; - public String computeResultOutput() { + public String computeResultOutput(String ruleLabel) { StringBuilder builder = new StringBuilder(); ImmutableList<String> missingClasses = resultCollector.getSortedMissingClassInternalNames(); for (String missing : missingClasses) { @@ -156,9 +162,51 @@ public final class ImportDepsChecker implements Closeable { .append(missingMembers.size()) .append('\n'); } + + ImmutableList<Path> indirectJars = resultCollector.getSortedIndirectDeps(); + if (!indirectJars.isEmpty()) { + ImmutableList<String> labels = extractLabels(indirectJars); + if (ruleLabel.isEmpty() || labels.isEmpty()) { + builder + .append( + "*** Missing strict dependencies on the following Jars which don't carry " + + "rule labels.\nPlease determine the originating rules, e.g., using Bazel's " + + "'query' command, and add them to the dependencies of ") + .append(ruleLabel.isEmpty() ? inputJars : ruleLabel) + .append('\n'); + for (Path jar : indirectJars) { + builder.append(jar).append('\n'); + } + } else { + builder.append("*** Missing strict dependencies. Run the following command to fix ***\n\n"); + builder.append(" add_dep "); + for (String indirectLabel : labels) { + builder.append(indirectLabel).append(" "); + } + builder.append(ruleLabel).append('\n'); + } + } return builder.toString(); } + private static ImmutableList<String> extractLabels(ImmutableList<Path> jars) { + return jars.parallelStream() + .map(ImportDepsChecker::extractLabel) + .filter(Objects::nonNull) + .distinct() + .sorted() + .collect(ImmutableList.toImmutableList()); + } + + @Nullable + private static String extractLabel(Path jarPath) { + try (JarFile jar = new JarFile(jarPath.toFile())) { + return jar.getManifest().getMainAttributes().getValue("Target-Label"); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + @Override public void close() throws IOException { classCache.close(); diff --git a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java index 3726caed30..7c7c034c7e 100644 --- a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java +++ b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/Main.java @@ -19,6 +19,7 @@ import static com.google.common.io.MoreFiles.asCharSink; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; import com.google.devtools.build.lib.view.proto.Deps.Dependencies; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.EnumConverter; @@ -37,6 +38,7 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; /** @@ -61,6 +63,19 @@ public class Main { public List<Path> inputJars; @Option( + name = "directdep", + allowMultiple = true, + defaultValue = "", + category = "input", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + converter = ExistingPathConverter.class, + help = "Subset of Jars listed in --classpath_entry that --input Jars are allowed to depend " + + "on directly." + ) + public List<Path> directClasspath; + + @Option( name = "classpath_entry", allowMultiple = true, defaultValue = "", @@ -71,7 +86,7 @@ public class Main { help = "Ordered classpath (Jar) to resolve symbols in the --input jars, like javac's -cp flag." ) - public List<Path> classpath; + public List<Path> fullClasspath; @Option( name = "bootclasspath_entry", @@ -158,10 +173,14 @@ public class Main { try (ImportDepsChecker checker = new ImportDepsChecker( ImmutableList.copyOf(options.bootclasspath), - ImmutableList.copyOf(options.classpath), + // Consider everything direct if no direct classpath is given + options.directClasspath.isEmpty() + ? ImmutableList.copyOf(options.fullClasspath) + : ImmutableList.copyOf(options.directClasspath), + ImmutableList.copyOf(options.fullClasspath), ImmutableList.copyOf(options.inputJars))) { if (!checker.check() && options.checkingMode != CheckingMode.SILENCE) { - String result = checker.computeResultOutput(); + String result = checker.computeResultOutput(options.ruleLabel); checkState(!result.isEmpty(), "The result should NOT be empty."); exitCode = options.checkingMode == CheckingMode.ERROR ? DEPS_ERROR_EXIT_CODE : 0; printErrorMessage(result, options); @@ -213,6 +232,13 @@ public class Main { // Convert --fail_on_errors to --checking_mode options.checkingMode = options.failOnErrors ? CheckingMode.ERROR : CheckingMode.WARNING; } + if (!options.fullClasspath.containsAll(options.directClasspath)) { + ArrayList<Path> missing = Lists.newArrayList(options.directClasspath); + missing.removeAll(options.fullClasspath); + throw new IllegalArgumentException( + "--strictdeps must be a subset of --classpath_entry but has additional entries: " + + missing); + } return options; } diff --git a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ResultCollector.java b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ResultCollector.java index 734b24be2f..5288799f07 100644 --- a/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ResultCollector.java +++ b/src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps/ResultCollector.java @@ -21,6 +21,7 @@ import com.google.common.collect.ComparisonChain; import com.google.common.collect.ImmutableList; import com.google.devtools.build.importdeps.AbstractClassEntryState.IncompleteState; import com.google.devtools.build.importdeps.ClassInfo.MemberInfo; +import java.nio.file.Path; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -31,6 +32,7 @@ public class ResultCollector { private final HashSet<String> missingClasss = new HashSet<>(); private final HashMap<String, IncompleteState> incompleteClasses = new HashMap<>(); private final HashSet<MissingMember> missingMembers = new HashSet<>(); + private final HashSet<Path> indirectDeps = new HashSet<>(); public ResultCollector() {} @@ -56,13 +58,20 @@ public class ResultCollector { /** Returns {@literal true} if there is NO dependency issue, {@literal false} otherwise. */ public boolean isEmpty() { - return missingClasss.isEmpty() && incompleteClasses.isEmpty() && missingMembers.isEmpty(); + return missingClasss.isEmpty() + && incompleteClasses.isEmpty() + && missingMembers.isEmpty() + && indirectDeps.isEmpty(); } public void addMissingMember(String owner, MemberInfo member) { missingMembers.add(MissingMember.create(owner, member)); } + public void addIndirectDep(Path indirectDep) { + indirectDeps.add(indirectDep); + } + public ImmutableList<String> getSortedMissingClassInternalNames() { return ImmutableList.sortedCopyOf(missingClasss); } @@ -76,6 +85,10 @@ public class ResultCollector { return ImmutableList.sortedCopyOf(missingMembers); } + public ImmutableList<Path> getSortedIndirectDeps() { + return ImmutableList.sortedCopyOf(indirectDeps); + } + /** * A missing member on the classpath. This class does not contain the member name and description, * but also the owner of the member. 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 341fa013a0..09fcf9eadd 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 @@ -37,7 +37,10 @@ public class ClassCacheTest extends AbstractClassCacheTest { public void testLibraryJar() throws Exception { try (ClassCache cache = new ClassCache( - ImmutableList.of(bootclasspath), ImmutableList.of(libraryJar), ImmutableList.of())) { + ImmutableList.of(bootclasspath), + ImmutableList.of(libraryJar), + ImmutableList.of(libraryJar), + ImmutableList.of())) { assertCache( cache, libraryJarPositives, @@ -54,6 +57,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new ClassCache( ImmutableList.of(bootclasspath), ImmutableList.of(libraryJar, libraryInterfaceJar), + ImmutableList.of(libraryJar, libraryInterfaceJar), ImmutableList.of(clientJar))) { assertCache( cache, @@ -66,7 +70,10 @@ public class ClassCacheTest extends AbstractClassCacheTest { public void testClientJarWithoutSuperClasses() throws IOException { try (ClassCache cache = new ClassCache( - ImmutableList.of(bootclasspath), ImmutableList.of(), ImmutableList.of(clientJar))) { + ImmutableList.of(bootclasspath), + ImmutableList.of(), + ImmutableList.of(), + 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. { @@ -92,6 +99,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new ClassCache( ImmutableList.of(bootclasspath), ImmutableList.of(), + ImmutableList.of(), ImmutableList.of(libraryExceptionJar))) { assertCache( cache, @@ -106,6 +114,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new ClassCache( ImmutableList.of(bootclasspath), ImmutableList.of(), + ImmutableList.of(), ImmutableList.of(libraryAnnotationsJar))) { assertCache( cache, @@ -116,7 +125,9 @@ public class ClassCacheTest extends AbstractClassCacheTest { @Test public void testCannotAccessClosedCache() throws IOException { - ClassCache cache = new ClassCache(ImmutableList.of(), ImmutableList.of(), ImmutableList.of()); + ClassCache cache = + new ClassCache( + ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of()); cache.close(); cache.close(); // Can close multiple times. assertThrows(IllegalStateException.class, () -> cache.getClassState("empty")); @@ -132,6 +143,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new ClassCache( ImmutableList.of(), ImmutableList.of(libraryJar, libraryJar, libraryAnnotationsJar, libraryInterfaceJar), + ImmutableList.of(libraryJar, libraryJar, libraryAnnotationsJar, libraryInterfaceJar), ImmutableList.of(clientJar))) { assertThat( cache @@ -152,6 +164,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new ClassCache( ImmutableList.of(), ImmutableList.of(libraryJar, libraryJar, libraryAnnotationsJar, libraryInterfaceJar), + ImmutableList.of(libraryJar, libraryJar, libraryAnnotationsJar, libraryInterfaceJar), ImmutableList.of(clientJar))) { assertThat( cache @@ -185,6 +198,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new LazyClasspath( ImmutableList.of(libraryJar), ImmutableList.of(libraryWoMembersJar), + ImmutableList.of(libraryWoMembersJar), ImmutableList.of()); LazyClassEntry entry = classpath.getLazyEntry("com/google/devtools/build/importdeps/testdata/Library$Class4"); @@ -206,6 +220,7 @@ public class ClassCacheTest extends AbstractClassCacheTest { new LazyClasspath( ImmutableList.of(libraryWoMembersJar), ImmutableList.of(libraryJar), + ImmutableList.of(libraryJar), ImmutableList.of()); LazyClassEntry entry = classpath.getLazyEntry("com/google/devtools/build/importdeps/testdata/Library$Class4"); @@ -219,6 +234,36 @@ public class ClassCacheTest extends AbstractClassCacheTest { } } + @Test + public void testOriginInformation() throws IOException { + try (ClassCache cache = + new ClassCache( + ImmutableList.of(bootclasspath), + ImmutableList.of(libraryJar), + ImmutableList.of(libraryJar, libraryInterfaceJar), + ImmutableList.of(clientJar))) { + assertCache( + cache, + clientJarPositives, + combine(libraryExceptionJarPositives, libraryAnnotationsJarPositives)); + + { + AbstractClassEntryState state = cache.getClassState(PACKAGE_NAME + "Library"); + assertThat(state.isExistingState()).isTrue(); + assertThat(state.classInfo().isPresent()).isTrue(); + assertThat(state.classInfo().get().directDep()).isTrue(); + assertThat((Object) state.classInfo().get().jarPath()).isEqualTo(libraryJar); + } + { + AbstractClassEntryState state = cache.getClassState(PACKAGE_NAME + "LibraryInterface"); + assertThat(state.isExistingState()).isTrue(); + assertThat(state.classInfo().isPresent()).isTrue(); + assertThat(state.classInfo().get().directDep()).isFalse(); + assertThat((Object) state.classInfo().get().jarPath()).isEqualTo(libraryInterfaceJar); + } + } + } + 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/ClassInfoTest.java b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassInfoTest.java index 2a09eac8eb..864dc27d3d 100644 --- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassInfoTest.java +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ClassInfoTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.importdeps.ClassInfo.MemberInfo; +import java.nio.file.Paths; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -31,11 +32,20 @@ public class ClassInfoTest { private final MemberInfo sizeMethod = MemberInfo.create("clear", "()V"); private final ClassInfo objectClass = - ClassInfo.create(JAVA_LANG_OBJECT, ImmutableList.of(), ImmutableSet.of(hashCodeMethod)); + ClassInfo.create( + JAVA_LANG_OBJECT, + Paths.get("a"), + true, + ImmutableList.of(), + ImmutableSet.of(hashCodeMethod)); private final ClassInfo listClass = ClassInfo.create( - "java/util/List", ImmutableList.of(objectClass), ImmutableSet.of(sizeMethod)); + "java/util/List", + Paths.get("a"), + true, + ImmutableList.of(objectClass), + ImmutableSet.of(sizeMethod)); @Test public void testMemberInfo() { @@ -57,6 +67,8 @@ public class ClassInfoTest { assertThat(objectClass.containsMember(MemberInfo.create("hashCode", "()I"))).isTrue(); assertThat(listClass.internalName()).isEqualTo("java/util/List"); + assertThat((Object) listClass.jarPath()).isEqualTo(Paths.get("a")); + assertThat(listClass.directDep()).isTrue(); assertThat(listClass.declaredMembers()).containsExactly(sizeMethod); assertThat(listClass.containsMember(hashCodeMethod)).isTrue(); } 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 c31b185178..fcffb98c21 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 @@ -114,7 +114,10 @@ public class DepsCheckerClassVisitorTest extends AbstractClassCacheTest { ResultCollector resultCollector = new ResultCollector(); try (ClassCache cache = new ClassCache( - ImmutableList.copyOf(classpath), ImmutableList.of(), ImmutableList.of()); + ImmutableList.copyOf(classpath), + ImmutableList.of(), + ImmutableList.of(), + ImmutableList.of()); ZipFile zipFile = new ZipFile(clientJar.toFile())) { assertThat(cache.getClassState("java/lang/invoke/LambdaMetafactory").isExistingState()) .isTrue(); diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ImportDepsCheckerTest.java b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ImportDepsCheckerTest.java index a42b860623..be46cb3ffb 100644 --- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ImportDepsCheckerTest.java +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ImportDepsCheckerTest.java @@ -79,7 +79,7 @@ public class ImportDepsCheckerTest extends AbstractClassCacheTest { ImmutableList<Path> expectedJdeps) throws IOException { try (ImportDepsChecker checker = - new ImportDepsChecker(bootclasspath, regularClasspath, inputJars)) { + new ImportDepsChecker(bootclasspath, regularClasspath, regularClasspath, inputJars)) { assertThat(checker.check()).isEqualTo(expectedCheckResult); Dependencies deps = checker.emitJdepsProto(DUMMY_RULE_LABEL); assertThat(deps.getDependencyList()) diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/LazyClassEntryStateTest.java b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/LazyClassEntryStateTest.java index 6b0a65134f..f042c34600 100644 --- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/LazyClassEntryStateTest.java +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/LazyClassEntryStateTest.java @@ -22,6 +22,7 @@ import com.google.devtools.build.importdeps.AbstractClassEntryState.ExistingStat import com.google.devtools.build.importdeps.AbstractClassEntryState.IncompleteState; import com.google.devtools.build.importdeps.AbstractClassEntryState.MissingState; import com.google.devtools.build.importdeps.ClassInfo.MemberInfo; +import java.nio.file.Paths; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -34,7 +35,7 @@ public class LazyClassEntryStateTest { public static final ImmutableSet<MemberInfo> METHOD_LIST = ImmutableSet.of(MemberInfo.create("hashCode", "()I")); public static final ClassInfo LIST_CLASS_INFO = - ClassInfo.create(LIST_CLASS_NAME, ImmutableList.of(), METHOD_LIST); + ClassInfo.create(LIST_CLASS_NAME, Paths.get("a"), true, ImmutableList.of(), METHOD_LIST); @Test public void testExistingState() { diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ResultCollectorTest.java b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ResultCollectorTest.java index 3b672880e3..db76c26d7b 100644 --- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ResultCollectorTest.java +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/ResultCollectorTest.java @@ -22,6 +22,8 @@ import com.google.devtools.build.importdeps.AbstractClassEntryState.MissingState import com.google.devtools.build.importdeps.ClassInfo.MemberInfo; import com.google.devtools.build.importdeps.ResultCollector.MissingMember; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -43,6 +45,8 @@ public class ResultCollectorTest { assertThat(collector.getSortedMissingClassInternalNames()).isEmpty(); assertThat(collector.getSortedMissingMembers()).isEmpty(); assertThat(collector.getSortedIncompleteClasses()).isEmpty(); + assertThat(collector.getSortedIndirectDeps()).isEmpty(); + assertThat(collector.isEmpty()).isTrue(); } @Test @@ -55,6 +59,7 @@ public class ResultCollectorTest { assertThat(collector.getSortedMissingMembers()) .containsExactly(MissingMember.create("java/lang/Object", "field", "I")); assertThat(collector.getSortedMissingClassInternalNames()).containsExactly("java.lang.String"); + assertThat(collector.isEmpty()).isFalse(); } @Test @@ -62,11 +67,15 @@ public class ResultCollectorTest { collector.addMissingOrIncompleteClass( "java/lang/String", IncompleteState.create( - ClassInfo.create("java/lang/String", ImmutableList.of(), ImmutableSet.of()), + ClassInfo.create( + "java/lang/String", Paths.get("a"), true, ImmutableList.of(), ImmutableSet.of()), ImmutableList.of("java/lang/Object"))); assertThat(collector.getSortedIncompleteClasses()).hasSize(1); assertThat(collector.getSortedIncompleteClasses().get(0).classInfo().get()) - .isEqualTo(ClassInfo.create("java/lang/String", ImmutableList.of(), ImmutableSet.of())); + .isEqualTo( + ClassInfo.create( + "java/lang/String", Paths.get("a"), true, ImmutableList.of(), ImmutableSet.of())); + assertThat(collector.isEmpty()).isFalse(); } @Test @@ -81,6 +90,17 @@ public class ResultCollectorTest { } @Test + public void testIndirectDeps() { + Path a = Paths.get("a"); + Path b = Paths.get("b"); + collector.addIndirectDep(b); + collector.addIndirectDep(a); + collector.addIndirectDep(b); + assertThat(collector.getSortedIndirectDeps()).containsExactly(a, b).inOrder(); + assertThat(collector.isEmpty()).isFalse(); + } + + @Test public void testMissingMember() { String owner = "owner"; String name = "name"; diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/golden_strictdeps.stderr.txt b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/golden_strictdeps.stderr.txt new file mode 100644 index 0000000000..8337765a37 --- /dev/null +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/golden_strictdeps.stderr.txt @@ -0,0 +1,6 @@ +ERROR: The dependencies for the following 1 jar(s) are not complete. + 1.third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata/libtestdata_client.jar +The details are listed below: +*** Missing strict dependencies. Run the following command to fix *** + + add_dep //third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata:testdata_lib_LibraryAnnotations //third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata:testdata_lib_LibraryException //third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata:testdata_lib_LibraryInterface :strictdeps_golden_test diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/golden_strictdeps.txt b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/golden_strictdeps.txt new file mode 100644 index 0000000000..8f0e2c25dd --- /dev/null +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/golden_strictdeps.txt @@ -0,0 +1,3 @@ +*** Missing strict dependencies. Run the following command to fix *** + + add_dep //third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata:testdata_lib_LibraryAnnotations //third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata:testdata_lib_LibraryException //third_party/bazel/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/testdata:testdata_lib_LibraryInterface :strictdeps_golden_test diff --git a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/tests.bzl b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/tests.bzl index a777bcfc66..e05e3bafea 100644 --- a/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/tests.bzl +++ b/src/java_tools/import_deps_checker/javatests/com/google/devtools/build/importdeps/tests.bzl @@ -15,7 +15,7 @@ def create_golden_test(name, golden_output_file, golden_stderr_file, expect_errors, checking_mode, has_bootclasspath, testdata_pkg, import_deps_checker, rt_jar, - missing_jar = None, replacing_jar = None): + missing_jar = None, replacing_jar = None, direct_jars = []): '''Create a golden test for the dependency checker.''' all_dep_jars = [ "testdata_client", @@ -57,10 +57,17 @@ def create_golden_test(name, golden_output_file, golden_stderr_file, expect_erro args.append("--classpath_entry") args.append("$(location %s:%s)" % (testdata_pkg, dep)) + for dep in direct_jars: + args.append("--directdep") + args.append("$(location %s:%s)" % (testdata_pkg, dep)) + args = args + [ "--input", "$(location %s:testdata_client)" % testdata_pkg, ] + + args.append("--rule_label=:%s" % name) + native.sh_test( name=name, srcs = ["golden_test.sh"], |