Support extra checks for ErrorProne in a dedicated property

Previous extra checks for ErrorProne were added using the plugins
proeprty to get them into the -processorpath argument.  This works
fine for java-only modules, but fails for mixed java+kotlin modules
because the processorpath is given to kapt and not javac.

Add a dedicated errorprone.extra_check_modules property (mirroring
the lint.extra_check_modules property), and add that to a separate
processorpath that is used only for errorprone rules and not cleared
when kotlin is used.

Test: TestKapt/errorprone
Change-Id: Id6ef02ce758532d1df8b8d969fad83bb44fe93ab
This commit is contained in:
Colin Cross 2020-11-19 13:52:06 -08:00
parent b479459ac9
commit 748b2d829a
2 changed files with 137 additions and 66 deletions

View file

@ -248,6 +248,9 @@ type CompilerProperties struct {
Errorprone struct {
// List of javac flags that should only be used when running errorprone.
Javacflags []string
// List of java_plugin modules that provide extra errorprone checks.
Extra_check_modules []string
}
Proto struct {
@ -569,6 +572,7 @@ var (
libTag = dependencyTag{name: "javalib"}
java9LibTag = dependencyTag{name: "java9lib"}
pluginTag = dependencyTag{name: "plugin"}
errorpronePluginTag = dependencyTag{name: "errorprone-plugin"}
exportedPluginTag = dependencyTag{name: "exported-plugin"}
bootClasspathTag = dependencyTag{name: "bootclasspath"}
systemModulesTag = dependencyTag{name: "system modules"}
@ -765,6 +769,7 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) {
}
ctx.AddFarVariationDependencies(ctx.Config().BuildOSCommonTarget.Variations(), pluginTag, j.properties.Plugins...)
ctx.AddFarVariationDependencies(ctx.Config().BuildOSCommonTarget.Variations(), errorpronePluginTag, j.properties.Errorprone.Extra_check_modules...)
ctx.AddFarVariationDependencies(ctx.Config().BuildOSCommonTarget.Variations(), exportedPluginTag, j.properties.Exported_plugins...)
android.ProtoDeps(ctx, &j.protoProperties)
@ -852,21 +857,22 @@ func (j *Module) aidlFlags(ctx android.ModuleContext, aidlPreprocess android.Opt
}
type deps struct {
classpath classpath
java9Classpath classpath
bootClasspath classpath
processorPath classpath
processorClasses []string
staticJars android.Paths
staticHeaderJars android.Paths
staticResourceJars android.Paths
aidlIncludeDirs android.Paths
srcs android.Paths
srcJars android.Paths
systemModules *systemModules
aidlPreprocess android.OptionalPath
kotlinStdlib android.Paths
kotlinAnnotations android.Paths
classpath classpath
java9Classpath classpath
bootClasspath classpath
processorPath classpath
errorProneProcessorPath classpath
processorClasses []string
staticJars android.Paths
staticHeaderJars android.Paths
staticResourceJars android.Paths
aidlIncludeDirs android.Paths
srcs android.Paths
srcJars android.Paths
systemModules *systemModules
aidlPreprocess android.OptionalPath
kotlinStdlib android.Paths
kotlinAnnotations android.Paths
disableTurbine bool
}
@ -1068,6 +1074,12 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps {
} else {
ctx.PropertyErrorf("plugins", "%q is not a java_plugin module", otherName)
}
case errorpronePluginTag:
if plugin, ok := dep.(*Plugin); ok {
deps.errorProneProcessorPath = append(deps.errorProneProcessorPath, plugin.ImplementationAndResourcesJars()...)
} else {
ctx.PropertyErrorf("plugins", "%q is not a java_plugin module", otherName)
}
case exportedPluginTag:
if plugin, ok := dep.(*Plugin); ok {
if plugin.pluginProperties.Generates_api != nil && *plugin.pluginProperties.Generates_api {
@ -1191,7 +1203,7 @@ func (j *Module) collectBuilderFlags(ctx android.ModuleContext, deps deps) javaB
flags.javaVersion = getJavaVersion(ctx, String(j.properties.Java_version), sdkContext(j))
if ctx.Config().RunErrorProne() {
if config.ErrorProneClasspath == nil {
if config.ErrorProneClasspath == nil && ctx.Config().TestProductVariables == nil {
ctx.ModuleErrorf("cannot build with Error Prone, missing external/error_prone?")
}
@ -1211,6 +1223,7 @@ func (j *Module) collectBuilderFlags(ctx android.ModuleContext, deps deps) javaB
flags.classpath = append(flags.classpath, deps.classpath...)
flags.java9Classpath = append(flags.java9Classpath, deps.java9Classpath...)
flags.processorPath = append(flags.processorPath, deps.processorPath...)
flags.errorProneProcessorPath = append(flags.errorProneProcessorPath, deps.errorProneProcessorPath...)
flags.processors = append(flags.processors, deps.processorClasses...)
flags.processors = android.FirstUniqueStrings(flags.processors)

View file

@ -84,11 +84,14 @@ func TestKotlin(t *testing.T) {
}
func TestKapt(t *testing.T) {
ctx, _ := testJava(t, `
bp := `
java_library {
name: "foo",
srcs: ["a.java", "b.kt"],
plugins: ["bar", "baz"],
errorprone: {
extra_check_modules: ["my_check"],
},
}
java_plugin {
@ -102,64 +105,119 @@ func TestKapt(t *testing.T) {
processor_class: "com.baz",
srcs: ["b.java"],
}
`)
buildOS := android.BuildOs.String()
java_plugin {
name: "my_check",
srcs: ["b.java"],
}
`
t.Run("", func(t *testing.T) {
ctx, _ := testJava(t, bp)
kapt := ctx.ModuleForTests("foo", "android_common").Rule("kapt")
kotlinc := ctx.ModuleForTests("foo", "android_common").Rule("kotlinc")
javac := ctx.ModuleForTests("foo", "android_common").Rule("javac")
buildOS := android.BuildOs.String()
bar := ctx.ModuleForTests("bar", buildOS+"_common").Rule("javac").Output.String()
baz := ctx.ModuleForTests("baz", buildOS+"_common").Rule("javac").Output.String()
kapt := ctx.ModuleForTests("foo", "android_common").Rule("kapt")
kotlinc := ctx.ModuleForTests("foo", "android_common").Rule("kotlinc")
javac := ctx.ModuleForTests("foo", "android_common").Rule("javac")
// Test that the kotlin and java sources are passed to kapt and kotlinc
if len(kapt.Inputs) != 2 || kapt.Inputs[0].String() != "a.java" || kapt.Inputs[1].String() != "b.kt" {
t.Errorf(`foo kapt inputs %v != ["a.java", "b.kt"]`, kapt.Inputs)
}
if len(kotlinc.Inputs) != 2 || kotlinc.Inputs[0].String() != "a.java" || kotlinc.Inputs[1].String() != "b.kt" {
t.Errorf(`foo kotlinc inputs %v != ["a.java", "b.kt"]`, kotlinc.Inputs)
}
bar := ctx.ModuleForTests("bar", buildOS+"_common").Rule("javac").Output.String()
baz := ctx.ModuleForTests("baz", buildOS+"_common").Rule("javac").Output.String()
// Test that only the java sources are passed to javac
if len(javac.Inputs) != 1 || javac.Inputs[0].String() != "a.java" {
t.Errorf(`foo inputs %v != ["a.java"]`, javac.Inputs)
}
// Test that the kotlin and java sources are passed to kapt and kotlinc
if len(kapt.Inputs) != 2 || kapt.Inputs[0].String() != "a.java" || kapt.Inputs[1].String() != "b.kt" {
t.Errorf(`foo kapt inputs %v != ["a.java", "b.kt"]`, kapt.Inputs)
}
if len(kotlinc.Inputs) != 2 || kotlinc.Inputs[0].String() != "a.java" || kotlinc.Inputs[1].String() != "b.kt" {
t.Errorf(`foo kotlinc inputs %v != ["a.java", "b.kt"]`, kotlinc.Inputs)
}
// Test that the kapt srcjar is a dependency of kotlinc and javac rules
if !inList(kapt.Output.String(), kotlinc.Implicits.Strings()) {
t.Errorf("expected %q in kotlinc implicits %v", kapt.Output.String(), kotlinc.Implicits.Strings())
}
if !inList(kapt.Output.String(), javac.Implicits.Strings()) {
t.Errorf("expected %q in javac implicits %v", kapt.Output.String(), javac.Implicits.Strings())
}
// Test that only the java sources are passed to javac
if len(javac.Inputs) != 1 || javac.Inputs[0].String() != "a.java" {
t.Errorf(`foo inputs %v != ["a.java"]`, javac.Inputs)
}
// Test that the kapt srcjar is extracted by the kotlinc and javac rules
if kotlinc.Args["srcJars"] != kapt.Output.String() {
t.Errorf("expected %q in kotlinc srcjars %v", kapt.Output.String(), kotlinc.Args["srcJars"])
}
if javac.Args["srcJars"] != kapt.Output.String() {
t.Errorf("expected %q in javac srcjars %v", kapt.Output.String(), kotlinc.Args["srcJars"])
}
// Test that the kapt srcjar is a dependency of kotlinc and javac rules
if !inList(kapt.Output.String(), kotlinc.Implicits.Strings()) {
t.Errorf("expected %q in kotlinc implicits %v", kapt.Output.String(), kotlinc.Implicits.Strings())
}
if !inList(kapt.Output.String(), javac.Implicits.Strings()) {
t.Errorf("expected %q in javac implicits %v", kapt.Output.String(), javac.Implicits.Strings())
}
// Test that the processors are passed to kapt
expectedProcessorPath := "-P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + bar +
" -P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + baz
if kapt.Args["kaptProcessorPath"] != expectedProcessorPath {
t.Errorf("expected kaptProcessorPath %q, got %q", expectedProcessorPath, kapt.Args["kaptProcessorPath"])
}
expectedProcessor := "-P plugin:org.jetbrains.kotlin.kapt3:processors=com.bar -P plugin:org.jetbrains.kotlin.kapt3:processors=com.baz"
if kapt.Args["kaptProcessor"] != expectedProcessor {
t.Errorf("expected kaptProcessor %q, got %q", expectedProcessor, kapt.Args["kaptProcessor"])
}
// Test that the kapt srcjar is extracted by the kotlinc and javac rules
if kotlinc.Args["srcJars"] != kapt.Output.String() {
t.Errorf("expected %q in kotlinc srcjars %v", kapt.Output.String(), kotlinc.Args["srcJars"])
}
if javac.Args["srcJars"] != kapt.Output.String() {
t.Errorf("expected %q in javac srcjars %v", kapt.Output.String(), kotlinc.Args["srcJars"])
}
// Test that the processors are not passed to javac
if javac.Args["processorPath"] != "" {
t.Errorf("expected processorPath '', got %q", javac.Args["processorPath"])
}
if javac.Args["processor"] != "-proc:none" {
t.Errorf("expected processor '-proc:none', got %q", javac.Args["processor"])
}
// Test that the processors are passed to kapt
expectedProcessorPath := "-P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + bar +
" -P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + baz
if kapt.Args["kaptProcessorPath"] != expectedProcessorPath {
t.Errorf("expected kaptProcessorPath %q, got %q", expectedProcessorPath, kapt.Args["kaptProcessorPath"])
}
expectedProcessor := "-P plugin:org.jetbrains.kotlin.kapt3:processors=com.bar -P plugin:org.jetbrains.kotlin.kapt3:processors=com.baz"
if kapt.Args["kaptProcessor"] != expectedProcessor {
t.Errorf("expected kaptProcessor %q, got %q", expectedProcessor, kapt.Args["kaptProcessor"])
}
// Test that the processors are not passed to javac
if javac.Args["processorpath"] != "" {
t.Errorf("expected processorPath '', got %q", javac.Args["processorpath"])
}
if javac.Args["processor"] != "-proc:none" {
t.Errorf("expected processor '-proc:none', got %q", javac.Args["processor"])
}
})
t.Run("errorprone", func(t *testing.T) {
env := map[string]string{
"RUN_ERROR_PRONE": "true",
}
config := testConfig(env, bp, nil)
ctx, _ := testJavaWithConfig(t, config)
buildOS := android.BuildOs.String()
kapt := ctx.ModuleForTests("foo", "android_common").Rule("kapt")
//kotlinc := ctx.ModuleForTests("foo", "android_common").Rule("kotlinc")
javac := ctx.ModuleForTests("foo", "android_common").Description("javac")
errorprone := ctx.ModuleForTests("foo", "android_common").Description("errorprone")
bar := ctx.ModuleForTests("bar", buildOS+"_common").Description("javac").Output.String()
baz := ctx.ModuleForTests("baz", buildOS+"_common").Description("javac").Output.String()
myCheck := ctx.ModuleForTests("my_check", buildOS+"_common").Description("javac").Output.String()
// Test that the errorprone plugins are not passed to kapt
expectedProcessorPath := "-P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + bar +
" -P plugin:org.jetbrains.kotlin.kapt3:apclasspath=" + baz
if kapt.Args["kaptProcessorPath"] != expectedProcessorPath {
t.Errorf("expected kaptProcessorPath %q, got %q", expectedProcessorPath, kapt.Args["kaptProcessorPath"])
}
expectedProcessor := "-P plugin:org.jetbrains.kotlin.kapt3:processors=com.bar -P plugin:org.jetbrains.kotlin.kapt3:processors=com.baz"
if kapt.Args["kaptProcessor"] != expectedProcessor {
t.Errorf("expected kaptProcessor %q, got %q", expectedProcessor, kapt.Args["kaptProcessor"])
}
// Test that the errorprone plugins are not passed to javac
if javac.Args["processorpath"] != "" {
t.Errorf("expected processorPath '', got %q", javac.Args["processorpath"])
}
if javac.Args["processor"] != "-proc:none" {
t.Errorf("expected processor '-proc:none', got %q", javac.Args["processor"])
}
// Test that the errorprone plugins are passed to errorprone
expectedProcessorPath = "-processorpath " + myCheck
if errorprone.Args["processorpath"] != expectedProcessorPath {
t.Errorf("expected processorpath %q, got %q", expectedProcessorPath, errorprone.Args["processorpath"])
}
if errorprone.Args["processor"] != "-proc:none" {
t.Errorf("expected processor '-proc:none', got %q", errorprone.Args["processor"])
}
})
}
func TestKaptEncodeFlags(t *testing.T) {