Proposal-pattern-matching: Disallow unreachable clauses

[xtuc]

I would propose to ensure that no other clause is after the else one since it should be the last (probably throwing an error at that point).

Currently nothing in the spec disallow this:

match (4) {
  1: "one",
  else: "else",
  4: "abandoned"
}

Since the 4 cause will never match, I don't see why the developers could be able to write it. That's also a behavior linter will need to implement for case exhaustiveness.

[vincentdchan]

Yep! Allowing clauses after else is nonsense.

[leonardiwagner]

First of all, the else clause itself is already unnecessary, just a pattern of unit already works as elseproposal

x: 1 -> matches when unit = 1,
x    -> matches everything, as else proposal,
            and better, you can also use the unit value
            as console.log(`the value ${x}`)

Second, @xtuc throwing an error for something which isn't a error is not correct, do you agree? If you have else, and use else before and skip next match, the program will run just as expected.

Third, and most important, you can create useless match with any matchers, not only else

{x: 1      }: 'Number 1',
{x: Number }: 'Any number',
{x: 2      }: 'Another number will never reach here due pattern above'
{x: 3      }: 'Another number will never reach here, 
                   are you gonna throw error for that too??' 😕

If it's non-sense @vincentdchan just don't write it, as you don't write the example above. We can make "non-sense" statements like this with plain if else for decades also and is just fine, just don't write.

I don't see any reason to make this feature harsher than it should be, and it only cover 1% of wrong useless possibilities.

[vincentdchan]

@leonardiwagner No, else is necessary. In the proposal, there are no other patterns to match everything. The example you gave uses an identifier, but the identifier is used to match constructor or Array or something like these.

About the second and the third, I think @xtuc 's thought is to throw an error when it's compiling, not in the compiled code. Of cause, you can create many useless patterns to match, but clauses after else is absolutely meaningless.

[ljharb]

If there is a pattern that can be determined for all values to be unreachable, then yes, I would expect an error. Why would we want to permit someone to write something that's verifiably wrong?

[leonardiwagner]

@ljharb Following your way of thinking, then this proposal also has to throw errors also for number literals matchers after a Number type matcher, string literals after a String type matchers, etc.. etc.. and all cases of unreachable matchers as well. Only checking for the else is shallow.

@vincentdchan Not true. In the proposal have the "pattern to match everything" written on Else Leg Syntax on proposal that states else is only a convenience from the unit match (exemplified as _), unnecessary.

[vincentdchan]

@leonardiwagner Firstly, I don't agree. You can not determine some useless clauses, or you will cost a lot to do that, but it's sure that clauses after else pattern are absolutely useless. It's shadow, but it helps.

And the Else Leg Syntax on proposal you mentioned is below the subtitle Design Goals & Alternatives. That's not a strict requirement. Maybe you can open an issue for that.

BDW, introducing _ as unit match will mess the syntax. Look at the example:

match (obj.x) {
    // ...
    _: _ // the result of evaluating obj.x is bound as _ and returned
}

_ is used to match everything here, but _ is a legal name of a variable, it can be bounded to some custom type or RegExp, see identifier-patterns--symbolmatches. Using it to match everything will make programmer confused. _ cannot be bounded in FP languages, so it works well in those languages. In JavaScript, it's not good. Maybe you need a new issue.

[xtuc]

@ljharb you got my point.

This discussion isn't about how to write the else clause but rather if we allow other clauses after it.

[leonardiwagner]

@vincentdchan So do x in simplest match x: 1 -> console.log(x) is a legal variable name. I think the proposal means you can use any variable name, the _ was just the name used in the example. Maybe @ljharb can confirm that.

The title of issue open by @xtuc is "Clauses after else" , that's why I'm focusing on else and showing you guys this is only 1% of cases of unreachable matches. If the title was "Throw error for unreachable matches" I wouldn't talk about that and would agree.

Matchers after else is useless as matching a literal number after a match of Number type. Throwing a error only for else would be amateurish.

[ljharb]

I disagree that throwing an error only for else would be "amateurish".

Throwing on no errors when we could throw on some, however, might be. We should throw on everything that's practical and possible to throw on, and not being able to catch "everything" doesn't diminish the value of that in the slightest.

[vincentdchan]

@leonardiwagner No! An identifier cannot match anything, it matches only if it contains Symbol.matches method. Just see identifier-patterns--symbolmatches. Your example x: 1 -> console.log(x) is meaningless, it's illegal according to the proposal.

And I agree with @ljharb .

[leonardiwagner]

@vincentdchan it was an hypothetical example

@ljharb It would be amateurish due continuous effort to defend the slightest instead discussing if we could match other unreachable matchers and make a better feature. With your current reply and the title change, now we're agreeing 100% and the discussion is productive again!

Although I still personally dislike the idea of an application crashing due an error thrown by some perfectly runnable code (I would expect such blocker behaviour from a linting tool).

[baptistemanson]

Looking at the JS language, both points of views seem valid to me. We cannot write an _else_ before an _if_, but we can make a _case_ in a _switch_ after a _default_. By reading eslint most used rules, it seems that JS never took a stand for one side or the other. My point is it seems there is no philosophy or JS culture going against any of you and it stays as only a pragmatic engineering decision.

Disallowing patterns after else:

• Does it disallow legit use cases? No.
• Does it change the performance? Probably not a lot.
• Does it save some bugs? A few, except for tooled developers.
• Can it be done better by another tool? Certainly.
• Does it make the specs more complicated to implement? Not a lot in my opinion, but TBD.
• Does it make the specs more complicated to understand/learn? Not really.
• Can it be superseded by something better? TBD
• Will it introduce compatibility concern long term? No.

I may have missed some points in the discussion, and I don't want to misrepresent your stand.
You fundamentally agree on facts, and reach similar conclusions.

• If @leonardiwagner's suggestion of better matcher is doable, I think we agree it's better.
• If not, I'm sure the proposition of @xtuc is actually a step towards we can all agree.

So if I understood, the question is for Benedikt and other implementers right?

[js-choi]

We cannot write an else before an if, but we can make a case in a switch after a default.

I am not expressing a preference on this issue, but I would like to gently point out that this illustration may not be valid. case statements after default statements are very much valid and useful. case and default are not branching like if and else. They rather act as goto labels that flow down until the end of the switch block or a break statement. See also the answers in Stack Overflow: “Switch statement: must default be the last case?”.

[zkat]

Hey y'all! #65 has gotten merged, and a lot of issues have become irrelevant or significantly changed in context. Because of the magnitude of changes and subtle differences in things that seem similar, we've decided to just nuke all existing issues so we can start fresh. Thank you so much for the contributions and discussions and feel free to create new issues if something seems to still be relevant, and link to the original, related issue so we can have a paper trail (but have the benefit of that clean slate anyway).

I think this one _might_ still be relevant to the new spec, but I'm inclined to not specify any specific behavior on generalized unreachable clauses. If you're still concerned about this, please do create a new issue!

[xtuc]

As you said, this one could stay open and preserve the comments. I will update the description with the update you made.

What do you think?

[zkat]

@xtuc I would rather make a new one where new discussion happens in terms of the new format, since it might be confusing to scroll through an issue and see mostly the old grammar if you're trying to grok the new one. Do you think that'd be enough? I'm happy enough to open a new issue that does this and gives a summary of the requirements in the new context, too (or you could?).

[xtuc]

Ok i'm fine with that.

I would love if you could open that new issue? Thanks!

[zkat]

@xtuc Done: #73