Merge "Allow disabling errorprone even when RUN_ERROR_PRONE is true"

java/base.go

@@ -156,9 +156,11 @@ type CommonProperties struct {
 		// 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 @@ func (j *Module) collectBuilderFlags(ctx android.ModuleContext, deps deps) javaB
 	// 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 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) {
 			// 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

java/java_test.go

@@ -1409,7 +1409,7 @@ func TestErrorproneEnabled(t *testing.T) {
 	// 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 @@ func TestErrorproneEnabled(t *testing.T) {
 		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"])
+	}
+}