-
Notifications
You must be signed in to change notification settings - Fork 252
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
Resolver.overrides fails for generics #175
Comments
btw, there is more nuance to this: When the overriding type argument is a final class, KAPT generates java code that does not actually override the base implementation. I've not checked if it is a bug in KAPT or working as intended based on: Nevertheless, i think KSP should return values for kotlin code hence return true for both cases even when it does conflict w/ KAPT's java translation. |
This CL fixes another case on variance inheritance for overrides where if there is an overridden method w/ star projected type argument, we should not use start projection but instead pick covariant (out). To reproduce it, i've expanded method spec helper test to cover methods that are not overridden. Unfortunately, it revelaled an issue where if a kotlin interface overrides another interface with generics, it is not possible to generate kotlin override for it if the type argument is a final type. (b/174313780). Only one test case hits it so I've added a way to ignore them for now (we cannot fix it until room generates kotlin sources). I've also hit some KSP bugs: google/ksp#175 Overrides might fail when overriding with generics. Luckily, findOverridee method finds it so I've added a workaround by calling findOverridee. We could possibly only call it instead of calling both but resolver bug will be eventually fixed in KSP hence I kept that call. google/ksp#174 Since we call findOverridee for properties now, it might trigger another issue in KSP. Added a try catch for it now. google/ksp#173 KSP might throw when resolving deserialized type alias descriptors. Added a try catch for it where we hit the issue. (reading annotations from an element that also has @throws annotation). google/ksp#164 This issue also affects get getJvmName for which we have safeGetJvmName. In this CL, it happens when finding overridee for getter/setter methods in java. Added a try catch for it for now. Bug: 174313780 Bug: 160322705 Test: MethodSpecHelperTest Change-Id: Ie6347f46027c317ed7784e8d2a0c834012fb4b52
* Removed workarounds in KspTypeElement about detecting constructors since KSP now has an API to get check if a function is a constructor * Removed the workaround for google/ksp#175 which seems to be working now. Instead, now we revert some override calls if its jvm signatures won't match. * Updated ksp integration test (room kotlin test app) to use the new kspAndroidTest conffiguration. * Removed the workaround for google/ksp#173 * Re-enabled @generated annotation for KSP: google/ksp#198 * Removed the log for https://github.com/android/kotlin/issues/133 We'll still use custom logic because calling KSP will invoke the descriptor APIs and current implementation is working fine. * Fallback location test now also checks the KSP constructor case google/ksp#273 Bug: 160322705 Test: existing tests Change-Id: I1aef81b0c7475a86d40d38456e765a469ca79de6
Fixed in #205 already. |
For the given input
Calling
resolverImpl.overrides(impl.receiveList, base.receiveList)
returnsfalse
.I traced it and it is actually
OverridingUtil.DEFAULT.isOverridableBy
which returns false.Surprisingly, calling
impl.receiveList.findOverridee
returnsbase.receiveList
:/The text was updated successfully, but these errors were encountered: