proposal-pattern-matching: Forbidding unreachable clauses?

The usefulness of being able to temporarily make a branch always match, just so you can test that code, I think overwhelms the usefulness of getting a parse-time error for (statically analyzable) unreachability. Linters have exactly the same amount of power here as the language would, and they don’t harm exploratory coding.

So I’m definitely on the “allow unreachable branches” side.

I feel this is the job of a linter, not the language. Javascript doesn’t care about this:

return 10;
if (condition) {
  return 5;
}

So I don’t think adding in that complexity is helping anyone. At best, I see this as a warning. The spec would have to include the special case of exactly “a catch-all happens above this pattern”.

It’s not really “less often” because instead of it always being valid, they have to learn which unreachable clauses throw SyntaxErrors and which don’t. It’s the same amount of information. And if you expect to see an error when you have uneachable clauses, and you have a case that is unreachable but doesn’t throw, it could produce more confusion.

@ljharb Would you consider it to be a syntax error?

If not, there isn’t really a place for that error to be thrown, other than at evaluation time, in which case I would strongly argue against having it exception then.

I also agree with @tabatkins that being able to do this:

complexCase match {
  complexFoo => ...,
  complexBar => ...,
  debug => console.debug('We didn't pass?', debug),
  complexFooBar => ...,
}

while developing certainly outweighs accidentally leaving that case in.

…so why wouldn’t we tell the user about it. Other languages are doing so (OCaml, etc). I feel like we are helping the user by doing so.

@xtuc How would we tell and help user about that? Throwing a runtime error? Because besides an error is not just “telling”… throwing an error for a perfectly runnable code isn’t a help at all! 😄

You also gave us the OCaml example to make the argument stronger. It’s a compiled language (opposite of Javascript), then I’ll assume the mentioned warning is given at compile time, am I wrong? If so, this example isn’t relevant to justify a runtime error on Javascript. Besides that, I tried this code on https://try.ocamlpro.com/, and I didn’t get any error, runs just as expected:

let f x =
  match x with
     | "foo" -> "it is foo"
     | _     -> "our default: " ^ x
     | "bar" -> "will never get here, and I didnt get any sort of error";;

f "foo";;  (* prints "it is foo" *)
f "bar";;  (* prints "our default: bar" *)

Now, the @ljharb sentence:

I think it’s very valuable to bake into the language what would otherwise require a linter

Where comes the will to “bake into language” and FORCE the user to have a compile-time sort of warning (it’s not even an error) in an interpreted language? It’s against the reason why we choose to Javascript, and not a compiled language to develop software.

If you want something like a compiler in an interpreted environment, you should use a linter. If many people don’t use, it’s because is optional, and being optional is the awesome part!

I consider it a mistake that switch allows default in any position, and that it can be silently omitted. I think it’s very valuable to bake into the language what would otherwise require a linter - something far too many people don’t even use.

With the latest proposal, an unreachable match clause seems like the primary thing a linter rule would be needed for. I’d very much like to see any statically unreachable clause disallowed, even if we can’t prevent all unreachable clauses.

There’s also no precedent for this proposal, or a MatchError; i think this should remain open until the committee has discussed it.

I reiterate that I dislike the idea of having this break from precedent, and especially when it involves debugging-related situations – debugging misbehaving clauses can already be quite a pain. I think disallowing fallthroughs altogether would make the situation even worse.

I’d very much like to see any statically unreachable clause disallowed, even if we can’t prevent all unreachable clauses.

This behavior feels somewhat footgun-y. Allowing some unreachable clauses, but not all, to be valid still requires the developer to know the difference. Seems like an extra thing to learn without a lot of benefit.

Just to clarify, this syntax rule is explicitly the catch-all case can only be at the end, if it exists?

My apprehension is if we have one “unreachable code” error, how many should we have? For example, under the current way of null-punning, this is unreachable:

foo match {
  null => console.log('I match null and undefined'),
  undefined => console.error('unreachable code')
}

While I opened the issue in the first place, I have mixed feelings now:

Disallow

When we parse the clauses we already can know if it’s impossible/unreachable, so why wouldn’t we tell the user about it. Other languages are doing so (OCaml, etc).

I feel like we are helping the user by doing so.

Allow

Linters/type checkers will catch them (since they are basically made for that), but when you don’t use one it will fail silently for ever.

There are many construction that will fail silently for ever that we don’t prevent:

if (false) { ILikeTrains() }

const a = true
a = false // well that's not silent

Conclusion

🤷 (I hope that helps)

I will try to attend remotely the next meeting. Also note that it’s easier to remove restrictions that adding them after.

My own 2c:

I’m not sure there’s any precedent for this? switch definitely doesn’t care if you put something after default: { ...; break; }. if likewise has no special case for if (false) or other such literals.

I would argue that we should not disallow this, specially because during development, it might be reasonable for people to insert catchalls into different branches in order to try different behaviors, or to sketch out behavior they want to write in the future without having to comment out the rest of the code.

match (x) {
  1 => 'it was a one!',
  _ => throw new Error('we should not be here yet'),
  2 => 'future code',
  x => 'code that broke before'
}

It seems extreme to explicitly disallow this, as I think this is something better left to linters, as there’s many ways that a clause might become unreachable, not just fallthrough variables. So, I’m opposed. 😃