Merge pull request #157 from grab/collect-custom-lint-jars-from-exter…

…nal-libraries

collect custom lint jars from external libraries

rules/android/android_binary.bzl

@@ -81,6 +81,7 @@ def android_binary(
             manifest = attrs.get("manifest"),
             baseline = lint_baseline,
             lint_config = lint_options.get("config", None),
+            deps = kotlin_library_deps,
         )
 
         # Build deps

rules/android/android_library.bzl

@@ -65,6 +65,7 @@ def android_library(
             manifest = attrs.get("manifest"),
             baseline = lint_baseline,
             lint_config = lint_options.get("config", None),
+            deps = android_library_deps,
         )
         android_library_deps = android_library_deps + [lint_sources_target]
         lint(

rules/android/lint/collect_aar_aspect.bzl

@@ -0,0 +1,50 @@
+load("@grab_bazel_common//rules/android/lint:providers.bzl", "AarInfo", "AarNodeInfo")
+load("@grab_bazel_common//rules/android:utils.bzl", "utils")
+
+def _collect_aar_aspect(_, ctx):
+    transitive_aar_node_infos = depset(
+        transitive = [
+            aar_node_info.transitive
+            for aar_node_info in utils.collect_providers(
+                AarNodeInfo,
+                getattr(ctx.rule.attr, "deps", []),
+                getattr(ctx.rule.attr, "exports", []),
+                getattr(ctx.rule.attr, "associates", []),
+            )
+        ],
+    )
+    current_info = AarNodeInfo(
+        aar = None,
+        aar_dir = None,
+    )
+
+    if hasattr(ctx.rule.attr, "aar"):  # Check for rule name and only extract if it is aar_import
+        aar = ctx.rule.attr.aar.files.to_list()[0]
+        aar_extract = ctx.actions.declare_directory(ctx.label.name + "/lint/_extracted_aar")
+
+        ctx.actions.run_shell(
+            inputs = [aar],
+            outputs = [aar_extract],
+            mnemonic = "ExtractLintAar",
+            progress_message = "Extracting %s's " % (ctx.label.name),
+            command = ("unzip -q -o %s -d %s/ " % (aar.path, aar_extract.path)),
+        )
+        current_info = AarNodeInfo(
+            aar = aar,
+            aar_dir = aar_extract,
+        )
+
+    return [
+        AarInfo(
+            self = current_info,
+            transitive = depset(
+                [current_info],
+                transitive = [depset(transitive = [transitive_aar_node_infos])],
+            ),
+        ),
+    ]
+
+collect_aar_aspect = aspect(
+    implementation = _collect_aar_aspect,
+    attr_aspects = ["aar", "deps", "exports", "associates"],
+)

rules/android/lint/lint_aspect.bzl

@@ -1,6 +1,8 @@
 load("@grab_bazel_common//rules/android:utils.bzl", "utils")
 load(
     "@grab_bazel_common//rules/android/lint:providers.bzl",
+    "AarInfo",
+    "AarNodeInfo",
     "AndroidLintInfo",
     "AndroidLintNodeInfo",
     "AndroidLintSourcesInfo",
@@ -50,6 +52,7 @@ def _collect_sources(target, ctx, library):
         struct(
             srcs = dep[AndroidLintSourcesInfo].srcs,
             resources = dep[AndroidLintSourcesInfo].resources,
+            aars = dep[AndroidLintSourcesInfo].aar_deps,
             manifest = dep[AndroidLintSourcesInfo].manifest,
             merged_manifest = merged_manifest,
             baseline = dep[AndroidLintSourcesInfo].baseline,
@@ -105,6 +108,15 @@ def _encode_dependency(dependency_info):
         dependency_info.models_dir.path,
     )
 
+def _encode_aars(aar_info):
+    """
+    Flatten the dependency details in a string to simplify arguments to Lint ClI
+    """
+    return "%s^%s" % (
+        aar_info.aar.path,
+        aar_info.aar_dir.path,
+    )
+
 def _lint_common_args(
         ctx,
         args,
@@ -113,6 +125,8 @@ def _lint_common_args(
         compile_sdk_version,
         srcs,
         resources,
+        aars,
+        aar_infos,
         classpath,
         manifest,
         merged_manifest,
@@ -141,6 +155,12 @@ def _lint_common_args(
         join_with = ",",
         map_each = utils.to_path,
     )
+    args.add_joined(
+        "--aar_dirs",
+        aar_infos,
+        join_with = ",",
+        map_each = _encode_aars,
+    )
     args.add_joined(
         "--resource-files",
         resources,
@@ -188,6 +208,8 @@ def _lint_analyze_action(
         compile_sdk_version,
         srcs,
         resources,
+        aars,
+        aar_infos,
         classpath,
         manifest,
         merged_manifest,
@@ -211,6 +233,8 @@ def _lint_analyze_action(
         compile_sdk_version = compile_sdk_version,
         srcs = srcs,
         resources = resources,
+        aars = aars,
+        aar_infos = aar_infos,
         classpath = classpath,
         manifest = manifest,
         merged_manifest = merged_manifest,
@@ -250,6 +274,8 @@ def _lint_report_action(
         compile_sdk_version,
         srcs,
         resources,
+        aars,
+        aar_infos,
         classpath,
         manifest,
         merged_manifest,
@@ -276,6 +302,8 @@ def _lint_report_action(
         compile_sdk_version = compile_sdk_version,
         srcs = srcs,
         resources = resources,
+        aars = aars,
+        aar_infos = aar_infos,
         classpath = classpath,
         manifest = manifest,
         merged_manifest = merged_manifest,
@@ -313,6 +341,13 @@ def _lint_report_action(
     )
     return
 
+def _aar_node_infos(aar_deps):
+    return [
+        aar_node_info
+        for aar_node_info in aar_deps.to_list()
+        if aar_node_info.aar != None
+    ]
+
 def _lint_aspect_impl(target, ctx):
     if target.label.workspace_root.startswith("external"):
         # Run lint only on internal targets
@@ -355,6 +390,10 @@ def _lint_aspect_impl(target, ctx):
             dep_partial_results = [info.partial_results_dir for info in dep_lint_node_infos]
             dep_lint_models = [info.models_dir for info in dep_lint_node_infos]
 
+            aar_node_infos = _aar_node_infos(sources.aars)
+            aars = [info.aar for info in aar_node_infos]
+            aars_dir = [info.aar_dir for info in aar_node_infos]
+
             # Inputs
             baseline_inputs = []
             if sources.baseline:
@@ -382,6 +421,8 @@ def _lint_aspect_impl(target, ctx):
                 compile_sdk_version = compile_sdk_version,
                 srcs = sources.srcs,
                 resources = sources.resources,
+                aars = aars,
+                aar_infos = aar_node_infos,
                 classpath = sources.classpath,
                 manifest = sources.manifest,
                 merged_manifest = sources.merged_manifest,
@@ -396,6 +437,8 @@ def _lint_aspect_impl(target, ctx):
                 inputs = depset(
                     sources.srcs +
                     sources.resources +
+                    aars +
+                    aars_dir +
                     sources.manifest +
                     sources.merged_manifest +
                     [sources.lint_config_xml] +
@@ -418,6 +461,8 @@ def _lint_aspect_impl(target, ctx):
                 compile_sdk_version = compile_sdk_version,
                 srcs = sources.srcs,
                 resources = sources.resources,
+                aars = aars,
+                aar_infos = aar_node_infos,
                 classpath = sources.classpath,
                 manifest = sources.manifest,
                 merged_manifest = sources.merged_manifest,
@@ -435,6 +480,8 @@ def _lint_aspect_impl(target, ctx):
                 inputs = depset(
                     sources.srcs +
                     sources.resources +
+                    aars +
+                    aars_dir +
                     sources.manifest +
                     sources.merged_manifest +
                     [sources.lint_config_xml] +

rules/android/lint/lint_sources.bzl

@@ -1,4 +1,5 @@
-load("@grab_bazel_common//rules/android/lint:providers.bzl", "AndroidLintSourcesInfo")
+load("@grab_bazel_common//rules/android/lint:providers.bzl", "AarInfo", "AndroidLintSourcesInfo")
+load("@grab_bazel_common//rules/android/lint:collect_aar_aspect.bzl", "collect_aar_aspect")
 
 def _target_outputs(targets):
     results = []
@@ -20,6 +21,11 @@ def _lint_sources_impl(ctx):
             name = ctx.attr.name,
             srcs = _target_outputs(ctx.attr.srcs),
             resources = _target_outputs(ctx.attr.resources),
+            aar_deps = depset(transitive = [
+                dep[AarInfo].transitive
+                for dep in ctx.attr.deps
+                if AarInfo in dep
+            ]),
             manifest = _target_outputs([ctx.attr.manifest]),
             baseline = _target_outputs([ctx.attr.baseline]) if ctx.attr.baseline != None else None,
             lint_config = _target_outputs([ctx.attr.lint_config]) if ctx.attr.lint_config != None else _target_outputs([ctx.attr._default_lint_config]),
@@ -52,6 +58,12 @@ lint_sources = rule(
             allow_single_file = True,
             default = Label("//third_party:empty.jar"),
         ),
+        "deps": attr.label_list(
+            doc = """dependencies of the target, which we collect aars infos from with help of collect_aar_aspect""",
+            allow_empty = True,
+            default = [],
+            aspects = [collect_aar_aspect],
+        ),
         # TODO(arun) add assets
     },
     provides = [

rules/android/lint/providers.bzl

@@ -27,10 +27,27 @@ AndroidLintSourcesInfo = provider(
         name = "Name of target",
         srcs = "Java/Kotlin sources",
         resources = "Android resources",
+        aar_deps = "direct and transitive aars AarInfo",
         manifest = "Android manifest file",
         baseline = "Lint baseline XML",
         lint_config = "Lint config XML",
     ),
 )
 
+AarNodeInfo = provider(
+    "A provider to collect aar info of the current target",
+    fields = {
+        "aar": "aar path",
+        "aar_dir": "aar extracrted path",
+    },
+)
+
+AarInfo = provider(
+    "A provider to collect all aars from transitive dependencies",
+    fields = {
+        "self": "AarNodeInfo",
+        "transitive": "depset(AarNodeInfo)",
+    },
+)
+
 LINT_ENABLED = "lint_enabled"

tests/android/binary/lint_baseline.xml

@@ -19,6 +19,17 @@
             column="29"/>
     </issue>
 
+    <issue
+        id="PrivateResource"
+        message="The resource `@color/material_blue_grey_950` is marked as private in material-1.2.1.aar"
+        errorLine1="    val material_res = com.google.android.material.R.color.material_blue_grey_950"
+        errorLine2="                                                           ~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="../../../../../../../tests/android/binary/src/main/java/com/grab/test/TestActivity.kt"
+            line="9"
+            column="60"/>
+    </issue>
+
     <issue
         id="MissingVersion"
         message="Should set `android:versionCode` to specify the application version"

tools/lint/src/main/java/com/grab/lint/LintBaseCommand.kt

@@ -112,14 +112,18 @@ abstract class LintBaseCommand : CliktCommand() {
         "--path-variables"
     ).default("${PWD}=${System.getenv(PWD)}")
 
+
+    protected val aarDeps by option(
+        "--aar_dirs"
+    ).split(",").default(emptyList())
+
     override fun run() {
         preRun()
         prepareJdk()
         WorkingDirectory().use {
             val workingDir = it.dir
             val projectXml = if (!createProjectXml) projectXml else {
                 ProjectXmlCreator(
-                    workingDir = workingDir,
                     projectXml = projectXml
                 ).create(
                     name = name,
@@ -130,6 +134,7 @@ abstract class LintBaseCommand : CliktCommand() {
                     modelsDir = modelsDir,
                     srcs = srcs,
                     resources = resources,
+                    aarDeps = aarDeps,
                     classpath = classpath,
                     manifest = manifest,
                     mergedManifest = mergedManifest,

tools/lint/src/main/java/com/grab/lint/ProjectXmlCreator.kt

@@ -2,10 +2,15 @@ package com.grab.lint
 
 import com.android.SdkConstants
 import java.io.File
-import java.nio.file.Path
+import java.nio.file.Paths
+import kotlin.io.path.exists
+
+private fun List<String>.lintJars(): Sequence<String> =
+    this.asSequence().map {
+        Paths.get(it).resolve("lint.jar")
+    }.filter { it.exists() }.map { it.toString() }
 
 class ProjectXmlCreator(
-    private val workingDir: Path,
     private val projectXml: File,
 ) {
 
@@ -34,6 +39,7 @@ class ProjectXmlCreator(
         modelsDir: File,
         srcs: List<String>,
         resources: List<String>,
+        aarDeps: List<String>,
         classpath: List<String>,
         manifest: File?,
         mergedManifest: File?,
@@ -71,6 +77,13 @@ class ProjectXmlCreator(
             resFolders(resources).forEach { resource ->
                 appendLine("  <resource file=\"$resource\" />")
             }
+            aarDeps.lintJars().forEach {
+                appendLine("  <lint-checks jar=\"$it\" />")
+            }
+            aarDeps.forEach { aar ->
+                val aarInfo = aar.split("^")
+                appendLine("  <aar file=\"${aarInfo[0]}\" extracted=\"${aarInfo[1]}\" />")
+            }
             manifest?.let { manifest ->
                 appendLine("  <manifest file=\"$manifest\" />")
             }