From c0dac52975761684e82d158e532545873bd133d8 Mon Sep 17 00:00:00 2001 From: Alix Date: Thu, 4 May 2023 21:20:16 +0000 Subject: [PATCH 1/2] Make errorprone a configurable attribute for bazel conversion Change-Id: Icf2265e9f712c3255321456e977928163696dc22 Test: ./bp22build tests in child commit --- bazel/configurability.go | 31 ++++++++++++++++++++++++------- bazel/properties.go | 8 ++++---- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/bazel/configurability.go b/bazel/configurability.go index 8f63ec45b..d962a1dac 100644 --- a/bazel/configurability.go +++ b/bazel/configurability.go @@ -74,6 +74,8 @@ const ( InApex = "in_apex" NonApex = "non_apex" + + ErrorproneDisabled = "errorprone_disabled" ) func PowerSetWithoutEmptySet[T any](items []T) [][]T { @@ -216,6 +218,11 @@ var ( NonApex: "//build/bazel/rules/apex:non_apex", ConditionsDefaultConfigKey: ConditionsDefaultSelectKey, } + + errorProneMap = map[string]string{ + ErrorproneDisabled: "//build/bazel/rules/java/errorprone:errorprone_globally_disabled", + ConditionsDefaultConfigKey: ConditionsDefaultSelectKey, + } ) // basic configuration types @@ -229,6 +236,7 @@ const ( productVariables osAndInApex inApex + errorProneDisabled ) func osArchString(os string, arch string) string { @@ -237,13 +245,14 @@ func osArchString(os string, arch string) string { func (ct configurationType) String() string { return map[configurationType]string{ - noConfig: "no_config", - arch: "arch", - os: "os", - osArch: "arch_os", - productVariables: "product_variables", - osAndInApex: "os_in_apex", - inApex: "in_apex", + noConfig: "no_config", + arch: "arch", + os: "os", + osArch: "arch_os", + productVariables: "product_variables", + osAndInApex: "os_in_apex", + inApex: "in_apex", + errorProneDisabled: "errorprone_disabled", }[ct] } @@ -274,6 +283,10 @@ func (ct configurationType) validateConfig(config string) { if _, ok := inApexMap[config]; !ok { panic(fmt.Errorf("Unknown in_apex config: %s", config)) } + case errorProneDisabled: + if _, ok := errorProneMap[config]; !ok { + panic(fmt.Errorf("Unknown errorprone config: %s", config)) + } default: panic(fmt.Errorf("Unrecognized ConfigurationType %d", ct)) } @@ -303,6 +316,8 @@ func (ca ConfigurationAxis) SelectKey(config string) string { return config case inApex: return inApexMap[config] + case errorProneDisabled: + return errorProneMap[config] default: panic(fmt.Errorf("Unrecognized ConfigurationType %d", ca.configurationType)) } @@ -321,6 +336,8 @@ var ( OsAndInApexAxis = ConfigurationAxis{configurationType: osAndInApex} // An axis for in_apex-specific configurations InApexAxis = ConfigurationAxis{configurationType: inApex} + + ErrorProneAxis = ConfigurationAxis{configurationType: errorProneDisabled} ) // ProductVariableConfigurationAxis returns an axis for the given product variable diff --git a/bazel/properties.go b/bazel/properties.go index e22f4dbe7..15af09b45 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -766,7 +766,7 @@ func (lla *LabelListAttribute) SetSelectValue(axis ConfigurationAxis, config str switch axis.configurationType { case noConfig: lla.Value = list - case arch, os, osArch, productVariables, osAndInApex, inApex: + case arch, os, osArch, productVariables, osAndInApex, inApex, errorProneDisabled: if lla.ConfigurableValues == nil { lla.ConfigurableValues = make(configurableLabelLists) } @@ -782,7 +782,7 @@ func (lla *LabelListAttribute) SelectValue(axis ConfigurationAxis, config string switch axis.configurationType { case noConfig: return lla.Value - case arch, os, osArch, productVariables, osAndInApex, inApex: + case arch, os, osArch, productVariables, osAndInApex, inApex, errorProneDisabled: return lla.ConfigurableValues[axis][config] default: panic(fmt.Errorf("Unrecognized ConfigurationAxis %s", axis)) @@ -1346,7 +1346,7 @@ func (sla *StringListAttribute) SetSelectValue(axis ConfigurationAxis, config st switch axis.configurationType { case noConfig: sla.Value = list - case arch, os, osArch, productVariables, osAndInApex: + case arch, os, osArch, productVariables, osAndInApex, errorProneDisabled: if sla.ConfigurableValues == nil { sla.ConfigurableValues = make(configurableStringLists) } @@ -1362,7 +1362,7 @@ func (sla *StringListAttribute) SelectValue(axis ConfigurationAxis, config strin switch axis.configurationType { case noConfig: return sla.Value - case arch, os, osArch, productVariables, osAndInApex: + case arch, os, osArch, productVariables, osAndInApex, errorProneDisabled: return sla.ConfigurableValues[axis][config] default: panic(fmt.Errorf("Unrecognized ConfigurationAxis %s", axis)) From b1e5c6a69a3e06819e26c6d669c3a18e4ec5db1a Mon Sep 17 00:00:00 2001 From: Alix Date: Tue, 20 Jun 2023 21:00:40 +0000 Subject: [PATCH 2/2] Bp2build for errorprone modules that manually enabled/disabled it Test: go test ./bp2build Change-Id: Ie60c0959ee9ae8ce86c11a8e85a0bc7592f63df8 --- bp2build/java_library_conversion_test.go | 28 +++++++++++----- java/java.go | 42 +++++++++++++++--------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/bp2build/java_library_conversion_test.go b/bp2build/java_library_conversion_test.go index fd92e95c6..c501a7bcb 100644 --- a/bp2build/java_library_conversion_test.go +++ b/bp2build/java_library_conversion_test.go @@ -183,8 +183,8 @@ func TestJavaLibraryJavaVersion(t *testing.T) { }) } -func TestJavaLibraryErrorproneJavacflagsEnabledManually(t *testing.T) { - runJavaLibraryTestCase(t, Bp2buildTestCase{ +func TestJavaLibraryErrorproneEnabledManually(t *testing.T) { + runJavaLibraryTestCaseWithRegistrationCtxFunc(t, Bp2buildTestCase{ Blueprint: `java_library { name: "java-lib-1", srcs: ["a.java"], @@ -192,7 +192,13 @@ func TestJavaLibraryErrorproneJavacflagsEnabledManually(t *testing.T) { errorprone: { enabled: true, javacflags: ["-Xep:SpeedLimit:OFF"], + extra_check_modules: ["plugin2"], }, +} +java_plugin { + name: "plugin2", + srcs: ["a.java"], + bazel_module: { bp2build_available: false }, }`, ExpectedBazelTargets: []string{ MakeBazelTarget("java_library", "java-lib-1", AttrNameToString{ @@ -200,10 +206,14 @@ func TestJavaLibraryErrorproneJavacflagsEnabledManually(t *testing.T) { "-Xsuper-fast", "-Xep:SpeedLimit:OFF", ]`, - "srcs": `["a.java"]`, + "plugins": `[":plugin2"]`, + "srcs": `["a.java"]`, + "errorprone_force_enable": `True`, }), MakeNeverlinkDuplicateTarget("java_library", "java-lib-1"), }, + }, func(ctx android.RegistrationContext) { + ctx.RegisterModuleType("java_plugin", java.PluginFactory) }) } @@ -227,21 +237,23 @@ func TestJavaLibraryErrorproneJavacflagsErrorproneDisabledByDefault(t *testing.T }) } -func TestJavaLibraryErrorproneJavacflagsErrorproneDisabledManually(t *testing.T) { +func TestJavaLibraryErrorproneDisabledManually(t *testing.T) { runJavaLibraryTestCase(t, Bp2buildTestCase{ Blueprint: `java_library { name: "java-lib-1", srcs: ["a.java"], javacflags: ["-Xsuper-fast"], errorprone: { - enabled: false, - javacflags: ["-Xep:SpeedLimit:OFF"], + enabled: false, }, }`, ExpectedBazelTargets: []string{ MakeBazelTarget("java_library", "java-lib-1", AttrNameToString{ - "javacopts": `["-Xsuper-fast"]`, - "srcs": `["a.java"]`, + "javacopts": `[ + "-Xsuper-fast", + "-XepDisableAllChecks", + ]`, + "srcs": `["a.java"]`, }), MakeNeverlinkDuplicateTarget("java_library", "java-lib-1"), }, diff --git a/java/java.go b/java/java.go index 389cdcede..445eb61c9 100644 --- a/java/java.go +++ b/java/java.go @@ -2765,11 +2765,12 @@ func (m *Library) convertJavaResourcesAttributes(ctx android.TopDownMutatorConte type javaCommonAttributes struct { *javaResourcesAttributes *kotlinAttributes - Srcs bazel.LabelListAttribute - Plugins bazel.LabelListAttribute - Javacopts bazel.StringListAttribute - Sdk_version bazel.StringAttribute - Java_version bazel.StringAttribute + Srcs bazel.LabelListAttribute + Plugins bazel.LabelListAttribute + Javacopts bazel.StringListAttribute + Sdk_version bazel.StringAttribute + Java_version bazel.StringAttribute + Errorprone_force_enable bazel.BoolAttribute } type javaDependencyLabels struct { @@ -2911,26 +2912,35 @@ func (m *Library) convertLibraryAttrsBp2Build(ctx android.TopDownMutatorContext) staticDeps.Add(&bazel.Label{Label: ":" + javaAidlLibName}) } - var javacopts []string + var javacopts bazel.StringListAttribute //[]string + plugins := bazel.MakeLabelListAttribute( + android.BazelLabelForModuleDeps(ctx, m.properties.Plugins), + ) if m.properties.Javacflags != nil { - javacopts = append(javacopts, m.properties.Javacflags...) + javacopts = bazel.MakeStringListAttribute(m.properties.Javacflags) } epEnabled := m.properties.Errorprone.Enabled - //TODO(b/227504307) add configuration that depends on RUN_ERROR_PRONE environment variable - if Bool(epEnabled) { - javacopts = append(javacopts, m.properties.Errorprone.Javacflags...) + epJavacflags := m.properties.Errorprone.Javacflags + var errorproneForceEnable bazel.BoolAttribute + if epEnabled == nil { + //TODO(b/227504307) add configuration that depends on RUN_ERROR_PRONE environment variable + } else if *epEnabled { + plugins.Append(bazel.MakeLabelListAttribute(android.BazelLabelForModuleDeps(ctx, m.properties.Errorprone.Extra_check_modules))) + javacopts.Append(bazel.MakeStringListAttribute(epJavacflags)) + errorproneForceEnable.Value = epEnabled + } else { + javacopts.Append(bazel.MakeStringListAttribute([]string{"-XepDisableAllChecks"})) } commonAttrs := &javaCommonAttributes{ Srcs: javaSrcs, javaResourcesAttributes: m.convertJavaResourcesAttributes(ctx), - Plugins: bazel.MakeLabelListAttribute( - android.BazelLabelForModuleDeps(ctx, m.properties.Plugins), - ), - Javacopts: bazel.MakeStringListAttribute(javacopts), - Java_version: bazel.StringAttribute{Value: m.properties.Java_version}, - Sdk_version: bazel.StringAttribute{Value: m.deviceProperties.Sdk_version}, + Plugins: plugins, + Javacopts: javacopts, + Java_version: bazel.StringAttribute{Value: m.properties.Java_version}, + Sdk_version: bazel.StringAttribute{Value: m.deviceProperties.Sdk_version}, + Errorprone_force_enable: errorproneForceEnable, } for axis, configToProps := range archVariantProps {