-
Notifications
You must be signed in to change notification settings - Fork 264
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
Order of declarations do not match the source #250
Comments
I do have a repro for this now. very interestingly, it only affects the compiled kotlin classes. Or at least, thats the only one i could reproduce for now. Their .class representations are almost equal.
|
in the test, it prints
instead of:
|
so looks like there is something internal to make it happen: still digging to see how we can use that. |
looks like |
i also debugged the proto in the class declaration and actually it is where the property list does not have the properties in the order they were declared in the code. And as far as I can see, the parser only uses the proto. |
Thanks for doing the investigation. That was my concern that if such ordering info isn't available in both class file and meta data, then there is no way for KSP to restore the ordering, the only feasible solution seems to be to change compiler codegen logic, and also requires a rebuild for libraries. |
actually it is available in the class file. in fact, when i debug the class via |
i have a prototype: it is not great as it requires getting jvm signatures etc, but is |
This CL fixes a bug in KSP where the order of fields returned in a KSClassDeclaration does not necessarily match the order they were declared in the class. It only impacts compiled kotlin classes because Kotlin compiler orders field names when serializing the metadata. google/ksp#250 This particularly hurts Room as we generate the table structure based on this order. Even though Room is clever enough to consider the schema the same, the effects are still visible in both schema files and generated sql database. This CL is a port of a possible implementation in KSP: google/ksp#260 Unfortunately, merging that CL would mean KSP doing a lot more work for each class. The plan there is to first fix it in the kotlin compiler if possible and if not, add an API to get sorted fields (to avoid the cost when not necessary). Meanwhile, this CL is a band aid solution in Room so that developers won't have unexpected schema file changes when they move to KSP from KAPT. As this solution is best effort, it doesn't fail compilation if it fails to read fields. I've added a strict mode to XProcessing so that we can run it in tests with a requirement to always find the order but on developer's machine, it won't require this. This strict mode can be turned on via a system property, which we can use to collect more bug information when it doesn't work. Fixes: 177471948 Test: OrderOfFieldsTest Change-Id: If0cf346a7f92dbbf2094e0076ea9ed02de4e32fc
BaseDaoTest triggered a bug in XProcessing where we were over-extending variance resolution to java sources, which is incorrect. Unfortunately, KSP does not yet tell us if a Origin.CLASS was derived from a kotlin file or other (java etc). google/ksp#375 For that, I've expanded KspFieldOrdering to also detect true origin via reflection (and renamed it to KspClassFileUtility). I also added method ordering there as order of methods might matter for Auto Value classes. It adds a performance penalty but we'll eventually get rid of it when google/ksp#250 is fixed. Note that method ordering does not check for descriptions so it is not 100% perfect. Bug: 160322705 Test: BaseDaoTest, MethodSpecHelperTest, KspClassFileUtilityTest Change-Id: Iddfed0f72c438987a7f68ded62e9653275685189
#602 added a |
This CL fixes an issue with our [TypeName] calculation for types that are annotated with `@JvmInline`. These types have interesting rules for when they're actually replaced with the actual type or the `@JvmInline` type, which I've updated [KSTypeJavaPoetExt] to handle. In addition, some of the test cases I added to [KspTypeNamesGoldenTest] caused a breakage in [KspClassFileUtility] when ordering the methods/fields. However, KSP has since fixed the initial issue (google/ksp#250), so I went ahead and updated the code to use the new `Resolver#getDeclarationsInSourceOrder()` method, which fixed the breakages. Test: KspTypeNamesGoldenTest.kt Change-Id: I9ad48edda0fbb518b89be116df2999545d68f33b
Hey guys. I came across this issue here - having a ksp plugin that iterates property declarations. For me, it happens when i don't iterate properties of a containing class directly for a given source file in a given module, but when a class declaration of a nested property should be iterated over. Don't think it's too important what my usecase is, my question: What is the state of the feature "add the possibility to iterate declarations in order they appear in the source code"? I saw the merge request and that it's already merged to main, but since I use the "symbol-processing-api" artifact in my ksp plugin, i think i can't use it, because it resides in the impl package. Would you advice to use a different artifact and use the internal api or is there maybe a way to access the "declarationsInSourceOrder" property somehow and I overlooked it? When I can be of any help, let me know :) (I am using version 1.9.10-1.0.13) |
@hannomalie I see you asked a while back, but for posterity you have to call it from the Resolver:
|
Using the resolver is a bit of a different api surface that needs to be used, making the code somewhat worse compared to what i had before, but it works and is correct, was able to solve my issue, many thanks @ianlevesque for the hint!! |
For .class dependencies, the order KSP reports declarations do not match the class file.
This is the case with Java AP and it helps with consistent output between builds (I've not tested if KSP output is consistent or not though)
https://docs.oracle.com/javase/8/docs/api/javax/lang/model/element/TypeElement.html#getEnclosedElements--
A similar problem existed in KAPT which was solved here: (in 1.5.0)
JetBrains/kotlin@e7f30f3
We should ensure that:
The text was updated successfully, but these errors were encountered: