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

Android: add javac intermediates to classpath #3867

Merged
merged 2 commits into from
Jun 18, 2021

Conversation

damianw
Copy link
Contributor

@damianw damianw commented Jun 12, 2021

This adds the classes from build/intermediates/javac to the classpath used to compile the code, in order for the compiler to be able to resolve references to Java code (included that which is generated). This resolves issues such as #3488 among probably others.

A test is included which enables viewBinding on a sample project and verifies that the Detekt invocation doesn't produce any errors about unresolved references to the generated classes.

Downside: invoking Detekt (with type resolution) now depends on the full assembly of the variant. Before (I think?) this was not a requirement. This is a bit of a trade-off, although in practice I don't think it makes too much of a difference considering the "expensive" type resolution tasks are typically run on CI (we configure Detekt in this way already in a fairly large codebase and haven't found it to be especially burdensome). It would be possible to make this an opt-in option if needed though. 🙃

@codecov
Copy link

codecov bot commented Jun 12, 2021

Codecov Report

Merging #3867 (9f78146) into main (4d4a11a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #3867   +/-   ##
=========================================
  Coverage     83.64%   83.64%           
  Complexity     3116     3116           
=========================================
  Files           456      456           
  Lines          8966     8966           
  Branches       1747     1747           
=========================================
  Hits           7500     7500           
  Misses          552      552           
  Partials        914      914           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d4a11a...9f78146. Read the comment docs.

Copy link
Member

@chao2zhang chao2zhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the improvements. I think the long-term goal is to build detekt as a compiler plugin, but we should help improve the existing android integration at the same time.

@chao2zhang chao2zhang merged commit 69749f6 into detekt:main Jun 18, 2021
@cortinico cortinico added this to the 1.18.0 milestone Jun 20, 2021
@cortinico cortinico added the notable changes Marker for notable changes in the changelog label Jul 8, 2021
chao2zhang added a commit to chao2zhang/detekt that referenced this pull request Jul 16, 2021
schalkms pushed a commit that referenced this pull request Jul 16, 2021
@cortinico cortinico removed the notable changes Marker for notable changes in the changelog label Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants