ruby icon indicating copy to clipboard operation
ruby copied to clipboard

Capture lambda block-local variables in ripper

Open kddnewton opened this issue 2 years ago • 4 comments

You can declare block-local variables in lambda literals like this:

-> (; local) {}

but there's no way to access that information in ripper. This commit adds a new lambda_var ripper event (much like the block_var event) that captures both the params and the locals.

kddnewton avatar Apr 14 '22 22:04 kddnewton

It would be easier for me to review if you could show us how the behavior changes with code and example output.

require "ripper"

pp Ripper.sexp("-> (; local) {}")
#=> before: [:program, [[:lambda, [:paren, [:params, nil, nil, nil, nil, nil, nil, nil]], [[:void_stmt]]]]]
#=> after:  [:program, [[:lambda, [:lambda_var, [:paren, [:params, nil, nil, nil, nil, nil, nil, nil]], [[:@ident, "local", [1, 6]]]], [[:void_stmt]]]]]

pp Ripper.sexp("-> (local) {}")
#=> before: [:program, [[:lambda, [:paren, [:params, [[:@ident, "local", [1, 4]]], nil, nil, nil, nil, nil, nil]], [[:void_stmt]]]]]
#=> after:  [:program, [[:lambda, [:lambda_var, [:paren, [:params, [[:@ident, "local", [1, 4]]], nil, nil, nil, nil, nil, nil]], false], [[:void_stmt]]]]]

BTW, the parser gem adds a virtual argument called shadowarg for the syntax.

$ ruby-parse -e '-> (; local) {} '
warning: parser/current is loading parser/ruby31, which recognizes3.1.1-compliant syntax, but you are running 3.1.2.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
(block
  (lambda)
  (args
    (shadowarg :local)) nil)

This way is more compatible, but it will complicate parse.y just for ripper. IMO, I like @kddnewton 's PR which is simple enough.

@nobu Do you have any opinion? I will merge this as is unless there is objection.

mame avatar Apr 15 '22 02:04 mame

Thanks for the review @mame! In the future I will definitely add the diff.

BTW, I found this difference by trying to make https://github.com/ruby-syntax-tree/syntax_tree-parser_translator. There appear to be one or two other differences (it looks like parser has different precedence for unary + on number literals), but this is the first one that I couldn't get around.

kddnewton avatar Apr 15 '22 14:04 kddnewton

As Ripper is extensible by user classes inheriting Ripper, and can't know the dispatched results, I doubt that the way like "parser" gem, add shadowargs to params, is possible.

nobu avatar Apr 16 '22 00:04 nobu

@mame @nobu I just rebased this to make sure it's up to date. Anything else I can do to get this merged?

kddnewton avatar May 12 '22 13:05 kddnewton