Allow disabling errorprone even when RUN_ERROR_PRONE is true
Some modules use -XepDisableAllChecks to disable errorprone.
However, this still causes them to be compiled twice when
RUN_ERROR_PRONE is true. Allow the new enabled property to
be set to false to disable errorprone entirely, so that those
modules are only compiled once.
Bug: 190944875
Test: New unit tests
Change-Id: Ie68695db762fffcaf11bbbcb0509c4fcab73f5c5
diff --git a/java/base.go b/java/base.go
index 1daa108..a7cc58e 100644
--- a/java/base.go
+++ b/java/base.go
@@ -156,9 +156,11 @@
// List of java_plugin modules that provide extra errorprone checks.
Extra_check_modules []string
- // Whether to run errorprone on a normal build. If this is false, errorprone
- // will still be run if the RUN_ERROR_PRONE environment variable is true.
- // Default false.
+ // This property can be in 3 states. When set to true, errorprone will
+ // be run during the regular build. When set to false, errorprone will
+ // never be run. When unset, errorprone will be run when the RUN_ERROR_PRONE
+ // environment variable is true. Setting this to false will improve build
+ // performance more than adding -XepDisableAllChecks in javacflags.
Enabled *bool
}
@@ -706,7 +708,8 @@
// javaVersion flag.
flags.javaVersion = getJavaVersion(ctx, String(j.properties.Java_version), android.SdkContext(j))
- if ctx.Config().RunErrorProne() || Bool(j.properties.Errorprone.Enabled) {
+ epEnabled := j.properties.Errorprone.Enabled
+ if (ctx.Config().RunErrorProne() && epEnabled == nil) || Bool(epEnabled) {
if config.ErrorProneClasspath == nil && ctx.Config().TestProductVariables == nil {
ctx.ModuleErrorf("cannot build with Error Prone, missing external/error_prone?")
}
@@ -981,7 +984,7 @@
// If error-prone is enabled, enable errorprone flags on the regular
// build.
flags = enableErrorproneFlags(flags)
- } else if ctx.Config().RunErrorProne() {
+ } else if ctx.Config().RunErrorProne() && j.properties.Errorprone.Enabled == nil {
// Otherwise, if the RUN_ERROR_PRONE environment variable is set, create
// a new jar file just for compiling with the errorprone compiler to.
// This is because we don't want to cause the java files to get completely
diff --git a/java/java_test.go b/java/java_test.go
index 78d9ab4..0f9965d 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -1409,7 +1409,7 @@
// Test that the errorprone plugins are passed to javac
expectedSubstring := "-Xplugin:ErrorProne"
if !strings.Contains(javac.Args["javacFlags"], expectedSubstring) {
- t.Errorf("expected javacFlags to conain %q, got %q", expectedSubstring, javac.Args["javacFlags"])
+ t.Errorf("expected javacFlags to contain %q, got %q", expectedSubstring, javac.Args["javacFlags"])
}
// Modules with errorprone { enabled: true } will include errorprone checks
@@ -1420,3 +1420,67 @@
t.Errorf("expected errorprone build rule to not exist, but it did")
}
}
+
+func TestErrorproneDisabled(t *testing.T) {
+ bp := `
+ java_library {
+ name: "foo",
+ srcs: ["a.java"],
+ errorprone: {
+ enabled: false,
+ },
+ }
+ `
+ ctx := android.GroupFixturePreparers(
+ PrepareForTestWithJavaDefaultModules,
+ android.FixtureMergeEnv(map[string]string{
+ "RUN_ERROR_PRONE": "true",
+ }),
+ ).RunTestWithBp(t, bp)
+
+ javac := ctx.ModuleForTests("foo", "android_common").Description("javac")
+
+ // Test that the errorprone plugins are not passed to javac, like they would
+ // be if enabled was true.
+ expectedSubstring := "-Xplugin:ErrorProne"
+ if strings.Contains(javac.Args["javacFlags"], expectedSubstring) {
+ t.Errorf("expected javacFlags to not contain %q, got %q", expectedSubstring, javac.Args["javacFlags"])
+ }
+
+ // Check that no errorprone build rule is created, like there would be
+ // if enabled was unset and RUN_ERROR_PRONE was true.
+ errorprone := ctx.ModuleForTests("foo", "android_common").MaybeDescription("errorprone")
+ if errorprone.RuleParams.Description != "" {
+ t.Errorf("expected errorprone build rule to not exist, but it did")
+ }
+}
+
+func TestErrorproneEnabledOnlyByEnvironmentVariable(t *testing.T) {
+ bp := `
+ java_library {
+ name: "foo",
+ srcs: ["a.java"],
+ }
+ `
+ ctx := android.GroupFixturePreparers(
+ PrepareForTestWithJavaDefaultModules,
+ android.FixtureMergeEnv(map[string]string{
+ "RUN_ERROR_PRONE": "true",
+ }),
+ ).RunTestWithBp(t, bp)
+
+ javac := ctx.ModuleForTests("foo", "android_common").Description("javac")
+ errorprone := ctx.ModuleForTests("foo", "android_common").Description("errorprone")
+
+ // Check that the errorprone plugins are not passed to javac, because they
+ // will instead be passed to the separate errorprone compilation
+ expectedSubstring := "-Xplugin:ErrorProne"
+ if strings.Contains(javac.Args["javacFlags"], expectedSubstring) {
+ t.Errorf("expected javacFlags to not contain %q, got %q", expectedSubstring, javac.Args["javacFlags"])
+ }
+
+ // Check that the errorprone plugin is enabled
+ if !strings.Contains(errorprone.Args["javacFlags"], expectedSubstring) {
+ t.Errorf("expected errorprone to contain %q, got %q", expectedSubstring, javac.Args["javacFlags"])
+ }
+}