case when covering all types of a type restricted variable shouldn't need an else case

[ydnax]

i have a function that contains a single case…when construct that covers all allowed types. if the function calls itself the compiler determines the case expressions return type as String?

code:

def encode(value : Int|Array) : String
  case value
  when Int
    "int"
  when Array
    str = "list:("
      value.each do |v|
        str+= encode(v)
      end
      str+=")"
    end
  end
  encode([1,2,3]) 

error message:

in ./encode.cr:8: no overload matches 'String#+' with types String?
Overloads are:
 - String#+(other : self)
 - String#+(char : Char)
Couldn't find overloads for these types:
 - String#+(Nil)

                str+= encode(v)
                   ^

================================================================================

Nil trace:

  ./encode.cr:1

    def encode(value :(Int|Array)) :String
        ^~~~~~

  ./encode.cr:2

        case value
        ^

  ./encode.cr:2

        case value

playground: http://play.crystal-lang.org/#/r/lgo

[jhass]

The recrusive part is not important:

def encode(value : Int|Array) : String
  case value
  when Int
    "int"
  when Array
   "array"
  end
end

encode([1,2,3]) 

[refi64]

As a workaround, try something like http://carc.in/#/r/lh6:

def encode(value : Int|Array) : String
  case value
  when Int
    "int"
  when Array
   "array"
  else
    raise ""
  end
end

encode([1,2,3])

[jhass]

Or

def encode(value : Int|Array) : String
  ret = case value
  when Int
    "int"
  when Array
   "array"
  end
  ret.not_nil!
end

[asterite]

You can use method overloads for this:

def encode(value : Int) : String
  "int"
end

def encode(value : Array) : String
  str = "list:("
  value.each do |v|
    str += encode(v)
  end
  str += ")"
end

puts encode([1, 2, 3])

But yes, we need something that will tell the compiler "I know this case has all types covered, tell me if not".

As a side note, constructing strings this way is very slow, use MemoryIO or String.build:

def encode(value)
  String.build { |io| encode(value, io) }
end

def encode(value : Int, io)
  io << "int"
end

def encode(value : Array, io)
  io << "list:("
  value.each do |v|
    encode(v, io)
  end
  io << ")"
end

puts encode([1, 2, 3])

[asterite]

There's a way to do this without any additional language feature. For example:

def assert_empty_type(obj : T)
  {% if T.union? %}
    {{ raise "unhandled types: #{T.union_types.join(", ").id}" }}
  {% else %}
    {{ raise "unhandled type: #{T}" }}
  {% end %}
end

def encode(value : Int | Array) : String
  case value
  when Int
    "int"
  when Array
    "array"
  else
    assert_empty_type value
  end
end

index = "foo".index('o')
case index
when Nil
  puts "was nil"
when Int32
  puts "was int"
else
  assert_empty_type index
end

This works. If we remove one branch we get a compile error:

def assert_empty_type(obj : T)
  {% if T.union? %}
    {{ raise "unhandled types: #{T.union_types.join(", ").id}" }}
  {% else %}
    {{ raise "unhandled type: #{T}" }}
  {% end %}
end

index = "foo".index('o')
case index
when Nil
  puts "was nil"
  # when Int32
  #   puts "was int"
else
  assert_empty_type index # Error: unhandled type: Int32
end

That's a way to make sure you cover all types.

This works because when index is being filtered out, after discarding Nil and Int32 the resulting type is empty, which is represented as NoReturn in Crystal. Methods whose arguments are NoReutrn are not typed by the compiler, because the compiler assumes that they aren't reachable... well, because they are NoReturn, so something must have happened before reaching that method, like raising an exception or calling exit. So the method assert_empty_type will only be instantiated if index has a concrete type, and in that case will always give an error.

I'd even consider adding this in the standard library, maybe with a better name, and of course trying to improve the error message which is now a bit cryptic.

[rvion]

def assert_empty_type(obj : T)
  {% if T.union? %}
    {{ raise "unhandled types: #{T.union_types.join(", ").id}" }}
  {% else %}
    {{ raise "unhandled type: #{T}" }}
  {% end %}
end

@asterite: totality checking is an awesome feature of crystal. 🍕

I really think that totality checking is a must have when you want to write future-proof code in the presence of open types collections/unions.

How about a new match syntax, similar to case, but with totality checking on by default ?
(match could even be later extended for all kind of things where we can check total coverage, like enums, etc.)

match index # assert_empty_type automatically
when Int
when Bool
end

I understand new syntax has a heavy cost, but I think crystal should push programmers to use totality checking when appropriate. It will yield better code quality, catch more errors, and really ease some kind of refactoring / or new cases additions over large codebases .

if you don't like new syntax, how about chaning case to have totality checking on by default when posible ? (an empty else clause wwould still let the code compile when some cases are not matched) The else clause could even work both ways:

• generates a compile error when present with all cases matched already
• generate a compile error when absent with some cases not yet matched

[lbguilherme]

One problem is that not all things can be statically checked for totality. For example:

x = rand < 0.5 ? :blue : :red
case x
when :red; puts "red"
when :blue; puts "blue"
else # this is now required simply because the compiler can't
     # be smart enough to detect all cases
end

If limited to types and enums only, +1

[timjs]

I'd love to see this by default for Enums and Types. I actually just stumbled upon this trying out Crystal and wondering why my return type is SomeType | Nil instead of just SomeType after case matching on all types. Would be a huge win!

Any status update on this feature?

[RX14]

I really like the possibility of solving this using the assert_empty_type method. However, it still looks a tad ugly. The best I could get it to look was like so:

index = "foo".index('o')
case index
when Nil
  puts "was nil"
when Int32
  puts "was int"
else impossible!(index)
end

https://carc.in/#/r/21t3

The alternative is to make this a language construct, possibly with a case! keyword (although the precedence for symbols in keywords is weak) or use a modifier like complete case (which adds another keyword).

Overall I think the top-level method is my favourite.

[paulcsmith]

I would love to see this feature as well. I like the idea of automatically doing this for enums and types, but if that's too hard/unwanted, then I like the match keyword or case! as @RX14 mentioned. It's an extra keyword, but it's not too much to learn IMO

[straight-shoota]

Resolved by #8424