diff --git a/rules/impl.bzl b/rules/impl.bzl index 03d6a97..1b2e13d 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -152,7 +152,7 @@ def _run_android_lint( inputs.append(aar_dir) # Report-phase-only arguments: baseline, reporting filters, output, and the dependency - # partial results merged into the final verdict. + # partial results produced with this android_lint target's check configuration. if is_report: if not regenerate and baseline: args.add("--baseline-file", baseline) @@ -165,6 +165,7 @@ def _run_android_lint( args.add("--autofix") for dependency in dependency_modules: args.add("--dependency-model", dependency.model) + args.add("--dependency-partial-results", dependency.partial_results.path) inputs.append(dependency.model) inputs.append(dependency.partial_results) inputs.extend(dependency.inputs) @@ -231,6 +232,52 @@ def _collect_dependency_modules(ctx): modules.append(node) return modules +def _analyze_dependency_modules(ctx, dependencies, common): + """Analyzes dependency modules with the consuming android_lint target's check settings.""" + configured = [] + for index, dependency in enumerate(dependencies): + partial_results = ctx.actions.declare_directory( + "%s_lint_dependency_partial_results/%d" % (ctx.label.name, index), + ) + dependency_common = dict(common) + dependency_common.update({ + "aars": list(dependency.classpath_aars), + "deps": list(dependency.classpath_jars), + "is_android": dependency.is_android, + "is_library": dependency.is_library, + "is_test_sources": False, + "manifest": dependency.manifest, + "module_name": dependency.module_name, + "resource_files": list(dependency.resource_files), + "srcs": list(dependency.srcs), + }) + _run_android_lint( + ctx, + mode = "analyze", + output = None, + partial_results = partial_results, + dependency_modules = [], + baseline = None, + warnings_as_errors = False, + autofix = False, + regenerate = False, + **dependency_common + ) + configured.append(struct( + classpath_aars = dependency.classpath_aars, + classpath_jars = dependency.classpath_jars, + inputs = dependency.inputs, + is_android = dependency.is_android, + is_library = dependency.is_library, + manifest = dependency.manifest, + model = dependency.model, + module_name = dependency.module_name, + partial_results = partial_results, + resource_files = dependency.resource_files, + srcs = dependency.srcs, + )) + return configured + def process_android_lint_issues(ctx, regenerate): """Runs Android Lint for the given target @@ -318,6 +365,14 @@ def process_android_lint_issues(ctx, regenerate): java_runtime_info = java_runtime_info, ) + dependency_modules = [] + if toolchain.android_lint_enable_check_dependencies: + dependency_modules = _analyze_dependency_modules( + ctx, + _collect_dependency_modules(ctx), + common, + ) + # 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( @@ -334,10 +389,6 @@ def process_android_lint_issues(ctx, regenerate): ) # 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, diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl index e0dce2d..e334681 100644 --- a/rules/lint_analysis_aspect.bzl +++ b/rules/lint_analysis_aspect.bzl @@ -163,6 +163,7 @@ def _lint_analysis_aspect_impl(target, ctx): # AAR dependencies (extracted), so lint loads their classes and embedded lint.jar checks. classpath_aars = [] + classpath_aar_nodes = [] if _AndroidLintAARInfo in target: aar_info = target[_AndroidLintAARInfo] direct_aar = aar_info.aar.aar @@ -179,6 +180,7 @@ def _lint_analysis_aspect_impl(target, ctx): inputs.append(node.aar_dir) module_inputs.append(node.aar) module_inputs.append(node.aar_dir) + classpath_aar_nodes.append(node) classpath_aars.append({ "aar": node.aar.path, "extracted": node.aar_dir.path, @@ -234,12 +236,17 @@ def _lint_analysis_aspect_impl(target, ctx): ) direct = struct( + classpath_aars = tuple(classpath_aar_nodes), + classpath_jars = tuple(classpath_jars), is_android = android_model.is_android, is_library = is_library, inputs = tuple(module_inputs), + manifest = android_model.manifest, model = module_model, module_name = module_name, partial_results = partial_results, + resource_files = tuple(android_model.resource_files.to_list()), + srcs = tuple(srcs), ) return [_AndroidLintPartialResultsInfo( is_android = android_model.is_android, diff --git a/rules/providers.bzl b/rules/providers.bzl index c3e459e..75a9a09 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -20,7 +20,8 @@ AndroidLintPartialResultsInfo = provider( "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, model, inputs, " + - "is_android, is_library) for this target and all analyzed " + + "is_android, is_library, srcs, resource_files, manifest, " + + "classpath_jars, classpath_aars) for this target and all analyzed " + "transitive dependencies.", }, ) diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 600b638..5c0c374 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -73,6 +73,14 @@ internal class AndroidLintActionArgs( transform = argsParserPathTransformer, ).default { emptyList() } + // Per-model partial results produced by dependency analysis configured for this lint target. + val dependencyPartialResults: List by parser + .adding( + names = arrayOf("--dependency-partial-results"), + help = "", + transform = argsParserPathTransformer, + ).default { emptyList() } + val androidHome: String? by parser .storing( names = arrayOf("--android-home"), diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index ac0febf..c00cdb5 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -71,7 +71,7 @@ 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 dependencyModules = dependencyModules(args) val projectFile = writeProjectXml( args = args, @@ -258,6 +258,22 @@ internal class AndroidLintRunner( return projectFile } + /** + * Loads dependency module descriptors and pairs them with the partial state produced by analysis + * actions owned by this lint target. The descriptor's original path belongs to the + * unparameterized aspect and is deliberately not reused. + */ + private fun dependencyModules(args: AndroidLintActionArgs): List { + require(args.dependencyModels.size == args.dependencyPartialResults.size) { + "Every --dependency-model requires a --dependency-partial-results" + } + return args.dependencyModels + .map(::readLintDependencyModule) + .zip(args.dependencyPartialResults) { dependency, partialResults -> + dependency.copy(partialResultsDir = partialResults) + } + } + /** * 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. diff --git a/tests/rules/BUILD.bazel b/tests/rules/BUILD.bazel index 612f3ee..f27bdbe 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -84,8 +84,10 @@ toolchain( android_lint( name = "analysis_fixture_lint", srcs = ["Fixture.java"], + android_lint_config = "fixtures/lint.xml", custom_rules = ["@rules_android_lint_test_deps//:com_slack_lint_compose_compose_lint_checks"], disable_checks = ["DefaultLocale"], + enable_checks = ["HardcodedText"], lib = ":application_fixture_lib", tags = ["manual"], warnings_as_errors = True, diff --git a/tests/rules/android_lint_analysis_test.bzl b/tests/rules/android_lint_analysis_test.bzl index 7294e5f..a5d344c 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -51,6 +51,7 @@ def _android_lint_action_impl(ctx): module_name = _argument_value(argv, "--label") output = _argument_value(argv, "--output") dependency_models = _argument_values(argv, "--dependency-model") + dependency_partial_results = _argument_values(argv, "--dependency-partial-results") classpath_aars = _argument_values(argv, "--classpath-aar") asserts.true(env, module_name != None and module_name.endswith("%3Aanalysis_fixture_lint")) @@ -63,6 +64,8 @@ def _android_lint_action_impl(ctx): custom_rule != None and "compose-lint-checks" in custom_rule and custom_rule.endswith(".jar"), ) asserts.equals(env, "DefaultLocale", _argument_value(argv, "--disable-check")) + asserts.equals(env, "HardcodedText", _argument_value(argv, "--enable-check")) + asserts.true(env, _argument_value(argv, "--config-file").endswith("/fixtures/lint.xml")) asserts.true(env, "--warnings-as-errors" in argv) asserts.false(env, "--library" in argv) asserts.false(env, "--test-sources" in argv) @@ -71,6 +74,7 @@ def _android_lint_action_impl(ctx): asserts.false(env, "--regenerate-baseline-files" in argv) asserts.equals(env, 5, len(dependency_models)) + asserts.equals(env, 5, len(dependency_partial_results)) asserts.true( env, any([model.endswith("/_lint/android_dependency/module_model.json") for model in dependency_models]), @@ -108,17 +112,44 @@ 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) - asserts.equals( - env, - 5, - len([path for path in input_paths if path.endswith("/partial_results")]), - ) + asserts.equals(env, 5, len([path for path in dependency_partial_results if "_lint_dependency_partial_results/" in path])) for model in dependency_models: asserts.true(env, model in input_paths) + for partial_results in dependency_partial_results: + asserts.true(env, partial_results in input_paths) asserts.equals(env, "1", action.execution_info.get("supports-workers")) asserts.equals(env, "1", action.execution_info.get("supports-multiplex-workers")) + analyze_actions = [ + action + for action in analysistest.target_actions(env) + if action.mnemonic == "AndroidLintAnalyze" + ] + asserts.equals(env, 6, len(analyze_actions)) + if analyze_actions: + dependency_actions = [ + action + for action in analyze_actions + if not _argument_value(action.argv, "--label").endswith("%3Aanalysis_fixture_lint") + ] + asserts.equals(env, 5, len(dependency_actions)) + for action in dependency_actions: + argv = action.argv + custom_rule = _argument_value(argv, "--custom-rule") + asserts.true( + env, + custom_rule != None and "compose-lint-checks" in custom_rule, + ) + asserts.equals(env, "DefaultLocale", _argument_value(argv, "--disable-check")) + asserts.equals(env, "HardcodedText", _argument_value(argv, "--enable-check")) + asserts.true(env, _argument_value(argv, "--config-file").endswith("/fixtures/lint.xml")) + asserts.equals(env, 1, len([ + output + for output in action.outputs.to_list() + if "_lint_dependency_partial_results/" in output.path + ])) + return analysistest.end(env) _android_lint_action_test = analysistest.make( diff --git a/tests/rules/fixtures/lint.xml b/tests/rules/fixtures/lint.xml new file mode 100644 index 0000000..ba73421 --- /dev/null +++ b/tests/rules/fixtures/lint.xml @@ -0,0 +1,2 @@ + + diff --git a/tests/src/cli/AndroidLintRunnerTest.kt b/tests/src/cli/AndroidLintRunnerTest.kt index 66c5caf..287d259 100644 --- a/tests/src/cli/AndroidLintRunnerTest.kt +++ b/tests/src/cli/AndroidLintRunnerTest.kt @@ -161,8 +161,12 @@ class AndroidLintRunnerTest { ownPartialResults.toString(), "--dependency-model", androidDependencyModel.toString(), + "--dependency-partial-results", + androidDependencyResults.toString(), "--dependency-model", javaDependencyModel.toString(), + "--dependency-partial-results", + javaDependencyResults.toString(), "--output", output.toString(), "--warnings-as-errors",