Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update documentation in lint.bzl and pass merged manifest as input #124

Closed
wants to merge 2 commits into from
Closed

Update documentation in lint.bzl and pass merged manifest as input #124

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e36303
Add docs
arunkumar9t2 Oct 27, 2023
b0e3b42
Update documentation in lint
arunkumar9t2 Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 74 additions & 27 deletions rules/android/lint.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ def _get_target_outputs(target, generator_name):

def _get_files(attr, source_attribute):
"""
Ensure sources are files. For any of non file entry, try to extract it from output group
Extract files from the given `source_attribute`. If any of them are generated or output of a target, extract
them as well.

The result will be list of struct(file, generator_name). generator_name is the current macro that expanded
this target and cab be used to filter out sources of particular target when eventually everything is
accumulated through the build graph.
"""
generator_name = attr.generator_name
raw_values = getattr(attr, source_attribute, [])
Expand All @@ -47,6 +52,9 @@ def _get_files(attr, source_attribute):
return result

def _transitive_sources(target, ctx, source_attribute):
"""
Collect transitive sources for the `target` across attributes in _LINT_ASPECTS_ATTR
"""
return depset(
_get_files(ctx.rule.attr, source_attribute),
transitive = [
Expand All @@ -57,7 +65,41 @@ def _transitive_sources(target, ctx, source_attribute):
],
)

def _merged_manifest(target, ctx, default):
"""
Return list of struct(file, generator_name) where file is merged manifest from android provider.

Return default if android provider does not exist.
"""
generator_name = ctx.rule.attr.generator_name
if getattr(target, "android", None) != None:
if getattr(target.android, "merged_manifest", None) != None:
return [
struct(
file = target.android.merged_manifest,
generator_name = generator_name,
),
]
return default

def _direct_sources(transitive_sources, generator_name):
"""
Consider a macro A that expands as below

A --> A_gen -> A_src --> A_kt

For context of lint, A is the logical linting unit, but because of macros it is spread across
several sub targets like `_gen`, `_src` etc. This is especially common in Kotlin android support
where we have `_kt` for Kotlin and `_base` for android resources. If we blindly assume aspect traverses
over build graph these individual targets also appear as node but this is not something we want since
then we will run lint over each sub target macro which can often produce wrong results.

What we need instead is a sources for logical target under test: A. To do this, we rely on `generator_name`
to filter out the sources from entire transitive closure. This also means we need to call `to_list` on the
depset which is unfortunate for analysis performance.

Returns sources belonging to current macro alone.
"""
results = []
for src_data in transitive_sources.to_list():
if src_data.generator_name == generator_name:
Expand All @@ -75,26 +117,20 @@ def _classpath(target):
return classpath

def _dep_lint_infos(ctx):
result = []
for target in ctx.rule.attr.deps:
if AndroidLintInfo in target:
lint_info = target[AndroidLintInfo]
if (lint_info.enabled or True):
result.append({
"module": str(target.label).lstrip("@"),
"android": lint_info.android,
"library": lint_info.library,
"partial_results_dir": lint_info.partial_results_dir,
"lint_result_xml": lint_info.lint_result_xml,
})
return result

def _collect_partial_results(ctx):
"""
Collect dependencies lint info from the transitive closure and extract relevant information like `partial_results_dir`.
"""
return [
t[AndroidLintInfo].partial_results_dir
{
"module": str(target.label).lstrip("@"),
"android": target[AndroidLintInfo].android,
"library": target[AndroidLintInfo].library,
"partial_results_dir": target[AndroidLintInfo].partial_results_dir,
"lint_result_xml": target[AndroidLintInfo].lint_result_xml,
}
for attr in _LINT_ASPECTS_ATTR
for t in getattr(ctx.rule.attr, attr, [])
if AndroidLintInfo in t and t[AndroidLintInfo].enabled
for target in getattr(ctx.rule.attr, attr, [])
if AndroidLintInfo in target # and target[AndroidLintInfo].enabled
]

def _module_xml_content(name, android, library, partial_results_dir):
Expand All @@ -117,6 +153,10 @@ def _project_xml_content(
merged_manifest = [],
partial_results_dir = None,
dep_lint_infos = []):
"""
Project descriptor XML acts as the overall input arg file for Lint CLI. This method constructs the XML content
from various inputs. No absolute path is used for passing the files.
"""
project_xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
project_xml += "<project>\n"
project_xml += _module_xml_content(
Expand Down Expand Up @@ -203,26 +243,31 @@ def _lint_aspect_impl(target, ctx):
# Run lint only on internal targets
return []
else:
# Lint is enabled only for top level module target generated by the macro
# Lint is enabled only for top level module target generated by the macro, use generator_name to determine
# top level or not
enabled = target.label.name == ctx.rule.attr.generator_name

# Output
partial_results_dir = ctx.actions.declare_directory(target.label.name + "_partial_results_dir")
lint_result_xml_file = ctx.actions.declare_file(target.label.name + "_lint_result.xml")

# Data
transitive_srcs = _transitive_sources(target, ctx, "srcs")
transitive_resources = _transitive_sources(target, ctx, "resource_files")
generator_name = ctx.rule.attr.generator_name
rule_kind = ctx.rule.kind
android = rule_kind == "android_library" or rule_kind == "android_binary" or True
library = rule_kind != "android_binary"
is_test = rule_kind.endswith("_test")

transitive_srcs = _transitive_sources(target, ctx, "srcs")
transitive_resources = _transitive_sources(target, ctx, "resource_files")
srcs = _direct_sources(transitive_srcs, generator_name)
resources = _direct_sources(transitive_resources, generator_name)

# Manifest
manifest = _get_files(ctx.rule.attr, "manifest")
merged_manifest = _merged_manifest(target, ctx, manifest)

if enabled and len(srcs + resources) != 0:
manifest = _get_files(ctx.rule.attr, "manifest")
classpath = _classpath(target)
dep_lint_infos = _dep_lint_infos(ctx) # Collect dependencies' lint data

Expand All @@ -238,7 +283,7 @@ def _lint_aspect_impl(target, ctx):
is_test = is_test,
classpath = classpath,
manifest = manifest,
merged_manifest = manifest,
merged_manifest = merged_manifest,
partial_results_dir = partial_results_dir,
dep_lint_infos = dep_lint_infos,
)
Expand All @@ -253,11 +298,13 @@ def _lint_aspect_impl(target, ctx):
lint_config_xml_file,
partial_results_dir,
inputs = depset(
srcs +
resources +
[project_xml_file] +
[dep_info["partial_results_dir"] for dep_info in dep_lint_infos] +
[lint_config_xml_file] +
srcs +
resources,
[src.file for src in manifest] +
[src.file for src in merged_manifest],
transitive = [classpath],
),
)
Expand Down Expand Up @@ -299,7 +346,7 @@ def _lint_aspect_impl(target, ctx):

lint_aspect = aspect(
implementation = _lint_aspect_impl,
attr_aspects = ["deps", "exports", "runtime_deps"], # Define attributes that aspect will propagate to
attr_aspects = _LINT_ASPECTS_ATTR, # Define attributes that aspect will propagate to
attrs = {
"_lint_cli": attr.label(
executable = True,
Expand Down