Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require semicolon or newline before method body #13386

Open
straight-shoota opened this issue Apr 24, 2023 · 5 comments
Open

Require semicolon or newline before method body #13386

straight-shoota opened this issue Apr 24, 2023 · 5 comments

Comments

@straight-shoota
Copy link
Member

The compiler currently does not require a clear delimiter between the head and body of a method.

For example, def foo : Int32 1 end is valid syntax.

There are some oddities: def foo 1 end is not valid, but def foo() 1 end is.

Some typos can cause surprising behaviour. The following code is valid:

def foo : Array (String)
  [1]
end

The return type restriction is Array. (String) is parsed as the first expression in the method body.

A variation of that appears in #6191:

def foo(x : String) : Tuple (String, String) # Syntax error: unterminated parenthesized expression
  {x, x}
end

I think requiring an explicit delimiter could improve the last examples because they would lead to (meaningful) syntax errors.

This comment form @FnControlOption in #11854 (comment) makes a concrete suggestion:

def f(x) puts x end is both valid Ruby and Crystal, and requiring a semicolon in this case seems unnecessarily restrictive. I think a semicolon/newline should only be required if the method header (a) has both no parameters and no parentheses; or (b) has anything after the parentheses (i.e. an explicit return type and/or any generic type parameters).

I'm not sure about (a). A def without parenthesis cannot have parameters, so this case should be pretty unambiguous.

@FnControlOption
Copy link
Contributor

FnControlOption commented Apr 24, 2023

I'm not sure about (a). A def without parenthesis cannot have parameters, so this case should be pretty unambiguous.

I agree—at least for Crystal—but it should be noted that this is already enforced in the parser, most likely to mirror Ruby.

For context, it's possible in Ruby to define a parenthesis-less, parameterized method. For example, the following is valid:

def f x
  puts x
end

In Crystal, however, this would raise a syntax error because all method headers must have parentheses around parameters.

In both languages, def foo bar end raises a syntax error. In Ruby, it's impossible to determine whether bar is a parameter to foo (def foo(bar) end) or if foo is parameterless and calls some bar method defined elsewhere (def foo() bar end). In Crystal, bar cannot be a parameter since it isn't enclosed by parentheses. However, this wouldn't be immediately obvious to a Rubyist who is new to Crystal.

@straight-shoota
Copy link
Member Author

Yeah, it's probably better to be strict about parameterless defs, even though it's technically unambiguous in Crystal.

@straight-shoota
Copy link
Member Author

straight-shoota commented May 5, 2023

Actually, the formatter already takes care of removing the potentially ambiguous syntax.

The first example from the OP formats like this:

def foo : Array
  (String)
  [1]
end

So the alternative is definitely considered ill-formed. But it's actually unambiguous and maybe it's fine to keep accepting it as valid code?
If you want your code to be formatted in a sane way, you should use the formatter anyway =)

@stefandd
Copy link

stefandd commented May 16, 2024

I believe this is related!

def foo : Int32
    switch = true
    if switch 10 else 0 end # syntax error: Error: unexpected token: "else"
end
puts foo

The above doesn't compile but with an inserted ; it prints 10.

def foo : Int32
    switch = true
    if switch ; 10 else 0 end # works
end
puts foo

@ysbaddaden
Copy link
Contributor

@stefandd switch 10 is interpreted as a function call with args, and else appears out of nowhere (syntax error). Adding the semicolon tells crystal to not look for args, and it parses correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants