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

Support multiple level inheritance for lib and app modules in case of KSP usage #5344

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Support multiple level inheritance for lib and app modules in case of KSP usage #5344

merged 3 commits into from
Mar 20, 2024

Conversation

0xnm
Copy link
Contributor

@0xnm 0xnm commented Dec 28, 2023

Description

In Datadog we have instrumentation for Glide by the means of creating DatadogGlideModule which inherits from AppGlideModule. Customers can extend DatadogGlideModule instead of AppGlideModule and get their Glide calls instrumented with Datadog.

This worked fine for KAPT, but doesn't work for KSP, because in case of KSP usage compilation with some class which inherits DatadogGlideModule instead of AppGlideModule directly gives the same error as in #5328.

This happens because supertypes call here checks only types at declaration site and doesn't go through the whole inheritance chain up.

This PR fixes that issue by traversing the whole inheritance chain, so that functionality is aligned with KAPT compiler (which permits such inheritance chains).

NB: Since we have to traverse the whole inheritance chain and AppGlideModule extends actually LibraryGlideModule, it may be tricky to not register app module as library module as well, but since this PR is using first item in the supertype match, app module will be still registered as app module and not as both.

Copy link

google-cla bot commented Dec 28, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@sjudd
Copy link
Collaborator

sjudd commented Jan 26, 2024

Thanks for the fix, I really appreciate the thorough tests!

@sjudd sjudd enabled auto-merge (rebase) January 26, 2024 04:39
@0xnm
Copy link
Contributor Author

0xnm commented Feb 12, 2024

Hi @sjudd! I had to sync the branch with main for the auto-merge, but it seems workflow needs your approval.

@sjudd sjudd merged commit 651d796 into bumptech:master Mar 20, 2024
4 checks passed
@0xnm 0xnm deleted the support-multiple-level-inheritance-for-modules-in-ksp branch March 20, 2024 16:09
@manishindiainc
Copy link

This is awesome. Do we have any idea when the next version containing this fix will be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants