From b76ae6583def2dd1f7dea2e2be140ee6b80ab681 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Wed, 10 Jun 2026 08:46:18 -0700 Subject: [PATCH 01/12] poc-split-into-analysis --- rules/BUILD.bazel | 15 + rules/attrs.bzl | 6 +- rules/collect_aar_outputs_aspect.bzl | 1 + rules/impl.bzl | 158 ++++++--- rules/lint_analysis_aspect.bzl | 162 +++++++++ rules/providers.bzl | 12 + src/cli/AndroidLintActionArgs.kt | 48 ++- src/cli/AndroidLintProject.kt | 47 +++ src/cli/AndroidLintRunner.kt | 362 +++++++++++++++------ tests/src/cli/AndroidLintActionArgsTest.kt | 39 +++ tests/src/cli/AndroidLintProjectTest.kt | 67 ++++ 11 files changed, 768 insertions(+), 149 deletions(-) create mode 100644 rules/lint_analysis_aspect.bzl diff --git a/rules/BUILD.bazel b/rules/BUILD.bazel index dffdf38..d0f9b30 100644 --- a/rules/BUILD.bazel +++ b/rules/BUILD.bazel @@ -62,6 +62,7 @@ bzl_library( visibility = ["//visibility:public"], deps = [ ":collect_aar_outputs_aspect", + ":lint_analysis_aspect", ], ) @@ -106,6 +107,20 @@ bzl_library( visibility = ["//visibility:public"], ) +bzl_library( + name = "lint_analysis_aspect", + srcs = ["lint_analysis_aspect.bzl"], + visibility = ["//visibility:public"], + deps = [ + ":collect_aar_outputs_aspect", + ":providers", + ":utils", + "@rules_android//providers:providers_bzl", + "@rules_java//java:rules", + "@rules_java//java/private:proto_support", + ], +) + filegroup( name = "all_files", srcs = glob(["*"]), diff --git a/rules/attrs.bzl b/rules/attrs.bzl index 3e84256..e82da5f 100644 --- a/rules/attrs.bzl +++ b/rules/attrs.bzl @@ -5,6 +5,10 @@ load( ":collect_aar_outputs_aspect.bzl", _collect_aar_outputs_aspect = "collect_aar_outputs_aspect", ) +load( + ":lint_analysis_aspect.bzl", + _lint_analysis_aspect = "lint_analysis_aspect", +) ATTRS = dict( _lint_wrapper = attr.label( @@ -38,7 +42,7 @@ ATTRS = dict( mandatory = False, allow_empty = True, default = [], - aspects = [_collect_aar_outputs_aspect], + aspects = [_collect_aar_outputs_aspect, _lint_analysis_aspect], doc = "Dependencies that should be on the classpath during execution.", ), android_lint_config = attr.label( diff --git a/rules/collect_aar_outputs_aspect.bzl b/rules/collect_aar_outputs_aspect.bzl index 1663945..dc4f177 100644 --- a/rules/collect_aar_outputs_aspect.bzl +++ b/rules/collect_aar_outputs_aspect.bzl @@ -74,4 +74,5 @@ def _collect_aar_outputs_aspect(tgt, ctx): collect_aar_outputs_aspect = aspect( implementation = _collect_aar_outputs_aspect, attr_aspects = ["aar", "deps", "exports", "associates"], + provides = [AndroidLintAARInfo], ) diff --git a/rules/impl.bzl b/rules/impl.bzl index 41e71aa..6dcf4a3 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -12,6 +12,7 @@ load( ) load( ":providers.bzl", + _AndroidLintPartialResultsInfo = "AndroidLintPartialResultsInfo", _AndroidLintResultsInfo = "AndroidLintResultsInfo", ) load( @@ -22,9 +23,12 @@ load( def _run_android_lint( ctx, + mode, android_lint, module_name, output, + partial_results, + dependency_modules, srcs, deps, aars, @@ -41,17 +45,20 @@ def _run_android_lint( enable_checks, autofix, regenerate, - android_lint_enable_check_dependencies, android_lint_skip_bytecode_verifier, android_lint_toolchain, java_runtime_info): - """Constructs the Android Lint actions + """Constructs an Android Lint action for the given phase. Args: ctx: The target context + mode: One of "analyze" (write partial results) or "report" (consume them, emit XML) android_lint: The Android Lint binary to use module_name: The name of the module - output: The output file + output: The XML output file (report mode only; None in analyze mode) + partial_results: The partial-results directory (output in analyze, input in report) + dependency_modules: List of structs(module_name, partial_results) for first-party deps + whose partial results should be merged (report mode only) srcs: The source files deps: Depset of aars and jars to include on the classpath aars: Depset of the aar nodes @@ -68,13 +75,13 @@ def _run_android_lint( enable_checks: List of additional checks to enable autofix: Whether to autofix (This is a no-op feature right now) regenerate: Whether to regenerate the baseline files - android_lint_enable_check_dependencies: Enables dependency checking during analysis android_lint_skip_bytecode_verifier: Disables bytecode verification android_lint_toolchain: The android lint toolchain java_runtime_info: The java runtime toolchain info """ + is_report = mode == "report" inputs = [] - outputs = [output] + outputs = [] args = ctx.actions.args() args.set_param_file_format("multiline") @@ -82,7 +89,16 @@ def _run_android_lint( args.add("--android-lint-cli-tool", android_lint) inputs.append(android_lint) - args.add("--label", "{}".format(module_name)) + args.add("--label", module_name) + args.add("--mode", mode) + + # The partial-results directory is an output of analyze and an input of report. + args.add("--partial-results", partial_results.path) + if is_report: + inputs.append(partial_results) + else: + outputs.append(partial_results) + if compile_sdk_version: args.add("--compile-sdk-version", compile_sdk_version) if java_language_level: @@ -98,21 +114,12 @@ def _run_android_lint( if manifest: args.add("--android-manifest", manifest) inputs.append(manifest) - if not regenerate and baseline: - args.add("--baseline-file", baseline) - inputs.append(baseline) - if regenerate: - args.add("--regenerate-baseline-files") if config: args.add("--config-file", config) inputs.append(config) - if warnings_as_errors: - args.add("--warnings-as-errors") for custom_rule in _utils.list_or_depset_to_list(custom_rules): args.add("--custom-rule", custom_rule) inputs.append(custom_rule) - if autofix == True: - args.add("--autofix") for check in disable_checks: args.add("--disable-check", check) for check in enable_checks: @@ -129,12 +136,24 @@ def _run_android_lint( args.add("--classpath-aar", "%s:%s" % (aar.path, aar_dir.path)) inputs.append(aar) inputs.append(aar_dir) - if android_lint_enable_check_dependencies: - args.add("--enable-check-dependencies") - # Declare the output file - args.add("--output", output) - outputs.append(output) + # Report-phase-only arguments: baseline, reporting filters, output, and the dependency + # partial results merged into the final verdict. + if is_report: + if not regenerate and baseline: + args.add("--baseline-file", baseline) + inputs.append(baseline) + if regenerate: + args.add("--regenerate-baseline-files") + if warnings_as_errors: + args.add("--warnings-as-errors") + if autofix == True: + args.add("--autofix") + for dependency in dependency_modules: + args.add("--dependency-partial-results", "%s=%s" % (dependency.module_name, dependency.partial_results.path)) + inputs.append(dependency.partial_results) + args.add("--output", output) + outputs.append(output) if android_lint_toolchain.android_home != None: args.add("--android-home", android_lint_toolchain.android_home.label.workspace_root) @@ -145,11 +164,14 @@ def _run_android_lint( inputs.extend(java_runtime_info.files.to_list()) ctx.actions.run( - mnemonic = "AndroidLint", + mnemonic = "AndroidLintAnalyze" if mode == "analyze" else "AndroidLint", inputs = inputs, outputs = outputs, executable = ctx.executable._lint_wrapper, - progress_message = "Running Android Lint {}".format(str(ctx.label)), + progress_message = "{} Android Lint {}".format( + "Analyzing" if mode == "analyze" else "Reporting", + str(ctx.label), + ), arguments = [args], tools = [ctx.executable._lint_wrapper], toolchain = _ANDROID_LINT_TOOLCHAIN_TYPE, @@ -180,9 +202,32 @@ def _get_module_name(ctx): return "%s_%s" % (path.replace("/", "_").replace("-", "_"), ctx.attr.name) return name +def _collect_dependency_modules(ctx): + """Collects the transitive partial-results modules from the rule's dependencies. + + Returns: + A deduplicated list of structs(module_name, partial_results) for every analyzed + transitive dependency. + """ + transitive = [] + for dep in ctx.attr.deps: + if _AndroidLintPartialResultsInfo in dep: + transitive.append(dep[_AndroidLintPartialResultsInfo].transitive_results) + seen = {} + modules = [] + for node in depset(transitive = transitive).to_list(): + if node.module_name and node.module_name not in seen: + seen[node.module_name] = True + modules.append(node) + return modules + def process_android_lint_issues(ctx, regenerate): """Runs Android Lint for the given target + Runs the analysis phase on the target's own sources to produce partial results, then the + report phase to merge those (and, when check-dependencies is enabled, the dependencies' + partial results produced by lint_analysis_aspect) into the final XML report. + Args: ctx: The target context regenerate: Whether to regenerate the baseline files @@ -195,7 +240,7 @@ def process_android_lint_issues(ctx, regenerate): # exactly `AndroidManifest.xml`. manifest = ctx.file.manifest if manifest and manifest.basename != "AndroidManifest.xml": - manifest = ctx.actions.declare_file("AndroidManifest.xml") + manifest = ctx.actions.declare_file("{}/AndroidManifest.xml".format(ctx.label.name)) ctx.actions.symlink(output = manifest, target_file = ctx.file.manifest) # Collect the transitive classpath jars to run lint against. @@ -229,32 +274,65 @@ def process_android_lint_issues(ctx, regenerate): _utils.list_or_depset_to_list(_utils.get_android_lint_toolchain(ctx).android_lint_config.files), ) - output = ctx.actions.declare_file("{}.xml".format(ctx.label.name)) - _run_android_lint( - ctx, - android_lint = _utils.only(_utils.list_or_depset_to_list(_utils.get_android_lint_toolchain(ctx).android_lint.files)), - module_name = _get_module_name(ctx), - output = output, + toolchain = _utils.get_android_lint_toolchain(ctx) + android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) + module_name = _get_module_name(ctx) + deps_depset = depset(transitive = deps) + aars_depset = depset(transitive = aars) + java_runtime_info = ctx.attr._javabase[java_common.JavaRuntimeInfo] + + common = dict( + android_lint = android_lint, + module_name = module_name, srcs = ctx.files.srcs, - deps = depset(transitive = deps), - aars = depset(transitive = aars), + deps = deps_depset, + aars = aars_depset, resource_files = ctx.files.resource_files, manifest = manifest, - compile_sdk_version = _utils.get_android_lint_toolchain(ctx).compile_sdk_version, - java_language_level = _utils.get_android_lint_toolchain(ctx).java_language_level, - kotlin_language_level = _utils.get_android_lint_toolchain(ctx).kotlin_language_level, - baseline = getattr(ctx.file, "baseline", None), + compile_sdk_version = toolchain.compile_sdk_version, + java_language_level = toolchain.java_language_level, + kotlin_language_level = toolchain.kotlin_language_level, config = config, - warnings_as_errors = ctx.attr.warnings_as_errors, custom_rules = ctx.files.custom_rules, disable_checks = ctx.attr.disable_checks, enable_checks = ctx.attr.enable_checks, + android_lint_skip_bytecode_verifier = toolchain.android_lint_skip_bytecode_verifier, + android_lint_toolchain = toolchain, + java_runtime_info = java_runtime_info, + ) + + # Analysis phase: analyze this target's own sources, producing partial results. + own_partial_results = ctx.actions.declare_directory("{}_lint_partial_results".format(ctx.label.name)) + _run_android_lint( + ctx, + mode = "analyze", + output = None, + partial_results = own_partial_results, + dependency_modules = [], + baseline = None, + warnings_as_errors = False, + autofix = False, + regenerate = False, + **common + ) + + # Report phase: merge this target's own partial results and, when enabled, the dependencies'. + dependency_modules = [] + if toolchain.android_lint_enable_check_dependencies: + dependency_modules = _collect_dependency_modules(ctx) + + output = ctx.actions.declare_file("{}.xml".format(ctx.label.name)) + _run_android_lint( + ctx, + mode = "report", + output = output, + partial_results = own_partial_results, + dependency_modules = dependency_modules, + baseline = getattr(ctx.file, "baseline", None), + warnings_as_errors = ctx.attr.warnings_as_errors, autofix = ctx.attr.autofix, regenerate = regenerate, - android_lint_enable_check_dependencies = _utils.get_android_lint_toolchain(ctx).android_lint_enable_check_dependencies, - android_lint_skip_bytecode_verifier = _utils.get_android_lint_toolchain(ctx).android_lint_skip_bytecode_verifier, - android_lint_toolchain = _utils.get_android_lint_toolchain(ctx), - java_runtime_info = ctx.attr._javabase[java_common.JavaRuntimeInfo], + **common ) return struct( diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl new file mode 100644 index 0000000..66783bb --- /dev/null +++ b/rules/lint_analysis_aspect.bzl @@ -0,0 +1,162 @@ +"""Aspect that runs the Android Lint analysis (`--analyze-only`) phase per target. + +For each first-party target it visits, the aspect runs lint in analyze mode, producing a +per-target partial-results directory, and propagates those outputs up the dependency graph via +[AndroidLintPartialResultsInfo]. A leaf rule later runs the report (`--report-only`) phase over +the transitive set. The analysis of a target is isolated: it depends only on that target's own +sources plus its dependencies' compiled artifacts, never on another target's partial results. +""" + +load( + "@rules_android//providers:providers.bzl", + "AndroidLibraryResourceClassJarProvider", +) +load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") +load( + ":collect_aar_outputs_aspect.bzl", + _AndroidLintAARInfo = "AndroidLintAARInfo", +) +load( + ":providers.bzl", + _AndroidLintPartialResultsInfo = "AndroidLintPartialResultsInfo", +) +load( + ":utils.bzl", + _ANDROID_LINT_TOOLCHAIN_TYPE = "ANDROID_LINT_TOOLCHAIN_TYPE", + _utils = "utils", +) + +# Edges traversed by the analysis aspect, matching collect_aar_outputs_aspect. +_ANALYSIS_ATTR_ASPECTS = ["deps", "exports", "associates"] + +def _module_name(label): + """Derives a stable, unique lint module name from a target label.""" + return ("%s_%s" % (label.package, label.name)).replace("/", "_").replace("-", "_").replace(".", "_").replace(":", "_") + +def _aspect_deps(ctx): + deps = [] + for attr in _ANALYSIS_ATTR_ASPECTS: + deps.extend(getattr(ctx.rule.attr, attr, [])) + return deps + +def _collect_transitive(deps): + return [ + dep[_AndroidLintPartialResultsInfo].transitive_results + for dep in deps + if _AndroidLintPartialResultsInfo in dep + ] + +def _lint_analysis_aspect_impl(target, ctx): + deps = _aspect_deps(ctx) + transitive = _collect_transitive(deps) + + # Skip nodes that have nothing to analyze (no JavaInfo or no sources), but keep propagating + # the transitive results gathered from their dependencies. + srcs = getattr(ctx.rule.files, "srcs", []) + if JavaInfo not in target or not srcs: + return [_AndroidLintPartialResultsInfo( + partial_results = None, + module_name = None, + transitive_results = depset(transitive = transitive), + )] + + toolchain = _utils.get_android_lint_toolchain(ctx) + module_name = _module_name(ctx.label) + partial_results = ctx.actions.declare_directory("_lint/%s/partial_results" % ctx.label.name) + android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) + java_runtime_info = ctx.attr._javabase[java_common.JavaRuntimeInfo] + + inputs = [android_lint] + inputs.extend(srcs) + + args = ctx.actions.args() + args.set_param_file_format("multiline") + args.use_param_file("@%s", use_always = True) + + args.add("--android-lint-cli-tool", android_lint) + args.add("--label", module_name) + args.add("--mode", "analyze") + args.add("--partial-results", partial_results.path) + if toolchain.compile_sdk_version: + args.add("--compile-sdk-version", toolchain.compile_sdk_version) + if toolchain.java_language_level: + args.add("--java-language-level", toolchain.java_language_level) + if toolchain.kotlin_language_level: + args.add("--kotlin-language-level", toolchain.kotlin_language_level) + + for src in srcs: + args.add("--src", src) + + # Classpath for symbol resolution: this target's full compile classpath (its own outputs plus + # its transitive dependencies) and any compiled R classes. + classpath = [target[JavaInfo].transitive_compile_time_jars] + if AndroidLibraryResourceClassJarProvider in target: + classpath.append(target[AndroidLibraryResourceClassJarProvider].jars) + for jar in depset(transitive = classpath).to_list(): + args.add("--classpath-jar", jar) + inputs.append(jar) + + # AAR dependencies (extracted), so lint loads their classes and embedded lint.jar checks. + if _AndroidLintAARInfo in target: + for node in target[_AndroidLintAARInfo].transitive_aar_artifacts.to_list(): + if node.aar and node.aar_dir: + args.add("--classpath-aar", "%s:%s" % (node.aar.path, node.aar_dir.path)) + inputs.append(node.aar) + inputs.append(node.aar_dir) + + if toolchain.android_lint_config: + config = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint_config.files)) + args.add("--config-file", config) + inputs.append(config) + + if toolchain.android_home != None: + args.add("--android-home", toolchain.android_home.label.workspace_root) + + if java_runtime_info: + args.add("--jdk-home", java_runtime_info.java_home) + inputs.extend(java_runtime_info.files.to_list()) + + ctx.actions.run( + mnemonic = "AndroidLintAnalyze", + inputs = inputs, + outputs = [partial_results], + executable = ctx.executable._lint_wrapper, + progress_message = "Analyzing Android Lint {}".format(str(ctx.label)), + arguments = [args], + tools = [ctx.executable._lint_wrapper], + toolchain = _ANDROID_LINT_TOOLCHAIN_TYPE, + execution_requirements = { + "supports-workers": "1", + "supports-multiplex-workers": "1", + }, + env = { + "ANDROID_LINT_SKIP_BYTECODE_VERIFIER": ( + "true" if toolchain.android_lint_skip_bytecode_verifier else "false" + ), + }, + ) + + direct = struct(module_name = module_name, partial_results = partial_results) + return [_AndroidLintPartialResultsInfo( + partial_results = partial_results, + module_name = module_name, + transitive_results = depset(direct = [direct], transitive = transitive), + )] + +lint_analysis_aspect = aspect( + implementation = _lint_analysis_aspect_impl, + attr_aspects = _ANALYSIS_ATTR_ASPECTS, + attrs = { + "_lint_wrapper": attr.label( + default = Label("//src/cli"), + executable = True, + cfg = "exec", + ), + "_javabase": attr.label( + default = "@bazel_tools//tools/jdk:current_java_runtime", + ), + }, + provides = [_AndroidLintPartialResultsInfo], + required_aspect_providers = [[_AndroidLintAARInfo]], + toolchains = ["//toolchains:toolchain_type"], +) diff --git a/rules/providers.bzl b/rules/providers.bzl index 2992a84..2590d97 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -7,3 +7,15 @@ AndroidLintResultsInfo = provider( "output": "The Android Lint baseline output", }, ) + +AndroidLintPartialResultsInfo = provider( + "Per-target Android Lint analysis (--analyze-only) outputs, propagated up the dependency " + + "graph so a leaf rule can run the report (--report-only) phase over the transitive set.", + fields = { + "partial_results": "Directory File of this target's partial results, or None if this " + + "node was not analyzed (e.g. no sources).", + "module_name": "The lint module name for this target, or None if not analyzed.", + "transitive_results": "depset of structs(module_name, partial_results) for this target " + + "and all analyzed transitive dependencies.", + }, +) diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 574f1bc..b6cb67a 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -18,6 +18,16 @@ internal class AndroidLintActionArgs( Pair(Paths.get(aar), Paths.get(aarDir)) } + private val argsParserDependencyModuleTransformer: String.() -> Pair = { + // Format: =. The name is opaque and may itself contain + // no '=', so split on the first occurrence only. + val separator = this.indexOf('=') + require(separator > 0) { + "Error: --dependency-partial-results expects =, got: $this" + } + Pair(this.substring(0, separator), Paths.get(this.substring(separator + 1))) + } + val androidLintCliTool: Path by parser.storing( names = arrayOf("--android-lint-cli-tool"), help = "", @@ -29,6 +39,32 @@ internal class AndroidLintActionArgs( help = "", ) + // Execution mode. "legacy" runs analysis and reporting in a single invocation (the original + // behavior). "analyze" runs `--analyze-only` and writes partial results. "report" runs + // `--report-only`, merging the module's own and its dependencies' partial results into a report. + val mode: String by parser + .storing( + names = arrayOf("--mode"), + help = "One of: legacy, analyze, report", + ).default { "legacy" } + + // In analyze mode, the directory lint writes partial results into. In report mode, the directory + // lint reads the module's own partial results from. + val partialResults: Path? by parser + .storing( + names = arrayOf("--partial-results"), + help = "", + transform = argsParserPathTransformer, + ).default { null } + + // First-party dependency partial results consumed in report mode, as = pairs. + val dependencyPartialResults: List> by parser + .adding( + names = arrayOf("--dependency-partial-results"), + help = "", + transform = argsParserDependencyModuleTransformer, + ).default { emptyList() } + val androidHome: String? by parser .storing( names = arrayOf("--android-home"), @@ -48,11 +84,13 @@ internal class AndroidLintActionArgs( transform = argsParserPathTransformer, ) - val output: Path by parser.storing( - names = arrayOf("--output"), - help = "", - transform = argsParserPathTransformer, - ) + // The XML report output. Required in legacy and report modes; absent in analyze mode. + val output: Path? by parser + .storing( + names = arrayOf("--output"), + help = "", + transform = argsParserPathTransformer, + ).default { null } val resources: List by parser .adding( diff --git a/src/cli/AndroidLintProject.kt b/src/cli/AndroidLintProject.kt index eb1a239..b58a26a 100644 --- a/src/cli/AndroidLintProject.kt +++ b/src/cli/AndroidLintProject.kt @@ -10,6 +10,18 @@ import javax.xml.transform.stream.StreamResult import kotlin.io.path.absolutePathString import kotlin.io.path.pathString +/** + * A first-party dependency module contributing partial analysis results to the report phase. + * + * In the report (`--report-only`) phase lint merges these modules' partial results into the main + * module's verdict without re-analyzing their sources. Each is registered as a library module + * carrying its own `partial-results-dir` and linked from the main module via a `` element. + */ +internal data class LintDependencyModule( + val name: String, + val partialResultsDir: Path, +) + internal fun createProjectXMLString( moduleName: String, rootDir: String, @@ -20,6 +32,12 @@ internal fun createProjectXMLString( classpathAars: List, classpathExtractedAarDirectories: List>, customLintChecks: List, + partialResultsDir: Path? = null, + dependencyModules: List = emptyList(), + // Scratch partial-results directory assigned to AAR dependency projects during partial + // analysis. These projects are not analyzed, but lint's partial-analysis detectors dereference + // every project's partial-results-dir (e.g. JoinEffectDetector), so it must be non-null. + aarPartialResultsScratchDir: Path? = null, ): String { val document = DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument() @@ -35,6 +53,11 @@ internal fun createProjectXMLString( document.createElement("module").also { it.setAttribute("name", moduleName) it.setAttribute("android", if (androidManifest != null) "true" else "false") + // The partial-results-dir is where lint writes results in `--analyze-only` and reads them + // back in `--report-only`. Absent in the legacy single-shot mode. + if (partialResultsDir != null) { + it.setAttribute("partial-results-dir", partialResultsDir.absolutePathString()) + } // it.setAttribute("library", "false") // it.setAttribute("compile-sdk-version", "get-actual-value-here") projectElement.appendChild(it) @@ -74,6 +97,9 @@ internal fun createProjectXMLString( classpathAars.forEach { aar -> val element = document.createElement("aar") element.setAttribute("file", aar.absolutePathString()) + if (aarPartialResultsScratchDir != null) { + element.setAttribute("partial-results-dir", aarPartialResultsScratchDir.absolutePathString()) + } moduleElement.appendChild(element) } @@ -81,9 +107,30 @@ internal fun createProjectXMLString( val element = document.createElement("aar") element.setAttribute("file", aar.absolutePathString()) element.setAttribute("extracted", unzippedDir.absolutePathString()) + if (aarPartialResultsScratchDir != null) { + element.setAttribute("partial-results-dir", aarPartialResultsScratchDir.absolutePathString()) + } moduleElement.appendChild(element) } + // Link the main module to each first-party dependency that contributed partial results, then + // register those dependencies as library modules carrying their own partial-results-dir. + dependencyModules.forEach { dependency -> + document.createElement("dep").also { + it.setAttribute("module", dependency.name) + moduleElement.appendChild(it) + } + } + + dependencyModules.forEach { dependency -> + document.createElement("module").also { + it.setAttribute("name", dependency.name) + it.setAttribute("library", "true") + it.setAttribute("partial-results-dir", dependency.partialResultsDir.absolutePathString()) + projectElement.appendChild(it) + } + } + return StringWriter() .apply { val transformer = TransformerFactory.newInstance().newTransformer() diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index 4b5658f..00e4dd1 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -15,26 +15,223 @@ internal class AndroidLintRunner( internal fun runAndroidLint( args: AndroidLintActionArgs, workingDirectory: Path, + ): Int = + when (args.mode) { + "analyze" -> runAnalyze(args, workingDirectory) + "report" -> runReport(args, workingDirectory) + "legacy" -> runLegacy(args, workingDirectory) + else -> error("Unknown --mode '${args.mode}'; expected one of: legacy, analyze, report") + } + + /** + * Analysis phase (`--analyze-only`). Analyzes this module in isolation and writes partial + * results into [AndroidLintActionArgs.partialResults]. Produces no report and never fails on + * lint issues — baseline, severity, and reporting are deferred to the report phase. + */ + private fun runAnalyze( + args: AndroidLintActionArgs, + workingDirectory: Path, ): Int { - // Create the input baseline file. This is either a copy of the existing baseline - // or a new temp one that can be written to - val baselineFile = workingDirectory.resolve("${args.label}_lint_baseline") - if (!args.regenerateBaselineFile && args.baselineFile != null) { - Files.copy(args.baselineFile!!, baselineFile) + val partialResults = + requireNotNull(args.partialResults) { "--partial-results is required in analyze mode" } + val rootDir = rootDir() + val projectFile = + writeProjectXml( + args = args, + workingDirectory = workingDirectory, + rootDir = rootDir, + partialResultsDir = partialResults, + dependencyModules = emptyList(), + ) + + val lintArgs = + buildList { + add("--project") + add(projectFile.pathString) + add("--analyze-only") + addCommonArgs(args, rootDir, workingDirectory) + } + + return when (invoke(args, lintArgs, workingDirectory)) { + AndroidLintCliInvoker.ERRNO_SUCCESS -> 0 + else -> AndroidLintCliInvoker.ERRNO_ERRORS } + } + + /** + * Report phase (`--report-only`). Merges this module's own partial results and those of its + * first-party dependencies into the final XML report, applying baseline and severity. + */ + private fun runReport( + args: AndroidLintActionArgs, + workingDirectory: Path, + ): Int { + val output = requireNotNull(args.output) { "--output is required in report mode" } + val partialResults = + requireNotNull(args.partialResults) { "--partial-results is required in report mode" } + val baselineFile = stageBaseline(args, workingDirectory) + val rootDir = rootDir() + val projectFile = + writeProjectXml( + args = args, + workingDirectory = workingDirectory, + rootDir = rootDir, + partialResultsDir = partialResults, + dependencyModules = + args.dependencyPartialResults.map { (name, dir) -> LintDependencyModule(name, dir) }, + ) + + val lintArgs = + buildList { + add("--project") + add(projectFile.pathString) + add("--report-only") + add("--xml") + add(output.pathString) + add("--exitcode") + add("--baseline") + add(baselineFile.pathString) + add("--update-baseline") + addReportFilters(args) + addCommonArgs(args, rootDir, workingDirectory) + } + + // When first-party dependency partial results are present, enable check-dependencies so the + // merge phase reports their incidents (rather than analyzing — the partial results already + // exist). Lint reads each dependency module's partial-results-dir instead of its sources. + return reportExitCode( + invoke( + args, + lintArgs, + workingDirectory, + enableCheckDependencies = args.dependencyPartialResults.isNotEmpty(), + ), + ) + } + + /** Legacy single-invocation behavior: analysis and reporting in one pass. */ + private fun runLegacy( + args: AndroidLintActionArgs, + workingDirectory: Path, + ): Int { + val output = requireNotNull(args.output) { "--output is required in legacy mode" } + val baselineFile = stageBaseline(args, workingDirectory) + val rootDir = rootDir() + val projectFile = + writeProjectXml( + args = args, + workingDirectory = workingDirectory, + rootDir = rootDir, + partialResultsDir = null, + dependencyModules = emptyList(), + ) + + val lintArgs = + buildList { + add("--project") + add(projectFile.pathString) + add("--xml") + add(output.pathString) + add("--exitcode") + add("--baseline") + add(baselineFile.pathString) + add("--update-baseline") + addReportFilters(args) + addCommonArgs(args, rootDir, workingDirectory) + } - // Collect the custom lint rules from the unpacked aars - val aarLintRuleJars = - args.classpathAarPairs - .asSequence() - .map { it.second.resolve("lint.jar") } - .filter { it.exists() && it.isRegularFile() } + return reportExitCode( + invoke( + args, + lintArgs, + workingDirectory, + enableCheckDependencies = args.enableCheckDependencies, + ), + ) + } + + /** Arguments shared by all modes: project description, language levels, cache, SDK/JDK. */ + private fun MutableList.addCommonArgs( + args: AndroidLintActionArgs, + rootDir: String, + workingDirectory: Path, + ) { + add("--path-variables") + add("PWD=$rootDir") + add("--compile-sdk-version") + add(args.compileSdkVersion) + add("--java-language-level") + add(args.javaLanguageLevel) + add("--kotlin-language-level") + add(args.kotlinLanguageLevel) + add("--stacktrace") + add("--quiet") + add("--offline") + // The lint analysis cache is ephemeral scratch in the Bazel sandbox: Bazel, not lint, owns + // incrementality, and a persisted cache would be hidden, non-hermetic state. It is created + // under the per-request working directory and discarded with it. + val cacheDir = workingDirectory.resolve("android-cache") + Files.createDirectories(cacheDir) + add("--cache-dir") + add(cacheDir.pathString) + add("--client-id") + add("cli") - // Create the project configuration file for lint + // Check selection must be consistent across analyze and report: the analyze phase decides + // which detectors run, and the report phase finalizes them with the same set. + if (args.config != null) { + add("--config") + add(args.config!!.pathString) + } + if (args.enableChecks.isNotEmpty()) { + add("--enable") + add(args.enableChecks.joinToString(",")) + } + if (args.disableChecks.isNotEmpty()) { + add("--disable") + add(args.disableChecks.joinToString(",")) + } + + if (args.androidHome?.isNotEmpty() == true) { + add("--sdk-home") + add(Paths.get(rootDir, args.androidHome).absolutePathString()) + } + if (args.jdkHome != null) { + add("--jdk-home") + add(args.jdkHome!!.absolutePathString()) + } + } + + /** Reporting-only filters: warnings-as-errors handling, applied in report and legacy modes. */ + private fun MutableList.addReportFilters(args: AndroidLintActionArgs) { + if (args.warningsAsErrors) { + add("-Werror") + } else { + add("--nowarn") + } + } + + private fun writeProjectXml( + args: AndroidLintActionArgs, + workingDirectory: Path, + rootDir: String, + partialResultsDir: Path?, + dependencyModules: List, + ): Path { + val aarLintRuleJars = collectAarLintRuleJars(args.classpathAarPairs) val projectFile = workingDirectory.resolve("${args.label}_project_config.xml") Files.createFile(projectFile) - val rootDir = System.getenv("PWD") + // During partial analysis (analyze/report), AAR dependency projects must carry a non-null + // partial-results-dir or lint's partial-analysis detectors NPE dereferencing it. They are not + // analyzed, so an ephemeral shared scratch directory suffices. + val aarPartialResultsScratchDir = + if (partialResultsDir != null) { + workingDirectory.resolve("aar-partial-results").also { Files.createDirectories(it) } + } else { + null + } + projectFile.writeText( createProjectXMLString( moduleName = args.label, @@ -46,108 +243,67 @@ internal class AndroidLintRunner( classpathAars = emptyList(), classpathExtractedAarDirectories = args.classpathAarPairs, customLintChecks = (args.customChecks + aarLintRuleJars).sortedDescending(), + partialResultsDir = partialResultsDir, + dependencyModules = dependencyModules, + aarPartialResultsScratchDir = aarPartialResultsScratchDir, ), ) + return projectFile + } - // Run Android Lint - val androidCacheFolder = workingDirectory.resolve("android-cache") - Files.createDirectory(androidCacheFolder) + /** + * Stages the input baseline into the working directory so lint can update it in place without + * mutating the (read-only) source baseline. Returns the path of the staged baseline. + */ + private fun stageBaseline( + args: AndroidLintActionArgs, + workingDirectory: Path, + ): Path { + val baselineFile = workingDirectory.resolve("${args.label}_lint_baseline") + if (!args.regenerateBaselineFile && args.baselineFile != null) { + Files.copy(args.baselineFile!!, baselineFile) + } + return baselineFile + } + + private fun invoke( + args: AndroidLintActionArgs, + lintArgs: List, + workingDirectory: Path, + enableCheckDependencies: Boolean = false, + ): Int { val lintUserHomeFolder = workingDirectory.resolve("lint-user-home") - Files.createDirectory(lintUserHomeFolder) + Files.createDirectories(lintUserHomeFolder) val invoker = invokerCache.acquire(listOf(args.androidLintCliTool)) - val exitCode = - try { - System.setProperty("user.home", lintUserHomeFolder.toString()) - invokeAndroidLintCLI( - invoker = invoker, - actionArgs = args, - rootDirPath = rootDir, - projectFilePath = projectFile, - baselineFilePath = baselineFile, - cacheDirectoryPath = androidCacheFolder, - ) - } finally { - invokerCache.release(invoker) - } + return try { + System.setProperty("user.home", lintUserHomeFolder.toString()) + invoker.invoke( + args = lintArgs.toTypedArray(), + enableCheckDependencies = enableCheckDependencies, + ) + } finally { + invokerCache.release(invoker) + } + } - return when (exitCode) { + private fun reportExitCode(exitCode: Int): Int = + when (exitCode) { AndroidLintCliInvoker.ERRNO_SUCCESS, AndroidLintCliInvoker.ERRNO_CREATED_BASELINE, -> 0 else -> exitCode } - } - private fun invokeAndroidLintCLI( - invoker: AndroidLintCliInvoker, - actionArgs: AndroidLintActionArgs, - rootDirPath: String, - projectFilePath: Path, - baselineFilePath: Path, - cacheDirectoryPath: Path, - ): Int { - val args = - mutableListOf( - "--project", - projectFilePath.pathString, - "--xml", - actionArgs.output.pathString, - "--path-variables", - "PWD=$rootDirPath", - "--exitcode", - "--compile-sdk-version", - actionArgs.compileSdkVersion, - "--java-language-level", - actionArgs.javaLanguageLevel, - "--kotlin-language-level", - actionArgs.kotlinLanguageLevel, - "--stacktrace", - "--quiet", - "--offline", - "--baseline", - baselineFilePath.pathString, - "--update-baseline", - "--cache-dir", - cacheDirectoryPath.pathString, - "--client-id", - "cli", - ) - if (actionArgs.warningsAsErrors) { - args.add("-Werror") - } else { - args.add("--nowarn") - } - if (actionArgs.config != null) { - args.add("--config") - args.add(actionArgs.config!!.pathString) - } - if (actionArgs.enableChecks.isNotEmpty()) { - args.add("--enable") - args.add(actionArgs.enableChecks.joinToString(",")) - } - if (actionArgs.disableChecks.isNotEmpty()) { - args.add("--disable") - args.add(actionArgs.disableChecks.joinToString(",")) - } - - if (actionArgs.androidHome?.isNotEmpty() == true) { - val androidHomePath = - Paths.get(System.getenv("PWD"), actionArgs.androidHome).absolutePathString() - args.add("--sdk-home") - args.add(androidHomePath) - } - - if (actionArgs.jdkHome != null) { - val jdkHome = actionArgs.jdkHome!! - args.add("--jdk-home") - args.add(jdkHome.absolutePathString()) - } - - return invoker.invoke( - args = args.toTypedArray(), - enableCheckDependencies = actionArgs.enableCheckDependencies, - ) - } + private fun rootDir(): String = System.getenv("PWD") ?: Paths.get("").toAbsolutePath().pathString } + +/** + * Collects the custom lint rule jars embedded in AARs. Each pair is (aar file, extracted + * directory); an AAR's lint.jar lives at the root of the extracted directory. + */ +internal fun collectAarLintRuleJars(classpathAarPairs: List>): List = + classpathAarPairs + .map { (_, extractedDirectory) -> extractedDirectory.resolve("lint.jar") } + .filter { it.exists() && it.isRegularFile() } diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index 6023a52..3c20dcd 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/tests/src/cli/AndroidLintActionArgsTest.kt @@ -76,5 +76,44 @@ class AndroidLintActionArgsTest { assertThat(parseArgs.javaLanguageLevel).isEqualTo("1.7") assertThat(parseArgs.kotlinLanguageLevel).isEqualTo("1.8") assertThat(parseArgs.enableCheckDependencies).isTrue() + // Mode defaults to legacy when not specified. + assertThat(parseArgs.mode).isEqualTo("legacy") + } + + @Test + fun `does parse analyze and report mode arguments`() { + val parseArgs = + AndroidLintActionArgs.parseArgs( + args = + listOf( + "--label", + "test", + "--android-lint-cli-tool", + "path/to/cli.jar", + "--mode", + "report", + "--partial-results", + "path/to/partial", + "--dependency-partial-results", + "dep_a=path/to/dep_a/partial", + "--dependency-partial-results", + "dep_b=path/to/dep_b/partial", + "--compile-sdk-version", + "34", + "--java-language-level", + "17", + "--kotlin-language-level", + "1.9", + ), + ) + + assertThat(parseArgs.mode).isEqualTo("report") + assertThat(parseArgs.partialResults).isEqualTo(Paths.get("path/to/partial")) + assertThat(parseArgs.dependencyPartialResults).containsExactly( + "dep_a" to Paths.get("path/to/dep_a/partial"), + "dep_b" to Paths.get("path/to/dep_b/partial"), + ) + // Output is optional now; absent here. + assertThat(parseArgs.output).isNull() } } diff --git a/tests/src/cli/AndroidLintProjectTest.kt b/tests/src/cli/AndroidLintProjectTest.kt index 824c4c9..17e93b9 100644 --- a/tests/src/cli/AndroidLintProjectTest.kt +++ b/tests/src/cli/AndroidLintProjectTest.kt @@ -55,4 +55,71 @@ class AndroidLintProjectTest { """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), ) } + + @Test + fun `analyze mode emits a partial-results-dir on the module`() { + val partialResults = tmpDirectory.newFolder("partial").toPath() + assertThat( + createProjectXMLString( + moduleName = "test_module_name", + rootDir = tmpDirectory.root.absolutePath, + srcs = listOf(tmpDirectory.newPath("Foo.kt")), + resources = emptyList(), + androidManifest = null, + classpathJars = emptyList(), + classpathAars = emptyList(), + classpathExtractedAarDirectories = emptyList(), + customLintChecks = emptyList(), + partialResultsDir = partialResults, + ), + ).isEqualTo( + """ + + + + + + + + + """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), + ) + } + + @Test + fun `report mode links dependency modules carrying their own partial-results-dir`() { + val ownPartial = tmpDirectory.newFolder("own").toPath() + val depPartial = tmpDirectory.newFolder("depA").toPath() + assertThat( + createProjectXMLString( + moduleName = "test_module_name", + rootDir = tmpDirectory.root.absolutePath, + srcs = listOf(tmpDirectory.newPath("Foo.kt")), + resources = emptyList(), + androidManifest = null, + classpathJars = emptyList(), + classpathAars = emptyList(), + classpathExtractedAarDirectories = emptyList(), + customLintChecks = emptyList(), + partialResultsDir = ownPartial, + dependencyModules = + listOf( + LintDependencyModule(name = "dep_a", partialResultsDir = depPartial), + ), + ), + ).isEqualTo( + """ + + + + + + + + + + + """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), + ) + } } From 5ba68f625d00d61981f2d484ce2052b2d7e25615 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 07:17:21 -0700 Subject: [PATCH 02/12] Android dependency model fixes --- rules/impl.bzl | 12 ++++ rules/lint_analysis_aspect.bzl | 67 +++++++++++++++++++-- rules/providers.bzl | 9 ++- src/cli/AndroidLintActionArgs.kt | 13 ++++ src/cli/AndroidLintProject.kt | 5 +- src/cli/AndroidLintRunner.kt | 9 ++- tests/rules/BUILD.bazel | 9 +++ tests/rules/android_lint_analysis_test.bzl | 57 +++++++++++++++++- tests/rules/fixtures/AndroidManifest.xml | 2 + tests/rules/fixtures/res/values/strings.xml | 3 + tests/src/cli/AndroidLintActionArgsTest.kt | 5 ++ tests/src/cli/AndroidLintProjectTest.kt | 15 +++-- 12 files changed, 189 insertions(+), 17 deletions(-) create mode 100644 tests/rules/fixtures/AndroidManifest.xml create mode 100644 tests/rules/fixtures/res/values/strings.xml diff --git a/rules/impl.bzl b/rules/impl.bzl index 6dcf4a3..74789c7 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -4,6 +4,7 @@ load( "@rules_android//providers:providers.bzl", "AndroidLibraryResourceClassJarProvider", + "StarlarkAndroidResourcesInfo", ) load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") load( @@ -25,6 +26,7 @@ def _run_android_lint( ctx, mode, android_lint, + is_android, module_name, output, partial_results, @@ -54,6 +56,7 @@ def _run_android_lint( ctx: The target context mode: One of "analyze" (write partial results) or "report" (consume them, emit XML) android_lint: The Android Lint binary to use + is_android: Whether the module is an Android module module_name: The name of the module output: The XML output file (report mode only; None in analyze mode) partial_results: The partial-results directory (output in analyze, input in report) @@ -91,6 +94,8 @@ def _run_android_lint( inputs.append(android_lint) args.add("--label", module_name) args.add("--mode", mode) + if is_android: + args.add("--android") # The partial-results directory is an output of analyze and an input of report. args.add("--partial-results", partial_results.path) @@ -151,6 +156,8 @@ def _run_android_lint( args.add("--autofix") for dependency in dependency_modules: args.add("--dependency-partial-results", "%s=%s" % (dependency.module_name, dependency.partial_results.path)) + if dependency.is_android: + args.add("--android-dependency", dependency.module_name) inputs.append(dependency.partial_results) args.add("--output", output) outputs.append(output) @@ -283,6 +290,11 @@ def process_android_lint_issues(ctx, regenerate): common = dict( android_lint = android_lint, + is_android = ( + StarlarkAndroidResourcesInfo in ctx.attr.lib or + manifest != None or + bool(ctx.files.resource_files) + ), module_name = module_name, srcs = ctx.files.srcs, deps = deps_depset, diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index 66783bb..9f7228c 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -10,11 +10,13 @@ sources plus its dependencies' compiled artifacts, never on another target's par load( "@rules_android//providers:providers.bzl", "AndroidLibraryResourceClassJarProvider", + "StarlarkAndroidResourcesInfo", ) load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") load( ":collect_aar_outputs_aspect.bzl", _AndroidLintAARInfo = "AndroidLintAARInfo", + _collect_aar_outputs_aspect = "collect_aar_outputs_aspect", ) load( ":providers.bzl", @@ -46,16 +48,52 @@ def _collect_transitive(deps): if _AndroidLintPartialResultsInfo in dep ] +def _android_model(target): + """Returns the direct Android manifest and resources owned by target.""" + if StarlarkAndroidResourcesInfo not in target: + return struct( + is_android = False, + manifest = None, + resource_files = depset(), + ) + + resources_info = target[StarlarkAndroidResourcesInfo] + nodes = ( + resources_info.direct_resources_nodes.to_list() + + resources_info.transitive_resources_nodes.to_list() + ) + own_nodes = [node for node in nodes if node.label == target.label] + manifests = [node.manifest for node in own_nodes if node.manifest] + if len(manifests) > 1: + fail("Expected at most one Android manifest for %s, found %s" % (target.label, manifests)) + + return struct( + is_android = True, + manifest = manifests[0] if manifests else None, + resource_files = depset( + transitive = [node.resource_files for node in own_nodes], + ), + ) + def _lint_analysis_aspect_impl(target, ctx): deps = _aspect_deps(ctx) transitive = _collect_transitive(deps) + android_model = _android_model(target) - # Skip nodes that have nothing to analyze (no JavaInfo or no sources), but keep propagating - # the transitive results gathered from their dependencies. + # Skip nodes that have nothing to analyze, but keep propagating the transitive results + # gathered from their dependencies. Android resource- or manifest-only targets still need an + # analysis action because many Android lint detectors do not inspect source code. srcs = getattr(ctx.rule.files, "srcs", []) - if JavaInfo not in target or not srcs: + if ( + ctx.rule.kind == "aar_import" or + JavaInfo not in target or + not srcs and not android_model.resource_files.to_list() and not android_model.manifest + ): return [_AndroidLintPartialResultsInfo( + is_android = android_model.is_android, + manifest = android_model.manifest, partial_results = None, + resource_files = android_model.resource_files, module_name = None, transitive_results = depset(transitive = transitive), )] @@ -68,6 +106,9 @@ def _lint_analysis_aspect_impl(target, ctx): inputs = [android_lint] inputs.extend(srcs) + inputs.extend(android_model.resource_files.to_list()) + if android_model.manifest: + inputs.append(android_model.manifest) args = ctx.actions.args() args.set_param_file_format("multiline") @@ -77,6 +118,8 @@ def _lint_analysis_aspect_impl(target, ctx): args.add("--label", module_name) args.add("--mode", "analyze") args.add("--partial-results", partial_results.path) + if android_model.is_android: + args.add("--android") if toolchain.compile_sdk_version: args.add("--compile-sdk-version", toolchain.compile_sdk_version) if toolchain.java_language_level: @@ -86,6 +129,13 @@ def _lint_analysis_aspect_impl(target, ctx): for src in srcs: args.add("--src", src) + for resource_file in android_model.resource_files.to_list(): + # A processed Android resource may be a TreeArtifact (for example, data binding output). + # Passing its path keeps it as one project resource root rather than expanding its files + # into separate command-line arguments. + args.add("--resource", resource_file.path) + if android_model.manifest: + args.add("--android-manifest", android_model.manifest) # Classpath for symbol resolution: this target's full compile classpath (its own outputs plus # its transitive dependencies) and any compiled R classes. @@ -136,9 +186,16 @@ def _lint_analysis_aspect_impl(target, ctx): }, ) - direct = struct(module_name = module_name, partial_results = partial_results) + direct = struct( + is_android = android_model.is_android, + module_name = module_name, + partial_results = partial_results, + ) return [_AndroidLintPartialResultsInfo( + is_android = android_model.is_android, + manifest = android_model.manifest, partial_results = partial_results, + resource_files = android_model.resource_files, module_name = module_name, transitive_results = depset(direct = [direct], transitive = transitive), )] @@ -157,6 +214,6 @@ lint_analysis_aspect = aspect( ), }, provides = [_AndroidLintPartialResultsInfo], - required_aspect_providers = [[_AndroidLintAARInfo]], + requires = [_collect_aar_outputs_aspect], toolchains = ["//toolchains:toolchain_type"], ) diff --git a/rules/providers.bzl b/rules/providers.bzl index 2590d97..910af92 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -12,10 +12,13 @@ AndroidLintPartialResultsInfo = provider( "Per-target Android Lint analysis (--analyze-only) outputs, propagated up the dependency " + "graph so a leaf rule can run the report (--report-only) phase over the transitive set.", fields = { + "is_android": "Whether this target is an Android module.", + "manifest": "The direct Android manifest used for this target's analysis, or None.", "partial_results": "Directory File of this target's partial results, or None if this " + - "node was not analyzed (e.g. no sources).", + "node had no lintable inputs.", + "resource_files": "Depset of direct Android resources used for this target's analysis.", "module_name": "The lint module name for this target, or None if not analyzed.", - "transitive_results": "depset of structs(module_name, partial_results) for this target " + - "and all analyzed transitive dependencies.", + "transitive_results": "depset of structs(module_name, partial_results, is_android) for " + + "this target and all analyzed transitive dependencies.", }, ) diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index b6cb67a..031a07f 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -48,6 +48,12 @@ internal class AndroidLintActionArgs( help = "One of: legacy, analyze, report", ).default { "legacy" } + val isAndroid: Boolean by parser + .flagging( + names = arrayOf("--android"), + help = "Model this project as an Android module.", + ).default { false } + // In analyze mode, the directory lint writes partial results into. In report mode, the directory // lint reads the module's own partial results from. val partialResults: Path? by parser @@ -65,6 +71,13 @@ internal class AndroidLintActionArgs( transform = argsParserDependencyModuleTransformer, ).default { emptyList() } + // Names of dependency modules whose partial results came from Android projects. + val androidDependencies: List by parser + .adding( + names = arrayOf("--android-dependency"), + help = "", + ).default { emptyList() } + val androidHome: String? by parser .storing( names = arrayOf("--android-home"), diff --git a/src/cli/AndroidLintProject.kt b/src/cli/AndroidLintProject.kt index b58a26a..1f2a542 100644 --- a/src/cli/AndroidLintProject.kt +++ b/src/cli/AndroidLintProject.kt @@ -20,6 +20,7 @@ import kotlin.io.path.pathString internal data class LintDependencyModule( val name: String, val partialResultsDir: Path, + val isAndroid: Boolean = false, ) internal fun createProjectXMLString( @@ -28,6 +29,7 @@ internal fun createProjectXMLString( srcs: List, resources: List, androidManifest: Path?, + isAndroid: Boolean = androidManifest != null, classpathJars: List, classpathAars: List, classpathExtractedAarDirectories: List>, @@ -52,7 +54,7 @@ internal fun createProjectXMLString( val moduleElement = document.createElement("module").also { it.setAttribute("name", moduleName) - it.setAttribute("android", if (androidManifest != null) "true" else "false") + it.setAttribute("android", isAndroid.toString()) // The partial-results-dir is where lint writes results in `--analyze-only` and reads them // back in `--report-only`. Absent in the legacy single-shot mode. if (partialResultsDir != null) { @@ -125,6 +127,7 @@ internal fun createProjectXMLString( dependencyModules.forEach { dependency -> document.createElement("module").also { it.setAttribute("name", dependency.name) + it.setAttribute("android", dependency.isAndroid.toString()) it.setAttribute("library", "true") it.setAttribute("partial-results-dir", dependency.partialResultsDir.absolutePathString()) projectElement.appendChild(it) diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index 00e4dd1..04ce6b1 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -78,7 +78,13 @@ internal class AndroidLintRunner( rootDir = rootDir, partialResultsDir = partialResults, dependencyModules = - args.dependencyPartialResults.map { (name, dir) -> LintDependencyModule(name, dir) }, + args.dependencyPartialResults.map { (name, dir) -> + LintDependencyModule( + name = name, + partialResultsDir = dir, + isAndroid = name in args.androidDependencies, + ) + }, ) val lintArgs = @@ -239,6 +245,7 @@ internal class AndroidLintRunner( srcs = args.srcs.sortedDescending(), resources = args.resources.sortedDescending(), androidManifest = args.androidManifest, + isAndroid = args.isAndroid || args.androidManifest != null, classpathJars = args.classpath.sortedDescending(), classpathAars = emptyList(), classpathExtractedAarDirectories = args.classpathAarPairs, diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index e7d7b71..a885db0 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -1,3 +1,4 @@ +load("@rules_android//android:rules.bzl", "android_library") load("//rules:defs.bzl", "android_lint") load(":android_lint_analysis_test.bzl", "android_lint_analysis_test_suite") @@ -6,6 +7,14 @@ filegroup( srcs = ["Fixture.java"], ) +android_library( + name = "android_dependency", + custom_package = "tests.rules.android", + manifest = "fixtures/AndroidManifest.xml", + resource_files = ["fixtures/res/values/strings.xml"], + tags = ["manual"], +) + android_lint( name = "analysis_fixture_lint", srcs = ["Fixture.java"], diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index ab85e33..6e74a1e 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -1,7 +1,12 @@ """Analysis tests for android_lint action and provider wiring.""" load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") -load("//rules:providers.bzl", "AndroidLintResultsInfo") +load("//rules:lint_analysis_aspect.bzl", "lint_analysis_aspect") +load( + "//rules:providers.bzl", + "AndroidLintPartialResultsInfo", + "AndroidLintResultsInfo", +) def _argument_value(argv, flag): for index in range(len(argv)): @@ -57,6 +62,41 @@ def _android_lint_action_impl(ctx): _android_lint_action_test = analysistest.make(_android_lint_action_impl) +def _android_dependency_analysis_impl(ctx): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + asserts.true(env, AndroidLintPartialResultsInfo in target) + if AndroidLintPartialResultsInfo in target: + info = target[AndroidLintPartialResultsInfo] + asserts.true(env, info.is_android) + asserts.true(env, info.partial_results != None) + asserts.equals(env, "AndroidManifest.xml", info.manifest.basename) + asserts.true( + env, + "strings.xml" in [file.basename for file in info.resource_files.to_list()], + ) + + return analysistest.end(env) + +_android_dependency_analysis_test = analysistest.make(_android_dependency_analysis_impl) + +def _android_dependency_subject_impl(ctx): + info = ctx.attr.dep[AndroidLintPartialResultsInfo] + return [ + DefaultInfo(files = depset([info.partial_results])), + info, + ] + +_android_dependency_subject = rule( + implementation = _android_dependency_subject_impl, + attrs = { + "dep": attr.label( + aspects = [lint_analysis_aspect], + providers = [AndroidLintPartialResultsInfo], + ), + }, +) + def android_lint_analysis_test_suite(name): """Defines the android_lint analysis test suite. @@ -68,7 +108,20 @@ def android_lint_analysis_test_suite(name): name = action_test, target_under_test = ":analysis_fixture_lint", ) + android_dependency_subject = name + "_android_dependency_subject" + _android_dependency_subject( + name = android_dependency_subject, + dep = ":android_dependency", + ) + android_dependency_test = name + "_android_dependency_test" + _android_dependency_analysis_test( + name = android_dependency_test, + target_under_test = ":" + android_dependency_subject, + ) native.test_suite( name = name, - tests = [":" + action_test], + tests = [ + ":" + action_test, + ":" + android_dependency_test, + ], ) diff --git a/tests/rules/fixtures/AndroidManifest.xml b/tests/rules/fixtures/AndroidManifest.xml new file mode 100644 index 0000000..6ec267f --- /dev/null +++ b/tests/rules/fixtures/AndroidManifest.xml @@ -0,0 +1,2 @@ + diff --git a/tests/rules/fixtures/res/values/strings.xml b/tests/rules/fixtures/res/values/strings.xml new file mode 100644 index 0000000..9fa37d4 --- /dev/null +++ b/tests/rules/fixtures/res/values/strings.xml @@ -0,0 +1,3 @@ + + Android dependency + diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index 3c20dcd..c1ead70 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/tests/src/cli/AndroidLintActionArgsTest.kt @@ -92,10 +92,13 @@ class AndroidLintActionArgsTest { "path/to/cli.jar", "--mode", "report", + "--android", "--partial-results", "path/to/partial", "--dependency-partial-results", "dep_a=path/to/dep_a/partial", + "--android-dependency", + "dep_a", "--dependency-partial-results", "dep_b=path/to/dep_b/partial", "--compile-sdk-version", @@ -108,11 +111,13 @@ class AndroidLintActionArgsTest { ) assertThat(parseArgs.mode).isEqualTo("report") + assertThat(parseArgs.isAndroid).isTrue() assertThat(parseArgs.partialResults).isEqualTo(Paths.get("path/to/partial")) assertThat(parseArgs.dependencyPartialResults).containsExactly( "dep_a" to Paths.get("path/to/dep_a/partial"), "dep_b" to Paths.get("path/to/dep_b/partial"), ) + assertThat(parseArgs.androidDependencies).containsExactly("dep_a") // Output is optional now; absent here. assertThat(parseArgs.output).isNull() } diff --git a/tests/src/cli/AndroidLintProjectTest.kt b/tests/src/cli/AndroidLintProjectTest.kt index 17e93b9..c2ab116 100644 --- a/tests/src/cli/AndroidLintProjectTest.kt +++ b/tests/src/cli/AndroidLintProjectTest.kt @@ -57,7 +57,7 @@ class AndroidLintProjectTest { } @Test - fun `analyze mode emits a partial-results-dir on the module`() { + fun `analyze mode preserves Android identity without requiring a manifest`() { val partialResults = tmpDirectory.newFolder("partial").toPath() assertThat( createProjectXMLString( @@ -66,6 +66,7 @@ class AndroidLintProjectTest { srcs = listOf(tmpDirectory.newPath("Foo.kt")), resources = emptyList(), androidManifest = null, + isAndroid = true, classpathJars = emptyList(), classpathAars = emptyList(), classpathExtractedAarDirectories = emptyList(), @@ -77,7 +78,7 @@ class AndroidLintProjectTest { - + @@ -87,7 +88,7 @@ class AndroidLintProjectTest { } @Test - fun `report mode links dependency modules carrying their own partial-results-dir`() { + fun `report mode preserves dependency Android identity and partial-results-dir`() { val ownPartial = tmpDirectory.newFolder("own").toPath() val depPartial = tmpDirectory.newFolder("depA").toPath() assertThat( @@ -104,7 +105,11 @@ class AndroidLintProjectTest { partialResultsDir = ownPartial, dependencyModules = listOf( - LintDependencyModule(name = "dep_a", partialResultsDir = depPartial), + LintDependencyModule( + name = "dep_a", + partialResultsDir = depPartial, + isAndroid = true, + ), ), ), ).isEqualTo( @@ -116,7 +121,7 @@ class AndroidLintProjectTest { - + """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), From d222b7376fcd93c45ba11313f9d1ccef40d4d4be Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 07:18:59 -0700 Subject: [PATCH 03/12] Pass android_home --- rules/lint_analysis_aspect.bzl | 1 + tests/rules/android_lint_analysis_test.bzl | 58 ++++++++++++++++++---- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index 9f7228c..28926f7 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -161,6 +161,7 @@ def _lint_analysis_aspect_impl(target, ctx): if toolchain.android_home != None: args.add("--android-home", toolchain.android_home.label.workspace_root) + inputs.extend(toolchain.android_home.files.to_list()) if java_runtime_info: args.add("--jdk-home", java_runtime_info.java_home) diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index 6e74a1e..db5ace6 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -65,9 +65,10 @@ _android_lint_action_test = analysistest.make(_android_lint_action_impl) def _android_dependency_analysis_impl(ctx): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) - asserts.true(env, AndroidLintPartialResultsInfo in target) - if AndroidLintPartialResultsInfo in target: - info = target[AndroidLintPartialResultsInfo] + asserts.true(env, _AndroidDependencyAnalysisInfo in target) + if _AndroidDependencyAnalysisInfo in target: + analysis = target[_AndroidDependencyAnalysisInfo] + info = analysis.lint_info asserts.true(env, info.is_android) asserts.true(env, info.partial_results != None) asserts.equals(env, "AndroidManifest.xml", info.manifest.basename) @@ -76,23 +77,60 @@ def _android_dependency_analysis_impl(ctx): "strings.xml" in [file.basename for file in info.resource_files.to_list()], ) + actions = [action for action in analysis.actions if action.mnemonic == "AndroidLintAnalyze"] + asserts.equals(env, 1, len(actions)) + if actions: + action = actions[0] + argv = action.argv + resource = _argument_value(argv, "--resource") + manifest = _argument_value(argv, "--android-manifest") + android_home = _argument_value(argv, "--android-home") + + asserts.true(env, "--android" in argv) + asserts.true(env, resource != None and resource.endswith("/res/values/strings.xml")) + asserts.true(env, manifest != None and manifest.endswith("/AndroidManifest.xml")) + asserts.true(env, android_home != None and "androidsdk" in android_home) + + inputs = [file.path for file in action.inputs.to_list()] + asserts.true(env, any([path.endswith("/res/values/strings.xml") for path in inputs])) + asserts.true(env, any([path.endswith("/AndroidManifest.xml") for path in inputs])) + asserts.true( + env, + any([ + "/platforms/android-" in path and path.endswith("/android.jar") + for path in inputs + ]), + ) + return analysistest.end(env) _android_dependency_analysis_test = analysistest.make(_android_dependency_analysis_impl) +_AndroidDependencyAnalysisInfo = provider( + "Test-only view of the dependency lint aspect's provider and registered actions.", + fields = ["actions", "lint_info"], +) + +def _android_dependency_analysis_aspect_impl(target, _ctx): + return [_AndroidDependencyAnalysisInfo( + actions = target.actions, + lint_info = target[AndroidLintPartialResultsInfo], + )] + +_android_dependency_analysis_aspect = aspect( + implementation = _android_dependency_analysis_aspect_impl, + requires = [lint_analysis_aspect], +) + def _android_dependency_subject_impl(ctx): - info = ctx.attr.dep[AndroidLintPartialResultsInfo] - return [ - DefaultInfo(files = depset([info.partial_results])), - info, - ] + return [ctx.attr.dep[_AndroidDependencyAnalysisInfo]] _android_dependency_subject = rule( implementation = _android_dependency_subject_impl, attrs = { "dep": attr.label( - aspects = [lint_analysis_aspect], - providers = [AndroidLintPartialResultsInfo], + aspects = [_android_dependency_analysis_aspect], + providers = [_AndroidDependencyAnalysisInfo], ), }, ) From dfdf1be7263bd2170902dc56cd5bcddc2e2f7125 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 07:37:45 -0700 Subject: [PATCH 04/12] Addressed the positive dependency test gap. --- rules/impl.bzl | 37 +++---- rules/lint_analysis_aspect.bzl | 6 +- rules/utils.bzl | 29 +++++- tests/rules/BUILD.bazel | 31 ++++++ tests/rules/android_lint_analysis_test.bzl | 107 ++++++++++++++++++++- 5 files changed, 180 insertions(+), 30 deletions(-) diff --git a/rules/impl.bzl b/rules/impl.bzl index 74789c7..96d9991 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -192,23 +192,6 @@ def _run_android_lint( }, ) -def _get_module_name(ctx): - """Extracts the module name from the target - - This module name will be embedded in the Android Lint project configuration. - - Args: - ctx: The target context - - Returns: - A string representing the module name - """ - path = ctx.build_file_path.split("BUILD")[0].replace("/", "_").replace("-", "_").replace(".", "_") - name = ctx.attr.name - if path: - return "%s_%s" % (path.replace("/", "_").replace("-", "_"), ctx.attr.name) - return name - def _collect_dependency_modules(ctx): """Collects the transitive partial-results modules from the rule's dependencies. @@ -223,9 +206,21 @@ def _collect_dependency_modules(ctx): seen = {} modules = [] for node in depset(transitive = transitive).to_list(): - if node.module_name and node.module_name not in seen: - seen[node.module_name] = True - modules.append(node) + if not node.module_name: + continue + previous = seen.get(node.module_name) + if previous: + if previous.partial_results.path != node.partial_results.path: + fail( + "Android Lint module ID collision for %s: %s and %s" % ( + node.module_name, + previous.partial_results.path, + node.partial_results.path, + ), + ) + continue + seen[node.module_name] = node + modules.append(node) return modules def process_android_lint_issues(ctx, regenerate): @@ -283,7 +278,7 @@ def process_android_lint_issues(ctx, regenerate): toolchain = _utils.get_android_lint_toolchain(ctx) android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) - module_name = _get_module_name(ctx) + module_name = _utils.module_name(ctx.label) deps_depset = depset(transitive = deps) aars_depset = depset(transitive = aars) java_runtime_info = ctx.attr._javabase[java_common.JavaRuntimeInfo] diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index 28926f7..4f78d55 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -31,10 +31,6 @@ load( # Edges traversed by the analysis aspect, matching collect_aar_outputs_aspect. _ANALYSIS_ATTR_ASPECTS = ["deps", "exports", "associates"] -def _module_name(label): - """Derives a stable, unique lint module name from a target label.""" - return ("%s_%s" % (label.package, label.name)).replace("/", "_").replace("-", "_").replace(".", "_").replace(":", "_") - def _aspect_deps(ctx): deps = [] for attr in _ANALYSIS_ATTR_ASPECTS: @@ -99,7 +95,7 @@ def _lint_analysis_aspect_impl(target, ctx): )] toolchain = _utils.get_android_lint_toolchain(ctx) - module_name = _module_name(ctx.label) + module_name = _utils.module_name(ctx.label) partial_results = ctx.actions.declare_directory("_lint/%s/partial_results" % ctx.label.name) android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) java_runtime_info = ctx.attr._javabase[java_common.JavaRuntimeInfo] diff --git a/rules/utils.bzl b/rules/utils.bzl index 730e201..06f8a07 100644 --- a/rules/utils.bzl +++ b/rules/utils.bzl @@ -26,9 +26,34 @@ def _list_or_depset_to_list(list_or_depset): else: return fail("Error: Expected a list or a depset. Got %s" % type(list_or_depset)) +def _module_name(label): + """Returns an injective, filesystem-safe module ID for a canonical Bazel label. + + Keeping the full label preserves repository, package, and target identity. Percent is escaped + first so the remaining replacements are reversible and cannot collide with literal escape + sequences in a label. + """ + result = str(label) + for character, replacement in [ + ("%", "%25"), + ("/", "%2F"), + ("\\", "%5C"), + (":", "%3A"), + ("=", "%3D"), + ("<", "%3C"), + (">", "%3E"), + ("\"", "%22"), + ("|", "%7C"), + ("?", "%3F"), + ("*", "%2A"), + ]: + result = result.replace(character, replacement) + return result + utils = struct( first = _first, - only = _only, - list_or_depset_to_list = _list_or_depset_to_list, get_android_lint_toolchain = _get_android_lint_toolchain, + list_or_depset_to_list = _list_or_depset_to_list, + module_name = _module_name, + only = _only, ) diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index a885db0..ea08004 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -1,5 +1,7 @@ load("@rules_android//android:rules.bzl", "android_library") +load("@rules_java//java:defs.bzl", "java_library") load("//rules:defs.bzl", "android_lint") +load("//toolchains:toolchain.bzl", "android_lint_toolchain") load(":android_lint_analysis_test.bzl", "android_lint_analysis_test_suite") filegroup( @@ -15,6 +17,30 @@ android_library( tags = ["manual"], ) +java_library( + name = "collision-dep", + srcs = ["Fixture.java"], + tags = ["manual"], +) + +java_library( + name = "collision.dep", + srcs = ["Fixture.java"], + tags = ["manual"], +) + +android_lint_toolchain( + name = "dependency_analysis_enabled", + android_home = "@androidsdk//:files", + android_lint_enable_check_dependencies = True, +) + +toolchain( + name = "dependency_analysis_enabled_toolchain", + toolchain = ":dependency_analysis_enabled", + toolchain_type = "//toolchains:toolchain_type", +) + android_lint( name = "analysis_fixture_lint", srcs = ["Fixture.java"], @@ -23,6 +49,11 @@ android_lint( lib = ":fixture_lib", tags = ["manual"], warnings_as_errors = True, + deps = [ + ":android_dependency", + ":collision.dep", + ":collision-dep", + ], ) android_lint_analysis_test_suite(name = "tests") diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index db5ace6..d31ad9d 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -16,6 +16,13 @@ def _argument_value(argv, flag): return None return None +def _argument_values(argv, flag): + return [ + argv[index + 1] + for index in range(len(argv) - 1) + if argv[index] == flag + ] + def _android_lint_action_impl(ctx): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) @@ -33,8 +40,15 @@ def _android_lint_action_impl(ctx): argv = action.argv source = _argument_value(argv, "--src") custom_rule = _argument_value(argv, "--custom-rule") + module_name = _argument_value(argv, "--label") output = _argument_value(argv, "--output") + dependency_partial_results = _argument_values(argv, "--dependency-partial-results") + android_dependencies = _argument_values(argv, "--android-dependency") + asserts.true(env, module_name != None and module_name.endswith("%3Aanalysis_fixture_lint")) + asserts.false(env, "/" in module_name) + asserts.false(env, ":" in module_name) + asserts.false(env, "=" in module_name) asserts.true(env, source != None and source.endswith("/Fixture.java")) asserts.true( env, @@ -46,7 +60,27 @@ def _android_lint_action_impl(ctx): asserts.false(env, "--baseline-file" in argv) asserts.false(env, "--regenerate-baseline-files" in argv) - inputs = [file.basename for file in action.inputs.to_list()] + dependency_module_names = [value.split("=")[0] for value in dependency_partial_results] + asserts.equals(env, 3, len(dependency_partial_results)) + asserts.true( + env, + any([name.endswith("%3Aandroid_dependency") for name in dependency_module_names]), + ) + asserts.true( + env, + any([name.endswith("%3Acollision-dep") for name in dependency_module_names]), + ) + asserts.true( + env, + any([name.endswith("%3Acollision.dep") for name in dependency_module_names]), + ) + asserts.equals(env, 1, len(android_dependencies)) + if android_dependencies: + asserts.true(env, android_dependencies[0].endswith("%3Aandroid_dependency")) + asserts.true(env, android_dependencies[0] in dependency_module_names) + + action_inputs = action.inputs.to_list() + inputs = [file.basename for file in action_inputs] outputs = [file.basename for file in action.outputs.to_list()] asserts.true(env, "Fixture.java" in inputs) asserts.true( @@ -54,13 +88,23 @@ def _android_lint_action_impl(ctx): any(["compose-lint-checks" in input and input.endswith(".jar") for input in inputs]), ) asserts.true(env, "analysis_fixture_lint.xml" in outputs) + for value in dependency_partial_results: + partial_results_path = value.split("=", 1)[1] + asserts.true(env, partial_results_path in [file.path for file in action_inputs]) asserts.equals(env, "1", action.execution_info.get("supports-workers")) asserts.equals(env, "1", action.execution_info.get("supports-multiplex-workers")) return analysistest.end(env) -_android_lint_action_test = analysistest.make(_android_lint_action_impl) +_android_lint_action_test = analysistest.make( + _android_lint_action_impl, + config_settings = { + "//command_line_option:extra_toolchains": [ + "//tests/rules:dependency_analysis_enabled_toolchain", + ], + }, +) def _android_dependency_analysis_impl(ctx): env = analysistest.begin(ctx) @@ -85,8 +129,10 @@ def _android_dependency_analysis_impl(ctx): resource = _argument_value(argv, "--resource") manifest = _argument_value(argv, "--android-manifest") android_home = _argument_value(argv, "--android-home") + module_name = _argument_value(argv, "--label") asserts.true(env, "--android" in argv) + asserts.true(env, module_name != None and module_name.endswith("%3Aandroid_dependency")) asserts.true(env, resource != None and resource.endswith("/res/values/strings.xml")) asserts.true(env, manifest != None and manifest.endswith("/AndroidManifest.xml")) asserts.true(env, android_home != None and "androidsdk" in android_home) @@ -135,6 +181,49 @@ _android_dependency_subject = rule( }, ) +_ModuleNamesInfo = provider( + "Test-only collection of module IDs propagated by dependency lint aspects.", + fields = ["module_names"], +) + +def _module_names_subject_impl(ctx): + module_names = [] + for dep in ctx.attr.deps: + module_names.extend([ + node.module_name + for node in dep[AndroidLintPartialResultsInfo].transitive_results.to_list() + ]) + return [_ModuleNamesInfo(module_names = module_names)] + +_module_names_subject = rule( + implementation = _module_names_subject_impl, + attrs = { + "deps": attr.label_list( + aspects = [lint_analysis_aspect], + providers = [AndroidLintPartialResultsInfo], + ), + }, +) + +def _module_name_collision_impl(ctx): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + module_names = target[_ModuleNamesInfo].module_names + + asserts.equals(env, 2, len(module_names)) + if len(module_names) == 2: + asserts.false(env, module_names[0] == module_names[1]) + asserts.true(env, any([name.endswith("%3Acollision-dep") for name in module_names])) + asserts.true(env, any([name.endswith("%3Acollision.dep") for name in module_names])) + for module_name in module_names: + asserts.false(env, "/" in module_name) + asserts.false(env, ":" in module_name) + asserts.false(env, "=" in module_name) + + return analysistest.end(env) + +_module_name_collision_test = analysistest.make(_module_name_collision_impl) + def android_lint_analysis_test_suite(name): """Defines the android_lint analysis test suite. @@ -156,10 +245,24 @@ def android_lint_analysis_test_suite(name): name = android_dependency_test, target_under_test = ":" + android_dependency_subject, ) + module_names_subject = name + "_module_names_subject" + _module_names_subject( + name = module_names_subject, + deps = [ + ":collision-dep", + ":collision.dep", + ], + ) + module_name_collision_test = name + "_module_name_collision_test" + _module_name_collision_test( + name = module_name_collision_test, + target_under_test = ":" + module_names_subject, + ) native.test_suite( name = name, tests = [ ":" + action_test, ":" + android_dependency_test, + ":" + module_name_collision_test, ], ) From f8dbfd3b8f8c4b2085224638ce2b621c6cab6538 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 07:55:39 -0700 Subject: [PATCH 05/12] Preserve library identity in dependency analysis --- rules/impl.bzl | 2 + rules/lint_analysis_aspect.bzl | 16 +++++++- rules/providers.bzl | 6 ++- src/cli/AndroidLintActionArgs.kt | 13 ++++++ src/cli/AndroidLintProject.kt | 11 +++-- src/cli/AndroidLintRunner.kt | 2 + tests/integration/BUILD.bazel | 5 +++ tests/rules/BUILD.bazel | 24 ++++++++++- tests/rules/android_lint_analysis_test.bzl | 47 +++++++++++++++++++++- tests/src/cli/AndroidLintActionArgsTest.kt | 5 +++ tests/src/cli/AndroidLintProjectTest.kt | 10 +++-- 11 files changed, 126 insertions(+), 15 deletions(-) diff --git a/rules/impl.bzl b/rules/impl.bzl index 96d9991..db535d8 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -158,6 +158,8 @@ def _run_android_lint( args.add("--dependency-partial-results", "%s=%s" % (dependency.module_name, dependency.partial_results.path)) if dependency.is_android: args.add("--android-dependency", dependency.module_name) + if dependency.is_library: + args.add("--library-dependency", dependency.module_name) inputs.append(dependency.partial_results) args.add("--output", output) outputs.append(output) diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index 4f78d55..11cf392 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -75,6 +75,7 @@ def _lint_analysis_aspect_impl(target, ctx): deps = _aspect_deps(ctx) transitive = _collect_transitive(deps) android_model = _android_model(target) + is_library = ctx.rule.kind != "android_binary" # Skip nodes that have nothing to analyze, but keep propagating the transitive results # gathered from their dependencies. Android resource- or manifest-only targets still need an @@ -87,6 +88,7 @@ def _lint_analysis_aspect_impl(target, ctx): ): return [_AndroidLintPartialResultsInfo( is_android = android_model.is_android, + is_library = is_library, manifest = android_model.manifest, partial_results = None, resource_files = android_model.resource_files, @@ -116,6 +118,8 @@ def _lint_analysis_aspect_impl(target, ctx): args.add("--partial-results", partial_results.path) if android_model.is_android: args.add("--android") + if is_library: + args.add("--library") if toolchain.compile_sdk_version: args.add("--compile-sdk-version", toolchain.compile_sdk_version) if toolchain.java_language_level: @@ -144,7 +148,15 @@ def _lint_analysis_aspect_impl(target, ctx): # AAR dependencies (extracted), so lint loads their classes and embedded lint.jar checks. if _AndroidLintAARInfo in target: - for node in target[_AndroidLintAARInfo].transitive_aar_artifacts.to_list(): + aar_info = target[_AndroidLintAARInfo] + direct_aar = aar_info.aar.aar + for node in aar_info.transitive_aar_artifacts.to_list(): + # AndroidLibraryAarInfo exposes this target's packaged AAR alongside its dependency + # AARs. Feeding that AAR back into its own lint model duplicates its manifest and, + # once the project is correctly identified as a library, makes lint attempt an + # unsupported library-manifest merge. + if direct_aar and node.aar == direct_aar: + continue if node.aar and node.aar_dir: args.add("--classpath-aar", "%s:%s" % (node.aar.path, node.aar_dir.path)) inputs.append(node.aar) @@ -185,11 +197,13 @@ def _lint_analysis_aspect_impl(target, ctx): direct = struct( is_android = android_model.is_android, + is_library = is_library, module_name = module_name, partial_results = partial_results, ) return [_AndroidLintPartialResultsInfo( is_android = android_model.is_android, + is_library = is_library, manifest = android_model.manifest, partial_results = partial_results, resource_files = android_model.resource_files, diff --git a/rules/providers.bzl b/rules/providers.bzl index 910af92..b4d02aa 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -13,12 +13,14 @@ AndroidLintPartialResultsInfo = provider( "graph so a leaf rule can run the report (--report-only) phase over the transitive set.", fields = { "is_android": "Whether this target is an Android module.", + "is_library": "Whether this target is a library rather than an application.", "manifest": "The direct Android manifest used for this target's analysis, or None.", "partial_results": "Directory File of this target's partial results, or None if this " + "node had no lintable inputs.", "resource_files": "Depset of direct Android resources used for this target's analysis.", "module_name": "The lint module name for this target, or None if not analyzed.", - "transitive_results": "depset of structs(module_name, partial_results, is_android) for " + - "this target and all analyzed transitive dependencies.", + "transitive_results": "depset of structs(module_name, partial_results, is_android, " + + "is_library) for this target and all analyzed transitive " + + "dependencies.", }, ) diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 031a07f..64186ee 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -54,6 +54,12 @@ internal class AndroidLintActionArgs( help = "Model this project as an Android module.", ).default { false } + val isLibrary: Boolean by parser + .flagging( + names = arrayOf("--library"), + help = "Model this project as a library module.", + ).default { false } + // In analyze mode, the directory lint writes partial results into. In report mode, the directory // lint reads the module's own partial results from. val partialResults: Path? by parser @@ -78,6 +84,13 @@ internal class AndroidLintActionArgs( help = "", ).default { emptyList() } + // Names of dependency modules whose partial results came from library projects. + val libraryDependencies: List by parser + .adding( + names = arrayOf("--library-dependency"), + help = "", + ).default { emptyList() } + val androidHome: String? by parser .storing( names = arrayOf("--android-home"), diff --git a/src/cli/AndroidLintProject.kt b/src/cli/AndroidLintProject.kt index 1f2a542..89878d1 100644 --- a/src/cli/AndroidLintProject.kt +++ b/src/cli/AndroidLintProject.kt @@ -14,13 +14,15 @@ import kotlin.io.path.pathString * A first-party dependency module contributing partial analysis results to the report phase. * * In the report (`--report-only`) phase lint merges these modules' partial results into the main - * module's verdict without re-analyzing their sources. Each is registered as a library module - * carrying its own `partial-results-dir` and linked from the main module via a `` element. + * module's verdict without re-analyzing their sources. Each is registered with its original + * module identity, carries its own `partial-results-dir`, and is linked from the main module via a + * `` element. */ internal data class LintDependencyModule( val name: String, val partialResultsDir: Path, val isAndroid: Boolean = false, + val isLibrary: Boolean = false, ) internal fun createProjectXMLString( @@ -30,6 +32,7 @@ internal fun createProjectXMLString( resources: List, androidManifest: Path?, isAndroid: Boolean = androidManifest != null, + isLibrary: Boolean = false, classpathJars: List, classpathAars: List, classpathExtractedAarDirectories: List>, @@ -55,12 +58,12 @@ internal fun createProjectXMLString( document.createElement("module").also { it.setAttribute("name", moduleName) it.setAttribute("android", isAndroid.toString()) + it.setAttribute("library", isLibrary.toString()) // The partial-results-dir is where lint writes results in `--analyze-only` and reads them // back in `--report-only`. Absent in the legacy single-shot mode. if (partialResultsDir != null) { it.setAttribute("partial-results-dir", partialResultsDir.absolutePathString()) } - // it.setAttribute("library", "false") // it.setAttribute("compile-sdk-version", "get-actual-value-here") projectElement.appendChild(it) } @@ -128,7 +131,7 @@ internal fun createProjectXMLString( document.createElement("module").also { it.setAttribute("name", dependency.name) it.setAttribute("android", dependency.isAndroid.toString()) - it.setAttribute("library", "true") + it.setAttribute("library", dependency.isLibrary.toString()) it.setAttribute("partial-results-dir", dependency.partialResultsDir.absolutePathString()) projectElement.appendChild(it) } diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index 04ce6b1..cf27cd6 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -83,6 +83,7 @@ internal class AndroidLintRunner( name = name, partialResultsDir = dir, isAndroid = name in args.androidDependencies, + isLibrary = name in args.libraryDependencies, ) }, ) @@ -246,6 +247,7 @@ internal class AndroidLintRunner( resources = args.resources.sortedDescending(), androidManifest = args.androidManifest, isAndroid = args.isAndroid || args.androidManifest != null, + isLibrary = args.isLibrary, classpathJars = args.classpath.sortedDescending(), classpathAars = emptyList(), classpathExtractedAarDirectories = args.classpathAarPairs, diff --git a/tests/integration/BUILD.bazel b/tests/integration/BUILD.bazel index 9282c5b..8676016 100644 --- a/tests/integration/BUILD.bazel +++ b/tests/integration/BUILD.bazel @@ -3,6 +3,11 @@ load("//rules:defs.bzl", "android_lint", "android_lint_test") load(":fixture_aar.bzl", "lint_aar_fixture") load(":lint_report_test.bzl", "lint_report_test") +exports_files( + ["report_assertion.py"], + visibility = ["//tests:__subpackages__"], +) + filegroup( name = "basic_clean_lib", srcs = ["fixtures/basic/Clean.java"], diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index ea08004..95904d0 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -1,8 +1,13 @@ load("@rules_android//android:rules.bzl", "android_library") load("@rules_java//java:defs.bzl", "java_library") load("//rules:defs.bzl", "android_lint") +load("//tests/integration:lint_report_test.bzl", "lint_report_test") load("//toolchains:toolchain.bzl", "android_lint_toolchain") -load(":android_lint_analysis_test.bzl", "android_lint_analysis_test_suite") +load( + ":android_lint_analysis_test.bzl", + "android_lint_analysis_test_suite", + "dependency_analysis_enabled_lint", +) filegroup( name = "fixture_lib", @@ -56,4 +61,19 @@ android_lint( ], ) -android_lint_analysis_test_suite(name = "tests") +dependency_analysis_enabled_lint( + name = "analysis_fixture_lint_dependency_analysis_enabled", + lint = ":analysis_fixture_lint", +) + +lint_report_test( + name = "dependency_library_identity_report_test", + expected_issues = ["UnusedResources"], + lint = ":analysis_fixture_lint_dependency_analysis_enabled", + rejected_issues = ["MissingVersion"], +) + +android_lint_analysis_test_suite( + name = "tests", + additional_tests = [":dependency_library_identity_report_test"], +) diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index d31ad9d..9d6f5c9 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -44,6 +44,7 @@ def _android_lint_action_impl(ctx): output = _argument_value(argv, "--output") dependency_partial_results = _argument_values(argv, "--dependency-partial-results") android_dependencies = _argument_values(argv, "--android-dependency") + library_dependencies = _argument_values(argv, "--library-dependency") asserts.true(env, module_name != None and module_name.endswith("%3Aanalysis_fixture_lint")) asserts.false(env, "/" in module_name) @@ -78,6 +79,9 @@ def _android_lint_action_impl(ctx): if android_dependencies: asserts.true(env, android_dependencies[0].endswith("%3Aandroid_dependency")) asserts.true(env, android_dependencies[0] in dependency_module_names) + asserts.equals(env, 3, len(library_dependencies)) + for library_dependency in library_dependencies: + asserts.true(env, library_dependency in dependency_module_names) action_inputs = action.inputs.to_list() inputs = [file.basename for file in action_inputs] @@ -114,6 +118,7 @@ def _android_dependency_analysis_impl(ctx): analysis = target[_AndroidDependencyAnalysisInfo] info = analysis.lint_info asserts.true(env, info.is_android) + asserts.true(env, info.is_library) asserts.true(env, info.partial_results != None) asserts.equals(env, "AndroidManifest.xml", info.manifest.basename) asserts.true( @@ -130,8 +135,11 @@ def _android_dependency_analysis_impl(ctx): manifest = _argument_value(argv, "--android-manifest") android_home = _argument_value(argv, "--android-home") module_name = _argument_value(argv, "--label") + classpath_aars = _argument_values(argv, "--classpath-aar") asserts.true(env, "--android" in argv) + asserts.true(env, "--library" in argv) + asserts.equals(env, 0, len(classpath_aars)) asserts.true(env, module_name != None and module_name.endswith("%3Aandroid_dependency")) asserts.true(env, resource != None and resource.endswith("/res/values/strings.xml")) asserts.true(env, manifest != None and manifest.endswith("/AndroidManifest.xml")) @@ -224,11 +232,46 @@ def _module_name_collision_impl(ctx): _module_name_collision_test = analysistest.make(_module_name_collision_impl) -def android_lint_analysis_test_suite(name): +def _dependency_analysis_enabled_transition_impl(_settings, _attr): + return { + "//command_line_option:extra_toolchains": [ + "//tests/rules:dependency_analysis_enabled_toolchain", + ], + } + +_dependency_analysis_enabled_transition = transition( + implementation = _dependency_analysis_enabled_transition_impl, + inputs = [], + outputs = ["//command_line_option:extra_toolchains"], +) + +def _dependency_analysis_enabled_lint_impl(ctx): + info = ctx.attr.lint[0][AndroidLintResultsInfo] + return [ + info, + DefaultInfo(files = depset([info.output])), + ] + +dependency_analysis_enabled_lint = rule( + implementation = _dependency_analysis_enabled_lint_impl, + attrs = { + "lint": attr.label( + cfg = _dependency_analysis_enabled_transition, + mandatory = True, + providers = [AndroidLintResultsInfo], + ), + "_allowlist_function_transition": attr.label( + default = "@bazel_tools//tools/allowlists/function_transition_allowlist", + ), + }, +) + +def android_lint_analysis_test_suite(name, additional_tests = []): """Defines the android_lint analysis test suite. Args: name: Name of the generated test suite. + additional_tests: Additional test labels to include in the suite. """ action_test = name + "_action_test" _android_lint_action_test( @@ -264,5 +307,5 @@ def android_lint_analysis_test_suite(name): ":" + action_test, ":" + android_dependency_test, ":" + module_name_collision_test, - ], + ] + additional_tests, ) diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index c1ead70..2027821 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/tests/src/cli/AndroidLintActionArgsTest.kt @@ -93,12 +93,15 @@ class AndroidLintActionArgsTest { "--mode", "report", "--android", + "--library", "--partial-results", "path/to/partial", "--dependency-partial-results", "dep_a=path/to/dep_a/partial", "--android-dependency", "dep_a", + "--library-dependency", + "dep_a", "--dependency-partial-results", "dep_b=path/to/dep_b/partial", "--compile-sdk-version", @@ -112,12 +115,14 @@ class AndroidLintActionArgsTest { assertThat(parseArgs.mode).isEqualTo("report") assertThat(parseArgs.isAndroid).isTrue() + assertThat(parseArgs.isLibrary).isTrue() assertThat(parseArgs.partialResults).isEqualTo(Paths.get("path/to/partial")) assertThat(parseArgs.dependencyPartialResults).containsExactly( "dep_a" to Paths.get("path/to/dep_a/partial"), "dep_b" to Paths.get("path/to/dep_b/partial"), ) assertThat(parseArgs.androidDependencies).containsExactly("dep_a") + assertThat(parseArgs.libraryDependencies).containsExactly("dep_a") // Output is optional now; absent here. assertThat(parseArgs.output).isNull() } diff --git a/tests/src/cli/AndroidLintProjectTest.kt b/tests/src/cli/AndroidLintProjectTest.kt index c2ab116..d2cd8c4 100644 --- a/tests/src/cli/AndroidLintProjectTest.kt +++ b/tests/src/cli/AndroidLintProjectTest.kt @@ -41,7 +41,7 @@ class AndroidLintProjectTest { - + @@ -57,7 +57,7 @@ class AndroidLintProjectTest { } @Test - fun `analyze mode preserves Android identity without requiring a manifest`() { + fun `analyze mode preserves Android library identity without requiring a manifest`() { val partialResults = tmpDirectory.newFolder("partial").toPath() assertThat( createProjectXMLString( @@ -67,6 +67,7 @@ class AndroidLintProjectTest { resources = emptyList(), androidManifest = null, isAndroid = true, + isLibrary = true, classpathJars = emptyList(), classpathAars = emptyList(), classpathExtractedAarDirectories = emptyList(), @@ -78,7 +79,7 @@ class AndroidLintProjectTest { - + @@ -109,6 +110,7 @@ class AndroidLintProjectTest { name = "dep_a", partialResultsDir = depPartial, isAndroid = true, + isLibrary = true, ), ), ), @@ -117,7 +119,7 @@ class AndroidLintProjectTest { - + From c39704ef1ce27726387b7e9889f275a8ae49aadd Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 07:59:43 -0700 Subject: [PATCH 06/12] Traverse runtime dependencies during lint analysis --- rules/collect_aar_outputs_aspect.bzl | 12 ++++--- rules/lint_analysis_aspect.bzl | 8 ++--- tests/rules/BUILD.bazel | 27 +++++++++++++++- tests/rules/android_lint_analysis_test.bzl | 32 +++++++++++++++---- .../fixtures/runtime/res/layout/runtime.xml | 5 +++ 5 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 tests/rules/fixtures/runtime/res/layout/runtime.xml diff --git a/rules/collect_aar_outputs_aspect.bzl b/rules/collect_aar_outputs_aspect.bzl index dc4f177..ce978bf 100644 --- a/rules/collect_aar_outputs_aspect.bzl +++ b/rules/collect_aar_outputs_aspect.bzl @@ -3,6 +3,8 @@ load("@rules_android//providers:providers.bzl", "AndroidLibraryAarInfo") +ANDROID_LINT_DEPENDENCY_ATTRS = ["deps", "runtime_deps", "exports", "associates"] + AndroidLintAARInfo = provider( "A provider to collect all aars from transitive dependencies", fields = { @@ -20,13 +22,13 @@ AndroidLintAARNodeInfo = provider( ) def _collect_aar_outputs_aspect(tgt, ctx): - deps = getattr(ctx.rule.attr, "deps", []) - exports = getattr(ctx.rule.attr, "exports", []) - associates = getattr(ctx.rule.attr, "associates", []) + deps = [] + for attr in ANDROID_LINT_DEPENDENCY_ATTRS: + deps.extend(getattr(ctx.rule.attr, attr, [])) # Collect the transitive aar artifacts for the given dependencies transitive_aar_depsets = [] - for dep in deps + exports + associates: + for dep in deps: if AndroidLintAARInfo in dep: transitive_aar_depsets.append(dep[AndroidLintAARInfo].transitive_aar_artifacts) @@ -73,6 +75,6 @@ def _collect_aar_outputs_aspect(tgt, ctx): collect_aar_outputs_aspect = aspect( implementation = _collect_aar_outputs_aspect, - attr_aspects = ["aar", "deps", "exports", "associates"], + attr_aspects = ["aar"] + ANDROID_LINT_DEPENDENCY_ATTRS, provides = [AndroidLintAARInfo], ) diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index 11cf392..f4fec52 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -15,6 +15,7 @@ load( load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") load( ":collect_aar_outputs_aspect.bzl", + _ANDROID_LINT_DEPENDENCY_ATTRS = "ANDROID_LINT_DEPENDENCY_ATTRS", _AndroidLintAARInfo = "AndroidLintAARInfo", _collect_aar_outputs_aspect = "collect_aar_outputs_aspect", ) @@ -28,12 +29,9 @@ load( _utils = "utils", ) -# Edges traversed by the analysis aspect, matching collect_aar_outputs_aspect. -_ANALYSIS_ATTR_ASPECTS = ["deps", "exports", "associates"] - def _aspect_deps(ctx): deps = [] - for attr in _ANALYSIS_ATTR_ASPECTS: + for attr in _ANDROID_LINT_DEPENDENCY_ATTRS: deps.extend(getattr(ctx.rule.attr, attr, [])) return deps @@ -213,7 +211,7 @@ def _lint_analysis_aspect_impl(target, ctx): lint_analysis_aspect = aspect( implementation = _lint_analysis_aspect_impl, - attr_aspects = _ANALYSIS_ATTR_ASPECTS, + attr_aspects = _ANDROID_LINT_DEPENDENCY_ATTRS, attrs = { "_lint_wrapper": attr.label( default = Label("//src/cli"), diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index 95904d0..4f1920c 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -22,6 +22,14 @@ android_library( tags = ["manual"], ) +android_library( + name = "runtime_android_dependency", + custom_package = "tests.rules.runtime", + manifest = "fixtures/AndroidManifest.xml", + resource_files = ["fixtures/runtime/res/layout/runtime.xml"], + tags = ["manual"], +) + java_library( name = "collision-dep", srcs = ["Fixture.java"], @@ -34,6 +42,13 @@ java_library( tags = ["manual"], ) +java_library( + name = "runtime_parent", + srcs = ["Fixture.java"], + tags = ["manual"], + runtime_deps = [":runtime_android_dependency"], +) + android_lint_toolchain( name = "dependency_analysis_enabled", android_home = "@androidsdk//:files", @@ -58,6 +73,7 @@ android_lint( ":android_dependency", ":collision.dep", ":collision-dep", + ":runtime_parent", ], ) @@ -73,7 +89,16 @@ lint_report_test( rejected_issues = ["MissingVersion"], ) +lint_report_test( + name = "runtime_dependency_report_test", + expected_issues = ["HardcodedText"], + lint = ":analysis_fixture_lint_dependency_analysis_enabled", +) + android_lint_analysis_test_suite( name = "tests", - additional_tests = [":dependency_library_identity_report_test"], + additional_tests = [ + ":dependency_library_identity_report_test", + ":runtime_dependency_report_test", + ], ) diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index 9d6f5c9..ea8a34f 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -45,6 +45,7 @@ def _android_lint_action_impl(ctx): dependency_partial_results = _argument_values(argv, "--dependency-partial-results") android_dependencies = _argument_values(argv, "--android-dependency") library_dependencies = _argument_values(argv, "--library-dependency") + classpath_aars = _argument_values(argv, "--classpath-aar") asserts.true(env, module_name != None and module_name.endswith("%3Aanalysis_fixture_lint")) asserts.false(env, "/" in module_name) @@ -62,7 +63,7 @@ def _android_lint_action_impl(ctx): asserts.false(env, "--regenerate-baseline-files" in argv) dependency_module_names = [value.split("=")[0] for value in dependency_partial_results] - asserts.equals(env, 3, len(dependency_partial_results)) + asserts.equals(env, 5, len(dependency_partial_results)) asserts.true( env, any([name.endswith("%3Aandroid_dependency") for name in dependency_module_names]), @@ -75,13 +76,32 @@ def _android_lint_action_impl(ctx): env, any([name.endswith("%3Acollision.dep") for name in dependency_module_names]), ) - asserts.equals(env, 1, len(android_dependencies)) - if android_dependencies: - asserts.true(env, android_dependencies[0].endswith("%3Aandroid_dependency")) - asserts.true(env, android_dependencies[0] in dependency_module_names) - asserts.equals(env, 3, len(library_dependencies)) + asserts.true( + env, + any([name.endswith("%3Aruntime_android_dependency") for name in dependency_module_names]), + ) + asserts.true( + env, + any([name.endswith("%3Aruntime_parent") for name in dependency_module_names]), + ) + asserts.equals(env, 2, len(android_dependencies)) + asserts.true( + env, + any([name.endswith("%3Aandroid_dependency") for name in android_dependencies]), + ) + asserts.true( + env, + any([name.endswith("%3Aruntime_android_dependency") for name in android_dependencies]), + ) + for android_dependency in android_dependencies: + asserts.true(env, android_dependency in dependency_module_names) + asserts.equals(env, 5, len(library_dependencies)) for library_dependency in library_dependencies: asserts.true(env, library_dependency in dependency_module_names) + asserts.true( + env, + any(["runtime_android_dependency.aar:" in aar for aar in classpath_aars]), + ) action_inputs = action.inputs.to_list() inputs = [file.basename for file in action_inputs] diff --git a/tests/rules/fixtures/runtime/res/layout/runtime.xml b/tests/rules/fixtures/runtime/res/layout/runtime.xml new file mode 100644 index 0000000..95df896 --- /dev/null +++ b/tests/rules/fixtures/runtime/res/layout/runtime.xml @@ -0,0 +1,5 @@ + + From e59ddaf3848b8761352ff0c9958ad9559736a15e Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 08:02:41 -0700 Subject: [PATCH 07/12] Execute dependency analysis positive test --- tests/integration/lint_report_test.bzl | 4 ++++ tests/integration/report_assertion.py | 19 +++++++++++++++- tests/rules/BUILD.bazel | 26 +++++++++++++++------- tests/rules/android_lint_analysis_test.bzl | 2 ++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/tests/integration/lint_report_test.bzl b/tests/integration/lint_report_test.bzl index 6b06b5b..25f4bb3 100644 --- a/tests/integration/lint_report_test.bzl +++ b/tests/integration/lint_report_test.bzl @@ -8,6 +8,7 @@ def lint_report_test( name, lint, expected_issues = [], + expected_location_suffixes = [], rejected_issues = [], **kwargs): """Defines a portable Python test that checks an android_lint report. @@ -16,6 +17,7 @@ def lint_report_test( name: Name of the generated test. lint: android_lint target whose XML output should be checked. expected_issues: Lint issue IDs that must be present. + expected_location_suffixes: File path suffixes that must be present in issue locations. rejected_issues: Lint issue IDs that must be absent. **kwargs: Additional arguments forwarded to py_test. """ @@ -25,6 +27,8 @@ def lint_report_test( ] for issue in expected_issues: arguments.extend(["--expect-issue", issue]) + for suffix in expected_location_suffixes: + arguments.extend(["--expect-location-suffix", suffix]) for issue in rejected_issues: arguments.extend(["--reject-issue", issue]) diff --git a/tests/integration/report_assertion.py b/tests/integration/report_assertion.py index 68c7e32..d3c47e3 100644 --- a/tests/integration/report_assertion.py +++ b/tests/integration/report_assertion.py @@ -9,6 +9,7 @@ def main() -> int: parser = argparse.ArgumentParser() parser.add_argument("--report", required=True) parser.add_argument("--expect-issue", action="append", default=[]) + parser.add_argument("--expect-location-suffix", action="append", default=[]) parser.add_argument("--reject-issue", action="append", default=[]) args = parser.parse_args() @@ -28,15 +29,31 @@ def main() -> int: return 1 issue_ids = {issue.get("id") for issue in root.findall("issue") if issue.get("id")} + location_files = { + location.get("file") + for location in root.findall(".//location") + if location.get("file") + } missing = sorted(set(args.expect_issue) - issue_ids) + missing_locations = sorted( + suffix + for suffix in set(args.expect_location_suffix) + if not any(location.endswith(suffix) for location in location_files) + ) unexpected = sorted(set(args.reject_issue) & issue_ids) - if missing or unexpected: + if missing or missing_locations or unexpected: if missing: print(f"Missing expected lint issues: {', '.join(missing)}", file=sys.stderr) + if missing_locations: + print( + f"Missing expected lint location suffixes: {', '.join(missing_locations)}", + file=sys.stderr, + ) if unexpected: print(f"Found rejected lint issues: {', '.join(unexpected)}", file=sys.stderr) print(f"Actual lint issues: {', '.join(sorted(issue_ids))}", file=sys.stderr) + print(f"Actual lint locations: {', '.join(sorted(location_files))}", file=sys.stderr) return 1 return 0 diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index 4f1920c..135f581 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -83,22 +83,32 @@ dependency_analysis_enabled_lint( ) lint_report_test( - name = "dependency_library_identity_report_test", - expected_issues = ["UnusedResources"], - lint = ":analysis_fixture_lint_dependency_analysis_enabled", - rejected_issues = ["MissingVersion"], + name = "dependency_analysis_disabled_report_test", + lint = ":analysis_fixture_lint", + rejected_issues = [ + "HardcodedText", + "UnusedResources", + ], ) lint_report_test( - name = "runtime_dependency_report_test", - expected_issues = ["HardcodedText"], + name = "dependency_analysis_enabled_report_test", + expected_issues = [ + "HardcodedText", + "UnusedResources", + ], + expected_location_suffixes = [ + "tests/rules/fixtures/res/values/strings.xml", + "tests/rules/fixtures/runtime/res/layout/runtime.xml", + ], lint = ":analysis_fixture_lint_dependency_analysis_enabled", + rejected_issues = ["MissingVersion"], ) android_lint_analysis_test_suite( name = "tests", additional_tests = [ - ":dependency_library_identity_report_test", - ":runtime_dependency_report_test", + ":dependency_analysis_disabled_report_test", + ":dependency_analysis_enabled_report_test", ], ) diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index ea8a34f..706b4f1 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -266,6 +266,7 @@ _dependency_analysis_enabled_transition = transition( ) def _dependency_analysis_enabled_lint_impl(ctx): + # An attribute transition exposes a singleton list even for attr.label. info = ctx.attr.lint[0][AndroidLintResultsInfo] return [ info, @@ -273,6 +274,7 @@ def _dependency_analysis_enabled_lint_impl(ctx): ] dependency_analysis_enabled_lint = rule( + doc = "Forwards a lint report built with dependency analysis enabled.", implementation = _dependency_analysis_enabled_lint_impl, attrs = { "lint": attr.label( From ef8d6af0d5987879a1b24166ae1dc44cc65558b4 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 08:05:49 -0700 Subject: [PATCH 08/12] Cover AndroidLintRunner analyze and report modes --- tests/src/cli/AndroidLintRunnerTest.kt | 168 +++++++++++++++++++++---- 1 file changed, 142 insertions(+), 26 deletions(-) diff --git a/tests/src/cli/AndroidLintRunnerTest.kt b/tests/src/cli/AndroidLintRunnerTest.kt index 3c50752..ebbc6cb 100644 --- a/tests/src/cli/AndroidLintRunnerTest.kt +++ b/tests/src/cli/AndroidLintRunnerTest.kt @@ -36,38 +36,22 @@ class AndroidLintRunnerTest { val workingDirectory = temporaryFolder.newFolder("working").toPath() val args = AndroidLintActionArgs.parseArgs( - listOf( - "--android-lint-cli-tool", - lintCli.toString(), - "--label", - "runner-test", - "--output", - output.toString(), - "--custom-rule", - customRule.toString(), - "--classpath-aar", - "$aar:$aarDirectory", - "--compile-sdk-version", - "34", - "--java-language-level", - "17", - "--kotlin-language-level", - "1.9", - ), + runnerArgs(lintCli, "runner-test") + + listOf( + "--output", + output.toString(), + "--custom-rule", + customRule.toString(), + "--classpath-aar", + "$aar:$aarDirectory", + ), ) val exitCode = AndroidLintRunner(AndroidLintCliInvokerCache(parentClassloader = javaClass.classLoader)) .runAndroidLint(args, workingDirectory) - val projectFile = - Paths.get( - Main.recordedRuns - .single() - .args - .argumentAfter("--project"), - ) - val projectXml = Files.readString(projectFile) + val projectXml = projectXml(Main.recordedRuns.single()) assertThat(exitCode).isEqualTo(0) assertThat(projectXml) .contains("") @@ -77,6 +61,138 @@ class AndroidLintRunnerTest { ) } + @Test + fun `analyze mode writes an isolated partial-results project`() { + val lintCli = writeStubLintJar("lint-cli.jar") + val source = temporaryFolder.newFile("Analyze.kt").toPath() + val resource = temporaryFolder.newFile("analyze.xml").toPath() + val manifest = temporaryFolder.newFile("AndroidManifest.xml").toPath() + val partialResults = temporaryFolder.newFolder("analyze-partial-results").toPath() + val workingDirectory = temporaryFolder.newFolder("analyze-working").toPath() + val args = + AndroidLintActionArgs.parseArgs( + runnerArgs(lintCli, "analyze-test") + + listOf( + "--mode", + "analyze", + "--android", + "--library", + "--partial-results", + partialResults.toString(), + "--src", + source.toString(), + "--resource", + resource.toString(), + "--android-manifest", + manifest.toString(), + ), + ) + + val exitCode = + AndroidLintRunner(AndroidLintCliInvokerCache(parentClassloader = javaClass.classLoader)) + .runAndroidLint(args, workingDirectory) + + val run = Main.recordedRuns.single() + val projectXml = projectXml(run) + assertThat(exitCode).isEqualTo(0) + assertThat(run.checkDependencies).isFalse() + assertThat(run.args) + .contains("--analyze-only") + .doesNotContain("--report-only", "--xml", "--exitcode", "--baseline", "--update-baseline") + assertThat(projectXml) + .contains( + "", + ).contains("") + .contains("") + .contains("") + .doesNotContain("", + ).contains("") + .contains("") + .contains( + "", + ).contains( + "", + ) + } + + private fun runnerArgs( + lintCli: Path, + label: String, + ): List = + listOf( + "--android-lint-cli-tool", + lintCli.toString(), + "--label", + label, + "--compile-sdk-version", + "34", + "--java-language-level", + "17", + "--kotlin-language-level", + "1.9", + ) + + private fun projectXml(run: Main.RunRecord): String = + Files.readString(Paths.get(run.args.argumentAfter("--project"))) + private fun writeStubLintJar(name: String): Path { val jar = temporaryFolder.newFile(name).toPath() JarOutputStream(Files.newOutputStream(jar)).use { stream -> From 80a48d78e7bf649d35e9338dd8e3f08f22208dfd Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 08:25:21 -0700 Subject: [PATCH 09/12] Skip disabled dependency lint analysis --- rules/lint_analysis_aspect.bzl | 18 +++++++-- tests/rules/android_lint_analysis_test.bzl | 46 +++++++++++++++++++++- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index f4fec52..8550d7b 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -70,11 +70,24 @@ def _android_model(target): ) def _lint_analysis_aspect_impl(target, ctx): - deps = _aspect_deps(ctx) - transitive = _collect_transitive(deps) + toolchain = _utils.get_android_lint_toolchain(ctx) android_model = _android_model(target) is_library = ctx.rule.kind != "android_binary" + if not toolchain.android_lint_enable_check_dependencies: + return [_AndroidLintPartialResultsInfo( + is_android = android_model.is_android, + is_library = is_library, + manifest = android_model.manifest, + partial_results = None, + resource_files = android_model.resource_files, + module_name = None, + transitive_results = depset(), + )] + + deps = _aspect_deps(ctx) + transitive = _collect_transitive(deps) + # Skip nodes that have nothing to analyze, but keep propagating the transitive results # gathered from their dependencies. Android resource- or manifest-only targets still need an # analysis action because many Android lint detectors do not inspect source code. @@ -94,7 +107,6 @@ def _lint_analysis_aspect_impl(target, ctx): transitive_results = depset(transitive = transitive), )] - toolchain = _utils.get_android_lint_toolchain(ctx) module_name = _utils.module_name(ctx.label) partial_results = ctx.actions.declare_directory("_lint/%s/partial_results" % ctx.label.name) android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index 706b4f1..1b8954f 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -178,7 +178,36 @@ def _android_dependency_analysis_impl(ctx): return analysistest.end(env) -_android_dependency_analysis_test = analysistest.make(_android_dependency_analysis_impl) +_android_dependency_analysis_test = analysistest.make( + _android_dependency_analysis_impl, + config_settings = { + "//command_line_option:extra_toolchains": [ + "//tests/rules:dependency_analysis_enabled_toolchain", + ], + }, +) + +def _android_dependency_analysis_disabled_impl(ctx): + env = analysistest.begin(ctx) + target = analysistest.target_under_test(env) + asserts.true(env, _AndroidDependencyAnalysisInfo in target) + if _AndroidDependencyAnalysisInfo in target: + analysis = target[_AndroidDependencyAnalysisInfo] + info = analysis.lint_info + asserts.true(env, info.is_android) + asserts.true(env, info.is_library) + asserts.equals(env, None, info.partial_results) + asserts.equals(env, None, info.module_name) + asserts.equals(env, 0, len(info.transitive_results.to_list())) + + actions = [action for action in analysis.actions if action.mnemonic == "AndroidLintAnalyze"] + asserts.equals(env, 0, len(actions)) + + return analysistest.end(env) + +_android_dependency_analysis_disabled_test = analysistest.make( + _android_dependency_analysis_disabled_impl, +) _AndroidDependencyAnalysisInfo = provider( "Test-only view of the dependency lint aspect's provider and registered actions.", @@ -250,7 +279,14 @@ def _module_name_collision_impl(ctx): return analysistest.end(env) -_module_name_collision_test = analysistest.make(_module_name_collision_impl) +_module_name_collision_test = analysistest.make( + _module_name_collision_impl, + config_settings = { + "//command_line_option:extra_toolchains": [ + "//tests/rules:dependency_analysis_enabled_toolchain", + ], + }, +) def _dependency_analysis_enabled_transition_impl(_settings, _attr): return { @@ -310,6 +346,11 @@ def android_lint_analysis_test_suite(name, additional_tests = []): name = android_dependency_test, target_under_test = ":" + android_dependency_subject, ) + android_dependency_disabled_test = name + "_android_dependency_disabled_test" + _android_dependency_analysis_disabled_test( + name = android_dependency_disabled_test, + target_under_test = ":" + android_dependency_subject, + ) module_names_subject = name + "_module_names_subject" _module_names_subject( name = module_names_subject, @@ -327,6 +368,7 @@ def android_lint_analysis_test_suite(name, additional_tests = []): name = name, tests = [ ":" + action_test, + ":" + android_dependency_disabled_test, ":" + android_dependency_test, ":" + module_name_collision_test, ] + additional_tests, From 6ba67db33ab72f78bfbc2c49d2ac699c227bde9f Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 08:36:23 -0700 Subject: [PATCH 10/12] Preserve dependency lint models in reports --- rules/impl.bzl | 15 +- rules/lint_analysis_aspect.bzl | 38 +++- rules/providers.bzl | 6 +- src/cli/AndroidLintActionArgs.kt | 32 +--- src/cli/AndroidLintProject.kt | 209 ++++++++++++++++----- src/cli/AndroidLintRunner.kt | 23 +-- src/cli/BUILD | 1 + tests/rules/android_lint_analysis_test.bzl | 51 +++-- tests/src/cli/AndroidLintActionArgsTest.kt | 20 +- tests/src/cli/AndroidLintProjectTest.kt | 19 +- tests/src/cli/AndroidLintRunnerTest.kt | 102 ++++++++-- 11 files changed, 353 insertions(+), 163 deletions(-) diff --git a/rules/impl.bzl b/rules/impl.bzl index db535d8..56c8bd9 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -60,8 +60,8 @@ def _run_android_lint( module_name: The name of the module output: The XML output file (report mode only; None in analyze mode) partial_results: The partial-results directory (output in analyze, input in report) - dependency_modules: List of structs(module_name, partial_results) for first-party deps - whose partial results should be merged (report mode only) + dependency_modules: List of structs(module_name, partial_results, model, inputs) for + first-party deps whose partial results should be merged (report mode only) srcs: The source files deps: Depset of aars and jars to include on the classpath aars: Depset of the aar nodes @@ -155,12 +155,10 @@ def _run_android_lint( if autofix == True: args.add("--autofix") for dependency in dependency_modules: - args.add("--dependency-partial-results", "%s=%s" % (dependency.module_name, dependency.partial_results.path)) - if dependency.is_android: - args.add("--android-dependency", dependency.module_name) - if dependency.is_library: - args.add("--library-dependency", dependency.module_name) + args.add("--dependency-model", dependency.model) + inputs.append(dependency.model) inputs.append(dependency.partial_results) + inputs.extend(dependency.inputs) args.add("--output", output) outputs.append(output) @@ -198,8 +196,7 @@ def _collect_dependency_modules(ctx): """Collects the transitive partial-results modules from the rule's dependencies. Returns: - A deduplicated list of structs(module_name, partial_results) for every analyzed - transitive dependency. + A deduplicated list of module model structs for every analyzed transitive dependency. """ transitive = [] for dep in ctx.attr.deps: diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index 8550d7b..e0dce2d 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -109,14 +109,17 @@ def _lint_analysis_aspect_impl(target, ctx): module_name = _utils.module_name(ctx.label) partial_results = ctx.actions.declare_directory("_lint/%s/partial_results" % ctx.label.name) + module_model = ctx.actions.declare_file("_lint/%s/module_model.json" % ctx.label.name) android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) java_runtime_info = ctx.attr._javabase[java_common.JavaRuntimeInfo] inputs = [android_lint] - inputs.extend(srcs) - inputs.extend(android_model.resource_files.to_list()) + module_inputs = [] + module_inputs.extend(srcs) + module_inputs.extend(android_model.resource_files.to_list()) if android_model.manifest: - inputs.append(android_model.manifest) + module_inputs.append(android_model.manifest) + inputs.extend(module_inputs) args = ctx.actions.args() args.set_param_file_format("multiline") @@ -152,11 +155,14 @@ def _lint_analysis_aspect_impl(target, ctx): classpath = [target[JavaInfo].transitive_compile_time_jars] if AndroidLibraryResourceClassJarProvider in target: classpath.append(target[AndroidLibraryResourceClassJarProvider].jars) - for jar in depset(transitive = classpath).to_list(): + classpath_jars = depset(transitive = classpath).to_list() + for jar in classpath_jars: args.add("--classpath-jar", jar) inputs.append(jar) + module_inputs.append(jar) # AAR dependencies (extracted), so lint loads their classes and embedded lint.jar checks. + classpath_aars = [] if _AndroidLintAARInfo in target: aar_info = target[_AndroidLintAARInfo] direct_aar = aar_info.aar.aar @@ -171,6 +177,28 @@ def _lint_analysis_aspect_impl(target, ctx): args.add("--classpath-aar", "%s:%s" % (node.aar.path, node.aar_dir.path)) inputs.append(node.aar) inputs.append(node.aar_dir) + module_inputs.append(node.aar) + module_inputs.append(node.aar_dir) + classpath_aars.append({ + "aar": node.aar.path, + "extracted": node.aar_dir.path, + }) + + ctx.actions.write( + output = module_model, + content = json.encode({ + "name": module_name, + "partialResultsDir": partial_results.path, + "isAndroid": android_model.is_android, + "isLibrary": is_library, + "srcs": [src.path for src in srcs], + "resources": [resource.path for resource in android_model.resource_files.to_list()], + "androidManifest": android_model.manifest.path if android_model.manifest else None, + "classpathJars": [jar.path for jar in classpath_jars], + "classpathAars": [], + "classpathExtractedAarDirectories": classpath_aars, + }), + ) if toolchain.android_lint_config: config = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint_config.files)) @@ -208,6 +236,8 @@ def _lint_analysis_aspect_impl(target, ctx): direct = struct( is_android = android_model.is_android, is_library = is_library, + inputs = tuple(module_inputs), + model = module_model, module_name = module_name, partial_results = partial_results, ) diff --git a/rules/providers.bzl b/rules/providers.bzl index b4d02aa..c3e459e 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -19,8 +19,8 @@ AndroidLintPartialResultsInfo = provider( "node had no lintable inputs.", "resource_files": "Depset of direct Android resources used for this target's analysis.", "module_name": "The lint module name for this target, or None if not analyzed.", - "transitive_results": "depset of structs(module_name, partial_results, is_android, " + - "is_library) for this target and all analyzed transitive " + - "dependencies.", + "transitive_results": "depset of structs(module_name, partial_results, model, inputs, " + + "is_android, is_library) for this target and all analyzed " + + "transitive dependencies.", }, ) diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 64186ee..75343b3 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -18,16 +18,6 @@ internal class AndroidLintActionArgs( Pair(Paths.get(aar), Paths.get(aarDir)) } - private val argsParserDependencyModuleTransformer: String.() -> Pair = { - // Format: =. The name is opaque and may itself contain - // no '=', so split on the first occurrence only. - val separator = this.indexOf('=') - require(separator > 0) { - "Error: --dependency-partial-results expects =, got: $this" - } - Pair(this.substring(0, separator), Paths.get(this.substring(separator + 1))) - } - val androidLintCliTool: Path by parser.storing( names = arrayOf("--android-lint-cli-tool"), help = "", @@ -69,26 +59,12 @@ internal class AndroidLintActionArgs( transform = argsParserPathTransformer, ).default { null } - // First-party dependency partial results consumed in report mode, as = pairs. - val dependencyPartialResults: List> by parser - .adding( - names = arrayOf("--dependency-partial-results"), - help = "", - transform = argsParserDependencyModuleTransformer, - ).default { emptyList() } - - // Names of dependency modules whose partial results came from Android projects. - val androidDependencies: List by parser - .adding( - names = arrayOf("--android-dependency"), - help = "", - ).default { emptyList() } - - // Names of dependency modules whose partial results came from library projects. - val libraryDependencies: List by parser + // Serialized descriptions of first-party dependency modules consumed in report mode. + val dependencyModels: List by parser .adding( - names = arrayOf("--library-dependency"), + names = arrayOf("--dependency-model"), help = "", + transform = argsParserPathTransformer, ).default { emptyList() } val androidHome: String? by parser diff --git a/src/cli/AndroidLintProject.kt b/src/cli/AndroidLintProject.kt index 89878d1..434799e 100644 --- a/src/cli/AndroidLintProject.kt +++ b/src/cli/AndroidLintProject.kt @@ -1,7 +1,10 @@ package com.rules.android.lint.cli +import com.google.gson.Gson import java.io.StringWriter +import java.nio.file.Files import java.nio.file.Path +import java.nio.file.Paths import javax.xml.parsers.DocumentBuilderFactory import javax.xml.transform.OutputKeys import javax.xml.transform.TransformerFactory @@ -14,17 +17,71 @@ import kotlin.io.path.pathString * A first-party dependency module contributing partial analysis results to the report phase. * * In the report (`--report-only`) phase lint merges these modules' partial results into the main - * module's verdict without re-analyzing their sources. Each is registered with its original - * module identity, carries its own `partial-results-dir`, and is linked from the main module via a - * `` element. + * module's verdict without re-analyzing their sources. Each is registered with the same module + * identity and inputs used during analysis, carries its own `partial-results-dir`, and is linked + * from the main module via a `` element. */ internal data class LintDependencyModule( val name: String, val partialResultsDir: Path, val isAndroid: Boolean = false, val isLibrary: Boolean = false, + val srcs: List = emptyList(), + val resources: List = emptyList(), + val androidManifest: Path? = null, + val classpathJars: List = emptyList(), + val classpathAars: List = emptyList(), + val classpathExtractedAarDirectories: List> = emptyList(), ) +private data class SerializedLintDependencyModule( + val name: String?, + val partialResultsDir: String?, + val isAndroid: Boolean, + val isLibrary: Boolean, + val srcs: List?, + val resources: List?, + val androidManifest: String?, + val classpathJars: List?, + val classpathAars: List?, + val classpathExtractedAarDirectories: List?, +) + +private data class SerializedExtractedAar( + val aar: String?, + val extracted: String?, +) + +internal fun readLintDependencyModule(model: Path): LintDependencyModule { + val serialized = + Gson().fromJson(Files.readString(model), SerializedLintDependencyModule::class.java) + return LintDependencyModule( + name = requireNotNull(serialized.name) { "Dependency model $model has no name" }, + partialResultsDir = + Paths.get( + requireNotNull(serialized.partialResultsDir) { + "Dependency model $model has no partialResultsDir" + }, + ), + isAndroid = serialized.isAndroid, + isLibrary = serialized.isLibrary, + srcs = serialized.srcs.orEmpty().map(Paths::get), + resources = serialized.resources.orEmpty().map(Paths::get), + androidManifest = serialized.androidManifest?.let(Paths::get), + classpathJars = serialized.classpathJars.orEmpty().map(Paths::get), + classpathAars = serialized.classpathAars.orEmpty().map(Paths::get), + classpathExtractedAarDirectories = + serialized.classpathExtractedAarDirectories.orEmpty().map { aar -> + Paths.get(requireNotNull(aar.aar) { "Dependency model $model has an AAR with no path" }) to + Paths.get( + requireNotNull(aar.extracted) { + "Dependency model $model has an AAR with no extracted directory" + }, + ) + }, + ) +} + internal fun createProjectXMLString( moduleName: String, rootDir: String, @@ -75,48 +132,17 @@ internal fun createProjectXMLString( } } - srcs.forEach { src -> - val element = document.createElement("src") - element.setAttribute("file", src.pathString) - moduleElement.appendChild(element) - } - - resources.forEach { res -> - val element = document.createElement("resource") - element.setAttribute("file", res.pathString) - moduleElement.appendChild(element) - } - - if (androidManifest != null) { - val element = document.createElement("manifest") - element.setAttribute("file", androidManifest.pathString) - moduleElement.appendChild(element) - } - - classpathJars.forEach { jar -> - val element = document.createElement("classpath") - element.setAttribute("jar", jar.absolutePathString()) - moduleElement.appendChild(element) - } - - classpathAars.forEach { aar -> - val element = document.createElement("aar") - element.setAttribute("file", aar.absolutePathString()) - if (aarPartialResultsScratchDir != null) { - element.setAttribute("partial-results-dir", aarPartialResultsScratchDir.absolutePathString()) - } - moduleElement.appendChild(element) - } - - classpathExtractedAarDirectories.forEach { (aar, unzippedDir) -> - val element = document.createElement("aar") - element.setAttribute("file", aar.absolutePathString()) - element.setAttribute("extracted", unzippedDir.absolutePathString()) - if (aarPartialResultsScratchDir != null) { - element.setAttribute("partial-results-dir", aarPartialResultsScratchDir.absolutePathString()) - } - moduleElement.appendChild(element) - } + appendModuleContents( + document = document, + moduleElement = moduleElement, + srcs = srcs, + resources = resources, + androidManifest = androidManifest, + classpathJars = classpathJars, + classpathAars = classpathAars, + classpathExtractedAarDirectories = classpathExtractedAarDirectories, + aarPartialResultsScratchDir = aarPartialResultsScratchDir, + ) // Link the main module to each first-party dependency that contributed partial results, then // register those dependencies as library modules carrying their own partial-results-dir. @@ -128,13 +154,25 @@ internal fun createProjectXMLString( } dependencyModules.forEach { dependency -> - document.createElement("module").also { - it.setAttribute("name", dependency.name) - it.setAttribute("android", dependency.isAndroid.toString()) - it.setAttribute("library", dependency.isLibrary.toString()) - it.setAttribute("partial-results-dir", dependency.partialResultsDir.absolutePathString()) - projectElement.appendChild(it) - } + val dependencyElement = + document.createElement("module").also { + it.setAttribute("name", dependency.name) + it.setAttribute("android", dependency.isAndroid.toString()) + it.setAttribute("library", dependency.isLibrary.toString()) + it.setAttribute("partial-results-dir", dependency.partialResultsDir.absolutePathString()) + projectElement.appendChild(it) + } + appendModuleContents( + document = document, + moduleElement = dependencyElement, + srcs = dependency.srcs, + resources = dependency.resources, + androidManifest = dependency.androidManifest, + classpathJars = dependency.classpathJars, + classpathAars = dependency.classpathAars, + classpathExtractedAarDirectories = dependency.classpathExtractedAarDirectories, + aarPartialResultsScratchDir = aarPartialResultsScratchDir, + ) } return StringWriter() @@ -146,3 +184,70 @@ internal fun createProjectXMLString( }.buffer .toString() } + +private fun appendModuleContents( + document: org.w3c.dom.Document, + moduleElement: org.w3c.dom.Element, + srcs: List, + resources: List, + androidManifest: Path?, + classpathJars: List, + classpathAars: List, + classpathExtractedAarDirectories: List>, + aarPartialResultsScratchDir: Path?, +) { + srcs.forEach { src -> + document.createElement("src").also { + it.setAttribute("file", src.pathString) + moduleElement.appendChild(it) + } + } + + resources.forEach { resource -> + document.createElement("resource").also { + it.setAttribute("file", resource.pathString) + moduleElement.appendChild(it) + } + } + + if (androidManifest != null) { + document.createElement("manifest").also { + it.setAttribute("file", androidManifest.pathString) + moduleElement.appendChild(it) + } + } + + classpathJars.forEach { jar -> + document.createElement("classpath").also { + it.setAttribute("jar", jar.absolutePathString()) + moduleElement.appendChild(it) + } + } + + classpathAars.forEach { aar -> + document.createElement("aar").also { + it.setAttribute("file", aar.absolutePathString()) + if (aarPartialResultsScratchDir != null) { + it.setAttribute( + "partial-results-dir", + aarPartialResultsScratchDir.absolutePathString(), + ) + } + moduleElement.appendChild(it) + } + } + + classpathExtractedAarDirectories.forEach { (aar, unzippedDir) -> + document.createElement("aar").also { + it.setAttribute("file", aar.absolutePathString()) + it.setAttribute("extracted", unzippedDir.absolutePathString()) + if (aarPartialResultsScratchDir != null) { + it.setAttribute( + "partial-results-dir", + aarPartialResultsScratchDir.absolutePathString(), + ) + } + moduleElement.appendChild(it) + } + } +} diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index cf27cd6..9aa6cda 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -71,21 +71,14 @@ internal class AndroidLintRunner( requireNotNull(args.partialResults) { "--partial-results is required in report mode" } val baselineFile = stageBaseline(args, workingDirectory) val rootDir = rootDir() + val dependencyModules = args.dependencyModels.map(::readLintDependencyModule) val projectFile = writeProjectXml( args = args, workingDirectory = workingDirectory, rootDir = rootDir, partialResultsDir = partialResults, - dependencyModules = - args.dependencyPartialResults.map { (name, dir) -> - LintDependencyModule( - name = name, - partialResultsDir = dir, - isAndroid = name in args.androidDependencies, - isLibrary = name in args.libraryDependencies, - ) - }, + dependencyModules = dependencyModules, ) val lintArgs = @@ -105,13 +98,13 @@ internal class AndroidLintRunner( // When first-party dependency partial results are present, enable check-dependencies so the // merge phase reports their incidents (rather than analyzing — the partial results already - // exist). Lint reads each dependency module's partial-results-dir instead of its sources. + // exist). The full module metadata remains available to detectors during reporting. return reportExitCode( invoke( args, lintArgs, workingDirectory, - enableCheckDependencies = args.dependencyPartialResults.isNotEmpty(), + enableCheckDependencies = dependencyModules.isNotEmpty(), ), ) } @@ -225,7 +218,11 @@ internal class AndroidLintRunner( partialResultsDir: Path?, dependencyModules: List, ): Path { - val aarLintRuleJars = collectAarLintRuleJars(args.classpathAarPairs) + val aarLintRuleJars = + collectAarLintRuleJars( + args.classpathAarPairs + + dependencyModules.flatMap(LintDependencyModule::classpathExtractedAarDirectories), + ) val projectFile = workingDirectory.resolve("${args.label}_project_config.xml") Files.createFile(projectFile) @@ -251,7 +248,7 @@ internal class AndroidLintRunner( classpathJars = args.classpath.sortedDescending(), classpathAars = emptyList(), classpathExtractedAarDirectories = args.classpathAarPairs, - customLintChecks = (args.customChecks + aarLintRuleJars).sortedDescending(), + customLintChecks = (args.customChecks + aarLintRuleJars).distinct().sortedDescending(), partialResultsDir = partialResultsDir, dependencyModules = dependencyModules, aarPartialResultsScratchDir = aarPartialResultsScratchDir, diff --git a/src/cli/BUILD b/src/cli/BUILD index 3daaa2c..82a035f 100644 --- a/src/cli/BUILD +++ b/src/cli/BUILD @@ -16,6 +16,7 @@ kt_jvm_library( visibility = ["//visibility:public"], deps = [ "//src/worker", + "@rules_android_lint_deps//:com_google_code_gson_gson", "@rules_android_lint_deps//:com_xenomachina_kotlin_argparser", ], ) diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index 1b8954f..97c6016 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -42,9 +42,7 @@ def _android_lint_action_impl(ctx): custom_rule = _argument_value(argv, "--custom-rule") module_name = _argument_value(argv, "--label") output = _argument_value(argv, "--output") - dependency_partial_results = _argument_values(argv, "--dependency-partial-results") - android_dependencies = _argument_values(argv, "--android-dependency") - library_dependencies = _argument_values(argv, "--library-dependency") + dependency_models = _argument_values(argv, "--dependency-model") classpath_aars = _argument_values(argv, "--classpath-aar") asserts.true(env, module_name != None and module_name.endswith("%3Aanalysis_fixture_lint")) @@ -62,42 +60,27 @@ def _android_lint_action_impl(ctx): asserts.false(env, "--baseline-file" in argv) asserts.false(env, "--regenerate-baseline-files" in argv) - dependency_module_names = [value.split("=")[0] for value in dependency_partial_results] - asserts.equals(env, 5, len(dependency_partial_results)) + asserts.equals(env, 5, len(dependency_models)) asserts.true( env, - any([name.endswith("%3Aandroid_dependency") for name in dependency_module_names]), + any([model.endswith("/_lint/android_dependency/module_model.json") for model in dependency_models]), ) asserts.true( env, - any([name.endswith("%3Acollision-dep") for name in dependency_module_names]), + any([model.endswith("/_lint/collision-dep/module_model.json") for model in dependency_models]), ) asserts.true( env, - any([name.endswith("%3Acollision.dep") for name in dependency_module_names]), + any([model.endswith("/_lint/collision.dep/module_model.json") for model in dependency_models]), ) asserts.true( env, - any([name.endswith("%3Aruntime_android_dependency") for name in dependency_module_names]), + any([model.endswith("/_lint/runtime_android_dependency/module_model.json") for model in dependency_models]), ) asserts.true( env, - any([name.endswith("%3Aruntime_parent") for name in dependency_module_names]), + any([model.endswith("/_lint/runtime_parent/module_model.json") for model in dependency_models]), ) - asserts.equals(env, 2, len(android_dependencies)) - asserts.true( - env, - any([name.endswith("%3Aandroid_dependency") for name in android_dependencies]), - ) - asserts.true( - env, - any([name.endswith("%3Aruntime_android_dependency") for name in android_dependencies]), - ) - for android_dependency in android_dependencies: - asserts.true(env, android_dependency in dependency_module_names) - asserts.equals(env, 5, len(library_dependencies)) - for library_dependency in library_dependencies: - asserts.true(env, library_dependency in dependency_module_names) asserts.true( env, any(["runtime_android_dependency.aar:" in aar for aar in classpath_aars]), @@ -105,16 +88,23 @@ def _android_lint_action_impl(ctx): action_inputs = action.inputs.to_list() inputs = [file.basename for file in action_inputs] + input_paths = [file.path for file in action_inputs] outputs = [file.basename for file in action.outputs.to_list()] asserts.true(env, "Fixture.java" in inputs) + asserts.true(env, "strings.xml" in inputs) + asserts.true(env, "runtime.xml" in inputs) asserts.true( env, any(["compose-lint-checks" in input and input.endswith(".jar") for input in inputs]), ) asserts.true(env, "analysis_fixture_lint.xml" in outputs) - for value in dependency_partial_results: - partial_results_path = value.split("=", 1)[1] - asserts.true(env, partial_results_path in [file.path for file in action_inputs]) + asserts.equals( + env, + 5, + len([path for path in input_paths if path.endswith("/partial_results")]), + ) + for model in dependency_models: + asserts.true(env, model in input_paths) asserts.equals(env, "1", action.execution_info.get("supports-workers")) asserts.equals(env, "1", action.execution_info.get("supports-multiplex-workers")) @@ -145,6 +135,13 @@ def _android_dependency_analysis_impl(ctx): env, "strings.xml" in [file.basename for file in info.resource_files.to_list()], ) + transitive_results = info.transitive_results.to_list() + asserts.equals(env, 1, len(transitive_results)) + if transitive_results: + asserts.equals(env, "module_model.json", transitive_results[0].model.basename) + model_inputs = [file.basename for file in transitive_results[0].inputs] + asserts.true(env, "AndroidManifest.xml" in model_inputs) + asserts.true(env, "strings.xml" in model_inputs) actions = [action for action in analysis.actions if action.mnemonic == "AndroidLintAnalyze"] asserts.equals(env, 1, len(actions)) diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index 2027821..571dd7f 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/tests/src/cli/AndroidLintActionArgsTest.kt @@ -96,14 +96,10 @@ class AndroidLintActionArgsTest { "--library", "--partial-results", "path/to/partial", - "--dependency-partial-results", - "dep_a=path/to/dep_a/partial", - "--android-dependency", - "dep_a", - "--library-dependency", - "dep_a", - "--dependency-partial-results", - "dep_b=path/to/dep_b/partial", + "--dependency-model", + "path/to/dep_a/model.json", + "--dependency-model", + "path/to/dep_b/model.json", "--compile-sdk-version", "34", "--java-language-level", @@ -117,12 +113,10 @@ class AndroidLintActionArgsTest { assertThat(parseArgs.isAndroid).isTrue() assertThat(parseArgs.isLibrary).isTrue() assertThat(parseArgs.partialResults).isEqualTo(Paths.get("path/to/partial")) - assertThat(parseArgs.dependencyPartialResults).containsExactly( - "dep_a" to Paths.get("path/to/dep_a/partial"), - "dep_b" to Paths.get("path/to/dep_b/partial"), + assertThat(parseArgs.dependencyModels).containsExactly( + Paths.get("path/to/dep_a/model.json"), + Paths.get("path/to/dep_b/model.json"), ) - assertThat(parseArgs.androidDependencies).containsExactly("dep_a") - assertThat(parseArgs.libraryDependencies).containsExactly("dep_a") // Output is optional now; absent here. assertThat(parseArgs.output).isNull() } diff --git a/tests/src/cli/AndroidLintProjectTest.kt b/tests/src/cli/AndroidLintProjectTest.kt index d2cd8c4..0b60525 100644 --- a/tests/src/cli/AndroidLintProjectTest.kt +++ b/tests/src/cli/AndroidLintProjectTest.kt @@ -92,6 +92,12 @@ class AndroidLintProjectTest { fun `report mode preserves dependency Android identity and partial-results-dir`() { val ownPartial = tmpDirectory.newFolder("own").toPath() val depPartial = tmpDirectory.newFolder("depA").toPath() + val depSource = tmpDirectory.newPath("Dep.kt") + val depResource = tmpDirectory.newPath("dep_strings.xml") + val depManifest = tmpDirectory.newPath("DepAndroidManifest.xml") + val depClasspath = tmpDirectory.newPath("Dep.jar") + val depAar = tmpDirectory.newPath("Dep.aar") + val depExtractedAar = tmpDirectory.newFolder("tmp/unpacked_aars/dep").toPath() assertThat( createProjectXMLString( moduleName = "test_module_name", @@ -111,6 +117,11 @@ class AndroidLintProjectTest { partialResultsDir = depPartial, isAndroid = true, isLibrary = true, + srcs = listOf(depSource), + resources = listOf(depResource), + androidManifest = depManifest, + classpathJars = listOf(depClasspath), + classpathExtractedAarDirectories = listOf(depAar to depExtractedAar), ), ), ), @@ -123,7 +134,13 @@ class AndroidLintProjectTest { - + + + + + + + """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), diff --git a/tests/src/cli/AndroidLintRunnerTest.kt b/tests/src/cli/AndroidLintRunnerTest.kt index ebbc6cb..1427ea6 100644 --- a/tests/src/cli/AndroidLintRunnerTest.kt +++ b/tests/src/cli/AndroidLintRunnerTest.kt @@ -116,6 +116,36 @@ class AndroidLintRunnerTest { val ownPartialResults = temporaryFolder.newFolder("report-partial-results").toPath() val androidDependencyResults = temporaryFolder.newFolder("android-dependency-results").toPath() val javaDependencyResults = temporaryFolder.newFolder("java-dependency-results").toPath() + val androidDependencySource = temporaryFolder.newFile("AndroidDependency.kt").toPath() + val androidDependencyResource = temporaryFolder.newFile("dependency_strings.xml").toPath() + val androidDependencyManifest = temporaryFolder.newFile("DependencyManifest.xml").toPath() + val androidDependencyClasspath = writeStubLintJar("android-dependency.jar") + val androidDependencyAar = temporaryFolder.newFile("android-dependency.aar").toPath() + val androidDependencyAarDirectory = + temporaryFolder.newFolder("android-dependency-aar").toPath() + val dependencyLintRule = androidDependencyAarDirectory.resolve("lint.jar") + Files.copy(writeStubLintJar("dependency-lint-rule.jar"), dependencyLintRule) + val javaDependencySource = temporaryFolder.newFile("JavaDependency.java").toPath() + val androidDependencyModel = + writeDependencyModel( + name = "android-dependency", + partialResults = androidDependencyResults, + isAndroid = true, + isLibrary = true, + srcs = listOf(androidDependencySource), + resources = listOf(androidDependencyResource), + androidManifest = androidDependencyManifest, + classpathJars = listOf(androidDependencyClasspath), + classpathAars = listOf(androidDependencyAar to androidDependencyAarDirectory), + ) + val javaDependencyModel = + writeDependencyModel( + name = "java-dependency", + partialResults = javaDependencyResults, + isAndroid = false, + isLibrary = true, + srcs = listOf(javaDependencySource), + ) val output = temporaryFolder.root.toPath().resolve("report.xml") val workingDirectory = temporaryFolder.newFolder("report-working").toPath() val args = @@ -128,16 +158,10 @@ class AndroidLintRunnerTest { "--library", "--partial-results", ownPartialResults.toString(), - "--dependency-partial-results", - "android-dependency=$androidDependencyResults", - "--android-dependency", - "android-dependency", - "--library-dependency", - "android-dependency", - "--dependency-partial-results", - "java-dependency=$javaDependencyResults", - "--library-dependency", - "java-dependency", + "--dependency-model", + androidDependencyModel.toString(), + "--dependency-model", + javaDependencyModel.toString(), "--output", output.toString(), "--warnings-as-errors", @@ -166,10 +190,20 @@ class AndroidLintRunnerTest { .contains("") .contains( "", - ).contains( + "partial-results-dir=\"${androidDependencyResults.toAbsolutePath()}\">", + ).contains("") + .contains("") + .contains("") + .contains("") + .contains( + "") + .contains( "", + "partial-results-dir=\"${javaDependencyResults.toAbsolutePath()}\">", + ).contains( + "", ) } @@ -202,8 +236,50 @@ class AndroidLintRunnerTest { } return jar } + + private fun writeDependencyModel( + name: String, + partialResults: Path, + isAndroid: Boolean, + isLibrary: Boolean, + srcs: List = emptyList(), + resources: List = emptyList(), + androidManifest: Path? = null, + classpathJars: List = emptyList(), + classpathAars: List> = emptyList(), + ): Path { + val model = temporaryFolder.newFile("$name-model.json").toPath() + val serializedAars = + classpathAars.joinToString(prefix = "[", postfix = "]") { (aar, extracted) -> + """{"aar": ${aar.toString().jsonString()}, "extracted": ${ + extracted.toString().jsonString() + }}""" + } + val json = + """ + { + "name": ${name.jsonString()}, + "partialResultsDir": ${partialResults.toString().jsonString()}, + "isAndroid": $isAndroid, + "isLibrary": $isLibrary, + "srcs": ${srcs.toJson()}, + "resources": ${resources.toJson()}, + "androidManifest": ${androidManifest?.toString()?.jsonString() ?: "null"}, + "classpathJars": ${classpathJars.toJson()}, + "classpathAars": [], + "classpathExtractedAarDirectories": $serializedAars + } + """.trimIndent() + Files.writeString(model, json) + return model + } } +private fun String.jsonString(): String = "\"${replace("\\", "\\\\").replace("\"", "\\\"")}\"" + +private fun List.toJson(): String = + joinToString(prefix = "[", postfix = "]") { it.toString().jsonString() } + private fun List.argumentAfter(argument: String): String { val index = indexOf(argument) check(index >= 0 && index + 1 < size) { "Missing value for $argument in $this" } From 6a9966bf4ff6cd9749748c2723ed07f0f4a1bbc0 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 17 Jul 2026 08:41:40 -0700 Subject: [PATCH 11/12] Preserve main lint module identity --- rules/impl.bzl | 11 +++++ src/cli/AndroidLintActionArgs.kt | 6 +++ src/cli/AndroidLintProject.kt | 7 +++ src/cli/AndroidLintRunner.kt | 1 + tests/rules/BUILD.bazel | 22 ++++++++- tests/rules/android_lint_analysis_test.bzl | 54 ++++++++++++++++++++++ tests/src/cli/AndroidLintActionArgsTest.kt | 2 + tests/src/cli/AndroidLintProjectTest.kt | 5 +- tests/src/cli/AndroidLintRunnerTest.kt | 3 +- 9 files changed, 107 insertions(+), 4 deletions(-) diff --git a/rules/impl.bzl b/rules/impl.bzl index 56c8bd9..03d6a97 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -4,6 +4,7 @@ load( "@rules_android//providers:providers.bzl", "AndroidLibraryResourceClassJarProvider", + "ApkInfo", "StarlarkAndroidResourcesInfo", ) load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") @@ -27,6 +28,8 @@ def _run_android_lint( mode, android_lint, is_android, + is_library, + is_test_sources, module_name, output, partial_results, @@ -57,6 +60,8 @@ def _run_android_lint( mode: One of "analyze" (write partial results) or "report" (consume them, emit XML) android_lint: The Android Lint binary to use is_android: Whether the module is an Android module + is_library: Whether the module is a library rather than an application + is_test_sources: Whether the module sources are test sources module_name: The name of the module output: The XML output file (report mode only; None in analyze mode) partial_results: The partial-results directory (output in analyze, input in report) @@ -96,6 +101,10 @@ def _run_android_lint( args.add("--mode", mode) if is_android: args.add("--android") + if is_library: + args.add("--library") + if is_test_sources: + args.add("--test-sources") # The partial-results directory is an output of analyze and an input of report. args.add("--partial-results", partial_results.path) @@ -289,6 +298,8 @@ def process_android_lint_issues(ctx, regenerate): manifest != None or bool(ctx.files.resource_files) ), + is_library = ApkInfo not in ctx.attr.lib, + is_test_sources = ctx.attr.is_test_sources, module_name = module_name, srcs = ctx.files.srcs, deps = deps_depset, diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 75343b3..600b638 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -50,6 +50,12 @@ internal class AndroidLintActionArgs( help = "Model this project as a library module.", ).default { false } + val isTestSources: Boolean by parser + .flagging( + names = arrayOf("--test-sources"), + help = "Model this project's sources as test sources.", + ).default { false } + // In analyze mode, the directory lint writes partial results into. In report mode, the directory // lint reads the module's own partial results from. val partialResults: Path? by parser diff --git a/src/cli/AndroidLintProject.kt b/src/cli/AndroidLintProject.kt index 434799e..d9d9c70 100644 --- a/src/cli/AndroidLintProject.kt +++ b/src/cli/AndroidLintProject.kt @@ -90,6 +90,7 @@ internal fun createProjectXMLString( androidManifest: Path?, isAndroid: Boolean = androidManifest != null, isLibrary: Boolean = false, + isTestSources: Boolean = false, classpathJars: List, classpathAars: List, classpathExtractedAarDirectories: List>, @@ -136,6 +137,7 @@ internal fun createProjectXMLString( document = document, moduleElement = moduleElement, srcs = srcs, + isTestSources = isTestSources, resources = resources, androidManifest = androidManifest, classpathJars = classpathJars, @@ -166,6 +168,7 @@ internal fun createProjectXMLString( document = document, moduleElement = dependencyElement, srcs = dependency.srcs, + isTestSources = false, resources = dependency.resources, androidManifest = dependency.androidManifest, classpathJars = dependency.classpathJars, @@ -189,6 +192,7 @@ private fun appendModuleContents( document: org.w3c.dom.Document, moduleElement: org.w3c.dom.Element, srcs: List, + isTestSources: Boolean, resources: List, androidManifest: Path?, classpathJars: List, @@ -199,6 +203,9 @@ private fun appendModuleContents( srcs.forEach { src -> document.createElement("src").also { it.setAttribute("file", src.pathString) + if (isTestSources) { + it.setAttribute("test", "true") + } moduleElement.appendChild(it) } } diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index 9aa6cda..ac0febf 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -245,6 +245,7 @@ internal class AndroidLintRunner( androidManifest = args.androidManifest, isAndroid = args.isAndroid || args.androidManifest != null, isLibrary = args.isLibrary, + isTestSources = args.isTestSources, classpathJars = args.classpath.sortedDescending(), classpathAars = emptyList(), classpathExtractedAarDirectories = args.classpathAarPairs, diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index 135f581..612f3ee 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -6,6 +6,7 @@ load("//toolchains:toolchain.bzl", "android_lint_toolchain") load( ":android_lint_analysis_test.bzl", "android_lint_analysis_test_suite", + "application_lib_subject", "dependency_analysis_enabled_lint", ) @@ -14,6 +15,25 @@ filegroup( srcs = ["Fixture.java"], ) +application_lib_subject( + name = "application_fixture_lib", +) + +android_lint( + name = "analysis_library_fixture_lint", + srcs = ["Fixture.java"], + lib = ":fixture_lib", + tags = ["manual"], +) + +android_lint( + name = "analysis_test_sources_fixture_lint", + srcs = ["Fixture.java"], + is_test_sources = True, + lib = ":fixture_lib", + tags = ["manual"], +) + android_library( name = "android_dependency", custom_package = "tests.rules.android", @@ -66,7 +86,7 @@ android_lint( srcs = ["Fixture.java"], custom_rules = ["@rules_android_lint_test_deps//:com_slack_lint_compose_compose_lint_checks"], disable_checks = ["DefaultLocale"], - lib = ":fixture_lib", + lib = ":application_fixture_lib", tags = ["manual"], warnings_as_errors = True, deps = [ diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index 97c6016..7294e5f 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -1,6 +1,7 @@ """Analysis tests for android_lint action and provider wiring.""" load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +load("@rules_android//providers:providers.bzl", "ApkInfo") load("//rules:lint_analysis_aspect.bzl", "lint_analysis_aspect") load( "//rules:providers.bzl", @@ -23,6 +24,13 @@ def _argument_values(argv, flag): if argv[index] == flag ] +def _application_lib_subject_impl(_ctx): + return [ApkInfo()] + +application_lib_subject = rule( + implementation = _application_lib_subject_impl, +) + def _android_lint_action_impl(ctx): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) @@ -56,6 +64,8 @@ def _android_lint_action_impl(ctx): ) asserts.equals(env, "DefaultLocale", _argument_value(argv, "--disable-check")) asserts.true(env, "--warnings-as-errors" in argv) + asserts.false(env, "--library" in argv) + asserts.false(env, "--test-sources" in argv) asserts.equals(env, target[AndroidLintResultsInfo].output.path, output) asserts.false(env, "--baseline-file" in argv) asserts.false(env, "--regenerate-baseline-files" in argv) @@ -120,6 +130,38 @@ _android_lint_action_test = analysistest.make( }, ) +def _android_library_action_impl(ctx): + env = analysistest.begin(ctx) + actions = [ + action + for action in analysistest.target_actions(env) + if action.mnemonic == "AndroidLint" + ] + asserts.equals(env, 1, len(actions)) + if actions: + argv = actions[0].argv + asserts.true(env, "--library" in argv) + asserts.false(env, "--test-sources" in argv) + return analysistest.end(env) + +_android_library_action_test = analysistest.make(_android_library_action_impl) + +def _android_test_sources_action_impl(ctx): + env = analysistest.begin(ctx) + actions = [ + action + for action in analysistest.target_actions(env) + if action.mnemonic == "AndroidLint" + ] + asserts.equals(env, 1, len(actions)) + if actions: + argv = actions[0].argv + asserts.true(env, "--library" in argv) + asserts.true(env, "--test-sources" in argv) + return analysistest.end(env) + +_android_test_sources_action_test = analysistest.make(_android_test_sources_action_impl) + def _android_dependency_analysis_impl(ctx): env = analysistest.begin(ctx) target = analysistest.target_under_test(env) @@ -333,6 +375,16 @@ def android_lint_analysis_test_suite(name, additional_tests = []): name = action_test, target_under_test = ":analysis_fixture_lint", ) + library_action_test = name + "_library_action_test" + _android_library_action_test( + name = library_action_test, + target_under_test = ":analysis_library_fixture_lint", + ) + test_sources_action_test = name + "_test_sources_action_test" + _android_test_sources_action_test( + name = test_sources_action_test, + target_under_test = ":analysis_test_sources_fixture_lint", + ) android_dependency_subject = name + "_android_dependency_subject" _android_dependency_subject( name = android_dependency_subject, @@ -367,6 +419,8 @@ def android_lint_analysis_test_suite(name, additional_tests = []): ":" + action_test, ":" + android_dependency_disabled_test, ":" + android_dependency_test, + ":" + library_action_test, ":" + module_name_collision_test, + ":" + test_sources_action_test, ] + additional_tests, ) diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index 571dd7f..22146f3 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/tests/src/cli/AndroidLintActionArgsTest.kt @@ -94,6 +94,7 @@ class AndroidLintActionArgsTest { "report", "--android", "--library", + "--test-sources", "--partial-results", "path/to/partial", "--dependency-model", @@ -112,6 +113,7 @@ class AndroidLintActionArgsTest { assertThat(parseArgs.mode).isEqualTo("report") assertThat(parseArgs.isAndroid).isTrue() assertThat(parseArgs.isLibrary).isTrue() + assertThat(parseArgs.isTestSources).isTrue() assertThat(parseArgs.partialResults).isEqualTo(Paths.get("path/to/partial")) assertThat(parseArgs.dependencyModels).containsExactly( Paths.get("path/to/dep_a/model.json"), diff --git a/tests/src/cli/AndroidLintProjectTest.kt b/tests/src/cli/AndroidLintProjectTest.kt index 0b60525..83d0e69 100644 --- a/tests/src/cli/AndroidLintProjectTest.kt +++ b/tests/src/cli/AndroidLintProjectTest.kt @@ -57,7 +57,7 @@ class AndroidLintProjectTest { } @Test - fun `analyze mode preserves Android library identity without requiring a manifest`() { + fun `analyze mode preserves Android library and test-source identity`() { val partialResults = tmpDirectory.newFolder("partial").toPath() assertThat( createProjectXMLString( @@ -68,6 +68,7 @@ class AndroidLintProjectTest { androidManifest = null, isAndroid = true, isLibrary = true, + isTestSources = true, classpathJars = emptyList(), classpathAars = emptyList(), classpathExtractedAarDirectories = emptyList(), @@ -80,7 +81,7 @@ class AndroidLintProjectTest { - + diff --git a/tests/src/cli/AndroidLintRunnerTest.kt b/tests/src/cli/AndroidLintRunnerTest.kt index 1427ea6..66c5caf 100644 --- a/tests/src/cli/AndroidLintRunnerTest.kt +++ b/tests/src/cli/AndroidLintRunnerTest.kt @@ -77,6 +77,7 @@ class AndroidLintRunnerTest { "analyze", "--android", "--library", + "--test-sources", "--partial-results", partialResults.toString(), "--src", @@ -103,7 +104,7 @@ class AndroidLintRunnerTest { .contains( "", - ).contains("") + ).contains("") .contains("") .contains("") .doesNotContain("