Add a build flag to always enable errorprone per-target

Currently, errorprone is only run if the RUN_ERROR_PRONE
enviornment variable is true. Add a flag that individual
modules can use to always enable errorprone.

In a followup cl, I plan to add another flag that will
force all errorprone checks to be errors, so that modules
can be confident that they're not ignoring any errorprone
checks.

Bug: 190944875
Test: New unit test and manually
Change-Id: Iab0c81642ed22a736add054147829e91a891d179

java/base.go

@@ -155,6 +155,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.
+		Enabled *bool
 	}
 
 	Proto struct {
@@ -701,7 +706,7 @@ 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() {
+	if ctx.Config().RunErrorProne() || Bool(j.properties.Errorprone.Enabled) {
 		if config.ErrorProneClasspath == nil && ctx.Config().TestProductVariables == nil {
 			ctx.ModuleErrorf("cannot build with Error Prone, missing external/error_prone?")
 		}
@@ -972,14 +977,23 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) {
 	}
 	if len(uniqueSrcFiles) > 0 || len(srcJars) > 0 {
 		var extraJarDeps android.Paths
-		if ctx.Config().RunErrorProne() {
+		if Bool(j.properties.Errorprone.Enabled) {
-			// If error-prone is enabled, add an additional rule to compile the java files into
+			// If error-prone is enabled, enable errorprone flags on the regular
-			// a separate set of classes (so that they don't overwrite the normal ones and require
+			// build.
-			// a rebuild when error-prone is turned off).
+			flags = enableErrorproneFlags(flags)
-			// TODO(ccross): Once we always compile with javac9 we may be able to conditionally
+		} else if ctx.Config().RunErrorProne() {
-			//    enable error-prone without affecting the output class files.
+			// 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
+			// rebuilt every time the state of the RUN_ERROR_PRONE variable changes.
+			// We also don't want to run this if errorprone is enabled by default for
+			// this module, or else we could have duplicated errorprone messages.
+			errorproneFlags := enableErrorproneFlags(flags)
 			errorprone := android.PathForModuleOut(ctx, "errorprone", jarName)
-			RunErrorProne(ctx, errorprone, uniqueSrcFiles, srcJars, flags)
+
+			transformJavaToClasses(ctx, errorprone, -1, uniqueSrcFiles, srcJars, errorproneFlags, nil,
+				"errorprone", "errorprone")
+
 			extraJarDeps = append(extraJarDeps, errorprone)
 		}
 
@@ -1303,6 +1317,21 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) {
 	j.outputFile = outputFile.WithoutRel()
 }
 
+// Returns a copy of the supplied flags, but with all the errorprone-related
+// fields copied to the regular build's fields.
+func enableErrorproneFlags(flags javaBuilderFlags) javaBuilderFlags {
+	flags.processorPath = append(flags.errorProneProcessorPath, flags.processorPath...)
+
+	if len(flags.errorProneExtraJavacFlags) > 0 {
+		if len(flags.javacFlags) > 0 {
+			flags.javacFlags += " " + flags.errorProneExtraJavacFlags
+		} else {
+			flags.javacFlags = flags.errorProneExtraJavacFlags
+		}
+	}
+	return flags
+}
+
 func (j *Module) compileJavaClasses(ctx android.ModuleContext, jarName string, idx int,
 	srcFiles, srcJars android.Paths, flags javaBuilderFlags, extraJarDeps android.Paths) android.WritablePath {
 

java/builder.go

@@ -279,23 +279,6 @@ func TransformJavaToClasses(ctx android.ModuleContext, outputFile android.Writab
 	transformJavaToClasses(ctx, outputFile, shardIdx, srcFiles, srcJars, flags, deps, "javac", desc)
 }
 
-func RunErrorProne(ctx android.ModuleContext, outputFile android.WritablePath,
-	srcFiles, srcJars android.Paths, flags javaBuilderFlags) {
-
-	flags.processorPath = append(flags.errorProneProcessorPath, flags.processorPath...)
-
-	if len(flags.errorProneExtraJavacFlags) > 0 {
-		if len(flags.javacFlags) > 0 {
-			flags.javacFlags += " " + flags.errorProneExtraJavacFlags
-		} else {
-			flags.javacFlags = flags.errorProneExtraJavacFlags
-		}
-	}
-
-	transformJavaToClasses(ctx, outputFile, -1, srcFiles, srcJars, flags, nil,
-		"errorprone", "errorprone")
-}
-
 // Emits the rule to generate Xref input file (.kzip file) for the given set of source files and source jars
 // to compile with given set of builder flags, etc.
 func emitXrefRule(ctx android.ModuleContext, xrefFile android.WritablePath, idx int,

java/java_test.go

@@ -1392,3 +1392,31 @@ func TestDefaultInstallable(t *testing.T) {
 	assertDeepEquals(t, "Default installable value should be true.", proptools.BoolPtr(true),
 		module.properties.Installable)
 }
+
+func TestErrorproneEnabled(t *testing.T) {
+	ctx, _ := testJava(t, `
+		java_library {
+			name: "foo",
+			srcs: ["a.java"],
+			errorprone: {
+				enabled: true,
+			},
+		}
+	`)
+
+	javac := ctx.ModuleForTests("foo", "android_common").Description("javac")
+
+	// 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"])
+	}
+
+	// Modules with errorprone { enabled: true } will include errorprone checks
+	// in the main javac build rule. Only when RUN_ERROR_PRONE is true will
+	// the explicit errorprone build rule be created.
+	errorprone := ctx.ModuleForTests("foo", "android_common").MaybeDescription("errorprone")
+	if errorprone.RuleParams.Description != "" {
+		t.Errorf("expected errorprone build rule to not exist, but it did")
+	}
+}

java/kotlin_test.go

@@ -185,7 +185,6 @@ func TestKapt(t *testing.T) {
 		buildOS := android.BuildOs.String()
 
 		kapt := result.ModuleForTests("foo", "android_common").Rule("kapt")
-		//kotlinc := ctx.ModuleForTests("foo", "android_common").Rule("kotlinc")
 		javac := result.ModuleForTests("foo", "android_common").Description("javac")
 		errorprone := result.ModuleForTests("foo", "android_common").Description("errorprone")