google / ksp

Kotlin Symbol Processing API
https://github.com/google/ksp
Apache License 2.0
2.84k stars 266 forks source link

Order of declarations do not match the source #250

Open yigit opened 3 years ago

yigit commented 3 years ago

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) https://github.com/JetBrains/kotlin/pull/4022/commits/e7f30f38fefa10143bc49546af6f1aeebec4f806

We should ensure that:

yigit commented 3 years ago

I do have a repro for this now. https://github.com/google/ksp/compare/master...yigit:traversal-order?expand=1#diff-041110caa10bb0996c760d70ec04b2ccddfbeba26cbea9e4661f2b5f32614c5d

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.

➜  support git:(boxed-primitive) javap -p lib/KotlinClass.class
Compiled from "KotlinClass.kt"
public final class lib.KotlinClass {
  private final java.lang.String name;
  private final java.lang.String lastName;
  public final java.lang.String getName();
  public final java.lang.String getLastName();
  public final java.lang.String nameFun();
  public final java.lang.String lastNameFun();
  public lib.KotlinClass();
}
➜  support git:(boxed-primitive) javap -p lib/JavaClass.class
Compiled from "JavaClass.java"
public class lib.JavaClass {
  java.lang.String name;
  java.lang.String lastName;
  public lib.JavaClass();
  void nameMethod();
  void lastNameMethod();
}
yigit commented 3 years ago

in the test, it prints

lib.KotlinClass
lastName
name
lastNameFun
nameFun

instead of:

lib.KotlinClass
name
lastName
nameFun
lastNameFun
yigit commented 3 years ago

so looks like there is something internal to make it happen: https://github.com/JetBrains/kotlin/blob/ebfbc2f6013be76a4e48e0998782c7d69ded7d9d/core/deserialization/src/org/jetbrains/kotlin/serialization/deserialization/descriptors/DeserializedMemberScope.kt#L168

still digging to see how we can use that.

yigit commented 3 years ago

looks like CompilerDeserializationConfiguration does not override that field so that option looks like a no-go for now. https://github.com/JetBrains/kotlin/blob/master/compiler/frontend/src/org/jetbrains/kotlin/resolve/CompilerDeserializationConfiguration.kt

yigit commented 3 years ago

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. That means, even if we could use NoReorderImplementation, it wouldn't help.

neetopia commented 3 years ago

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.

yigit commented 3 years ago

actually it is available in the class file. in fact, when i debug the class via javap -p, it has the right ordering (this is why we've not seen this issue in kapt). but as far i understood from reading the code, it only uses the metadata for descriptors (makes sense). "maybe" there is a way to extract that information from the descriptor, find the backing class and get the ordering from there. It would be expensive but for people moving from ksp to kapt, it could matter.

yigit commented 3 years ago

i have a prototype: https://github.com/google/ksp/pull/260

it is not great as it requires getting jvm signatures etc, but is a possible solution :)

ting-yuan commented 3 years ago

602 added a KSDeclarationContainer.declarationsInSourceOrder to work this around.

hannomalie commented 11 months ago

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)

ianlevesque commented 10 months ago

@hannomalie I see you asked a while back, but for posterity you have to call it from the Resolver:

resolver.getDeclarationsInSourceOrder(myDeclarationContainer)

hannomalie commented 10 months ago

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!!