FIx findOverridee for properties, fixes #174 #176
Conversation
This CL updates KSPropertyDecl.findOverridee to use the same infra that is used by KSFunctionDeclaration. Fixes: google#174
| * | ||
| * When there are multiple super classes / interfaces with the property, the closest declaration | ||
| * to the current containing declaration is selected. If they are in the same level, the | ||
| * property of the first specified interface (in source) will be returned. |
There was a problem hiding this comment.
nit: specified super class or interface instead of specified interface
There was a problem hiding this comment.
The overriding for properties usually get complicated when it involves Java, can you add a test case where Java code is overriding a property declared in Kotlin code? See this for example.
the findOverridee in the java implementation of property is not implemented (just returns null). I'm guessing that is because java fields do not override parents.
You mean there is a base kotlin class with val x:T and java overrides it with getX():T?
I'll give it a try but that makes me think, that would mean the findOverridee of KSFunction would need to return a KSPropertyAccessor that does not implement KSFunction :/.
I'll see what happens there.
There was also some work discussed before where KSP right now does not properly resolve property accessors in java source code. If we do, then we should also implement findOverridee in KSPropertyDeclaration.
There was a problem hiding this comment.
actually trying to reproduce that hits this issue: (same exception)
#94
There was a problem hiding this comment.
i'm actually able to workaround that issue by making KSJavaFileImpl use descriptors for the class instead of Java impls.
Java impls are causing lots of inconsistencies hence created this issue:
#177
There was a problem hiding this comment.
Thanks for looking into this so fast.
Question: is the merge of this PR blocked by the progress on #177? Or can this be merged already? It would help me a lot: until this is fixed, I need to maintain my fork. I use Kotlin only. :-)
There was a problem hiding this comment.
Sounds good, I'll merge this one.
There was a problem hiding this comment.
Thanks for the merge. Is it possible to create a release that contains this? I am using KSP as a dependency.
If not, what is a likely date for a new release, given your release schedule?
There was a problem hiding this comment.
I'll do a release this Friday PST.
This CL updates KSPropertyDecl.findOverridee to use the same infra
that is used by KSFunctionDeclaration.
Fixes: #174