Crystal: Incorrect typing in case statement with multiple conditions per clause

[RX14]

Correctly typed case (https://carc.in/#/r/1d0l):

def foo(foo)
  case foo
  when "foo"
    true
  else
    raise "Invalid operator"
  end
end

puts typeof(foo("foo")) # => Bool

Incorrectly typed case (https://carc.in/#/r/1d0m):

def foo(foo)
  case foo
  when "foo"
  when "bar"
    true
  else
    raise "Invalid operator"
  end
end

puts typeof(foo("foo")) # => (Bool | Nil)

Crystal 0.19.4

[Exilor]

Are you sure you didn't mean case "foo", "bar" in the second example?

[RX14]

oh wow thats ugly

[RX14]

I must have picked this up from another language... I was absolutely certain this was how it worked.

I think that the syntax for multiple conditions should become multiple adjacent "when" clauses. Using commas makes when foo, bar too close to when {foo, bar} (changing this frees up the former for special cases). When using long conditions it's ugly and you can't comment each clause:

case op
when "+", '+', :"+"
  Operator::Add
when "-", '-', :"-"
  Operator::Subtract
else
  raise "Invalid operator"
end

vs

case op
when "+"
when '+'
when :"+"
  Operator::Add
when "-"
when '-'
when :"-"
  Operator::Subtract
else
  raise "Invalid operator"
end

Empty clauses which do nothing can leave a blank line between whens. This encourages you to document it:

string.each_char do |chr|
  case chr
  when '0'..'9'
    stack << read_number
  when ' '
    # ignore whitespace
  else
    raise "invalid"
  end
end

At the very least make adjacent when clauses an error, the original example I gave looks like multiple conditions to me.

[Sija]

@RX14 👍, I've got bitten by the same perception once, although IMO it should be considered as alternative to listing conditions after comma.

[ysbaddaden]

Nope. This is very ugly in JavaScript, for example.

An empty when block is an empty block for the condition, not something that will skip to the next when block, skipping the when condition altogether, then skipping to next... until what? Without an explicit break, it's confusing. Do we mean to have a skip (like in JavaScript)? Or is it stopping whenever a block isn't empty? What about ignoring a value? A comment means an empty when, so... we'd need an explicit break, or require to specify nil so the block isn't empty? Or as you suggest, a blank line? But a blank line is an empty block... it's confusing, at best.

I, by far, prefer the Ruby syntax. You have a single when keyword followed by a comma separated list of possible conditions —that can be spanned on multiple lines— and a single block that applies. An empty block is merely an empty block, a there is nothing more. No confusion.

[RX14]

@ysbaddaden I prefer to think of it as grouping when statements when they are adjacent. There's no empty when block to be skipped (you certainly can't see any empty space when looking at adjacent when). Likening it to c-based langauges with the explicit break condition (or explicit skip) is wrong.

As for ignoring values, I think that having an empty when condition without a blank line is simply incredibly confusing. Because there is no indent, no "space", it simply doesn't look empty, it looks grouped with the following when condition. For this reason, placing an empty line creates a visual break which make it clear the clauses are seperate. As an additional benefit, it also places a nice space for a comment to describe the reason you have this confusing, empty when clause.

[RX14]

I think the core of my argument is that adjacent whens _look_ grouped, and that's confusing. Either we disallow that, or we change the grouping style to be adjacent when instead of ,. I believe that adjacent when is more powerful because it frees up syntax, and is much easier on the eyes than finding a ,.

[waj]

Some people might prefer one option, some people might prefer the other. Both have pros and cons and each of us will have an opinion about which reason is stronger but in these cases there isn't always a clear winner.

Crystal inherits from Ruby most of the syntax and semantics, so it makes sense to copy the same rules here to avoid confusion. Ruby uses , to separate cases, so does Crystal.

[asterite]

I don't think we'll change the syntax of case/when, so I'm closing this.