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

KE2: implement basic usage of properties, variables and flexible types #17884

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Oct 31, 2024

Basic implementation of property and variable accesses, and flexible types since they were needed in passing.

Choices for comment:

  • Keep KE1's choice to extract property accesses as desugared getter or setter calls.
    • Possible alternative: extract a reference expression, perhaps with the hint that the compiler knows this is targeting a property, and turn that into a getter/setter call QL-side
  • For the time being, extract flexible types as their lower bound (this means String! is extracted as String; the upper bound would mean we get String? instead; I don't know from memory if we get flexible types that are more complex than just flexible nullability
    • Possible alternatives: extract the other bound, or extract a flexible type explicitly (and do something with this QL-side so that Java query authors need not worry about flexible types)

@smowton smowton requested a review from a team as a code owner October 31, 2024 17:56
Copy link
Contributor

@tamasvajk tamasvajk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I've added a couple of comments.

args
)
}
else -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the types that we're handling here? Do we need to add any special handling for enum entries; java/kotlin field access?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not explored yet; noted a TODO.

when (val resolvedCall = ref.resolveCallTarget()) {
is KaSimpleVariableAccessCall -> {
when (val varSymbol = resolvedCall.symbol) {
is KaPropertySymbol -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this explicit that we're handling Kotlin properties here? Does the below work for synthetic java properties?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment clarifying that the path seems to work for both

@smowton smowton force-pushed the ke2/properties-and-variables branch from 317fdfc to f12818a Compare November 13, 2024 15:32
@smowton
Copy link
Contributor Author

smowton commented Nov 13, 2024

@tamasvajk comments applied; will merge

@smowton smowton merged commit efe20b2 into github:ke2 Nov 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants