From 69861aa57fe538f076be029d14df46733a0340b0 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Tue, 31 Jan 2023 15:49:07 -0800 Subject: [PATCH] Give lint the correct api levels database for the current module Before this change, lint was always working off of the public api database. Now, it will have the system, module-lib, or system-server databases if the current module is compiling against those SDKs. This means that a lot more methods are going to start being checked for NewApi issues. Bug: 193460475 Test: Errorprone presubmit Change-Id: I3eb2c617cd61554cd2a3f9d15fd40b3ec91f961d --- java/lint.go | 143 +++++++++++++++++++++++++++------------------- java/lint_test.go | 81 +++++++++++--------------- 2 files changed, 118 insertions(+), 106 deletions(-) diff --git a/java/lint.go b/java/lint.go index 81666bfbb..a457d4475 100644 --- a/java/lint.go +++ b/java/lint.go @@ -159,6 +159,50 @@ func (l LintDepSetsBuilder) Build() LintDepSets { } } +type lintDatabaseFiles struct { + apiVersionsModule string + apiVersionsCopiedName string + apiVersionsPrebuiltPath string + annotationsModule string + annotationCopiedName string + annotationPrebuiltpath string +} + +var allLintDatabasefiles = map[android.SdkKind]lintDatabaseFiles{ + android.SdkPublic: { + apiVersionsModule: "api_versions_public", + apiVersionsCopiedName: "api_versions_public.xml", + apiVersionsPrebuiltPath: "prebuilts/sdk/current/public/data/api-versions.xml", + annotationsModule: "sdk-annotations.zip", + annotationCopiedName: "annotations-public.zip", + annotationPrebuiltpath: "prebuilts/sdk/current/public/data/annotations.zip", + }, + android.SdkSystem: { + apiVersionsModule: "api_versions_system", + apiVersionsCopiedName: "api_versions_system.xml", + apiVersionsPrebuiltPath: "prebuilts/sdk/current/system/data/api-versions.xml", + annotationsModule: "sdk-annotations-system.zip", + annotationCopiedName: "annotations-system.zip", + annotationPrebuiltpath: "prebuilts/sdk/current/system/data/annotations.zip", + }, + android.SdkModule: { + apiVersionsModule: "api_versions_module_lib", + apiVersionsCopiedName: "api_versions_module_lib.xml", + apiVersionsPrebuiltPath: "prebuilts/sdk/current/module-lib/data/api-versions.xml", + annotationsModule: "sdk-annotations-module-lib.zip", + annotationCopiedName: "annotations-module-lib.zip", + annotationPrebuiltpath: "prebuilts/sdk/current/module-lib/data/annotations.zip", + }, + android.SdkSystemServer: { + apiVersionsModule: "api_versions_system_server", + apiVersionsCopiedName: "api_versions_system_server.xml", + apiVersionsPrebuiltPath: "prebuilts/sdk/current/system-server/data/api-versions.xml", + annotationsModule: "sdk-annotations-system-server.zip", + annotationCopiedName: "annotations-system-server.zip", + annotationPrebuiltpath: "prebuilts/sdk/current/system-server/data/annotations.zip", + }, +} + func (l *linter) LintDepSets() LintDepSets { return l.outputs.depSets } @@ -269,7 +313,12 @@ func (l *linter) writeLintProjectXML(ctx android.ModuleContext, rule *android.Ru cmd.FlagWithInput("@", android.PathForSource(ctx, "build/soong/java/lint_defaults.txt")) - cmd.FlagForEachArg("--error_check ", l.extraMainlineLintErrors) + 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("--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) @@ -415,26 +464,17 @@ func (l *linter) lint(ctx android.ModuleContext) { rule.Command().Text("mkdir -p").Flag(lintPaths.cacheDir.String()).Flag(lintPaths.homeDir.String()) rule.Command().Text("rm -f").Output(html).Output(text).Output(xml) - var apiVersionsName, apiVersionsPrebuilt string - if l.compileSdkKind == android.SdkModule || l.compileSdkKind == android.SdkSystemServer { - // When compiling an SDK module (or system server) we use the filtered - // database because otherwise lint's - // NewApi check produces too many false positives; This database excludes information - // about classes created in mainline modules hence removing those false positives. - apiVersionsName = "api_versions_public_filtered.xml" - apiVersionsPrebuilt = "prebuilts/sdk/current/public/data/api-versions-filtered.xml" - } else { - apiVersionsName = "api_versions.xml" - apiVersionsPrebuilt = "prebuilts/sdk/current/public/data/api-versions.xml" + files, ok := allLintDatabasefiles[l.compileSdkKind] + if !ok { + files = allLintDatabasefiles[android.SdkPublic] } - var annotationsZipPath, apiVersionsXMLPath android.Path if ctx.Config().AlwaysUsePrebuiltSdks() { - annotationsZipPath = android.PathForSource(ctx, "prebuilts/sdk/current/public/data/annotations.zip") - apiVersionsXMLPath = android.PathForSource(ctx, apiVersionsPrebuilt) + annotationsZipPath = android.PathForSource(ctx, files.annotationPrebuiltpath) + apiVersionsXMLPath = android.PathForSource(ctx, files.apiVersionsPrebuiltPath) } else { - annotationsZipPath = copiedAnnotationsZipPath(ctx) - apiVersionsXMLPath = copiedAPIVersionsXmlPath(ctx, apiVersionsName) + annotationsZipPath = copiedLintDatabaseFilesPath(ctx, files.annotationCopiedName) + apiVersionsXMLPath = copiedLintDatabaseFilesPath(ctx, files.apiVersionsCopiedName) } cmd := rule.Command() @@ -559,54 +599,39 @@ func (l *lintSingleton) copyLintDependencies(ctx android.SingletonContext) { return } - apiVersionsDb := findModuleOrErr(ctx, "api_versions_public") - if apiVersionsDb == nil { - if !ctx.Config().AllowMissingDependencies() { - ctx.Errorf("lint: missing module api_versions_public") + for _, sdk := range android.SortedKeys(allLintDatabasefiles) { + files := allLintDatabasefiles[sdk] + apiVersionsDb := findModuleOrErr(ctx, files.apiVersionsModule) + if apiVersionsDb == nil { + if !ctx.Config().AllowMissingDependencies() { + ctx.Errorf("lint: missing module api_versions_public") + } + return } - return - } - sdkAnnotations := findModuleOrErr(ctx, "sdk-annotations.zip") - if sdkAnnotations == nil { - if !ctx.Config().AllowMissingDependencies() { - ctx.Errorf("lint: missing module sdk-annotations.zip") + sdkAnnotations := findModuleOrErr(ctx, files.annotationsModule) + if sdkAnnotations == nil { + if !ctx.Config().AllowMissingDependencies() { + ctx.Errorf("lint: missing module sdk-annotations.zip") + } + return } - return + + ctx.Build(pctx, android.BuildParams{ + Rule: android.CpIfChanged, + Input: android.OutputFileForModule(ctx, sdkAnnotations, ""), + Output: copiedLintDatabaseFilesPath(ctx, files.annotationCopiedName), + }) + + ctx.Build(pctx, android.BuildParams{ + Rule: android.CpIfChanged, + Input: android.OutputFileForModule(ctx, apiVersionsDb, ".api_versions.xml"), + Output: copiedLintDatabaseFilesPath(ctx, files.apiVersionsCopiedName), + }) } - - filteredDb := findModuleOrErr(ctx, "api-versions-xml-public-filtered") - if filteredDb == nil { - if !ctx.Config().AllowMissingDependencies() { - ctx.Errorf("lint: missing api-versions-xml-public-filtered") - } - return - } - - ctx.Build(pctx, android.BuildParams{ - Rule: android.CpIfChanged, - Input: android.OutputFileForModule(ctx, sdkAnnotations, ""), - Output: copiedAnnotationsZipPath(ctx), - }) - - ctx.Build(pctx, android.BuildParams{ - Rule: android.CpIfChanged, - Input: android.OutputFileForModule(ctx, apiVersionsDb, ".api_versions.xml"), - Output: copiedAPIVersionsXmlPath(ctx, "api_versions.xml"), - }) - - ctx.Build(pctx, android.BuildParams{ - Rule: android.CpIfChanged, - Input: android.OutputFileForModule(ctx, filteredDb, ""), - Output: copiedAPIVersionsXmlPath(ctx, "api_versions_public_filtered.xml"), - }) } -func copiedAnnotationsZipPath(ctx android.PathContext) android.WritablePath { - return android.PathForOutput(ctx, "lint", "annotations.zip") -} - -func copiedAPIVersionsXmlPath(ctx android.PathContext, name string) android.WritablePath { +func copiedLintDatabaseFilesPath(ctx android.PathContext, name string) android.WritablePath { return android.PathForOutput(ctx, "lint", name) } diff --git a/java/lint_test.go b/java/lint_test.go index 1ab416c9e..ec901aa70 100644 --- a/java/lint_test.go +++ b/java/lint_test.go @@ -113,8 +113,9 @@ func TestJavaLintUsesCorrectBpConfig(t *testing.T) { t.Error("did not use the correct file for baseline") } - if !strings.Contains(*sboxProto.Commands[0].Command, "--error_check NewApi") { - t.Error("should check NewApi errors") + if !strings.Contains(*sboxProto.Commands[0].Command, "--warning_check NewApi") { + // TODO(b/268261262): Change this to check for --error_check + t.Error("should check NewApi warnings") } if !strings.Contains(*sboxProto.Commands[0].Command, "--error_check SomeCheck") { @@ -222,8 +223,35 @@ func TestJavaLintBypassUpdatableChecks(t *testing.T) { //} func TestJavaLintDatabaseSelectionFull(t *testing.T) { - testCases := []string{ - "current", "core_platform", "system_current", "S", "30", "10000", + testCases := []struct { + sdk_version string + expected_file string + }{ + { + "current", + "api_versions_public.xml", + }, { + "core_platform", + "api_versions_public.xml", + }, { + "system_current", + "api_versions_system.xml", + }, { + "module_current", + "api_versions_module_lib.xml", + }, { + "system_server_current", + "api_versions_system_server.xml", + }, { + "S", + "api_versions_public.xml", + }, { + "30", + "api_versions_public.xml", + }, { + "10000", + "api_versions_public.xml", + }, } bp := ` java_library { @@ -239,7 +267,7 @@ func TestJavaLintDatabaseSelectionFull(t *testing.T) { } ` for _, testCase := range testCases { - thisBp := strings.Replace(bp, "XXX", testCase, 1) + thisBp := strings.Replace(bp, "XXX", testCase.sdk_version, 1) result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules, FixtureWithPrebuiltApis(map[string][]string{ "30": {"foo"}, @@ -249,49 +277,8 @@ func TestJavaLintDatabaseSelectionFull(t *testing.T) { foo := result.ModuleForTests("foo", "android_common") sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) - if strings.Contains(*sboxProto.Commands[0].Command, - "/api_versions_public_filtered.xml") { - t.Error("used public-filtered lint api database for case", testCase) - } - if !strings.Contains(*sboxProto.Commands[0].Command, - "/api_versions.xml") { + if !strings.Contains(*sboxProto.Commands[0].Command, "/"+testCase.expected_file) { t.Error("did not use full api database for case", testCase) } } - -} - -func TestJavaLintDatabaseSelectionPublicFiltered(t *testing.T) { - testCases := []string{ - "module_current", "system_server_current", - } - bp := ` - java_library { - name: "foo", - srcs: [ - "a.java", - ], - min_sdk_version: "29", - sdk_version: "XXX", - lint: { - strict_updatability_linting: true, - }, - } -` - for _, testCase := range testCases { - thisBp := strings.Replace(bp, "XXX", testCase, 1) - result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules). - RunTestWithBp(t, thisBp) - - foo := result.ModuleForTests("foo", "android_common") - sboxProto := android.RuleBuilderSboxProtoForTests(t, foo.Output("lint.sbox.textproto")) - if !strings.Contains(*sboxProto.Commands[0].Command, - "/api_versions_public_filtered.xml") { - t.Error("did not use public-filtered lint api database for case", testCase) - } - if strings.Contains(*sboxProto.Commands[0].Command, - "/api_versions.xml") { - t.Error("used full api database for case", testCase) - } - } }