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

Improve error message on trailing comma in method definition #205

Open
schneems opened this issue Sep 27, 2023 · 0 comments
Open

Improve error message on trailing comma in method definition #205

schneems opened this issue Sep 27, 2023 · 0 comments

Comments

@schneems
Copy link
Collaborator

The error produced by this (invalid) code is confusing and not helpful:

def run_build_script(
   workspace_dir: ARGV[0],
   output_dir: ARGV[1],
   cache_dir: ARGV[2],
   stack: ENV.fetch("STACK"),
   ruby_version: ENV.fetch("STACK"),
   io: STDOUT,
)
 end
$ exe/syntax_suggest /tmp/bad.rb
--> /tmp/bad.rb

syntax error, unexpected ':', expecting end-of-input

  1  def run_build_script(
> 2     workspace_dir: ARGV[0],
> 3     output_dir: ARGV[1],
> 4     cache_dir: ARGV[2],
> 5     stack: ENV.fetch("STACK"),
> 6     ruby_version: ENV.fetch("STACK"),
> 7     io: STDOUT,
  8  )

The problem is that there's a trailing comma. This code works (when you remove the comma after STDOUT):

$ cat /tmp/bad.rb
def run_build_script(
   workspace_dir: ARGV[0],
   output_dir: ARGV[1],
   cache_dir: ARGV[2],
   stack: ENV.fetch("STACK"),
   ruby_version: ENV.fetch("STACK"),
   io: STDOUT
)
 end
$ exe/syntax_suggest /tmp/bad.rb
Syntax OK

Fix Idea(s)

I'm not sure how to approach this. Possibly we could check for some variation of this regex https://rubular.com/r/aocG0GtGFjEiG2 as a brute force check. There may be a more elegant option.

Sidebar: I would love to see trailing commas allowed in method definitions. I think adding this behavior is an issue for positional args as you want that feedback that your airity is off. Idea: Allow it if and only if the method ends in a kwarg (instead of a positional arg). I don't know if it's been discussed or not.

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

No branches or pull requests

1 participant