Skip to content
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

fix: KSP processing for package private properties #10777

Merged
merged 3 commits into from May 2, 2024

Conversation

timyates
Copy link
Member

@timyates timyates commented Apr 29, 2024

We seem to be processing package private properties when using Ksp.

This is incorrect and they should be skipped.

Package-private isn't really a concept in Kotlin, but the Kotlin classes may be based on Java classes where it is a thing.

I believe when I have a fix, that this will fix micronaut-projects/micronaut-sql#1324

Closes: micronaut-projects/micronaut-sql#1324

We seem to be processing package private properties when using Ksp.

This is incorrect and they should be skipped.

Package-private isn't really a concept in Kotlin, but the Kotlin classes may be based on Java classes where it is a thing.
@timyates timyates added type: bug Something isn't working lang: kotlin Issues or features specific to Kotlin labels Apr 29, 2024
@timyates timyates self-assigned this Apr 30, 2024
@timyates timyates changed the title wip: Fix KSP processing for package private properties fix: Fix KSP processing for package private properties Apr 30, 2024
@timyates timyates changed the title fix: Fix KSP processing for package private properties fix: KSP processing for package private properties Apr 30, 2024
@timyates timyates marked this pull request as ready for review April 30, 2024 14:29
@graemerocher
Copy link
Contributor

So if it is package private but in the same package that is not allowed? Does this align with java?

@timyates
Copy link
Member Author

The original issue is that the PersonRecord extends a public Jooq class UpdatableRecordImpl (which is in a different package)

https://github.com/davidsonnabend/micronaut-illegal-access-error/blob/main/src/main/generated/com/example/jooq/generated/tables/records/PersonRecord.kt

This UpdatableRecordImpl eventually extends AbstractRecord

https://github.com/jOOQ/jOOQ/blob/d0bf1d180fc2f7cd10f7a49e0974b4594211889e/jOOQ/src/main/java/org/jooq/impl/AbstractRecord.java#L122

and this is package private... 🤔

I thought I added the same class to all 3 languages (groovy, java and kotlin) and kotlin was different...

I will check again to make sure 🤔

@timyates timyates marked this pull request as draft May 1, 2024 08:19
@timyates
Copy link
Member Author

timyates commented May 1, 2024

@graemerocher You're right, I've found a difference between kotlin and java/groovy... working on it

@timyates timyates marked this pull request as ready for review May 1, 2024 08:55
@timyates
Copy link
Member Author

timyates commented May 1, 2024

@graemerocher In d1d7e9e I added a test for all 3 languages to make sure we see the same...

When the superclass is in the same package as the introspected bean, we do see the package private property in all 3

So I think this is right 🤔

Copy link

sonarcloud bot commented May 1, 2024

@timyates timyates requested a review from sdelamo May 2, 2024 09:40
@sdelamo sdelamo merged commit 32fd3c4 into 4.4.x May 2, 2024
17 checks passed
@sdelamo sdelamo deleted the ksp-support-for-package-private-java-supertypes branch May 2, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: kotlin Issues or features specific to Kotlin type: bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants