From 028b94cdc5d89e3f5a84b25b39067f902ffa448a Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Tue, 16 Jan 2024 17:17:11 -0800 Subject: Promote the NewApi check to an error again Now that all existing cases have been baselined. Ignore-AOSP-First: Requires a LSC that is difficult to do across branches. Bug: 268261262 Test: m lint-check Change-Id: I413f86ff7a1f33564465f3ba9cd52924a4242901 --- java/lint.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'java/lint.go') diff --git a/java/lint.go b/java/lint.go index c3d723b40..91e22a491 100644 --- a/java/lint.go +++ b/java/lint.go @@ -319,12 +319,7 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.FlagWithInput("@", android.PathForSource(ctx, "build/soong/java/lint_defaults.txt")) - if l.compileSdkKind == android.SdkPublic { - cmd.FlagForEachArg("--error_check ", l.extraMainlineLintErrors) - } else { - // TODO(b/268261262): Remove this branch. We're demoting NewApi to a warning due to pre-existing issues that need to be fixed. - cmd.FlagForEachArg("--warning_check ", l.extraMainlineLintErrors) - } + cmd.FlagForEachArg("--error_check ", l.extraMainlineLintErrors) cmd.FlagForEachArg("--disable_check ", l.properties.Lint.Disabled_checks) cmd.FlagForEachArg("--warning_check ", l.properties.Lint.Warning_checks) cmd.FlagForEachArg("--error_check ", l.properties.Lint.Error_checks) -- cgit v1.2.3-59-g8ed1b From 24e25c0499956d78fcbd2b8a9a9b00e773e6949f Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Fri, 19 Jan 2024 14:12:17 -0800 Subject: Re-enable strict updatability linting Now that all use cases where it would've errored on are removed. Ignore-AOSP-First: the properties were only removed in internal main to make the LSC smaller Bug: 320698986 Test: m nothing --no-skip-soong-tests, and also locally edited soong to add a quick build that runs all the strict updatability checks in the tree and ran that Change-Id: If9e23327a3c0944cc8c6849914fe51dc48bdb626 --- apex/apex_test.go | 372 ++++++++++++++++++++++++++++-------------------------- java/lint.go | 15 +-- java/lint_test.go | 95 +++++++------- 3 files changed, 246 insertions(+), 236 deletions(-) (limited to 'java/lint.go') diff --git a/apex/apex_test.go b/apex/apex_test.go index 7e67c0f9d..229175fbd 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -10273,188 +10273,196 @@ func ensureDoesNotContainRequiredDeps(t *testing.T, ctx *android.TestContext, mo } } -// TODO(b/193460475): Re-enable this test -//func TestApexStrictUpdtabilityLint(t *testing.T) { -// bpTemplate := ` -// apex { -// name: "myapex", -// key: "myapex.key", -// java_libs: ["myjavalib"], -// updatable: %v, -// min_sdk_version: "29", -// } -// apex_key { -// name: "myapex.key", -// } -// java_library { -// name: "myjavalib", -// srcs: ["MyClass.java"], -// apex_available: [ "myapex" ], -// lint: { -// strict_updatability_linting: %v, -// }, -// sdk_version: "current", -// min_sdk_version: "29", -// } -// ` -// fs := android.MockFS{ -// "lint-baseline.xml": nil, -// } -// -// testCases := []struct { -// testCaseName string -// apexUpdatable bool -// javaStrictUpdtabilityLint bool -// lintFileExists bool -// disallowedFlagExpected bool -// }{ -// { -// testCaseName: "lint-baseline.xml does not exist, no disallowed flag necessary in lint cmd", -// apexUpdatable: true, -// javaStrictUpdtabilityLint: true, -// lintFileExists: false, -// disallowedFlagExpected: false, -// }, -// { -// testCaseName: "non-updatable apex respects strict_updatability of javalib", -// apexUpdatable: false, -// javaStrictUpdtabilityLint: false, -// lintFileExists: true, -// disallowedFlagExpected: false, -// }, -// { -// testCaseName: "non-updatable apex respects strict updatability of javalib", -// apexUpdatable: false, -// javaStrictUpdtabilityLint: true, -// lintFileExists: true, -// disallowedFlagExpected: true, -// }, -// { -// testCaseName: "updatable apex sets strict updatability of javalib to true", -// apexUpdatable: true, -// javaStrictUpdtabilityLint: false, // will be set to true by mutator -// lintFileExists: true, -// disallowedFlagExpected: true, -// }, -// } -// -// for _, testCase := range testCases { -// bp := fmt.Sprintf(bpTemplate, testCase.apexUpdatable, testCase.javaStrictUpdtabilityLint) -// fixtures := []android.FixturePreparer{} -// if testCase.lintFileExists { -// fixtures = append(fixtures, fs.AddToFixture()) -// } -// -// result := testApex(t, bp, fixtures...) -// myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") -// sboxProto := android.RuleBuilderSboxProtoForTests(t, myjavalib.Output("lint.sbox.textproto")) -// disallowedFlagActual := strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml --disallowed_issues NewApi") -// -// if disallowedFlagActual != testCase.disallowedFlagExpected { -// t.Errorf("Failed testcase: %v \nActual lint cmd: %v", testCase.testCaseName, *sboxProto.Commands[0].Command) -// } -// } -//} -// -//func TestUpdatabilityLintSkipLibcore(t *testing.T) { -// bp := ` -// apex { -// name: "myapex", -// key: "myapex.key", -// java_libs: ["myjavalib"], -// updatable: true, -// min_sdk_version: "29", -// } -// apex_key { -// name: "myapex.key", -// } -// java_library { -// name: "myjavalib", -// srcs: ["MyClass.java"], -// apex_available: [ "myapex" ], -// sdk_version: "current", -// min_sdk_version: "29", -// } -// ` -// -// testCases := []struct { -// testCaseName string -// moduleDirectory string -// disallowedFlagExpected bool -// }{ -// { -// testCaseName: "lintable module defined outside libcore", -// moduleDirectory: "", -// disallowedFlagExpected: true, -// }, -// { -// testCaseName: "lintable module defined in libcore root directory", -// moduleDirectory: "libcore/", -// disallowedFlagExpected: false, -// }, -// { -// testCaseName: "lintable module defined in libcore child directory", -// moduleDirectory: "libcore/childdir/", -// disallowedFlagExpected: true, -// }, -// } -// -// for _, testCase := range testCases { -// lintFileCreator := android.FixtureAddTextFile(testCase.moduleDirectory+"lint-baseline.xml", "") -// bpFileCreator := android.FixtureAddTextFile(testCase.moduleDirectory+"Android.bp", bp) -// result := testApex(t, "", lintFileCreator, bpFileCreator) -// myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") -// sboxProto := android.RuleBuilderSboxProtoForTests(t, myjavalib.Output("lint.sbox.textproto")) -// cmdFlags := fmt.Sprintf("--baseline %vlint-baseline.xml --disallowed_issues NewApi", testCase.moduleDirectory) -// disallowedFlagActual := strings.Contains(*sboxProto.Commands[0].Command, cmdFlags) -// -// if disallowedFlagActual != testCase.disallowedFlagExpected { -// t.Errorf("Failed testcase: %v \nActual lint cmd: %v", testCase.testCaseName, *sboxProto.Commands[0].Command) -// } -// } -//} -// -//// checks transtive deps of an apex coming from bootclasspath_fragment -//func TestApexStrictUpdtabilityLintBcpFragmentDeps(t *testing.T) { -// bp := ` -// apex { -// name: "myapex", -// key: "myapex.key", -// bootclasspath_fragments: ["mybootclasspathfragment"], -// updatable: true, -// min_sdk_version: "29", -// } -// apex_key { -// name: "myapex.key", -// } -// bootclasspath_fragment { -// name: "mybootclasspathfragment", -// contents: ["myjavalib"], -// apex_available: ["myapex"], -// hidden_api: { -// split_packages: ["*"], -// }, -// } -// java_library { -// name: "myjavalib", -// srcs: ["MyClass.java"], -// apex_available: [ "myapex" ], -// sdk_version: "current", -// min_sdk_version: "29", -// compile_dex: true, -// } -// ` -// fs := android.MockFS{ -// "lint-baseline.xml": nil, -// } -// -// result := testApex(t, bp, dexpreopt.FixtureSetApexBootJars("myapex:myjavalib"), fs.AddToFixture()) -// myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") -// sboxProto := android.RuleBuilderSboxProtoForTests(t, myjavalib.Output("lint.sbox.textproto")) -// if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml --disallowed_issues NewApi") { -// t.Errorf("Strict updabality lint missing in myjavalib coming from bootclasspath_fragment mybootclasspath-fragment\nActual lint cmd: %v", *sboxProto.Commands[0].Command) -// } -//} +func TestApexStrictUpdtabilityLint(t *testing.T) { + bpTemplate := ` + apex { + name: "myapex", + key: "myapex.key", + java_libs: ["myjavalib"], + updatable: %v, + min_sdk_version: "29", + } + apex_key { + name: "myapex.key", + } + java_library { + name: "myjavalib", + srcs: ["MyClass.java"], + apex_available: [ "myapex" ], + lint: { + strict_updatability_linting: %v, + %s + }, + sdk_version: "current", + min_sdk_version: "29", + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + testCases := []struct { + testCaseName string + apexUpdatable bool + javaStrictUpdtabilityLint bool + lintFileExists bool + disallowedFlagExpected bool + }{ + { + testCaseName: "lint-baseline.xml does not exist, no disallowed flag necessary in lint cmd", + apexUpdatable: true, + javaStrictUpdtabilityLint: true, + lintFileExists: false, + disallowedFlagExpected: false, + }, + { + testCaseName: "non-updatable apex respects strict_updatability of javalib", + apexUpdatable: false, + javaStrictUpdtabilityLint: false, + lintFileExists: true, + disallowedFlagExpected: false, + }, + { + testCaseName: "non-updatable apex respects strict updatability of javalib", + apexUpdatable: false, + javaStrictUpdtabilityLint: true, + lintFileExists: true, + disallowedFlagExpected: true, + }, + { + testCaseName: "updatable apex sets strict updatability of javalib to true", + apexUpdatable: true, + javaStrictUpdtabilityLint: false, // will be set to true by mutator + lintFileExists: true, + disallowedFlagExpected: true, + }, + } + + for _, testCase := range testCases { + fixtures := []android.FixturePreparer{} + baselineProperty := "" + if testCase.lintFileExists { + fixtures = append(fixtures, fs.AddToFixture()) + baselineProperty = "baseline_filename: \"lint-baseline.xml\"" + } + bp := fmt.Sprintf(bpTemplate, testCase.apexUpdatable, testCase.javaStrictUpdtabilityLint, baselineProperty) + + result := testApex(t, bp, fixtures...) + myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") + sboxProto := android.RuleBuilderSboxProtoForTests(t, result, myjavalib.Output("lint.sbox.textproto")) + disallowedFlagActual := strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml --disallowed_issues NewApi") + + if disallowedFlagActual != testCase.disallowedFlagExpected { + t.Errorf("Failed testcase: %v \nActual lint cmd: %v", testCase.testCaseName, *sboxProto.Commands[0].Command) + } + } +} + +func TestUpdatabilityLintSkipLibcore(t *testing.T) { + bp := ` + apex { + name: "myapex", + key: "myapex.key", + java_libs: ["myjavalib"], + updatable: true, + min_sdk_version: "29", + } + apex_key { + name: "myapex.key", + } + java_library { + name: "myjavalib", + srcs: ["MyClass.java"], + apex_available: [ "myapex" ], + sdk_version: "current", + min_sdk_version: "29", + lint: { + baseline_filename: "lint-baseline.xml", + } + } + ` + + testCases := []struct { + testCaseName string + moduleDirectory string + disallowedFlagExpected bool + }{ + { + testCaseName: "lintable module defined outside libcore", + moduleDirectory: "", + disallowedFlagExpected: true, + }, + { + testCaseName: "lintable module defined in libcore root directory", + moduleDirectory: "libcore/", + disallowedFlagExpected: false, + }, + { + testCaseName: "lintable module defined in libcore child directory", + moduleDirectory: "libcore/childdir/", + disallowedFlagExpected: true, + }, + } + + for _, testCase := range testCases { + lintFileCreator := android.FixtureAddTextFile(testCase.moduleDirectory+"lint-baseline.xml", "") + bpFileCreator := android.FixtureAddTextFile(testCase.moduleDirectory+"Android.bp", bp) + result := testApex(t, "", lintFileCreator, bpFileCreator) + myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") + sboxProto := android.RuleBuilderSboxProtoForTests(t, result, myjavalib.Output("lint.sbox.textproto")) + cmdFlags := fmt.Sprintf("--baseline %vlint-baseline.xml --disallowed_issues NewApi", testCase.moduleDirectory) + disallowedFlagActual := strings.Contains(*sboxProto.Commands[0].Command, cmdFlags) + + if disallowedFlagActual != testCase.disallowedFlagExpected { + t.Errorf("Failed testcase: %v \nActual lint cmd: %v", testCase.testCaseName, *sboxProto.Commands[0].Command) + } + } +} + +// checks transtive deps of an apex coming from bootclasspath_fragment +func TestApexStrictUpdtabilityLintBcpFragmentDeps(t *testing.T) { + bp := ` + apex { + name: "myapex", + key: "myapex.key", + bootclasspath_fragments: ["mybootclasspathfragment"], + updatable: true, + min_sdk_version: "29", + } + apex_key { + name: "myapex.key", + } + bootclasspath_fragment { + name: "mybootclasspathfragment", + contents: ["myjavalib"], + apex_available: ["myapex"], + hidden_api: { + split_packages: ["*"], + }, + } + java_library { + name: "myjavalib", + srcs: ["MyClass.java"], + apex_available: [ "myapex" ], + sdk_version: "current", + min_sdk_version: "29", + compile_dex: true, + lint: { + baseline_filename: "lint-baseline.xml", + } + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + result := testApex(t, bp, dexpreopt.FixtureSetApexBootJars("myapex:myjavalib"), fs.AddToFixture()) + myjavalib := result.ModuleForTests("myjavalib", "android_common_apex29") + sboxProto := android.RuleBuilderSboxProtoForTests(t, result, myjavalib.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, "--baseline lint-baseline.xml --disallowed_issues NewApi") { + t.Errorf("Strict updabality lint missing in myjavalib coming from bootclasspath_fragment mybootclasspath-fragment\nActual lint cmd: %v", *sboxProto.Commands[0].Command) + } +} // updatable apexes should propagate updatable=true to its apps func TestUpdatableApexEnforcesAppUpdatability(t *testing.T) { diff --git a/java/lint.go b/java/lint.go index 91e22a491..bd6c1bae6 100644 --- a/java/lint.go +++ b/java/lint.go @@ -325,14 +325,13 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.FlagForEachArg("--error_check ", l.properties.Lint.Error_checks) cmd.FlagForEachArg("--fatal_check ", l.properties.Lint.Fatal_checks) - // TODO(b/193460475): Re-enable strict updatability linting - //if l.GetStrictUpdatabilityLinting() { - // // Verify the module does not baseline issues that endanger safe updatability. - // if baselinePath := l.getBaselineFilepath(ctx); baselinePath.Valid() { - // cmd.FlagWithInput("--baseline ", baselinePath.Path()) - // cmd.FlagForEachArg("--disallowed_issues ", updatabilityChecks) - // } - //} + if l.GetStrictUpdatabilityLinting() { + // Verify the module does not baseline issues that endanger safe updatability. + if l.properties.Lint.Baseline_filename != nil { + cmd.FlagWithInput("--baseline ", android.PathForModuleSrc(ctx, *l.properties.Lint.Baseline_filename)) + cmd.FlagForEachArg("--disallowed_issues ", updatabilityChecks) + } + } return lintPaths{ projectXML: projectXMLPath, diff --git a/java/lint_test.go b/java/lint_test.go index 11c999cea..6aee5946b 100644 --- a/java/lint_test.go +++ b/java/lint_test.go @@ -152,52 +152,55 @@ func TestJavaLintBypassUpdatableChecks(t *testing.T) { } } -// TODO(b/193460475): Re-enable this test -//func TestJavaLintStrictUpdatabilityLinting(t *testing.T) { -// bp := ` -// java_library { -// name: "foo", -// srcs: [ -// "a.java", -// ], -// static_libs: ["bar"], -// min_sdk_version: "29", -// sdk_version: "current", -// lint: { -// strict_updatability_linting: true, -// }, -// } -// -// java_library { -// name: "bar", -// srcs: [ -// "a.java", -// ], -// min_sdk_version: "29", -// sdk_version: "current", -// } -// ` -// fs := android.MockFS{ -// "lint-baseline.xml": nil, -// } -// -// result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules, fs.AddToFixture()). -// RunTestWithBp(t, bp) -// -// foo := result.ModuleForTests("foo", "android_common") -// sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) -// if !strings.Contains(*sboxProto.Commands[0].Command, -// "--baseline lint-baseline.xml --disallowed_issues NewApi") { -// t.Error("did not restrict baselining NewApi") -// } -// -// bar := result.ModuleForTests("bar", "android_common") -// sboxProto = android.RuleBuilderSboxProtoForTests(t, bar.Output("lint.sbox.textproto")) -// if !strings.Contains(*sboxProto.Commands[0].Command, -// "--baseline lint-baseline.xml --disallowed_issues NewApi") { -// t.Error("did not restrict baselining NewApi") -// } -//} +func TestJavaLintStrictUpdatabilityLinting(t *testing.T) { + bp := ` + java_library { + name: "foo", + srcs: [ + "a.java", + ], + static_libs: ["bar"], + min_sdk_version: "29", + sdk_version: "current", + lint: { + strict_updatability_linting: true, + baseline_filename: "lint-baseline.xml", + }, + } + + java_library { + name: "bar", + srcs: [ + "a.java", + ], + min_sdk_version: "29", + sdk_version: "current", + lint: { + baseline_filename: "lint-baseline.xml", + } + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules, fs.AddToFixture()). + RunTestWithBp(t, bp) + + foo := result.ModuleForTests("foo", "android_common") + sboxProto := android.RuleBuilderSboxProtoForTests(t, result.TestContext, foo.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, + "--baseline lint-baseline.xml --disallowed_issues NewApi") { + t.Error("did not restrict baselining NewApi") + } + + bar := result.ModuleForTests("bar", "android_common") + sboxProto = android.RuleBuilderSboxProtoForTests(t, result.TestContext, bar.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, + "--baseline lint-baseline.xml --disallowed_issues NewApi") { + t.Error("did not restrict baselining NewApi") + } +} func TestJavaLintDatabaseSelectionFull(t *testing.T) { testCases := []struct { -- cgit v1.2.3-59-g8ed1b From ada543ed86dc4828cecdb937844c6468c872d3e3 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Thu, 1 Feb 2024 17:36:57 -0800 Subject: Print the modules that had strict_updatability_linting set strict_updatability_linting imposes requirements on all of a module's transitive dependencies. So you may start getting errors on a module due to some random other module that is hard to track down. Print out the modules that caused the current module to use strict updatability linting so that these errors are easier to debug. Ignore-AOSP-First: The lint code in internal has diverged from AOSP quite a bit now Bug: 323366771 Test: m lint-check Change-Id: I43f2b1adb1ffa5b6e9516694c2bb87c7f94fed7d --- apex/apex.go | 3 ++- java/lint.go | 43 +++++++++++++++++++++--------- java/lint_test.go | 65 +++++++++++++++++++++++++++++++++++++++++++++ java/sdk_library.go | 10 +++---- scripts/lint_project_xml.py | 8 ++++-- 5 files changed, 108 insertions(+), 21 deletions(-) (limited to 'java/lint.go') diff --git a/apex/apex.go b/apex/apex.go index 276ac80d5..abab12b35 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -1104,6 +1104,7 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { return } if apex, ok := mctx.Module().(*apexBundle); ok && apex.checkStrictUpdatabilityLinting() { + parents := []string{mctx.ModuleName()} mctx.WalkDeps(func(child, parent android.Module) bool { // b/208656169 Do not propagate strict updatability linting to libcore/ // These libs are available on the classpath during compilation @@ -1117,7 +1118,7 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { return false } if lintable, ok := child.(java.LintDepSetsIntf); ok { - lintable.SetStrictUpdatabilityLinting(true) + lintable.SetStrictUpdatabilityLinting(parents) } // visit transitive deps return true diff --git a/java/lint.go b/java/lint.go index c79f6e7bb..7c06312ce 100644 --- a/java/lint.go +++ b/java/lint.go @@ -62,6 +62,10 @@ type LintProperties struct { // If true, baselining updatability lint checks (e.g. NewApi) is prohibited. Defaults to false. Strict_updatability_linting *bool + // The reverse dependency that had strict_updatability_linting set that caused this module to + // have it enabled, for use in error messages. + Strict_updatability_parents []string + // Treat the code in this module as test code for @VisibleForTesting enforcement. // This will be true by default for test module types, false otherwise. // If soong gets support for testonly, this flag should be replaced with that. @@ -117,8 +121,8 @@ type LintDepSetsIntf interface { LintDepSets() LintDepSets // Methods used to propagate strict_updatability_linting values. - GetStrictUpdatabilityLinting() bool - SetStrictUpdatabilityLinting(bool) + GetStrictUpdatabilityLinting(ctx android.BaseModuleContext) []string + SetStrictUpdatabilityLinting([]string) } type LintDepSets struct { @@ -213,12 +217,22 @@ func (l *linter) LintDepSets() LintDepSets { return l.outputs.depSets } -func (l *linter) GetStrictUpdatabilityLinting() bool { - return BoolDefault(l.properties.Lint.Strict_updatability_linting, false) +// GetStrictUpdatabilityLinting returns a list of names of modules +// that set strict_updatability_linting: true and affect the current module. +// Strict updatability linting should be enabled if the result is non-empty. +func (l *linter) GetStrictUpdatabilityLinting(ctx android.BaseModuleContext) []string { + result := l.properties.Lint.Strict_updatability_parents + if proptools.Bool(l.properties.Lint.Strict_updatability_linting) { + result = append(result, ctx.ModuleName()) + } + return result } -func (l *linter) SetStrictUpdatabilityLinting(strictLinting bool) { - l.properties.Lint.Strict_updatability_linting = &strictLinting +// SetStrictUpdatabilityLinting adds the given list of modules to this module's +// list of "strict updatable parent" modules. If then given list is non-empty, +// it causes strict updatability linting to be enabled on this module. +func (l *linter) SetStrictUpdatabilityLinting(parents []string) { + l.properties.Lint.Strict_updatability_parents = android.SortedUniqueStrings(append(l.properties.Lint.Strict_updatability_parents, parents...)) } var _ LintDepSetsIntf = (*linter)(nil) @@ -325,9 +339,10 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.FlagForEachArg("--error_check ", l.properties.Lint.Error_checks) cmd.FlagForEachArg("--fatal_check ", l.properties.Lint.Fatal_checks) - if l.GetStrictUpdatabilityLinting() { + if strict_mods := l.GetStrictUpdatabilityLinting(ctx); len(strict_mods) > 0 { // Verify the module does not baseline issues that endanger safe updatability. if l.properties.Lint.Baseline_filename != nil { + cmd.FlagWithArg("--strict_updatability_parents ", proptools.ShellEscape(strings.Join(strict_mods, ","))) cmd.FlagWithInput("--baseline ", android.PathForModuleSrc(ctx, *l.properties.Lint.Baseline_filename)) cmd.FlagForEachArg("--disallowed_issues ", updatabilityChecks) } @@ -733,11 +748,13 @@ func lintZip(ctx android.BuilderContext, paths android.Paths, outputPath android // Enforce the strict updatability linting to all applicable transitive dependencies. func enforceStrictUpdatabilityLintingMutator(ctx android.TopDownMutatorContext) { m := ctx.Module() - if d, ok := m.(LintDepSetsIntf); ok && d.GetStrictUpdatabilityLinting() { - ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) { - if a, ok := d.(LintDepSetsIntf); ok { - a.SetStrictUpdatabilityLinting(true) - } - }) + if d, ok := m.(LintDepSetsIntf); ok { + if strict_mods := d.GetStrictUpdatabilityLinting(ctx); len(strict_mods) > 0 { + ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) { + if a, ok := d.(LintDepSetsIntf); ok { + a.SetStrictUpdatabilityLinting(strict_mods) + } + }) + } } } diff --git a/java/lint_test.go b/java/lint_test.go index b51753f71..6b24daa5f 100644 --- a/java/lint_test.go +++ b/java/lint_test.go @@ -202,6 +202,71 @@ func TestJavaLintStrictUpdatabilityLinting(t *testing.T) { } } +func TestJavaLintStrictUpdatabilityLintingMultipleParents(t *testing.T) { + bp := ` + java_library { + name: "a", + srcs: ["a.java"], + static_libs: ["b"], + min_sdk_version: "29", + sdk_version: "current", + lint: { + strict_updatability_linting: true, + baseline_filename: "lint-baseline.xml", + }, + } + + java_library { + name: "b", + srcs: ["b.java"], + static_libs: ["c"], + min_sdk_version: "29", + sdk_version: "current", + lint: { + strict_updatability_linting: true, + }, + } + + java_library { + name: "d", + srcs: ["d.java"], + static_libs: ["c"], + min_sdk_version: "29", + sdk_version: "current", + lint: { + strict_updatability_linting: true, + }, + } + + java_library { + name: "c", + srcs: ["c.java"], + min_sdk_version: "29", + sdk_version: "current", + lint: { + baseline_filename: "lint-baseline.xml", + } + } + ` + fs := android.MockFS{ + "lint-baseline.xml": nil, + } + + result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules, fs.AddToFixture()). + RunTestWithBp(t, bp) + + c := result.ModuleForTests("c", "android_common") + sboxProto := android.RuleBuilderSboxProtoForTests(t, result.TestContext, c.Output("lint.sbox.textproto")) + if !strings.Contains(*sboxProto.Commands[0].Command, + "--baseline lint-baseline.xml --disallowed_issues NewApi") { + t.Error("did not restrict baselining NewApi") + } + if !strings.Contains(*sboxProto.Commands[0].Command, + "--strict_updatability_parents a,b,d") { + t.Errorf("Did not find correct strict_updatability_parents in command %q", *sboxProto.Commands[0].Command) + } +} + func TestJavaLintDatabaseSelectionFull(t *testing.T) { testCases := []struct { sdk_version string diff --git a/java/sdk_library.go b/java/sdk_library.go index 49e6727eb..570248ed3 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2968,17 +2968,17 @@ func (module *SdkLibraryImport) LintDepSets() LintDepSets { } } -func (module *SdkLibraryImport) GetStrictUpdatabilityLinting() bool { +func (module *SdkLibraryImport) GetStrictUpdatabilityLinting(ctx android.BaseModuleContext) []string { if module.implLibraryModule == nil { - return false + return nil } else { - return module.implLibraryModule.GetStrictUpdatabilityLinting() + return module.implLibraryModule.GetStrictUpdatabilityLinting(ctx) } } -func (module *SdkLibraryImport) SetStrictUpdatabilityLinting(strictLinting bool) { +func (module *SdkLibraryImport) SetStrictUpdatabilityLinting(parents []string) { if module.implLibraryModule != nil { - module.implLibraryModule.SetStrictUpdatabilityLinting(strictLinting) + module.implLibraryModule.SetStrictUpdatabilityLinting(parents) } } diff --git a/scripts/lint_project_xml.py b/scripts/lint_project_xml.py index c40b07d38..cdc968aa9 100755 --- a/scripts/lint_project_xml.py +++ b/scripts/lint_project_xml.py @@ -77,6 +77,8 @@ def parse_args(): help='file containing merged manifest for the module and its dependencies.') parser.add_argument('--baseline', dest='baseline_path', help='file containing baseline lint issues.') + parser.add_argument('--strict_updatability_parents', + help='reverse dependencies that set strict_updatability_linting: true, for use in error messages') parser.add_argument('--library', dest='library', action='store_true', help='mark the module as a library.') parser.add_argument('--test', dest='test', action='store_true', @@ -161,8 +163,10 @@ def main(): baseline = minidom.parse(args.baseline_path) disallowed_issues = check_baseline_for_disallowed_issues(baseline, args.disallowed_issues) if disallowed_issues: - sys.exit('disallowed issues %s found in lint baseline file %s for module %s' - % (disallowed_issues, args.baseline_path, args.name)) + error_message = f'disallowed issues {disallowed_issues} found in lint baseline file {args.baseline_path} for module {args.name}' + if args.strict_updatability_parents: + error_message += f'. The parent modules that set strict_updatability_linting are {args.strict_updatability_parent}' + sys.exit(error_message) if args.project_out: with open(args.project_out, 'w') as f: -- cgit v1.2.3-59-g8ed1b From 75dd41b57666ec2ad0179e9416ab2e8005627c86 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Mon, 5 Feb 2024 19:37:05 +0000 Subject: Revert "Print the modules that had strict_updatability_linting set" This reverts commit ada543ed86dc4828cecdb937844c6468c872d3e3. Reason for revert: b/323773738 Change-Id: Iaddd35b275392c8d0eb7ce1cc65523467462d7b3 --- apex/apex.go | 3 +-- java/lint.go | 43 +++++++++--------------------- java/lint_test.go | 65 --------------------------------------------- java/sdk_library.go | 10 +++---- scripts/lint_project_xml.py | 8 ++---- 5 files changed, 21 insertions(+), 108 deletions(-) (limited to 'java/lint.go') diff --git a/apex/apex.go b/apex/apex.go index abab12b35..276ac80d5 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -1104,7 +1104,6 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { return } if apex, ok := mctx.Module().(*apexBundle); ok && apex.checkStrictUpdatabilityLinting() { - parents := []string{mctx.ModuleName()} mctx.WalkDeps(func(child, parent android.Module) bool { // b/208656169 Do not propagate strict updatability linting to libcore/ // These libs are available on the classpath during compilation @@ -1118,7 +1117,7 @@ func apexStrictUpdatibilityLintMutator(mctx android.TopDownMutatorContext) { return false } if lintable, ok := child.(java.LintDepSetsIntf); ok { - lintable.SetStrictUpdatabilityLinting(parents) + lintable.SetStrictUpdatabilityLinting(true) } // visit transitive deps return true diff --git a/java/lint.go b/java/lint.go index 7c06312ce..c79f6e7bb 100644 --- a/java/lint.go +++ b/java/lint.go @@ -62,10 +62,6 @@ type LintProperties struct { // If true, baselining updatability lint checks (e.g. NewApi) is prohibited. Defaults to false. Strict_updatability_linting *bool - // The reverse dependency that had strict_updatability_linting set that caused this module to - // have it enabled, for use in error messages. - Strict_updatability_parents []string - // Treat the code in this module as test code for @VisibleForTesting enforcement. // This will be true by default for test module types, false otherwise. // If soong gets support for testonly, this flag should be replaced with that. @@ -121,8 +117,8 @@ type LintDepSetsIntf interface { LintDepSets() LintDepSets // Methods used to propagate strict_updatability_linting values. - GetStrictUpdatabilityLinting(ctx android.BaseModuleContext) []string - SetStrictUpdatabilityLinting([]string) + GetStrictUpdatabilityLinting() bool + SetStrictUpdatabilityLinting(bool) } type LintDepSets struct { @@ -217,22 +213,12 @@ func (l *linter) LintDepSets() LintDepSets { return l.outputs.depSets } -// GetStrictUpdatabilityLinting returns a list of names of modules -// that set strict_updatability_linting: true and affect the current module. -// Strict updatability linting should be enabled if the result is non-empty. -func (l *linter) GetStrictUpdatabilityLinting(ctx android.BaseModuleContext) []string { - result := l.properties.Lint.Strict_updatability_parents - if proptools.Bool(l.properties.Lint.Strict_updatability_linting) { - result = append(result, ctx.ModuleName()) - } - return result +func (l *linter) GetStrictUpdatabilityLinting() bool { + return BoolDefault(l.properties.Lint.Strict_updatability_linting, false) } -// SetStrictUpdatabilityLinting adds the given list of modules to this module's -// list of "strict updatable parent" modules. If then given list is non-empty, -// it causes strict updatability linting to be enabled on this module. -func (l *linter) SetStrictUpdatabilityLinting(parents []string) { - l.properties.Lint.Strict_updatability_parents = android.SortedUniqueStrings(append(l.properties.Lint.Strict_updatability_parents, parents...)) +func (l *linter) SetStrictUpdatabilityLinting(strictLinting bool) { + l.properties.Lint.Strict_updatability_linting = &strictLinting } var _ LintDepSetsIntf = (*linter)(nil) @@ -339,10 +325,9 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.FlagForEachArg("--error_check ", l.properties.Lint.Error_checks) cmd.FlagForEachArg("--fatal_check ", l.properties.Lint.Fatal_checks) - if strict_mods := l.GetStrictUpdatabilityLinting(ctx); len(strict_mods) > 0 { + if l.GetStrictUpdatabilityLinting() { // Verify the module does not baseline issues that endanger safe updatability. if l.properties.Lint.Baseline_filename != nil { - cmd.FlagWithArg("--strict_updatability_parents ", proptools.ShellEscape(strings.Join(strict_mods, ","))) cmd.FlagWithInput("--baseline ", android.PathForModuleSrc(ctx, *l.properties.Lint.Baseline_filename)) cmd.FlagForEachArg("--disallowed_issues ", updatabilityChecks) } @@ -748,13 +733,11 @@ func lintZip(ctx android.BuilderContext, paths android.Paths, outputPath android // Enforce the strict updatability linting to all applicable transitive dependencies. func enforceStrictUpdatabilityLintingMutator(ctx android.TopDownMutatorContext) { m := ctx.Module() - if d, ok := m.(LintDepSetsIntf); ok { - if strict_mods := d.GetStrictUpdatabilityLinting(ctx); len(strict_mods) > 0 { - ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) { - if a, ok := d.(LintDepSetsIntf); ok { - a.SetStrictUpdatabilityLinting(strict_mods) - } - }) - } + if d, ok := m.(LintDepSetsIntf); ok && d.GetStrictUpdatabilityLinting() { + ctx.VisitDirectDepsWithTag(staticLibTag, func(d android.Module) { + if a, ok := d.(LintDepSetsIntf); ok { + a.SetStrictUpdatabilityLinting(true) + } + }) } } diff --git a/java/lint_test.go b/java/lint_test.go index 6b24daa5f..b51753f71 100644 --- a/java/lint_test.go +++ b/java/lint_test.go @@ -202,71 +202,6 @@ func TestJavaLintStrictUpdatabilityLinting(t *testing.T) { } } -func TestJavaLintStrictUpdatabilityLintingMultipleParents(t *testing.T) { - bp := ` - java_library { - name: "a", - srcs: ["a.java"], - static_libs: ["b"], - min_sdk_version: "29", - sdk_version: "current", - lint: { - strict_updatability_linting: true, - baseline_filename: "lint-baseline.xml", - }, - } - - java_library { - name: "b", - srcs: ["b.java"], - static_libs: ["c"], - min_sdk_version: "29", - sdk_version: "current", - lint: { - strict_updatability_linting: true, - }, - } - - java_library { - name: "d", - srcs: ["d.java"], - static_libs: ["c"], - min_sdk_version: "29", - sdk_version: "current", - lint: { - strict_updatability_linting: true, - }, - } - - java_library { - name: "c", - srcs: ["c.java"], - min_sdk_version: "29", - sdk_version: "current", - lint: { - baseline_filename: "lint-baseline.xml", - } - } - ` - fs := android.MockFS{ - "lint-baseline.xml": nil, - } - - result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules, fs.AddToFixture()). - RunTestWithBp(t, bp) - - c := result.ModuleForTests("c", "android_common") - sboxProto := android.RuleBuilderSboxProtoForTests(t, result.TestContext, c.Output("lint.sbox.textproto")) - if !strings.Contains(*sboxProto.Commands[0].Command, - "--baseline lint-baseline.xml --disallowed_issues NewApi") { - t.Error("did not restrict baselining NewApi") - } - if !strings.Contains(*sboxProto.Commands[0].Command, - "--strict_updatability_parents a,b,d") { - t.Errorf("Did not find correct strict_updatability_parents in command %q", *sboxProto.Commands[0].Command) - } -} - func TestJavaLintDatabaseSelectionFull(t *testing.T) { testCases := []struct { sdk_version string diff --git a/java/sdk_library.go b/java/sdk_library.go index 570248ed3..49e6727eb 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2968,17 +2968,17 @@ func (module *SdkLibraryImport) LintDepSets() LintDepSets { } } -func (module *SdkLibraryImport) GetStrictUpdatabilityLinting(ctx android.BaseModuleContext) []string { +func (module *SdkLibraryImport) GetStrictUpdatabilityLinting() bool { if module.implLibraryModule == nil { - return nil + return false } else { - return module.implLibraryModule.GetStrictUpdatabilityLinting(ctx) + return module.implLibraryModule.GetStrictUpdatabilityLinting() } } -func (module *SdkLibraryImport) SetStrictUpdatabilityLinting(parents []string) { +func (module *SdkLibraryImport) SetStrictUpdatabilityLinting(strictLinting bool) { if module.implLibraryModule != nil { - module.implLibraryModule.SetStrictUpdatabilityLinting(parents) + module.implLibraryModule.SetStrictUpdatabilityLinting(strictLinting) } } diff --git a/scripts/lint_project_xml.py b/scripts/lint_project_xml.py index cdc968aa9..c40b07d38 100755 --- a/scripts/lint_project_xml.py +++ b/scripts/lint_project_xml.py @@ -77,8 +77,6 @@ def parse_args(): help='file containing merged manifest for the module and its dependencies.') parser.add_argument('--baseline', dest='baseline_path', help='file containing baseline lint issues.') - parser.add_argument('--strict_updatability_parents', - help='reverse dependencies that set strict_updatability_linting: true, for use in error messages') parser.add_argument('--library', dest='library', action='store_true', help='mark the module as a library.') parser.add_argument('--test', dest='test', action='store_true', @@ -163,10 +161,8 @@ def main(): baseline = minidom.parse(args.baseline_path) disallowed_issues = check_baseline_for_disallowed_issues(baseline, args.disallowed_issues) if disallowed_issues: - error_message = f'disallowed issues {disallowed_issues} found in lint baseline file {args.baseline_path} for module {args.name}' - if args.strict_updatability_parents: - error_message += f'. The parent modules that set strict_updatability_linting are {args.strict_updatability_parent}' - sys.exit(error_message) + sys.exit('disallowed issues %s found in lint baseline file %s for module %s' + % (disallowed_issues, args.baseline_path, args.name)) if args.project_out: with open(args.project_out, 'w') as f: -- cgit v1.2.3-59-g8ed1b