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

Possible bug with def obj.foo; end? #1435

Closed
seven1m opened this issue Sep 10, 2023 · 2 comments · Fixed by #1439
Closed

Possible bug with def obj.foo; end? #1435

seven1m opened this issue Sep 10, 2023 · 2 comments · Fixed by #1439
Labels
bug Something isn't working

Comments

@seven1m
Copy link
Contributor

seven1m commented Sep 10, 2023

The following Ruby code produces an unexpected (to me) AST:

o = Object.new
def o.foo; end

...which gives:

irb(main):001:0> YARP.parse('o = Object.new; def o.foo; end')
=> 
#<YARP::ParseResult:0x000055cdb0e77d58
 @comments=[],
 @errors=[],
 @source=#<YARP::Source:0x000055cdb0e7cd08 @offsets=[0], @source="o = Object.new; def o.foo; end">,
 @value=
  ProgramNode(0...30)(
    [:o],
    StatementsNode(0...30)(
      [LocalVariableWriteNode(0...14)(:o, 0, (0...1), CallNode(4...14)(ConstantReadNode(4...10)(:Object), (10...11), (11...14), nil, nil, nil, nil, 0, "new"), (2...3)),
       DefNode(16...30)(:foo, (22...25), CallNode(20...21)(nil, nil, (20...21), nil, nil, nil, nil, 2, "o"), nil, nil, [], (16...19), (21...22), nil, nil, nil, (27...30))]
    )
  ),
 @warnings=[]>

Specifically the receiver of the DefNode, e.g.

irb(main):011:0> node.child_nodes.first.child_nodes[1].receiver
=> CallNode(20...21)(nil, nil, (20...21), nil, nil, nil, nil, 2, "o")

Is this a bug? I would expect a LocalVariableReadNode or maybe a LocalVariableTargetNode instead of a CallNode.

This is what Whitequark Parser produces:

irb(main):002:0> Parser::CurrentRuby.parse('o = Object.new; def o.foo; end')
=> 
s(:begin,                                        
  s(:lvasgn, :o,                                 
    s(:send,                                     
      s(:const, nil, :Object), :new)),           
  s(:defs,                                       
    s(:lvar, :o), :foo,                          
    s(:args), nil))

...vs what it would produce if o wasn't a known local variable:

irb(main):001:0> Parser::CurrentRuby.parse('def o.foo; end')
=> 
s(:defs,
  s(:send, nil, :o), :foo,
  s(:args), nil)
seven1m added a commit to natalie-lang/natalie that referenced this issue Sep 10, 2023
I was trying to get test/natalie/method_test.rb parsing (and passing),
but I think I found a few unexpected results from YARP. I think these
are bugs, which I filed here:

ruby/prism#1435
ruby/prism#1436
seven1m added a commit to natalie-lang/natalie that referenced this issue Sep 10, 2023
I was trying to get test/natalie/method_test.rb parsing (and passing),
but I found a few unexpected results from YARP. I think these are bugs,
which I filed here:

ruby/prism#1435
ruby/prism#1436
@kddnewton
Copy link
Collaborator

Yup that's a bug! Looks like we're pushing the new scope too soon.

seven1m added a commit to natalie-lang/natalie that referenced this issue Sep 11, 2023
I was trying to get test/natalie/method_test.rb parsing (and passing),
but I found a few unexpected results from YARP. I think these are bugs,
which I filed here:

ruby/prism#1435
ruby/prism#1436
@eregon eregon added the bug Something isn't working label Sep 12, 2023
@seven1m
Copy link
Contributor Author

seven1m commented Sep 13, 2023

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants