diff options
author | 2015-08-03 23:57:23 +0000 | |
---|---|---|
committer | 2015-08-04 09:09:26 +0000 | |
commit | e927532a03addbcdd1e716b8453b53d9c3eb98e1 (patch) | |
tree | 85da330d9017e57f24f94237b62beaa7ccaadeee /src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac | |
parent | cd03ca221c4686473d44315e9b448b8c001bb998 (diff) |
Don't run plugins on compilations with errors.
--
MOS_MIGRATED_REVID=99775681
Diffstat (limited to 'src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac')
2 files changed, 108 insertions, 27 deletions
diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavaCompiler.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavaCompiler.java index 9494ce7f49..e362796ee7 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavaCompiler.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavaCompiler.java @@ -14,6 +14,7 @@ package com.google.devtools.build.buildjar.javac; +import com.google.common.base.Function; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin; import com.sun.tools.javac.comp.AttrContext; @@ -26,21 +27,26 @@ import java.util.ArrayList; import java.util.List; import java.util.Queue; +import javax.annotation.Nullable; + /** - * An extended version of the javac compiler, providing support for + * An extended version of the javac compiler, providing support for * composable static analyses via a plugin mechanism. BlazeJavaCompiler * keeps a list of plugins and calls callback methods in those plugins * after certain compiler phases. The plugins perform the actual static - * analyses. + * analyses. */ public class BlazeJavaCompiler extends JavaCompiler { + private int skippedFlowEvents = 0; + private int flowEvents = 0; + /** * A list of plugins to run at particular points in the compile */ private final List<BlazeJavaCompilerPlugin> plugins = new ArrayList<>(); - public BlazeJavaCompiler(Context context, Iterable<BlazeJavaCompilerPlugin> plugins) { + private BlazeJavaCompiler(Context context, Iterable<BlazeJavaCompilerPlugin> plugins) { super(context); // initialize all plugins @@ -51,28 +57,56 @@ public class BlazeJavaCompiler extends JavaCompiler { } /** - * Adds an initialization hook to the Context, such that each subsequent - * request for a JavaCompiler (i.e., a lookup for 'compilerKey' of our - * superclass, JavaCompiler) will actually construct and return our version - * of BlazeJavaCompiler. It's necessary since many new JavaCompilers may - * be requested for later stages of the compilation (annotation processing), - * within the same Context. And it's the preferred way for extending behavior - * within javac, per the documentation in {@link Context}. + * Adds an initialization hook to the Context, such that requests for a + * JavaCompiler (i.e., a lookup for 'compilerKey' of our superclass, + * JavaCompiler) will actually construct and return BlazeJavaCompiler. + * + * This is the preferred way for extending behavior within javac, + * per the documentation in {@link Context}. + * + * Prior to JDK-8038455 additional JavaCompilers were created for + * annotation processing rounds, but we now expect a single + * compiler instance per compilation. The factory is still seems + * to be necessary to avoid context-ordering issues, but we + * assert that the factory is only called once, and save the + * output after its call for introspection. */ - public static void preRegister(final Context context, - final Iterable<BlazeJavaCompilerPlugin> plugins) { - context.put(compilerKey, new Context.Factory<JavaCompiler>() { - @Override - public JavaCompiler make(Context c) { - return new BlazeJavaCompiler(c, plugins); - } - }); + public static void preRegister( + final Context context, + final Iterable<BlazeJavaCompilerPlugin> plugins, + @Nullable final Function<BlazeJavaCompiler, Void> listener) { + context.put( + compilerKey, + new Context.Factory<JavaCompiler>() { + boolean first = true; + + @Override + public JavaCompiler make(Context c) { + if (!first) { + throw new AssertionError("Expected a single creation of BlazeJavaCompiler."); + } + first = false; + BlazeJavaCompiler compiler = new BlazeJavaCompiler(c, plugins); + if (listener != null) { + listener.apply(compiler); + } + return compiler; + } + }); + } + + public static void preRegister( + final Context context, final Iterable<BlazeJavaCompilerPlugin> plugins) { + preRegister(context, plugins, null); } @Override public Env<AttrContext> attribute(Env<AttrContext> env) { Env<AttrContext> result = super.attribute(env); - + // don't run plugins if there were compilation errors + if (errorCount() > 0) { + return result; + } // Iterate over all plugins, calling their postAttribute methods for (BlazeJavaCompilerPlugin plugin : plugins) { plugin.postAttribute(result); @@ -83,11 +117,18 @@ public class BlazeJavaCompiler extends JavaCompiler { @Override protected void flow(Env<AttrContext> env, Queue<Env<AttrContext>> results) { - if (compileStates.isDone(env, CompileState.FLOW)) { - super.flow(env, results); + boolean isDone = compileStates.isDone(env, CompileState.FLOW); + super.flow(env, results); + if (isDone) { return; } - super.flow(env, results); + // don't run plugins if there were compilation errors + if (errorCount() > 0) { + skippedFlowEvents++; + return; + } + flowEvents++; + // Iterate over all plugins, calling their postFlow methods for (BlazeJavaCompilerPlugin plugin : plugins) { plugin.postFlow(env); @@ -103,6 +144,16 @@ public class BlazeJavaCompiler extends JavaCompiler { super.close(); } + /** The number of flow events that were skipped. */ + public int skippedFlowEvents() { + return skippedFlowEvents; + } + + /** The number of error-free flow events that were passed to plugins. */ + public int flowEvents() { + return flowEvents; + } + /** * Testing purposes only. Returns true if the collection of plugins in * this instance contains one of the provided type. diff --git a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java index 057c145a74..2ac756670f 100644 --- a/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java +++ b/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/BlazeJavacMain.java @@ -14,7 +14,12 @@ package com.google.devtools.build.buildjar.javac; +import static com.google.common.base.Verify.verifyNotNull; + import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; +import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import com.google.devtools.build.buildjar.InvalidCommandLineException; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin; import com.google.devtools.build.buildjar.javac.plugins.BlazeJavaCompilerPlugin.PluginException; @@ -59,6 +64,7 @@ public class BlazeJavacMain { private List<BlazeJavaCompilerPlugin> plugins; private final PrintWriter errOutput; private final String compilerName; + private BlazeJavaCompiler compiler = null; public BlazeJavacMain(PrintWriter errOutput, List<BlazeJavaCompilerPlugin> plugins) { this.compilerName = "blaze javac"; @@ -76,13 +82,22 @@ public class BlazeJavacMain { preRegister(context, plugins); } + private final Function<BlazeJavaCompiler, Void> compilerListener = + new Function<BlazeJavaCompiler, Void>() { + @Override + public Void apply(BlazeJavaCompiler compiler) { + Verify.verify(BlazeJavacMain.this.compiler == null); + BlazeJavacMain.this.compiler = Preconditions.checkNotNull(compiler); + return null; + } + }; + public void preRegister(Context context, List<BlazeJavaCompilerPlugin> plugins) { this.plugins = plugins; for (BlazeJavaCompilerPlugin plugin : plugins) { plugin.initializeContext(context); } - - BlazeJavaCompiler.preRegister(context, plugins); + BlazeJavaCompiler.preRegister(context, plugins, compilerListener); } public Result compile(String[] argv) { @@ -121,8 +136,9 @@ public class BlazeJavacMain { @VisibleForTesting public Result compile(String[] argv, Context context) { enableEndPositions(context); + Result result = Result.ABNORMAL; try { - return new Main(compilerName, errOutput).compile(argv, context); + result = new Main(compilerName, errOutput).compile(argv, context); } catch (PropagatedException e) { if (e.getCause() instanceof PluginException) { PluginException pluginException = (PluginException) e.getCause(); @@ -130,8 +146,17 @@ public class BlazeJavacMain { return pluginException.getResult(); } e.printStackTrace(errOutput); - return Result.ABNORMAL; + result = Result.ABNORMAL; + } finally { + if (result.isOK()) { + verifyNotNull(compiler); + if (compiler.flowEvents() == 0) { + errOutput.println("Expected at least one FLOW event"); + result = Result.ABNORMAL; + } + } } + return result; } // javac9 removes the ability to pass lists of {@link JavaFileObject}s or {@link Processors}s to @@ -140,7 +165,7 @@ public class BlazeJavacMain { // behaviour closer to stock javac, but it makes it harder to write integration tests. This class // provides a compile method that accepts file objects and processors, but it isn't // guaranteed to behave exactly the same way as JavaBuilder does when used from the command-line. - // TODO(bazel-team): either stop using Main and commit to using the the API for everything, or + // TODO(cushon): either stop using Main and commit to using the the API for everything, or // re-write integration tests for JavaBuilder to use the real compile() method. @VisibleForTesting @Deprecated @@ -197,4 +222,9 @@ public class BlazeJavacMain { } return processedArgs.toArray(new String[processedArgs.size()]); } + + @VisibleForTesting + BlazeJavaCompiler getCompiler() { + return verifyNotNull(compiler); + } } |