Improve resolution using expected type #379
Comments
|
|
|
Hi! First of all, I want to say that I really like this idea, and I've been missing this feature in Kotlin for a while. I have one question/comment about the decision not to use the Swift syntax I use both Kotlin and Swift, with Kotlin as my primary language. I personally strongly prefer the To be fair, this is partially because I only use the PascalCase naming convention for enum cases (which works better with the naming convention of sealed classes). Therefore, it's easy to create name collisions with another class or, worse, an object. The document mentions two issues why this wasn't the chosen solution:
Let's start with the second one. The issue here is that there is currently no other way to do it, so just because people use it doesn't necessarily mean it's the better option or that they wouldn't want to switch to the new syntax (also, the old syntax doesn't go away). As for the first issue, I can't think of any situation where this new syntax would create a problem because I don't think Kotlin has any expression and type declaration that can start with a It wouldn't make sense to bother with the issues mentioned above if there weren't any advantages to compensate for them. However, I think this dot syntax has significant benefits, or to put it better; the currently proposed syntax has some significant drawbacks. First of all, clarity. The Second: With Third: Having the Last but not least, the proposed syntax allows for creating nasty semantic bugs that the compiler cannot always catch. For example, given this code: sealed interface I {
object SomeClass : I
interface SomeInterface : I
}import some.package.*
fun foo(i: I) = when (i) {
SomeClass -> println("some case")
else -> println("default")
}Now add this object: package some.package
object SomeClass : I.SomeInterfaceThe result is a change in the semantics of the Yeah, this is a convoluted example, and in most cases, the result will be a compilation error (still not good, but better). However, I wouldn't be surprised if there were more realistic examples. I also don't like that this feature's behavior depends on the imports. So many times, I have experienced that the indices in IDEA get corrupted, and the IDE removes some "unnecessary" imports. I somehow have a feeling that this feature could be especially prone to this problem. The result would again be a potentially silent change to the program's behavior in the worst case. Note that this issue will occur more frequently if the project uses the camelCase naming convention for things like enum cases. I know that this is not in line with the official code style, but it will be the default used by Swift developers who use Kotlin as their secondary language and expect this feature not to have this problem. |
The problem arises when you combine thing
.foois not ambiguous: access member
I don't 100% follow here. The current proposal allows for situations like the following, where the resolution of enum class Status { ON, OFF }
enum class Switch { ON, OFF }
fun f(s: Status) = when(s) {
ON -> ...
OFF -> ...
} |
|
I like how this KEEP reduces clutter and improves readability for most scenarios.
Inline value classes have a special equals operator function that isn't overriding Some concerns:
Reversing the precedence order would address concerns 2 & 3 but I'm not sure if that introduces other problems. Alternatively, I think that using the |
|
I think One more advantage of Having it explicit is actually looks pretty good to me and .value syntax looks pretty natural for this use case and already well-tested in Swift and I don't see it as an issue for Kotlin developers, rather opposite, when existing importa approach is mixed with proposed expected type |
Oh, I see. So the issue is that there can be a I think that the use of infix function calls in this way should be prohibited. I'm not convinced that such a pattern should exist because it is confusing even for the developer, not just the parser. I also do not think that it would be used that often. Infix functions are generally used relatively rarely outside of DSLs, and in DSLs, it's common to achieve similar syntax (without the dot) by using contexts. Regular code can also do the same using the static imports. Alternatively, we can give priority to the other scopes - just like the proposal does for the name collisions. This kind of collision will be much rarer, but it would complicate the parsing, and I'm not sure it's worth it.
The issue I mentioned is about a situation in which you already have the case "ON" in scope. In that case it would always take precedence even though it's not the correct type (assuming I understand the KEEP correctly). For example, following is not possible: sealed interface Json {
class String : Json
class List : Json
}
fun f(json: Json) = when (json) {
is String -> ...
is List -> ...
}(However, it would be possible with the fun f(json: Json) = when (json) {
is Json.String -> ...
is List -> ...
}This code would work just fine until you import List in the file. |
|
Hi! I really like this proposal, I don't have much to say about it. I'm 50/50 on member functions being included. The main use-case I see for including member functions is when building complex objects: AComplexObject(
ANestedComplexObject(
…
)
)but since the current conventions are to create constructors/factories as top-level entities, and this KEEP explicitly excludes argument resolution, this isn't a very compelling use-case. I wonder if excluding member functions would be more confusing for beginners ("why does it work for some things and not for others") than including them.
I think this is mostly mitigated by the fact that IntelliJ will always let you write the name of the enum element, auto-complete, and will add the qualified type automatically if necessary (which is the current behavior). Now, on the Currently, there are two main ways enums are used:
If we are in a codebase that uses star-import (and many of them do), the arrival of this language feature with
To me, this is clearly a regression. If a codebase has to be made more verbose to accommodate a new language feature made to simplify this exact use-case, then I believe it is a failure of the language feature. Before: import TransferType.*
when (transfer) {
Entry -> …
InternalTransfer -> …
Exit -> …
}After: when (transfer) {
.Entry -> …
.InternalTransfer -> …
.Exit -> …
}Why add syntax when the original code worked already? I much prefer avoiding adding any new syntax at all, and simply codifying the existing pattern and conventions into the compiler. Without the
That's it. Before: import TransferType.*
when (transfer) {
Entry -> …
InternalTransfer -> …
Exit -> …
}After: when (transfer) {
Entry -> …
InternalTransfer -> …
Exit -> …
}Adding new syntax means having to teach users about the new syntax. Sure, the So, I'm in favor of keeping the KEEP as-is: purely about resolution, with no new syntax at all. |
The static imports do not go away, so existing code doesn't have to be migrated at all, even with the
I'm not convinced that it does, at least not universally. The static imports create name collisions. The syntax without the dot has the same issue, potentially much worse than the static imports. Depending on the kind of code you write they can be very common. For example when you write mapping between DTOs and domain objects - they tend to use the same names for nested types.
I think readability is more important than reducing verbosity (especially when we talk about an extra dot). I consider the |
I'm not sure that it's true, considering that the expected type resolution has the lowest priority, so at least from compiler point of view, import is used
Beginners will follow what IDE would suggest, now it's a non-static version, if it would change to |
|
I guess this is going to be an issue of taste, but I don't think looks like Kotlin, whereas this KEEP aims towards: Button(width(10.dp), BLUE, stringResource(string.OK))I don't think this KEEP (which is about type resolution, which is almost a detail to most users) should be the start of such a major syntactical change to the language. And anyway, this KEEP explicitly excludes function calls. |
|
Let me try to spell out some of the concerns and how important they were in the current design. ☣️ Full backward compatibility: every program which currently compiles should keep compiling with the same behavior and no additional warnings.
🛑 Toolability: it should not become way more difficult to provide tooling for this feature.
About
About having the last priority:
|
|
@CLOVIS-AI problem comes when different params use expected type, and some just regular function calls. Yes, this proposal doesn't support method invocation of expected type, but it's a very powerful feature; I see that it may save more code than in the case of when/enums, and the decision now will affect how it is implemented in the future, and even if it possible to implement. I would say that I'm not against the proposal; I just would like the KEEP to have a comparison for .value syntax (advantages and disadvantages) because I still see serious advantages, mostly because it's very explicit and concise. |
Note that there's a fine line into what counts as function here and what not. The main issue is that a constant may not have an explicit functional type, but there may still be an |
Sure, but the code wouldn't compile, so people would quickly learn about this difference. Also, I thought this might be only a temporary limitation that might change with some future KEEP. Therefore, it makes sense to consider what that future would look like, and I feel the current syntax will be very limiting. (For example, for the
Swift also supports the It's true that this would be slightly more restrictive and more or less prevent the usage of this feature in implicit returns in multi-line lambdas. However, I'd question how frequent this pattern would be in practice.
Yes, the issue with forward compatibility already exists, but I think it makes sense to consider not contributing more to this problem if there are alternatives.
I agree that you will usually (but not always) notice the issue because of a compilation error or at least a warning. However, my main problem is that if such a collision happens, you cannot use this feature without the You also need to go and fix your code, which can become annoying depending on how often you run into the issue - which I think can be more frequent than expected, especially if this feature is used with functions and properties rather than just the enum cases and classes. |
|
Let me reiterate that Swift had that syntax from the beginning, so whatever the chose back then has been stable. We have a different starting point, with lots of code in the wild, so we need to guarantee no change in parsing. Any proposal for new syntax should give strong reasons that this is the case. I’ve tried to come with such rules myself, and I never managed to convince myself of that property (which is of course not a proof of anything, but I wanted to mention it to dismiss that the team hasn’t looked at this possibility).
Let me take this as example (not trying to pinpoint the particular comment itself). Even if the pattern is not very frequent, a small percentage of Kotlin users is already a large amount of people that would be affected. This is why we strongly prefer full backward compatibility unless the benefits are really big. |
That's why I proposed to give priority to the member call in this case instead of this new feature. In other words: val a: () -> SomeEnum = {
foo()
.someCase
}would call the |
|
@serras I just tested with Kotlin 1.9.23 and confirmed that the current variable resolution is opposite to the way it would behave with this KEEP. Consider this example: Color.kt package com.danrusu.resolution
data class Color(val r: Int, val g: Int, val b: Int) {
companion object {
val RED = Color(255, 0, 0)
val GREEN = Color(0, 255, 0)
val BLUE = Color(0, 0, 255)
}
}CheckResolution.kt package com.danrusu.resolution
import com.danrusu.resolution.Color.Companion.RED
val RED = Color(200, 0, 0) // Same name & type as in the companion object
fun main() {
printRedIntensity(RED)
}
fun printRedIntensity(color: Color) {
println("Red: ${color.r}")
}
If I comment out the import then I expect this scenario to be much more common especially with variables & functions defined in other files. Adding unrelated variables with the same name & type or functions with the same signature will silently change the behavior based on the KEEP without any compiler errors or warnings to the developer adding these somewhere else. Edit: private fun describeColor(color: Color) = when (color) {
RED -> println("Red ${color.r}")
BLUE -> println("Blue")
else -> println("Other color")
}Using existing syntax in Kotlin 1.9.23, the import has a higher precedence than top-level variables that have the same name & type. It would be very confusing and error-prone if we use the new feature and then the precedence is reversed and behavior silently changes even though the code looks the same. |
Please be careful, as there's a special rule in the compiler that states that an imported identifier has priority over locally-defined ones (in fact, you can "import" the local declaration to reverse this fact). If you want to have a more realistic example, you need to define values in two different files, and import it in a third one. |
I just tested that now: Color.kt package com.danrusu.resolution
data class Color(val r: Int, val g: Int, val b: Int) {
companion object {
val RED = Color(255, 0, 0)
val GREEN = Color(0, 255, 0)
val BLUE = Color(0, 0, 255)
}
}OtherFile.kt package com.danrusu.resolution
val RED = Color(200, 0, 0) // Same name & type as in the companion objectUsage.kt package com.danrusu.resolution
import com.danrusu.resolution.Color.Companion.RED
fun describeColor(color: Color) = when (color) {
RED -> println("Red ${color.r}")
else -> println("Other color")
}With 3 files, I see the same behavior where the import has higher precedence so |
Sorry, my bad, I said "three files", but I meant "three packages". But yes, indeed imported elements have a high priority. But I don't see how this directly relates to the KEEP: in a similar situation the value is resolved to the imported |
|
I've taken some time to compare Swift's and Kotlin's grammar, to figure out whether and how Swift defines implicit member expressions using the following grammar rules, where Note that this is not enough to disambiguate all the expressions, so the reference contains an additional note:
Swift allows defining custom operators, but they need to be made from a special set of symbols: Let's now move to Kotlin's grammar for expressions. The grammar is made out of levels, following the usual technique for avoiding left-recursion in formal grammars. We show here the relevant fragments. In addition, we have the following rule about tokenization of whitespace. Note that Kotlin's grammar clearly differentiates between newlines (
As a result, this means that an expression of the form The only possibility I see with the current grammar is to make That means that you'd be able to use when (x) {
.ENTRY -> ...
is .Foo -> ...
}
return .Left(3)
val x: Either<Int, String> = .Left(3)As hinted in one of the comments above, the main problem would be implicit returns. In the following fragment the val x: (Int) -> Either<Int, String> = { n ->
val m = n + 1
.Left(m)
}
// equivalent to
val x: (Int) -> Either<Int, String> = { n ->
val m = n + 1.Left(m)
}It would also be impossible to use an infix function followed by This lies a possible path to include However, the main question still remain: is this syntax something worth having in Kotlin, or does the current syntax serves us well? Personally, I think that including a major new syntactic form in the language is too big of a change. Some of the reasons why |
|
I have mixed impression. On one hand, the proposed syntax is optional so it should be possible just to skip this part if needed. On the other hand, I tried to understand how and when I should be able to omit qualifier and failed. It could create additional entry level barrier. Also, I do not see how this feature interacts with other language features like namespaces/statics and context parameters. As I stated before, I do not like language features that solve only one problem. Also, I am not sure if it is related, I just remembered another proposal: #209. If we combine it with namespaces, then the feature you want to implement could be brought down to auto-import of a namespace as a file or class level receiver. |
|
Thanks for looking into how the
I don't think this will be a problem in practice because the probability that you create compilable code, in this case, is very low, much lower than the probability of name collisions that occur with the proposed syntax. Also, the IDE will help you recognize this issue thanks to auto-completion and, more importantly, by automatically adding a tab in front of the
The However, I want to point out that this argument can be dangerous in certain situations because without bringing the IDE team into the discussion, we could get into a situation in which the language design team says: This is a problem that IDE will solve so it's a problem for the IDE team. Then the IDE team says this is really difficult to solve in IDE, so it's a problem for the language design team. (It's not necessarily the case here, but I just wanted to point that out.)
I'm not necessarily fixated on the My primary issue with the proposed syntax is that it pollutes the scope with additional names (with all the problems this can cause that were mentioned above), which will be even more pronounced if Kotlin adds this support for function calls. Kotlin already has a considerable issue with scope pollution. This problem was also a big part of all the discussion about replacing context receivers, where the user at least has control over how much they pollute their scope. So, I think that we should try not to contribute more to this problem if there are viable alternatives without other significant downsides. |
|
I don't think making these imports implicit is a good idea. I very often review code on GitHub or other platforms and it won't be easy to understand where things are coming from. Can't this be solved solely as an IDE feature, with the compiler providing suggestions for the most relevant types? For example, in |
|
One general rule we try to follow is that developers can easily track where some of the names are coming from. In this case, we do so by propagating only types which are somehow close to the place where the new resolution takes place. That is, if you write: fun f(s: Status) = when (s) {
is Status.Ok -> ...
}the fun f(): Status = Ok
fun g() = Status.Okbecause in
But it introduces some "traps" about how users may expect their code to be parsed. Thinking about it, the problem of implicit returns from blocks is quite bad; for example the following is wrongly parsed as fun blah(n: Int): Either<String, Int> = when {
n < 0 -> {
val abs = - n
.Right(abs)
}
}
Interestingly, I see this quite the opposite. Imports (or context receivers) pollute in the sense that you cannot hide the new names in any way once you have them. This new resolution is more precise, as it adds only a few new names, based on the information it already has about the type.
Definitely a good IDE experience can help. But it's not the end of the story, as the new resolution in this KEEP takes the types into account, so it can help you reduce conflicts whenever two different subclasses share a name. |
Yeah, but the problems mentioned more or less do not exist in these simple expressions. Here is an example of how this feature will definitely end up being used: fun foo(): Status {
// Many lines of code
...
return Ok
}In this example, it's no longer that easy to track where this expression comes from, and it can get much worse even without introducing this feature to function calls (where it can be quite difficult to determine the type): fun foo(): SomeClass {
// Many lines of code
...
return from("something")
}In this case, you can no longer make reasonable assumptions about the origin of this function - even after you read through all of the code around this expression. This will be true for basically all classes and functions. It's not like with the contexts where you at least know that this feature is used in the given function. But it still can get worse: // In some other file
fun foo(action: () -> SomeClass) {
}
fun callFoo() {
foo {
from("something")
}
}It's not even clear here if this function returns a value and of which type, let alone that the Now that I think about this example. What happens if I add the following function into some scope accessible from Will it get the priority even though this function wouldn't get priority under standard overloading resolution rules? (Assuming I understand the KEEP correctly, this seems to be the case, which is highly counterintuitive)
I agree it's not ideal, but I'd be okay with this limitation if other solutions cannot be found. As I mentioned above, the code will generally not compile, and the IDE will immediately add a tab while you write this expression. So, this limitation is clearly communicated to developers. Additionally, this limitation prevents developers from abusing this feature with implicit returns from complex multiline blocks, which I already try to avoid. If the developer insists on not writing the full name, they can always add the static import. However, maybe we can find some other special character that would work better instead of |
|
On second thought, I realize the reason behind the trap introduced by enum class E { A }
expr()
.A // No pass - Collide & Type unknown
val value = .A // No pass - Type unknown
val value: E = .A // Pass
val value = when {
true -> .A // No pass - Type unknown
}
val value: E = when {
true -> .A // Pass
}
val value = when {
true -> {
expr()
.A // No pass - Collide & Type unknown
}
}
val value: E = when {
true -> {
expr()
.A // No pass - Collide
}
}
val lambda = {
expr()
.A // No pass - Collide & Type unknown
}
val lambda: () -> E = {
expr()
.A // No pass - Collide
}
fun foo(e: E): E {
expr()
.A // No pass - Collide & Type unknown
return .A // Pass
}
foo(.A) // Pass
foo(
.A // Pass
)
when (value) {
.A -> {} // Pass
}As you can see, the rule could be regarded as obvious but strange. It makes me wonder if we should limit that only to |
I'm not sure I understand. The limitation is that you cannot use this syntax if there is some other expression in front of the All of the examples you shared work if the type can be derived (except for the function calls, but those might work in the future). This is not a surprising behavior and doesn't seem to cause issues in languages like Swift. The same is the case even without the To clarify: fun foo(e: E): E {
.A // First & Unknown - this will not compile because the type of the expression cannot be derived
return .A // Not first & Known - this will compile just fine even though there is an expression above because the "return" keyword acts as a delimiter.
} |
I exactly meant that. ;) Note that some places don't require or can't have val l: () -> E = {
expr()
.A // No pass - Collide
}
val e: E = when {
true -> {
expr()
.A // No pass - Collide
}
} |
|
To me, I think the proponents of the Apart from the syntactic part, it feels that most people here agree this is a useful feature to have (not surprising, as it was highly voted in YouTrack). |
|
I feel like it's too unobvious for the syntax without |
Agreed.
Also agreed. However, that is only one criterion to decide if it should be added to the language or not. Union types, for example, is a highly requested feature, but there are good reasons not to add it. Also, people who vote for a feature are often looking at the positive aspects of it and are not weighing the costs like we're doing here. Any feature that causes detriments to compilation speed, type system integrity, program correctness, IDE experience, etc. must have big benefits. I don't think that's the case here, and that's why I think it should be an IDE feature. |
|
These are very good points, and my biggest concern. There are already pitfalls with the later addition of member functions replacing extension function calls without any errors. We shouldn't double down on these behaviors, otherwise the language will be less and less reliable over time.
|
|
I do support this feature, as |
|
@gildor note that this feature is more powerful than a simple IDE improvement. If you add an import to |
It looks an anti feature to me. |
|
@serras yes, it's fair, indeed. |
|
I forgot to update the thread here after a discussion we had around a week ago in the team:
|
I've added more cases where this feature could show up in my comment before. Please review.
What were the comments on this? @serras |
|
@Peanuuutz in general, parsing cannot depend on typing. Parsing is performed independently as a first phase, and typing and resolution come later. So your suggestion is not feasible without major changes on the architecture. About limiting |
|
Thanks. That I understand now.
I do feel like the benefit of For
For
|
|
We've held a few meetings within the Kotlin team, and managed to come with a design in which function arguments also work. You're all welcome to read the new design and give feedback on it. |
|
I much prefer there to be some kind of explicit indication that this kind of inferencing is happening. I think that without an indication like this, it becomes very difficult to tell where a value is coming from. enum Color { RED, GREEN, BLUE }
val RED : Color = BLUE
val a : ()->Color = { RED } // Hmm.... which one is meant?
I would also like to point out that almost all of the problems involving syntactic ambiguity disappear if something other than a single "." is used as a prefix. For instance we could use "*." as a prefix instead of just ".": val a: () -> SomeEnum = {
foo()
*.someCase // This is clearly not a member access on whatever 'foo()' returns.
}Some other prefix candidates might be "...", "@", etc. The ".." token wouldn't work though since it could be interpreted as a range. I kind of like "*." as a prefix though, since it mirrors the "import foo.bar.baz.*" syntax a bit: The "*" seems like a wildcard for a type whose name is deliberately omitted, which is what happens in the 'import' as well. fun f(s: Status) = when(s) {
*.ON -> ...
*.OFF -> ...
}
|
|
I'm against adding new syntax for this. Kotlin already has quite a few weird symbols ( IMO, syntax should only be added when it allows us to do something that wasn't possible before (extension functions, data classes…). This is such a minor feature that it shouldn't have such a large impact on the language. |
This is an issue to discuss improve resolution using expected type. The current full text of the proposal can be found here.
We propose an improvement of the name resolution rules of Kotlin based on the expected type of the expression. The goal is to decrease the amount of qualifications when the type of the expression is known.
The text was updated successfully, but these errors were encountered: