diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java index f3e7cc7a56c5e3..6319df0c28cc88 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java @@ -57,22 +57,11 @@ public final class DependencyModule { public static enum StrictJavaDeps { /** Legacy behavior: Silently allow referencing transitive dependencies. */ - OFF(false), + OFF, /** Warn about transitive dependencies being used directly. */ - WARN(true), + WARN, /** Fail the build when transitive dependencies are used directly. */ - ERROR(true); - - private final boolean enabled; - - StrictJavaDeps(boolean enabled) { - this.enabled = enabled; - } - - /** Convenience method for just checking if it's not OFF */ - public boolean isEnabled() { - return enabled; - } + ERROR } private final StrictJavaDeps strictJavaDeps; @@ -169,11 +158,6 @@ Dependencies buildDependenciesProto(ImmutableList classpath, boolean succe return deps.build(); } - /** Returns whether strict dependency checks (strictJavaDeps) are enabled. */ - public boolean isStrictDepsEnabled() { - return strictJavaDeps.isEnabled(); - } - /** Returns the paths of direct dependencies. */ public ImmutableSet directJars() { return directJars; diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java index 5fcb35d6060126..ca8d007065f1d1 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java @@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.buildjar.JarOwner; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin; -import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.StrictJavaDeps; import com.google.devtools.build.lib.view.proto.Deps; import com.google.devtools.build.lib.view.proto.Deps.Dependency; import com.sun.tools.javac.code.Flags; @@ -172,10 +171,14 @@ public void finish() { for (SjdDiagnostic diagnostic : diagnostics) { JavaFileObject prev = log.useSource(diagnostic.source()); try { - if (dependencyModule.getStrictJavaDeps() == ERROR) { - log.error(diagnostic.pos(), "proc.messager", diagnostic.message()); - } else { - log.warning(diagnostic.pos(), "proc.messager", diagnostic.message()); + switch (dependencyModule.getStrictJavaDeps()) { + case ERROR: + log.error(diagnostic.pos(), "proc.messager", diagnostic.message()); + break; + case WARN: + log.warning(diagnostic.pos(), "proc.messager", diagnostic.message()); + break; + case OFF: // continue below } } finally { log.useSource(prev); @@ -214,9 +217,6 @@ private static class CheckingTreeScanner extends TreeScanner { /** Strict deps diagnostics. */ private final List diagnostics; - /** The strict_java_deps mode */ - private final StrictJavaDeps strictJavaDepsMode; - /** Missing targets */ private final Set missingTargets; @@ -245,7 +245,6 @@ public CheckingTreeScanner( Set missingTargets, Set platformJars) { this.directJars = dependencyModule.directJars(); - this.strictJavaDepsMode = dependencyModule.getStrictJavaDeps(); this.diagnostics = diagnostics; this.missingTargets = missingTargets; this.directDependenciesMap = dependencyModule.getExplicitDependenciesMap(); @@ -275,7 +274,7 @@ private void checkTypeLiteral(JCTree node, Symbol sym) { * strict_java_deps is enabled, it emits a [strict] compiler warning/error. */ private void collectExplicitDependency(Path jarPath, JCTree node, Symbol sym) { - if (strictJavaDepsMode.isEnabled() && !isStrictDepsExempt) { + if (!isStrictDepsExempt) { // Does it make sense to emit a warning/error for this pair of (type, owner)? // We want to emit only one error/warning per owner. if (!directJars.contains(jarPath) && seenStrictDepsViolatingJars.add(jarPath)) { diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java index 1cc9391ca34a90..dd61582bd2d65d 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbine.java @@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.buildjar.javac.JavacOptions; import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule; -import com.google.devtools.build.buildjar.javac.plugins.dependency.DependencyModule.StrictJavaDeps; import com.google.devtools.build.buildjar.javac.plugins.dependency.StrictJavaDepsPlugin; import com.google.turbine.binder.ClassPathBinder; import com.google.turbine.options.TurbineOptions; @@ -115,8 +114,7 @@ Result compile() throws IOException { // To avoid having to apply the same exemptions here, we just ignore strict deps errors // and leave enforcement to JavaBuilder. ImmutableSet platformJars = ImmutableSet.copyOf(asPaths(turbineOptions.bootClassPath())); - DependencyModule dependencyModule = - buildDependencyModule(turbineOptions, StrictJavaDeps.WARN, platformJars); + DependencyModule dependencyModule = buildDependencyModule(turbineOptions, platformJars); if (sources.isEmpty()) { // accept compilations with an empty source list for compatibility with JavaBuilder @@ -261,15 +259,13 @@ static ImmutableList processJavacopts(TurbineOptions turbineOptions) { private static DependencyModule buildDependencyModule( TurbineOptions turbineOptions, - StrictJavaDeps strictDepsMode, ImmutableSet platformJars) { DependencyModule.Builder dependencyModuleBuilder = new DependencyModule.Builder() .setReduceClasspath() .setTargetLabel(turbineOptions.targetLabel().orNull()) .addDepsArtifacts(asPaths(turbineOptions.depsArtifacts())) - .setPlatformJars(platformJars) - .setStrictJavaDeps(strictDepsMode.toString()); + .setPlatformJars(platformJars); ImmutableSet.Builder directJars = ImmutableSet.builder(); for (String path : turbineOptions.directJars()) { directJars.add(Paths.get(path)); diff --git a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java index b7b8250f17138e..c598edf538254d 100644 --- a/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java +++ b/src/java_tools/buildjar/javatests/com/google/devtools/build/java/turbine/javac/JavacTurbineTest.java @@ -1207,6 +1207,37 @@ public void ignoreStrictDepsErrors() throws Exception { assertThat(result).isNotEqualTo(Result.OK_WITH_REDUCED_CLASSPATH); } + @Test + public void ignoreStrictDepsErrorsForFailingCompilations() throws Exception { + + Path lib = + createClassJar( + "deps.jar", + AbstractJavacTurbineCompilationTest.class, + JavacTurbineTest.class, + Lib.class); + + addSourceLines( + "Hello.java", + "import " + Lib.class.getCanonicalName() + ";", + "import no.such.Class;", + "class Hello extends Lib {", + "}"); + + optionsBuilder.addClassPathEntries(ImmutableList.of(lib.toString())); + + optionsBuilder.addSources(ImmutableList.copyOf(Iterables.transform(sources, TO_STRING))); + + StringWriter errOutput = new StringWriter(); + Result result; + try (JavacTurbine turbine = + new JavacTurbine(new PrintWriter(errOutput, true), optionsBuilder.build())) { + result = turbine.compile(); + } + assertThat(errOutput.toString()).doesNotContain("[strict]"); + assertThat(result).isEqualTo(Result.ERROR); + } + @Test public void clinit() throws Exception { addSourceLines(