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

Correctly handle empty Paren nodes as args #368

Merged
merged 2 commits into from
Nov 26, 2022

Conversation

reese
Copy link
Collaborator

@reese reese commented Nov 22, 2022

Resolves #367

Method calls like foo (arg) are represented with paren nodes by Ripper, and our deserialization previously assumed that these always had an Expression in the parens. However, you can still do things like foo (), which Ripper represents as [:paren, false] instead of something like [:paren,[:void]] or whatever.


Note: it's worth calling out that foo () and foo are not technically the same, and that if we're being pedantic the actual output should be foo(()). Take as example the following:

def foo(arg)
  arg
end

foo () # => nil
foo # => wrong number of arguments (given 0, expected 1) (ArgumentError)

However, simply removing empty parens is the way we currently handle all equivalent cases currently, so that's the method I'm going with for this PR. I also think it's pretty fair to assume that most users would be somewhat surprised by the foo(()) transformation, and that if that's what you really mean, we can just tell users to use foo(nil) instead.

@reese reese merged commit 0db5183 into trunk Nov 26, 2022
@reese reese deleted the reese-empty-parens-in-args-add-block branch November 26, 2022 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruby Tree Deserialization Error
2 participants