feat(ksp): add option to convert KSAnnotation to AnnotationSpec while omitting default values

[mcarleio]

Adds the possibility in ksp annotaitons extension functions to define to not include the default values when converting a KSAnnotation to an AnnotationSpec.
This is the ksp "equivalent" of AnnotationSpec.get and does not change the current behaviour.

[mcarleio]

Any feedback or reason why not to merge this?

[mcarleio]

Is there anything preventing the acceptance of this PR?

[ZacSweers]

Is there anything preventing the acceptance of this PR?

Yes, the two comments left in code review

[mcarleio]

But I am not the one who should answer them, as the questions are (in my point of view) related to the library owners.

This seems binary-incompatible looking at the changes to the ksp.api file, should we care?

I have no idea what the ksp.api file is and why it seems to be incompatible. I just added a boolean parameter with a default value. Most (or probably any) existing code should work as before and its behavior will remains unchanged.

I wonder if there's a different API we can use, if we ever want to get this code to run without the JDK, having java.* dependencies will make it harder.

I have no idea if there is a plan to ever run this without java. Is there a plan to run KSP without java? Or is this even already possible? I just oriented on the solution already existing in kotlinpoet (without ksp): https://github.com/square/kotlinpoet/blob/master/kotlinpoet/src/main/java/com/squareup/kotlinpoet/AnnotationSpec.kt#L210

[ZacSweers]

I just added a boolean parameter with a default value. Most (or probably any) existing code should work as before and its behavior will remains unchanged.

That's not true. You can see it in the updated binary compatibility file you committed here: https://github.com/square/kotlinpoet/pull/1538/files#diff-03a0dab0a934dcdd578aba4e325fd542f74158af6c2e6a4d3380b1c024720d04R2-R3. You should add @JvmOverloads to this to preserve existing compatibility, we don't only support source compatiblity.

For the second one, Egor was just asking if there's a non JVM-only (i.e. in the kotlin stdlib or kotlin-reflect) option to do deep equals checks.

[mcarleio]

That's not true. You can see it in the updated binary compatibility file you committed here: https://github.com/square/kotlinpoet/pull/1538/files#diff-03a0dab0a934dcdd578aba4e325fd542f74158af6c2e6a4d3380b1c024720d04R2-R3. You should add @JvmOverloads to this to preserve existing compatibility, we don't only support source compatiblity.

As I said, I had no idea what that file is for and what the problem was. For future readers, have a look here to understand the background of these files: https://github.com/Kotlin/binary-compatibility-validator
I added @JvmOverloads to the function and hope that this is fine now.

For the second one, Egor was just asking if there's a non JVM-only (i.e. in the kotlin stdlib or kotlin-reflect) option to do deep equals checks.

I had thought about that and why I added it in the first place. I think it is not necessary, as the values seem to always be Lists instead of arrays and for that I already implemented the check.
Therefore, I just replaced it with a simple equals check and everything seems to work 👍

[ZacSweers]

Thanks for the PR and addressing the comments!

As I said, I had no idea what that file is for and what the problem was.

Then ask :). It's the maintainers' responsibility to help contributors understand the impact of what they're doing, but it's also on the contributors to seek that understanding when they are changing code they don't understand. As you didn't respond to the review comment, it wasn't clear what your position was.