Anyway, I guess I can just ask here. Why is there ...

# random

[iex]

Anyway, I guess I can just ask here. Why is there no error (or at least a warning) here?

sealed class Foo {
    data object Foo1 : Foo()
}

sealed class Bar {
    data object Bar1 : Bar()
}

fun abc() {
    val foo: Foo = Foo.Foo1
    when (foo) {
        Foo.Foo1 -> {}
        Bar.Bar1 -> {} // <-- invalid type!
    }
}

[Youssef Shoaib [MOD]]

Purely an educated guess here:
You're comparing for equality, not is checks. Hence, a rogue implementation of Foo can make itself always equal to Bar1. Obviously, Foo is sealed, and no implementation of it has any such strange equality, but the compiler is not aware of that I guess. If you write this with is checks, you do get an error:

sealed class Foo {
    data object Foo1 : Foo()
}

sealed class Bar {
    data object Bar1 : Bar()
}

fun abc() {
    val foo: Foo = Foo.Foo1
    when (foo) {
        is Foo.Foo1 -> {}
        is Bar.Bar1 -> {} // Check for instance is always 'false'.
    }
}

Such strange equals implementations are still respected by the compiler, perhaps because in other situations they can make sense (different Set implementations compare equal still, so 2 objects being equal doesn't imply they have the same type)

[iex]

indeed with is there's a warning (not error). Maybe I should use always is

[Youssef Shoaib [MOD]]

Seems to be only an error in 2.3.0-Beta2. At least when testing in playground

[iex]

is there a way to "upgrade" specific warnings to errors, btw?

[iex]

using 2.2.21

[Youssef Shoaib [MOD]]

https://kotlinlang.org/docs/compiler-reference.html#xwarning-level for a specific warning, but you can also do it for all warnings

[iex]

alright, thanks! is there any drawback of using always is vs skipping it?

[Youssef Shoaib [MOD]]

Semantics are slightly different if you have a rouge equals implementation. It used to be the case that is was always the correct way to do it, but then we got `data object`s, whose equals implementation just uses is internally.
Personally, I still like the is. There's no drawback AFAIK

[Youssef Shoaib [MOD]]

Btw, for a general kotlin channel, maybe see #C0B8MA7FA or #C3PQML5NU

[iex]

okay, thanks again! if you (or anyone else) happen to know about a youtrack issue addressing this please add it to upvote.. couldn't find in a quick search. While possibly somehow justified it seems really bad ergonomics

[iex]

for now I'll add is to everything

[Youssef Shoaib [MOD]]

I think it might be intentional behaviour tbh. It doesn't matter much btw since at runtime you'd never hit that Bar1 branch, and in fact, you can just remove it and it'd still compile fine.
Maybe it's worth a ticket because of that though. The fact that you can remove that last branch and the when is still exhaustive should tell the compiler that the last branch is dead

[iex]

it's bad if you do a refactoring where the sealed class changes. I usually let the compiler guide me in large refactorings, but here it just doesn't notice

[iex]

I should have added, in combination with else clause. Otherwise you'd notice via the missing cases

[Youssef Shoaib [MOD]]

I've opened this issue for the dead branches
Hmmm, in combination with else this might be intentional behaviour, though, or at least difficult for the compiler to track. I'm guessing you mean a case like:

sealed class Foo {
    data object Foo1 : Foo()
}

sealed class Bar {
    data object Bar1 : Bar()
}

fun abc() {
    val foo: Foo = Foo.Foo1
    when (foo) {
        Bar.Bar1 -> {} // <-- invalid type!
        else -> {}
    }
}

It's difficult to accurately produce a warning here. The compiler would have to determine all possible Foo subtypes, and then know that they don't override equals in any crazy way.

[iex]

Alright, upvoted! yeah I don't know about the implementation details, it just bit me while refactoring my api errors, I was checking for some error cases to show a paywall, added an error wrapper sealed class.. the original error list was very long so I was using the else (I try to do exhaustive checking everywhere but here's just a very long list and am interested in only 2 of them), and well after the refactoring the paywall was not being triggered anymore and no idea why

[iex]

I made it now exhaustive since the refactoring happens to group the long list so it's less verbose

[Youssef Shoaib [MOD]]

I completely see your point here. I think you're right in expecting a warning, but it's a little difficult for the compiler to gather all the info necessary to produce such a warning. I think using is for now might be the best workaround.

[Youssef Shoaib [MOD]]

I've filed an issue. I discovered that this was a K2 change, likely made because of the equals business I mentioned above, but it thus should try to warn in cases where it makes sense, like here.

[iex]

ok, also upvoted 🙂

[Youssef Shoaib [MOD]]

https://youtrack.jetbrains.com/issue/KTIJ-13081/Report-IDE-inspection-when-equality-comparing-when-incompatible-types Seems like there might be an IDE inspection instead of a compiler warning here. Is that not present in your case?
EDIT: seems that this is planned to be an IDE inspection, but it hasn't been implemented yet

[Youssef Shoaib [MOD]]

As I suspected, the behaviour of not warning on equality is on purpose, but hopefully they'll accept checking for equals definitely not being overridden as a worthwhile addition

[iex]

And another one. This is covered by what we already discussed, just leaving it here as I haven't shown the actual problem code yet

when {
        this is ApiClientError.Api && (error == RawApiError.UserNotFound || error == RawApiError.ExpiredProduct) -> {
            // this is never activated now
            log.t { "Polling iteration: user not found or expired" }
            // ...
        }

        else -> {
            log.e(pay) { "Subscription polling unsuccessful" }
            // ...
        }
    }

The refactoring was renaming previous ApiError into RawApiError, then changing the error in ApiClientError.Api to reference a new ApiError

sealed class ApiClientError {
    data class Api(val error: ApiError) : ApiClientError()
    // ...
} 

sealed class ApiError {
    data object ExpiredProduct : ApiError()
    data object UserNotFound : ApiError()
    // ...
}


sealed class RawApiError {
    data object ExpiredProduct : RawApiError()
    data object UserNotFound : RawApiError()
    // ...
}