From 18233a2b47a45faf4682129a7c0bf2ab064e7054 Mon Sep 17 00:00:00 2001 From: Pedro Loureiro Date: Tue, 8 Jun 2021 18:11:21 +0000 Subject: [PATCH] Use trimmed lint database for mainline modules Lint's NewApi checks currently produce a lot of false positive findings. The filtered lint database removes information of classes defined by mainline modules which are the cases that might become a false positive. This commit updates soong to use this database instead of the normal one when linting mainline modules. Test: m lint-check Fixes: 186478867 Change-Id: Ica646081b9189303c393b36b2f02914d69eee291 --- java/base.go | 1 + java/lint.go | 65 ++++++++++++++++++++++++++++++++------------ java/lint_test.go | 69 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 17 deletions(-) diff --git a/java/base.go b/java/base.go index a7cc58e33..82576b55f 100644 --- a/java/base.go +++ b/java/base.go @@ -1292,6 +1292,7 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { j.linter.minSdkVersion = lintSDKVersionString(j.MinSdkVersion(ctx)) j.linter.targetSdkVersion = lintSDKVersionString(j.TargetSdkVersion(ctx)) j.linter.compileSdkVersion = lintSDKVersionString(j.SdkVersion(ctx)) + j.linter.compileSdkKind = j.SdkVersion(ctx).Kind j.linter.javaLanguageLevel = flags.javaVersion.String() j.linter.kotlinLanguageLevel = "1.3" if !apexInfo.IsForPlatform() && ctx.Config().UnbundledBuildApps() { diff --git a/java/lint.go b/java/lint.go index 1511cfe26..dd5e4fb13 100644 --- a/java/lint.go +++ b/java/lint.go @@ -78,6 +78,7 @@ type linter struct { minSdkVersion string targetSdkVersion string compileSdkVersion string + compileSdkKind android.SdkKind javaLanguageLevel string kotlinLanguageLevel string outputs lintOutputs @@ -389,13 +390,25 @@ 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 { + // When compiling an SDK module 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" + } + var annotationsZipPath, apiVersionsXMLPath android.Path if ctx.Config().AlwaysUsePrebuiltSdks() { annotationsZipPath = android.PathForSource(ctx, "prebuilts/sdk/current/public/data/annotations.zip") - apiVersionsXMLPath = android.PathForSource(ctx, "prebuilts/sdk/current/public/data/api-versions.xml") + apiVersionsXMLPath = android.PathForSource(ctx, apiVersionsPrebuilt) } else { annotationsZipPath = copiedAnnotationsZipPath(ctx) - apiVersionsXMLPath = copiedAPIVersionsXmlPath(ctx) + apiVersionsXMLPath = copiedAPIVersionsXmlPath(ctx, apiVersionsName) } cmd := rule.Command() @@ -487,23 +500,27 @@ func (l *lintSingleton) GenerateBuildActions(ctx android.SingletonContext) { l.copyLintDependencies(ctx) } +func findModuleOrErr(ctx android.SingletonContext, moduleName string) android.Module { + var res android.Module + ctx.VisitAllModules(func(m android.Module) { + if ctx.ModuleName(m) == moduleName { + if res == nil { + res = m + } else { + ctx.Errorf("lint: multiple %s modules found: %s and %s", moduleName, + ctx.ModuleSubDir(m), ctx.ModuleSubDir(res)) + } + } + }) + return res +} + func (l *lintSingleton) copyLintDependencies(ctx android.SingletonContext) { if ctx.Config().AlwaysUsePrebuiltSdks() { return } - var frameworkDocStubs android.Module - ctx.VisitAllModules(func(m android.Module) { - if ctx.ModuleName(m) == "framework-doc-stubs" { - if frameworkDocStubs == nil { - frameworkDocStubs = m - } else { - ctx.Errorf("lint: multiple framework-doc-stubs modules found: %s and %s", - ctx.ModuleSubDir(m), ctx.ModuleSubDir(frameworkDocStubs)) - } - } - }) - + frameworkDocStubs := findModuleOrErr(ctx, "framework-doc-stubs") if frameworkDocStubs == nil { if !ctx.Config().AllowMissingDependencies() { ctx.Errorf("lint: missing framework-doc-stubs") @@ -511,6 +528,14 @@ func (l *lintSingleton) copyLintDependencies(ctx android.SingletonContext) { return } + 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, frameworkDocStubs, ".annotations.zip"), @@ -520,7 +545,13 @@ func (l *lintSingleton) copyLintDependencies(ctx android.SingletonContext) { ctx.Build(pctx, android.BuildParams{ Rule: android.CpIfChanged, Input: android.OutputFileForModule(ctx, frameworkDocStubs, ".api_versions.xml"), - Output: copiedAPIVersionsXmlPath(ctx), + 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"), }) } @@ -528,8 +559,8 @@ func copiedAnnotationsZipPath(ctx android.PathContext) android.WritablePath { return android.PathForOutput(ctx, "lint", "annotations.zip") } -func copiedAPIVersionsXmlPath(ctx android.PathContext) android.WritablePath { - return android.PathForOutput(ctx, "lint", "api_versions.xml") +func copiedAPIVersionsXmlPath(ctx android.PathContext, name string) android.WritablePath { + return android.PathForOutput(ctx, "lint", name) } func (l *lintSingleton) generateLintReportZips(ctx android.SingletonContext) { diff --git a/java/lint_test.go b/java/lint_test.go index 6d64de7e9..9cf1c33fe 100644 --- a/java/lint_test.go +++ b/java/lint_test.go @@ -219,3 +219,72 @@ func TestJavaLintStrictUpdatabilityLinting(t *testing.T) { t.Error("did not restrict baselining NewApi") } } + +func TestJavaLintDatabaseSelectionFull(t *testing.T) { + testCases := []string{ + "current", "core_platform", "system_current", "S", "30", "10000", + } + 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, FixtureWithPrebuiltApis(map[string][]string{ + "30": {"foo"}, + "10000": {"foo"}, + })). + 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("used public-filtered lint api database for case", testCase) + } + if !strings.Contains(*sboxProto.Commands[0].Command, + "/api_versions.xml") { + t.Error("did not use full api database for case", testCase) + } + } + +} + +func TestJavaLintDatabaseSelectionPublicFiltered(t *testing.T) { + bp := ` + java_library { + name: "foo", + srcs: [ + "a.java", + ], + min_sdk_version: "29", + sdk_version: "module_current", + lint: { + strict_updatability_linting: true, + }, + } +` + result := android.GroupFixturePreparers(PrepareForTestWithJavaDefaultModules). + 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, + "/api_versions_public_filtered.xml") { + t.Error("did not use public-filtered lint api database", *sboxProto.Commands[0].Command) + } + if strings.Contains(*sboxProto.Commands[0].Command, + "/api_versions.xml") { + t.Error("used full api database") + } +}