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/AndroidLintCliInvoker.kt b/src/cli/AndroidLintCliInvoker.kt index 2d2ceaf..642bb5a 100644 --- a/src/cli/AndroidLintCliInvoker.kt +++ b/src/cli/AndroidLintCliInvoker.kt @@ -85,7 +85,12 @@ class AndroidLintCliInvoker( const val ERRNO_INTERNAL_CONTINUE = 100 fun createUsingJars( - parentClassloader: ClassLoader = this::class.java.classLoader, + // Parent on the platform classloader (JDK classes only) so lint loads its entire runtime — + // crucially kotlin-stdlib AND kotlin-reflect — from its own deploy jar. Parenting on the + // worker's classloader instead lets the worker's kotlin-stdlib shadow lint's, and since the + // worker has no kotlin-reflect, Kotlin reflection inside lint detectors (e.g. the partial + // analysis "fx" checks) silently fails. This mirrors AGP's AndroidLintWorkAction. + parentClassloader: ClassLoader = ClassLoader.getPlatformClassLoader(), vararg jars: Path, ): AndroidLintCliInvoker { require(jars.isNotEmpty()) { diff --git a/src/cli/AndroidLintCliInvokerCache.kt b/src/cli/AndroidLintCliInvokerCache.kt index e5cda61..8f3ea7a 100644 --- a/src/cli/AndroidLintCliInvokerCache.kt +++ b/src/cli/AndroidLintCliInvokerCache.kt @@ -15,7 +15,12 @@ import java.nio.file.Path * while a request against the old jar is still running, so the old classloader is only disposed * once its last lease is released. */ -internal class AndroidLintCliInvokerCache { +internal class AndroidLintCliInvokerCache( + // The classloader lint's deploy jar is parented on. Defaults to the platform classloader so lint + // is fully isolated from the worker's classpath. Tests override this to resolve stub lint classes + // from the test classpath. + private val parentClassloader: ClassLoader = ClassLoader.getPlatformClassLoader(), +) { private class Entry( val key: String, val invoker: AndroidLintCliInvoker, @@ -44,7 +49,11 @@ internal class AndroidLintCliInvokerCache { } current = null } - val invoker = AndroidLintCliInvoker.createUsingJars(jars = jars.toTypedArray()) + val invoker = + AndroidLintCliInvoker.createUsingJars( + parentClassloader = parentClassloader, + jars = jars.toTypedArray(), + ) createdCount += 1 current = Entry(key = key, invoker = invoker, activeLeases = 1) return invoker 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 31d7b7f..e3c7daf 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -15,26 +15,217 @@ 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)) { + 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) + } - // 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() } + // 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, + 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) + } + + return reportExitCode( + invoke(args, lintArgs, 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") + + // 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(",")) + } - // Create the project configuration file for lint + 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,104 +237,62 @@ 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) - val invoker = invokerCache.acquire(listOf(args.androidLintCliTool)) - val exitCode = - try { - invokeAndroidLintCLI( - invoker = invoker, - actionArgs = args, - rootDirPath = rootDir, - projectFilePath = projectFile, - baselineFilePath = baselineFile, - cacheDirectoryPath = androidCacheFolder, - ) - } finally { - invokerCache.release(invoker) - } - - return when (exitCode) { - AndroidLintCliInvoker.ERRNO_SUCCESS, - AndroidLintCliInvoker.ERRNO_CREATED_BASELINE, - -> 0 - - else -> exitCode + /** + * 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 invokeAndroidLintCLI( - invoker: AndroidLintCliInvoker, - actionArgs: AndroidLintActionArgs, - rootDirPath: String, - projectFilePath: Path, - baselineFilePath: Path, - cacheDirectoryPath: Path, + private fun invoke( + args: AndroidLintActionArgs, + lintArgs: List, + enableCheckDependencies: Boolean = false, ): 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", + val invoker = invokerCache.acquire(listOf(args.androidLintCliTool)) + return try { + invoker.invoke( + args = lintArgs.toTypedArray(), + enableCheckDependencies = enableCheckDependencies, ) - 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(",")) + } finally { + invokerCache.release(invoker) } + } - if (actionArgs.androidHome?.isNotEmpty() != null) { - var androidHomePath = - Paths.get(System.getenv("PWD"), actionArgs.androidHome).absolutePathString() - args.add("--sdk-home") - args.add(androidHomePath) - } + private fun reportExitCode(exitCode: Int): Int = + when (exitCode) { + AndroidLintCliInvoker.ERRNO_SUCCESS, + AndroidLintCliInvoker.ERRNO_CREATED_BASELINE, + -> 0 - if (actionArgs.jdkHome != null) { - val jdkHome = actionArgs.jdkHome!! - args.add("--jdk-home") - args.add(jdkHome.absolutePathString()) + else -> exitCode } - 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/scripts/android_lint_scenarios_test.sh b/tests/scripts/android_lint_scenarios_test.sh index a31649a..f5a1a9a 100755 --- a/tests/scripts/android_lint_scenarios_test.sh +++ b/tests/scripts/android_lint_scenarios_test.sh @@ -137,4 +137,25 @@ function test_custom_rules_jar_accepts_clean_composable() { || fail "Expected lint test with custom compose-lints rules to pass" } +function test_check_dependencies_surfaces_dependency_issue() { + enable_check_dependencies + write_dependency_fanin_targets + + # The leaf's own sources are clean, but its dependency trips DefaultLocale. With check + # dependencies enabled the report phase merges the dependency's partial results, so the + # issue surfaces and the lint test fails. + "${BIT_BAZEL_BINARY}" test --test_output=all //:leaf_lint_test >& "$TEST_log" \ + && fail "Expected dependency issue to surface with check dependencies enabled" || true + expect_log "DefaultLocale" +} + +function test_without_check_dependencies_dependency_issue_is_not_reported() { + # Default toolchain: check dependencies disabled. The dependency is only on the classpath as + # compiled bytecode and is never analyzed, so its source issue does not reach the report. + write_dependency_fanin_targets + + "${BIT_BAZEL_BINARY}" test //:leaf_lint_test >& "$TEST_log" \ + || fail "Expected leaf lint test to pass when check dependencies is disabled" +} + run_suite "android_lint rule behavior scenarios" diff --git a/tests/scripts/lint_helper.sh b/tests/scripts/lint_helper.sh index e991ec7..5c38a2f 100755 --- a/tests/scripts/lint_helper.sh +++ b/tests/scripts/lint_helper.sh @@ -95,6 +95,9 @@ local_path_override( module_name = "rules_android_lint", path = "${dest}", ) + +# Available to consumer BUILD files that declare first-party java_library dependencies. +bazel_dep(name = "rules_java", version = "9.3.0") EOF cat > .bazelrc < "${dest}/toolchains/BUILD.bazel" < Dep.java < Leaf.java < BUILD.bazel < + + + + + + + + """.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), + ) + } }