Confusing rescue syntax edge case

[keidax]

Discussion

Much like Ruby, Crystal allows rescue to have a variable and/or type:

begin
  raise "uh oh"
rescue ex
  puts "it's okay"
end

begin
  raise "uh oh"
rescue Exception
  puts "it's okay"
end

begin
  raise "uh oh"
rescue ex : Exception
  puts "it's okay"
end

But what about this?

begin
  raise "uh oh"
rescue : Exception # <-- here's the problem
  puts "it's okay"
end
# => Unhandled exception: uh oh (Exception)

Surprise! This doesn't rescue anything. Instead it just declares a local variable named rescue.

This is very unexpected behavior (at least to me). It doesn't help that rescue usually still has the syntax highlighting of a keyword, rather than a variable. This could even be dangerous, if it lets someone slip code past a code review that appears to handle an error but actually doesn't.

I propose that this becomes a syntax error. It seems unlikely anyone is purposefully using the current behavior.

[asterite]

This syntax is there so you can do:

property rescue : String

You can see that for that to compile, we have to allow that syntax.

That said, we could probably disallow that syntax when it happens inside method definitions.

[straight-shoota]

Assignment rescue = 1 is already a syntax error and accessing a variable named rescue is also somewhat limited.

This works, though:

rescue : String = "foo"
p!(rescue)

@asterite I'd expect restricting the syntax to specific locations to be somewhat fuzzy. For example, outside of method definitions, it could either be a valid macro argument, but also a mistyped rescue clause.
I think a better solution would be to apply the restriction at the semantic stage and disallow a type declaration of a local variable rescue without a value.
The use case with the property macro doesn't actually declare a local variable, so that still works.

[straight-shoota]

For example, outside of method definitions, it could either be a valid macro argument, but also a mistyped rescue clause.

Hm, maybe that's not actually true. A mistyped rescue clause can't be in a call argument. So it can probably be determined at syntax level. rescue : Foo outside a call argument would be a syntax error.

[HertzDevil]

You cannot do this to any keyword even if the variable declaration is legal:

rescue : String = "foo"
p!(rescue) # okay

def : String = "foo"
p!(def) # Error: expecting any of these tokens: IDENT, CONST, `, <<, <, <=, ==, ===, !=, =~, !~, >>, >, >=, +, -, *, /, //, !, ~, %, &, |, ^, **, [], []?, []=, <=>, &+, &-, &*, &** (not 'EOF')

begin : String = "foo"
p!(begin) # Error: unterminated call

class : String = "foo"
p!(class) # Error: expecting token 'CONST', not ')'

Because of this I'd argue we should disallow using any keyword as the name of a local variable.

[straight-shoota]

This works with many keywords, actually. Some don't even require any convincing to be parsed as a variable declaration:

type = "foo"
p! type

union = "foo"
p! union
    
forall = "foo"
p! forall
    
do : String = "foo"
p!(do)

Keywords are always context specific. Disallowing any keyword as a variable name in any context would be very restricting and not reasonable I think. It would definitely be a major breaking change.
This repo alone contains dozens of local variables with such names.

I suppose restricting the use of names that could only be declared via explicit variable declaration (such as rescue and do) might be acceptable, though. They're not easy to use anyways.