From 819821c53f0bc731064bff1ae6f25fbdc26b0f34 Mon Sep 17 00:00:00 2001 From: Ben Lee Date: Fri, 31 Jul 2026 22:59:13 -0400 Subject: [PATCH] Revert "Split into analysis and report (#128)" This reverts commit fd6e27ed0144901f83a9ccbd1861ce7ebe0ddec1. --- .github/workflows/ci.yaml | 4 +- rules/BUILD.bazel | 15 - rules/attrs.bzl | 6 +- rules/collect_aar_outputs_aspect.bzl | 13 +- rules/impl.bzl | 201 +++------- rules/lint_analysis_aspect.bzl | 270 ------------- rules/providers.bzl | 17 - rules/utils.bzl | 29 +- src/cli/AndroidLintActionArgs.kt | 56 +-- src/cli/AndroidLintProject.kt | 227 ++--------- src/cli/AndroidLintRunner.kt | 371 +++++------------- src/cli/BUILD | 1 - tests/integration/BUILD.bazel | 5 - tests/integration/lint_report_test.bzl | 4 - tests/integration/report_assertion.py | 19 +- tests/rules/BUILD.bazel | 121 +----- tests/rules/android_lint_analysis_test.bzl | 362 +---------------- tests/rules/fixtures/AndroidManifest.xml | 2 - tests/rules/fixtures/res/values/strings.xml | 3 - .../fixtures/runtime/res/layout/runtime.xml | 5 - tests/src/cli/AndroidLintActionArgsTest.kt | 45 --- tests/src/cli/AndroidLintProjectTest.kt | 94 +---- tests/src/cli/AndroidLintRunnerTest.kt | 245 ++---------- 23 files changed, 239 insertions(+), 1876 deletions(-) delete mode 100644 rules/lint_analysis_aspect.bzl delete mode 100644 tests/rules/fixtures/AndroidManifest.xml delete mode 100644 tests/rules/fixtures/res/values/strings.xml delete mode 100644 tests/rules/fixtures/runtime/res/layout/runtime.xml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c472ecd..e8baa13 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -36,7 +36,7 @@ jobs: - name: "Running tests //..." env: USE_BAZEL_VERSION: ${{ matrix.bazel_version }} - run: bazel test //... --strategy=AndroidLint=${{ matrix.strategy }} --strategy=AndroidLintAnalyze=${{ matrix.strategy }} + run: bazel test //... --strategy=AndroidLint=${{ matrix.strategy }} integration-tests: strategy: matrix: @@ -58,4 +58,4 @@ jobs: env: USE_BAZEL_VERSION: ${{ matrix.bazel_version }} working-directory: ${{ matrix.directory }} - run: bazel test //... --strategy=AndroidLint=${{ matrix.strategy }} --strategy=AndroidLintAnalyze=${{ matrix.strategy }} + run: bazel test //... --strategy=AndroidLint=${{ matrix.strategy }} diff --git a/rules/BUILD.bazel b/rules/BUILD.bazel index d0f9b30..dffdf38 100644 --- a/rules/BUILD.bazel +++ b/rules/BUILD.bazel @@ -62,7 +62,6 @@ bzl_library( visibility = ["//visibility:public"], deps = [ ":collect_aar_outputs_aspect", - ":lint_analysis_aspect", ], ) @@ -107,20 +106,6 @@ 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 e82da5f..3e84256 100644 --- a/rules/attrs.bzl +++ b/rules/attrs.bzl @@ -5,10 +5,6 @@ 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( @@ -42,7 +38,7 @@ ATTRS = dict( mandatory = False, allow_empty = True, default = [], - aspects = [_collect_aar_outputs_aspect, _lint_analysis_aspect], + aspects = [_collect_aar_outputs_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 ce978bf..1663945 100644 --- a/rules/collect_aar_outputs_aspect.bzl +++ b/rules/collect_aar_outputs_aspect.bzl @@ -3,8 +3,6 @@ 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 = { @@ -22,13 +20,13 @@ AndroidLintAARNodeInfo = provider( ) def _collect_aar_outputs_aspect(tgt, ctx): - deps = [] - for attr in ANDROID_LINT_DEPENDENCY_ATTRS: - deps.extend(getattr(ctx.rule.attr, attr, [])) + deps = getattr(ctx.rule.attr, "deps", []) + exports = getattr(ctx.rule.attr, "exports", []) + associates = getattr(ctx.rule.attr, "associates", []) # Collect the transitive aar artifacts for the given dependencies transitive_aar_depsets = [] - for dep in deps: + for dep in deps + exports + associates: if AndroidLintAARInfo in dep: transitive_aar_depsets.append(dep[AndroidLintAARInfo].transitive_aar_artifacts) @@ -75,6 +73,5 @@ def _collect_aar_outputs_aspect(tgt, ctx): collect_aar_outputs_aspect = aspect( implementation = _collect_aar_outputs_aspect, - attr_aspects = ["aar"] + ANDROID_LINT_DEPENDENCY_ATTRS, - provides = [AndroidLintAARInfo], + attr_aspects = ["aar", "deps", "exports", "associates"], ) diff --git a/rules/impl.bzl b/rules/impl.bzl index 03d6a97..41e71aa 100644 --- a/rules/impl.bzl +++ b/rules/impl.bzl @@ -4,8 +4,6 @@ load( "@rules_android//providers:providers.bzl", "AndroidLibraryResourceClassJarProvider", - "ApkInfo", - "StarlarkAndroidResourcesInfo", ) load("@rules_java//java:defs.bzl", "JavaInfo", "java_common") load( @@ -14,7 +12,6 @@ load( ) load( ":providers.bzl", - _AndroidLintPartialResultsInfo = "AndroidLintPartialResultsInfo", _AndroidLintResultsInfo = "AndroidLintResultsInfo", ) load( @@ -25,15 +22,9 @@ load( def _run_android_lint( ctx, - mode, android_lint, - is_android, - is_library, - is_test_sources, module_name, output, - partial_results, - dependency_modules, srcs, deps, aars, @@ -50,23 +41,17 @@ 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 an Android Lint action for the given phase. + """Constructs the Android Lint actions 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 - 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) - dependency_modules: List of structs(module_name, partial_results, model, inputs) for - first-party deps whose partial results should be merged (report mode only) + output: The output file srcs: The source files deps: Depset of aars and jars to include on the classpath aars: Depset of the aar nodes @@ -83,13 +68,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 = [] + outputs = [output] args = ctx.actions.args() args.set_param_file_format("multiline") @@ -97,22 +82,7 @@ def _run_android_lint( args.add("--android-lint-cli-tool", android_lint) inputs.append(android_lint) - args.add("--label", module_name) - 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) - if is_report: - inputs.append(partial_results) - else: - outputs.append(partial_results) - + args.add("--label", "{}".format(module_name)) if compile_sdk_version: args.add("--compile-sdk-version", compile_sdk_version) if java_language_level: @@ -128,12 +98,21 @@ 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: @@ -150,26 +129,12 @@ 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") - # 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-model", dependency.model) - inputs.append(dependency.model) - inputs.append(dependency.partial_results) - inputs.extend(dependency.inputs) - args.add("--output", output) - outputs.append(output) + # Declare the output file + 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) @@ -180,14 +145,11 @@ def _run_android_lint( inputs.extend(java_runtime_info.files.to_list()) ctx.actions.run( - mnemonic = "AndroidLintAnalyze" if mode == "analyze" else "AndroidLint", + mnemonic = "AndroidLint", inputs = inputs, outputs = outputs, executable = ctx.executable._lint_wrapper, - progress_message = "{} Android Lint {}".format( - "Analyzing" if mode == "analyze" else "Reporting", - str(ctx.label), - ), + progress_message = "Running Android Lint {}".format(str(ctx.label)), arguments = [args], tools = [ctx.executable._lint_wrapper], toolchain = _ANDROID_LINT_TOOLCHAIN_TYPE, @@ -201,43 +163,26 @@ def _run_android_lint( }, ) -def _collect_dependency_modules(ctx): - """Collects the transitive partial-results modules from the rule's dependencies. +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 deduplicated list of module model structs for every analyzed transitive dependency. + A string representing the module name """ - 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 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 + 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 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 @@ -250,7 +195,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".format(ctx.label.name)) + manifest = ctx.actions.declare_file("AndroidManifest.xml") ctx.actions.symlink(output = manifest, target_file = ctx.file.manifest) # Collect the transitive classpath jars to run lint against. @@ -284,72 +229,32 @@ def process_android_lint_issues(ctx, regenerate): _utils.list_or_depset_to_list(_utils.get_android_lint_toolchain(ctx).android_lint_config.files), ) - toolchain = _utils.get_android_lint_toolchain(ctx) - android_lint = _utils.only(_utils.list_or_depset_to_list(toolchain.android_lint.files)) - 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] - - common = dict( - android_lint = android_lint, - is_android = ( - StarlarkAndroidResourcesInfo in ctx.attr.lib or - 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, + 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, srcs = ctx.files.srcs, - deps = deps_depset, - aars = aars_depset, + deps = depset(transitive = deps), + aars = depset(transitive = aars), resource_files = ctx.files.resource_files, manifest = manifest, - compile_sdk_version = toolchain.compile_sdk_version, - java_language_level = toolchain.java_language_level, - kotlin_language_level = toolchain.kotlin_language_level, + 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), 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, - **common + 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], ) return struct( diff --git a/rules/lint_analysis_aspect.bzl b/rules/lint_analysis_aspect.bzl deleted file mode 100644 index e0dce2d..0000000 --- a/rules/lint_analysis_aspect.bzl +++ /dev/null @@ -1,270 +0,0 @@ -"""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", - "StarlarkAndroidResourcesInfo", -) -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", -) -load( - ":providers.bzl", - _AndroidLintPartialResultsInfo = "AndroidLintPartialResultsInfo", -) -load( - ":utils.bzl", - _ANDROID_LINT_TOOLCHAIN_TYPE = "ANDROID_LINT_TOOLCHAIN_TYPE", - _utils = "utils", -) - -def _aspect_deps(ctx): - deps = [] - for attr in _ANDROID_LINT_DEPENDENCY_ATTRS: - 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 _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): - 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. - srcs = getattr(ctx.rule.files, "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, - is_library = is_library, - manifest = android_model.manifest, - partial_results = None, - resource_files = android_model.resource_files, - module_name = None, - transitive_results = depset(transitive = transitive), - )] - - 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] - module_inputs = [] - module_inputs.extend(srcs) - module_inputs.extend(android_model.resource_files.to_list()) - if android_model.manifest: - module_inputs.append(android_model.manifest) - inputs.extend(module_inputs) - - 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 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: - 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) - 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. - classpath = [target[JavaInfo].transitive_compile_time_jars] - if AndroidLibraryResourceClassJarProvider in target: - classpath.append(target[AndroidLibraryResourceClassJarProvider].jars) - 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 - 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) - 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)) - args.add("--config-file", config) - inputs.append(config) - - 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) - 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( - 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, - ) - 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, - module_name = module_name, - transitive_results = depset(direct = [direct], transitive = transitive), - )] - -lint_analysis_aspect = aspect( - implementation = _lint_analysis_aspect_impl, - attr_aspects = _ANDROID_LINT_DEPENDENCY_ATTRS, - 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], - requires = [_collect_aar_outputs_aspect], - toolchains = ["//toolchains:toolchain_type"], -) diff --git a/rules/providers.bzl b/rules/providers.bzl index c3e459e..2992a84 100644 --- a/rules/providers.bzl +++ b/rules/providers.bzl @@ -7,20 +7,3 @@ 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 = { - "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, model, inputs, " + - "is_android, is_library) for this target and all analyzed " + - "transitive dependencies.", - }, -) diff --git a/rules/utils.bzl b/rules/utils.bzl index 06f8a07..730e201 100644 --- a/rules/utils.bzl +++ b/rules/utils.bzl @@ -26,34 +26,9 @@ 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, - get_android_lint_toolchain = _get_android_lint_toolchain, - list_or_depset_to_list = _list_or_depset_to_list, - module_name = _module_name, only = _only, + list_or_depset_to_list = _list_or_depset_to_list, + get_android_lint_toolchain = _get_android_lint_toolchain, ) diff --git a/src/cli/AndroidLintActionArgs.kt b/src/cli/AndroidLintActionArgs.kt index 600b638..574f1bc 100644 --- a/src/cli/AndroidLintActionArgs.kt +++ b/src/cli/AndroidLintActionArgs.kt @@ -29,50 +29,6 @@ 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" } - - val isAndroid: Boolean by parser - .flagging( - names = arrayOf("--android"), - 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 } - - 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 - .storing( - names = arrayOf("--partial-results"), - help = "", - transform = argsParserPathTransformer, - ).default { null } - - // Serialized descriptions of first-party dependency modules consumed in report mode. - val dependencyModels: List by parser - .adding( - names = arrayOf("--dependency-model"), - help = "", - transform = argsParserPathTransformer, - ).default { emptyList() } - val androidHome: String? by parser .storing( names = arrayOf("--android-home"), @@ -92,13 +48,11 @@ internal class AndroidLintActionArgs( 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 output: Path by parser.storing( + names = arrayOf("--output"), + help = "", + transform = argsParserPathTransformer, + ) val resources: List by parser .adding( diff --git a/src/cli/AndroidLintProject.kt b/src/cli/AndroidLintProject.kt index d9d9c70..eb1a239 100644 --- a/src/cli/AndroidLintProject.kt +++ b/src/cli/AndroidLintProject.kt @@ -1,10 +1,7 @@ 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 @@ -13,94 +10,16 @@ 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 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, srcs: List, resources: List, androidManifest: Path?, - isAndroid: Boolean = androidManifest != null, - isLibrary: Boolean = false, - isTestSources: Boolean = false, classpathJars: List, 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() @@ -115,13 +34,8 @@ internal fun createProjectXMLString( val moduleElement = 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("android", if (androidManifest != null) "true" else "false") + // it.setAttribute("library", "false") // it.setAttribute("compile-sdk-version", "get-actual-value-here") projectElement.appendChild(it) } @@ -133,128 +47,49 @@ internal fun createProjectXMLString( } } - appendModuleContents( - document = document, - moduleElement = moduleElement, - srcs = srcs, - isTestSources = isTestSources, - 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. - dependencyModules.forEach { dependency -> - document.createElement("dep").also { - it.setAttribute("module", dependency.name) - moduleElement.appendChild(it) - } - } - - dependencyModules.forEach { dependency -> - 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, - isTestSources = false, - resources = dependency.resources, - androidManifest = dependency.androidManifest, - classpathJars = dependency.classpathJars, - classpathAars = dependency.classpathAars, - classpathExtractedAarDirectories = dependency.classpathExtractedAarDirectories, - aarPartialResultsScratchDir = aarPartialResultsScratchDir, - ) - } - - return StringWriter() - .apply { - val transformer = TransformerFactory.newInstance().newTransformer() - transformer.setOutputProperty(OutputKeys.INDENT, "yes") - transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2") - transformer.transform(DOMSource(document), StreamResult(this)) - }.buffer - .toString() -} - -private fun appendModuleContents( - document: org.w3c.dom.Document, - moduleElement: org.w3c.dom.Element, - srcs: List, - isTestSources: Boolean, - resources: List, - androidManifest: Path?, - classpathJars: List, - classpathAars: List, - classpathExtractedAarDirectories: List>, - aarPartialResultsScratchDir: Path?, -) { srcs.forEach { src -> - document.createElement("src").also { - it.setAttribute("file", src.pathString) - if (isTestSources) { - it.setAttribute("test", "true") - } - moduleElement.appendChild(it) - } + val element = document.createElement("src") + element.setAttribute("file", src.pathString) + moduleElement.appendChild(element) } - resources.forEach { resource -> - document.createElement("resource").also { - it.setAttribute("file", resource.pathString) - moduleElement.appendChild(it) - } + resources.forEach { res -> + val element = document.createElement("resource") + element.setAttribute("file", res.pathString) + moduleElement.appendChild(element) } if (androidManifest != null) { - document.createElement("manifest").also { - it.setAttribute("file", androidManifest.pathString) - moduleElement.appendChild(it) - } + val element = document.createElement("manifest") + element.setAttribute("file", androidManifest.pathString) + moduleElement.appendChild(element) } classpathJars.forEach { jar -> - document.createElement("classpath").also { - it.setAttribute("jar", jar.absolutePathString()) - moduleElement.appendChild(it) - } + val element = document.createElement("classpath") + element.setAttribute("jar", jar.absolutePathString()) + moduleElement.appendChild(element) } 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) - } + val element = document.createElement("aar") + element.setAttribute("file", aar.absolutePathString()) + moduleElement.appendChild(element) } 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) - } + val element = document.createElement("aar") + element.setAttribute("file", aar.absolutePathString()) + element.setAttribute("extracted", unzippedDir.absolutePathString()) + moduleElement.appendChild(element) } + + return StringWriter() + .apply { + val transformer = TransformerFactory.newInstance().newTransformer() + transformer.setOutputProperty(OutputKeys.INDENT, "yes") + transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2") + transformer.transform(DOMSource(document), StreamResult(this)) + }.buffer + .toString() } diff --git a/src/cli/AndroidLintRunner.kt b/src/cli/AndroidLintRunner.kt index ac0febf..4b5658f 100644 --- a/src/cli/AndroidLintRunner.kt +++ b/src/cli/AndroidLintRunner.kt @@ -15,227 +15,26 @@ 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 { - 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 dependencyModules = args.dependencyModels.map(::readLintDependencyModule) - val projectFile = - writeProjectXml( - args = args, - workingDirectory = workingDirectory, - rootDir = rootDir, - partialResultsDir = partialResults, - dependencyModules = dependencyModules, - ) - - 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). The full module metadata remains available to detectors during reporting. - return reportExitCode( - invoke( - args, - lintArgs, - workingDirectory, - enableCheckDependencies = dependencyModules.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, - 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") - - // 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") + // 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) } - } - private fun writeProjectXml( - args: AndroidLintActionArgs, - workingDirectory: Path, - rootDir: String, - partialResultsDir: Path?, - dependencyModules: List, - ): Path { + // Collect the custom lint rules from the unpacked aars val aarLintRuleJars = - collectAarLintRuleJars( - args.classpathAarPairs + - dependencyModules.flatMap(LintDependencyModule::classpathExtractedAarDirectories), - ) + args.classpathAarPairs + .asSequence() + .map { it.second.resolve("lint.jar") } + .filter { it.exists() && it.isRegularFile() } + + // Create the project configuration file for lint val projectFile = workingDirectory.resolve("${args.label}_project_config.xml") Files.createFile(projectFile) - // 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 - } - + val rootDir = System.getenv("PWD") projectFile.writeText( createProjectXMLString( moduleName = args.label, @@ -243,74 +42,112 @@ internal class AndroidLintRunner( srcs = args.srcs.sortedDescending(), resources = args.resources.sortedDescending(), androidManifest = args.androidManifest, - isAndroid = args.isAndroid || args.androidManifest != null, - isLibrary = args.isLibrary, - isTestSources = args.isTestSources, classpathJars = args.classpath.sortedDescending(), classpathAars = emptyList(), classpathExtractedAarDirectories = args.classpathAarPairs, - customLintChecks = (args.customChecks + aarLintRuleJars).distinct().sortedDescending(), - partialResultsDir = partialResultsDir, - dependencyModules = dependencyModules, - aarPartialResultsScratchDir = aarPartialResultsScratchDir, + customLintChecks = (args.customChecks + aarLintRuleJars).sortedDescending(), ), ) - return projectFile - } - /** - * 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 { + // Run Android Lint + val androidCacheFolder = workingDirectory.resolve("android-cache") + Files.createDirectory(androidCacheFolder) val lintUserHomeFolder = workingDirectory.resolve("lint-user-home") - Files.createDirectories(lintUserHomeFolder) + Files.createDirectory(lintUserHomeFolder) val invoker = invokerCache.acquire(listOf(args.androidLintCliTool)) - return try { - System.setProperty("user.home", lintUserHomeFolder.toString()) - invoker.invoke( - args = lintArgs.toTypedArray(), - enableCheckDependencies = enableCheckDependencies, - ) - } finally { - invokerCache.release(invoker) - } - } + 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) + } - private fun reportExitCode(exitCode: Int): Int = - when (exitCode) { + return when (exitCode) { AndroidLintCliInvoker.ERRNO_SUCCESS, AndroidLintCliInvoker.ERRNO_CREATED_BASELINE, -> 0 else -> exitCode } + } - private fun rootDir(): String = System.getenv("PWD") ?: Paths.get("").toAbsolutePath().pathString -} + 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) + } -/** - * 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() } + 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, + ) + } +} diff --git a/src/cli/BUILD b/src/cli/BUILD index 82a035f..3daaa2c 100644 --- a/src/cli/BUILD +++ b/src/cli/BUILD @@ -16,7 +16,6 @@ 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/integration/BUILD.bazel b/tests/integration/BUILD.bazel index 8676016..9282c5b 100644 --- a/tests/integration/BUILD.bazel +++ b/tests/integration/BUILD.bazel @@ -3,11 +3,6 @@ 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/integration/lint_report_test.bzl b/tests/integration/lint_report_test.bzl index 25f4bb3..6b06b5b 100644 --- a/tests/integration/lint_report_test.bzl +++ b/tests/integration/lint_report_test.bzl @@ -8,7 +8,6 @@ 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. @@ -17,7 +16,6 @@ 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. """ @@ -27,8 +25,6 @@ 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 d3c47e3..68c7e32 100644 --- a/tests/integration/report_assertion.py +++ b/tests/integration/report_assertion.py @@ -9,7 +9,6 @@ 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() @@ -29,31 +28,15 @@ 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 missing_locations or unexpected: + if missing 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 612f3ee..e7d7b71 100644 --- a/tests/rules/BUILD.bazel +++ b/tests/rules/BUILD.bazel @@ -1,134 +1,19 @@ -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", - "application_lib_subject", - "dependency_analysis_enabled_lint", -) +load(":android_lint_analysis_test.bzl", "android_lint_analysis_test_suite") filegroup( name = "fixture_lib", 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", - manifest = "fixtures/AndroidManifest.xml", - resource_files = ["fixtures/res/values/strings.xml"], - 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"], - tags = ["manual"], -) - -java_library( - name = "collision.dep", - srcs = ["Fixture.java"], - 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", - 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"], custom_rules = ["@rules_android_lint_test_deps//:com_slack_lint_compose_compose_lint_checks"], disable_checks = ["DefaultLocale"], - lib = ":application_fixture_lib", + lib = ":fixture_lib", tags = ["manual"], warnings_as_errors = True, - deps = [ - ":android_dependency", - ":collision.dep", - ":collision-dep", - ":runtime_parent", - ], -) - -dependency_analysis_enabled_lint( - name = "analysis_fixture_lint_dependency_analysis_enabled", - lint = ":analysis_fixture_lint", ) -lint_report_test( - name = "dependency_analysis_disabled_report_test", - lint = ":analysis_fixture_lint", - rejected_issues = [ - "HardcodedText", - "UnusedResources", - ], -) - -lint_report_test( - 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_analysis_disabled_report_test", - ":dependency_analysis_enabled_report_test", - ], -) +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 7294e5f..ab85e33 100644 --- a/tests/rules/android_lint_analysis_test.bzl +++ b/tests/rules/android_lint_analysis_test.bzl @@ -1,13 +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", - "AndroidLintPartialResultsInfo", - "AndroidLintResultsInfo", -) +load("//rules:providers.bzl", "AndroidLintResultsInfo") def _argument_value(argv, flag): for index in range(len(argv)): @@ -17,20 +11,6 @@ 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 _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) @@ -48,15 +28,8 @@ 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_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")) - 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, @@ -64,363 +37,38 @@ 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) - asserts.equals(env, 5, len(dependency_models)) - asserts.true( - env, - any([model.endswith("/_lint/android_dependency/module_model.json") for model in dependency_models]), - ) - asserts.true( - env, - any([model.endswith("/_lint/collision-dep/module_model.json") for model in dependency_models]), - ) - asserts.true( - env, - any([model.endswith("/_lint/collision.dep/module_model.json") for model in dependency_models]), - ) - asserts.true( - env, - any([model.endswith("/_lint/runtime_android_dependency/module_model.json") for model in dependency_models]), - ) - asserts.true( - env, - any([model.endswith("/_lint/runtime_parent/module_model.json") for model in dependency_models]), - ) - 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] - input_paths = [file.path for file in action_inputs] + inputs = [file.basename for file in action.inputs.to_list()] 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) - 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")) return analysistest.end(env) -_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_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) +_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, _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.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()], - ) - 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)) - 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") - 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")) - 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, - 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.", - 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): - return [ctx.attr.dep[_AndroidDependencyAnalysisInfo]] - -_android_dependency_subject = rule( - implementation = _android_dependency_subject_impl, - attrs = { - "dep": attr.label( - aspects = [_android_dependency_analysis_aspect], - providers = [_AndroidDependencyAnalysisInfo], - ), - }, -) - -_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, - config_settings = { - "//command_line_option:extra_toolchains": [ - "//tests/rules:dependency_analysis_enabled_toolchain", - ], - }, -) - -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): - # An attribute transition exposes a singleton list even for attr.label. - info = ctx.attr.lint[0][AndroidLintResultsInfo] - return [ - info, - DefaultInfo(files = depset([info.output])), - ] - -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( - 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 = []): +def android_lint_analysis_test_suite(name): """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( 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, - dep = ":android_dependency", - ) - android_dependency_test = name + "_android_dependency_test" - _android_dependency_analysis_test( - 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, - 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_disabled_test, - ":" + android_dependency_test, - ":" + library_action_test, - ":" + module_name_collision_test, - ":" + test_sources_action_test, - ] + additional_tests, + tests = [":" + action_test], ) diff --git a/tests/rules/fixtures/AndroidManifest.xml b/tests/rules/fixtures/AndroidManifest.xml deleted file mode 100644 index 6ec267f..0000000 --- a/tests/rules/fixtures/AndroidManifest.xml +++ /dev/null @@ -1,2 +0,0 @@ - diff --git a/tests/rules/fixtures/res/values/strings.xml b/tests/rules/fixtures/res/values/strings.xml deleted file mode 100644 index 9fa37d4..0000000 --- a/tests/rules/fixtures/res/values/strings.xml +++ /dev/null @@ -1,3 +0,0 @@ - - Android dependency - diff --git a/tests/rules/fixtures/runtime/res/layout/runtime.xml b/tests/rules/fixtures/runtime/res/layout/runtime.xml deleted file mode 100644 index 95df896..0000000 --- a/tests/rules/fixtures/runtime/res/layout/runtime.xml +++ /dev/null @@ -1,5 +0,0 @@ - - diff --git a/tests/src/cli/AndroidLintActionArgsTest.kt b/tests/src/cli/AndroidLintActionArgsTest.kt index 22146f3..6023a52 100644 --- a/tests/src/cli/AndroidLintActionArgsTest.kt +++ b/tests/src/cli/AndroidLintActionArgsTest.kt @@ -76,50 +76,5 @@ 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", - "--android", - "--library", - "--test-sources", - "--partial-results", - "path/to/partial", - "--dependency-model", - "path/to/dep_a/model.json", - "--dependency-model", - "path/to/dep_b/model.json", - "--compile-sdk-version", - "34", - "--java-language-level", - "17", - "--kotlin-language-level", - "1.9", - ), - ) - - 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"), - Paths.get("path/to/dep_b/model.json"), - ) - // 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 83d0e69..824c4c9 100644 --- a/tests/src/cli/AndroidLintProjectTest.kt +++ b/tests/src/cli/AndroidLintProjectTest.kt @@ -41,7 +41,7 @@ class AndroidLintProjectTest { - + @@ -55,96 +55,4 @@ class AndroidLintProjectTest { """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), ) } - - @Test - fun `analyze mode preserves Android library and test-source identity`() { - 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, - isAndroid = true, - isLibrary = true, - isTestSources = true, - classpathJars = emptyList(), - classpathAars = emptyList(), - classpathExtractedAarDirectories = emptyList(), - customLintChecks = emptyList(), - partialResultsDir = partialResults, - ), - ).isEqualTo( - """ - - - - - - - - - """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), - ) - } - - @Test - 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", - 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, - isAndroid = true, - isLibrary = true, - srcs = listOf(depSource), - resources = listOf(depResource), - androidManifest = depManifest, - classpathJars = listOf(depClasspath), - classpathExtractedAarDirectories = listOf(depAar to depExtractedAar), - ), - ), - ), - ).isEqualTo( - """ - - - - - - - - - - - - - - - - - """.trimIndent().replace("{root}", tmpDirectory.root.absolutePath), - ) - } } diff --git a/tests/src/cli/AndroidLintRunnerTest.kt b/tests/src/cli/AndroidLintRunnerTest.kt index 66c5caf..3c50752 100644 --- a/tests/src/cli/AndroidLintRunnerTest.kt +++ b/tests/src/cli/AndroidLintRunnerTest.kt @@ -36,22 +36,38 @@ class AndroidLintRunnerTest { val workingDirectory = temporaryFolder.newFolder("working").toPath() val args = AndroidLintActionArgs.parseArgs( - runnerArgs(lintCli, "runner-test") + - listOf( - "--output", - output.toString(), - "--custom-rule", - customRule.toString(), - "--classpath-aar", - "$aar:$aarDirectory", - ), + 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", + ), ) val exitCode = AndroidLintRunner(AndroidLintCliInvokerCache(parentClassloader = javaClass.classLoader)) .runAndroidLint(args, workingDirectory) - val projectXml = projectXml(Main.recordedRuns.single()) + val projectFile = + Paths.get( + Main.recordedRuns + .single() + .args + .argumentAfter("--project"), + ) + val projectXml = Files.readString(projectFile) assertThat(exitCode).isEqualTo(0) assertThat(projectXml) .contains("") @@ -61,173 +77,6 @@ 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", - "--test-sources", - "--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("") - .contains("") - .contains("") - .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 -> @@ -237,50 +86,8 @@ 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" }