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

Missing support for context parameters on properties #518

Open
koljakube opened this issue Oct 10, 2024 · 5 comments
Open

Missing support for context parameters on properties #518

koljakube opened this issue Oct 10, 2024 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@koljakube
Copy link

Hello!

While introducing context parameters in my code, I noticed that ktfmt is not able to handle them on properties (at least properties in classes/interfaces, global scope seems to work for some reason).

Two possible errors are raised:

0

context(Unit)
val someOuterProp: String get() = ""

This is fine.
I use Unit for simplicity's sake, but "real" interfaces exhibit the same behavior.

1

Simple property with context parameter:

interface SomeInterface {
    context(Unit)
    val someInnerProp: String get() = ""
}

error: expected token: 'context'; generated val instead

2

interface BaseInterface {
    context(Unit)
    open val someInnerProp: String get() = ""
}

interface SomeInterface {
    context(Unit)
    override val someInnerProp: String get() = ""
}

Both of them can, in isolation, cause this:

error: did not generate token "context"


A related issue I could find is #471, but that pertains to methods, not properties.
If I read the KEEP correctly, these cases should be legal, albeit experimental, Kotlin.

@davidtorosyan
Copy link
Contributor

I'm unclear on how much work we should be putting into supporting experimental features. On the one hand, why not, on the other hand things change around so the value is unclear.

Maybe I'm just questioning it because no matter how many times I read the spec, I can't understand what context receivers / parameters are :P


In this case we should probably put some effort in here, since we previously accepted changes related to this (#400).

In general, as long as we don't support line-by-line formatting, I'd feel bad crashing on stuff like this and preventing usage of ktfmt.


Will take a look, but also tagging @bddckr in case you're interested.

@bddckr
Copy link
Contributor

bddckr commented Oct 17, 2024

Let's clarify some confusion here first:

Kotlin first introduced the experimental context receivers feature. By now it's been decided to change the design of those, which leads us to the context parameters feature that was linked above.

Context parameters aren't available yet - they're still in the design phase. So all we have right now is context receivers, which are kinda at an end-of-life stage since context parameters will eventually fully replace them:

[...] We understand that context receivers are already being used by a large number of developers. Therefore, we will begin gradually removing support for context receivers. Our migration plan starts with Kotlin 2.0.20, where warnings are issued in your code when context receivers are used with the -Xcontext-receivers compiler option. [...]

[...] Alternatively, you can wait until the Kotlin release where context parameters are supported in the compiler. Note that context parameters will initially be introduced as an Experimental feature.

https://kotlinlang.org/docs/whatsnew2020.html#phased-replacement-of-context-receivers-with-context-parameters


So my recommendation is to not put any further effort into supporting context receivers. Once context parameters are available, those could be supported - assuming it's checking the boxes for the maintainers to take on that responsibility. Especially considering that it will start as an experimental feature once again...

At a bare minimum, it would be great for ktfmt to not crash and somehow still work when using these features. I personally would be fine with these code parts being left unformatted and kept as-is, as long as the rest is formatted. I understand that's easier said than done, though.

@koljakube
Copy link
Author

Thank you for the clarification @bddckr. Yes, it's true that receivers are on their way out. But context parameters are syntactically very similar (they mostly add a parameterName: to the context(SomeType) contents, so I would (naively, not knowing ktfmt's internals) expect you to be able to reuse a good part of the implementation.

I can work around this for now, so no pressure. But according to my research, ktfmt is the only reliable zero-config formatter for Kotlin out there. I think it might make sense to start work on supporting incubating syntax as soon as the stable language features are well supported, to provide formatting to the broadest spectrum of Kotlin code bases.

@hick209
Copy link
Contributor

hick209 commented Oct 30, 2024

@koljakube this might actually be a simple PR, do you want to take a shot at this?

@hick209 hick209 added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Oct 30, 2024
@koljakube
Copy link
Author

I would first need to look into the code to get an idea about how much work that would be, I don't have a lot of spare time at the moment.

But I won't sign your CLA, so I guess that disqualifies me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants