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.

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

Implement Lint aspect to lint and collect partial results from dependencies #121

Closed
wants to merge 5 commits into from

Conversation

arunkumar9t2
Copy link
Contributor

@arunkumar9t2 arunkumar9t2 commented Oct 24, 2023

This PR continues #119 and enhances running lint.

For proper linting, the Lint CLI requires partial linting data from dependencies as well. For example, running lint on the root binary target it needs access to all library target's partial result data as described here.

Since we have variety of macro and rule implementations, it is difficult to modify the graph and add a Provider to all targets we can talk to and get this information.
Alternatively this PR implements a new Bazel aspect that traverses the build graph and decides whether partial results are required, runs and accumulates and only then runs the root binary lint with all partial results available. Without aspect, we might need to manually connect the linting targets via Grazel which is less than ideal.

Although appears straight forward, there were some challenges.

  • For linting actions, we need all sources and resources for current target. Target here in practice is a very loose term due to complications of build like needing build_config, resources, kotlin, databinding. Thus sources for a android_library/binary mean it can come from variety of sub targets inside the macro. As aspect is traversing the build graph, it should be able to distinguish all sources vs sub target sources. Thankfuly, there is generator_name attribute that can be used to filter sources only from current macro expansion.
  • Because of above, it also means sources here could be generated or just input. For example, for build config the BuildConfig.java is generated but also a source. So instead of assuming this, a new _get_files function correctly expands the source/generated file. Then these files can be used as expected with action and inputs.

Pending

  • Currently entire classpath is dumped which is less than ideal.
  • Android output like R.txt and merged manifest are not yet passed to the linter
  • Lint config.xml needs to be user configurable.
  • Baseline generation / updation.

@arunsampathkumar-grabtaxi arunsampathkumar-grabtaxi added the enhancement New feature or request label Oct 24, 2023
@arunsampathkumar-grabtaxi arunsampathkumar-grabtaxi changed the title Implement Lint aspect to lint and collect partial results from dependencies Implement Lint aspect to lint and collect partial results from dependencies Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants