JEP 441: Pattern Matching for switch (suggestion re: handling sealed type selector expressions)

[David Alayachew]

Hello,

> I'm not sure I understand the problem you present, are
> you saying that if I have the following (I'm on my phone,
> so I apologize about the formatting):
>
> sealed interface S permits A {}
>
> switch(x) {
>     case A a -> ...
>     case S s -> ...
> }
>
> Then when I modify S to be:
>
> sealed interface S permits A, B {}
>
> My switch expression now has a hidden case? I.e. the
> compiler doesn't help me know that I may want to add an
> explicit B case?

Well, not necessarily. By definition, case S s and the default clause hide
new inclusions into the type domain. That is their job and we are not
trying to take away this functionality.

The problem is that there are a handful of situations where case S s and
default are 1-to-1 matches in matched values. This is a very tricky problem
in my eyes because it took me a long time of looking at Robert's example
before I realized that the default clause and the case S s are subsets of
each other, which is to say they cover the exact same cases - no more and
no less.

> I can see few problems with disallowing a "catchall" clause.
>
> Here is a textbook example of sealed types:
>
>     sealed interface Json permits JPrimitive, JArray, JObject {}
>
>     sealed interface JPrimitive<T> extends Json permits JNull, JBool,
JNumber,
>     JString {
>         T unbox();
>     }
>
>     // the implementation of unbox just returns the underlying object, for
>     JNull it returns null.
>     record JNull() implements JPrimitive<Void> {}
>     record JBool(bool val) implements JPrimitive<Boolean> {}
>     ...
>
> Will:
>
>     switch(x) {
>         JArray a -> ...
>         JObject o -> ...
>         JPrimitive<?> p -> ...
>     }
>
> Be allowed? It has the same problem, if we extend the permits list of
> JPrimitive, the compiler won't notify you, but we really want a single
> "primitive" clause, semantically we know that it won't ever change (but
the
> compiler has no way of knowing it).

So to be clear, the part that Robert is pointing out is that this only
applies in a situation where the switch selector expression returns the
type S, which is the sealed type. So in the example that you provided, the
type of the selector expression would have to be JPrimitive for the
situation that Robert is describing to apply (though you address this
later).

> If you only care about the "first layer", there are still
> cases were you have a sealed types that permits quite a
> lot of types (say, 10, which is a lot but possible), you
> may have a function that receive that type, and handle 7
> out of the 10 ways the same way, but 3 of the types
> require aspecial handling (think subtypes that requires
> external resources, or thread locks), not having a
> catchall will create a lot of boilerplate (or will make
> you switch to if-else chain (pun intended))

So I think I understand what you mean by first layer, but I will dive into
an example anyways to be explicit.

Let's rework your first example to be exactly what we need -- a sealed type
with 10 permitted subclasses. Also, we will apply the restriction that
Robert mentioned -- the "case S s" must also be the type of the switch
selector expression.

    sealed interface S permits A, B, C, D, E, F, G, H, I, J { /** Methods,
etc. */ }

    record A(long a)   implements S { /** Methods, etc. */ }
    record B(int b)    implements S { /** Methods, etc. */ }
    record C(short c)  implements S { /** Methods, etc. */ }
    record D(String d) implements S { /** Methods, etc. */ }
    /** Record E and the rest. */

    final S sealedType = new D("idc");

Ok, now we have the foundation we need to make examples.

And let me repeat this part of your quote specifically.

> you may have a function that receive that type (S), and
> handle 7 out of the 10 ways the same way, but 3 of the
> types require a special handling (think subtypes that
> requires external resources, or thread locks), not having
> a catchall will create a lot of boilerplate (or will make
> you switch to if-else chain (pun intended))

Well, let's start off by making sure we accurately capture what you are
saying

    final SomeType output =
        switch (sealedType) //from earlier
        {

            case A a -> { /** Some complex logic */ }
            case B b -> { /** Some complex logic */ }
            case C c -> { /** Some complex logic */ }
            case S s -> { /** Do something with s that handles the
remaining 7 cases all the same way */ }

        };

Am I capturing your point correctly here? Because if so, why not just do
this instead?

    final SomeType output =
        switch (sealedType) //from earlier
        {

            case A a -> { /** Some complex logic */ }
            case B b -> { /** Some complex logic */ }
            case C c -> { /** Some complex logic */ }
            default  -> { /** Do something with sealedType that handles the
remaining 7 cases all the same way */ }

        };

Remembed, both (s) and (sealedType) have the type S. So really, anything
that we can use (s) for, we can 100% of the time replace it with
(sealedType) assuming no when clauses or other fluff.

That's the point that Robert is getting at here. There is no situation that
could be represented with just case S s by itself that could not also be
captured with default when the switch selector expression is of type S.
Both of these situations are subsets of each other, they cover 100% of the
exact same cases. And therefore, he is proposing that we do something when
this occurs. He suggests error, I suggest warning. And the error/warning
should tell you to use a default clause instead.

> If you still think it is a problem, I'm sure you could
> make a rule in your linter/build pipeline to disallow
> catchall clauses

Can you think of any examples where you would want to have a case S s
(excluding when clauses of course) that could not be better communicated
and handled with a default clause and using the existing sealedType
variable? If there aren't any other cases, then both Robert and I think
that this ambiguity should be prevented. The purpose of the default clause
is to make it LOUD and clear to the reader that all cases left are to be
captured under this clause. To instead use case S s can provide just enough
indirection such that if there is enough noise above the switch expression,
then this is a pretty easy pothole to miss.

And based on that, we think that this should be some sort of warning/error
on the language level. Not just for some third party linter.

Thank you for your time and insight!
David Alayachew
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20230409/7c7a340d/attachment.htm>