Bug #16008
closedRipper hshptn node drops information
Description
Hi - I'm the author of the prettier plugin for ruby. I'm trying to get it to support 2.7 pattern matching. It works well for everything to do with the array pattern. However, for hash patterns, it drops certain information if you touch the on_label
method (which I do to add metadata about the location of the token). For example,
require 'ripper'
class Parser < Ripper::SexpBuilder
private
def on_label(token)
[:@label, token]
end
end
content = <<~EORUBY
case foo
in { a: 1 }
bar
else
baz
end
EORUBY
pp Ripper::SexpBuilder.new(content).parse
pp Parser.new(content).parse
This script results in:
[:program,
[:stmts_add,
[:stmts_new],
[:case,
[:vcall, [:@ident, "foo", [1, 5]]],
[:in,
[:hshptn, nil, [[[:@label, "a:", [2, 5]], [:@int, "1", [2, 8]]]], nil],
[:stmts_add, [:stmts_new], [:vcall, [:@ident, "bar", [3, 2]]]],
[:else, [:stmts_add, [:stmts_new], [:vcall, [:@ident, "baz", [5, 2]]]]]]]]]
[:program,
[:stmts_add,
[:stmts_new],
[:case,
[:vcall, [:@ident, "foo", [1, 5]]],
[:in,
[:hshptn, nil, nil, nil],
[:stmts_add, [:stmts_new], [:vcall, [:@ident, "bar", [3, 2]]]],
[:else, [:stmts_add, [:stmts_new], [:vcall, [:@ident, "baz", [5, 2]]]]]]]]]
As you can see, in the second example the hashptn
node has no label on it. I'm not entirely sure what's going on there. Any insight would be appreciated.
Files
Updated by ko1 (Koichi Sasada) over 5 years ago
- Assignee set to nobu (Nobuyoshi Nakada)
Updated by jeremyevans0 (Jeremy Evans) about 5 years ago
I believe this issue comes from new_unique_key_hash
, which expects the kw_args
member of the hshptn
node to be in a certain format (see https://github.com/ruby/ruby/blob/master/parse.y#L811). If you modify the size of the array, it can't recognize the results to determine if the hash is unique, and treats it as an error, which results in a nil
value being used.
You can currently work around this by including your metadata as an extra member of the location array:
def on_label(token)
super.tap{|a| a.last.push(:metadata=>nil)}
end
It may be better to return the array as-is in case the format doesn't match what is expected, and only treat actual duplicate keys as errors. The attached patch implements that approach.
Updated by nobu (Nobuyoshi Nakada) about 5 years ago
It works in this case, but not generic.
My WIP patch is https://github.com/nobu/ruby/tree/ripper.value
Updated by nobu (Nobuyoshi Nakada) almost 5 years ago
- Status changed from Open to Closed
- Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN to 2.5: DONTNEED, 2.6: DONTNEED
Updated by nobu (Nobuyoshi Nakada) 11 months ago
- Related to Bug #20055: Ripper seems to skip some checks like `void value expression` and `duplicated variable name` added