From 17a66bdf5712686fa8eca4a7bd10ffab45c1d754 Mon Sep 17 00:00:00 2001 From: Ilya Bylich Date: Tue, 18 Jun 2019 00:16:31 +0300 Subject: [PATCH] - builder.rb, processor.rb: Changed format of the procarg0 node. (#587) Previously a single non-mlhs argument was emitted as `s(:procarg0, :a)`, this commit introduces a compatibility flag `Parser::Builders::Default.emit_arg_inside_procarg0` that causes a single argument to be emitted as `s(:procarg0, s(:arg, :a))`. --- doc/AST_FORMAT.md | 9 ++++-- lib/parser/ast/processor.rb | 17 ++++++++++- lib/parser/builders/default.rb | 55 +++++++++++++++++++++++++++------- test/test_parser.rb | 39 ++++++++++++++++++++++-- 4 files changed, 104 insertions(+), 16 deletions(-) diff --git a/doc/AST_FORMAT.md b/doc/AST_FORMAT.md index 54af5f80e..b3a878273 100644 --- a/doc/AST_FORMAT.md +++ b/doc/AST_FORMAT.md @@ -933,10 +933,15 @@ the sole argument (e.g. `|foo,|`). Format: ~~~ -(procarg0 :foo) +(procarg0 (arg :foo)) "|foo|" ~~~ expression - ~~~ name + +(procarg0 (arg :foo) (arg :bar)) +"|(foo, bar)|" + ~ begin + ~ end + ~~~~~~~~~~ expression ~~~ ### Expression arguments diff --git a/lib/parser/ast/processor.rb b/lib/parser/ast/processor.rb index 2c04546e0..a4d73f08a 100644 --- a/lib/parser/ast/processor.rb +++ b/lib/parser/ast/processor.rb @@ -124,7 +124,22 @@ def process_argument_node(node) alias on_kwarg process_argument_node alias on_kwoptarg process_argument_node alias on_kwrestarg process_argument_node - alias on_procarg0 process_argument_node + + def on_procarg0(node) + if node.children[0].is_a?(Symbol) + # This branch gets executed when the builder + # is not configured to emit and 'arg' inside 'procarg0', i.e. when + # Parser::Builders::Default.emit_arg_inside_procarg0 + # is set to false. + # + # If this flag is set to true this branch is unreachable. + # s(:procarg0, :a) + on_argument(node) + else + # s(:procarg0, s(:arg, :a), s(:arg, :b)) + process_regular_node(node) + end + end alias on_arg_expr process_regular_node alias on_restarg_expr process_regular_node diff --git a/lib/parser/builders/default.rb b/lib/parser/builders/default.rb index 851cae000..8fd94dacf 100644 --- a/lib/parser/builders/default.rb +++ b/lib/parser/builders/default.rb @@ -80,6 +80,21 @@ class << self attr_accessor :emit_index end + class << self + ## + # AST compatibility attribute; causes a single non-mlhs + # block argument to be wrapped in s(:procarg0). + # + # If set to false (the default), block arguments `|a|` are emitted as + # `s(:args, s(:procarg0, :a))` + # + # If set to true, block arguments `|a|` are emitted as + # `s(:args, s(:procarg0, s(:arg, :a))` + # + # @return [Boolean] + attr_accessor :emit_arg_inside_procarg0 + end + @emit_index = false class << self @@ -90,6 +105,7 @@ def modernize @emit_procarg0 = true @emit_encoding = true @emit_index = true + @emit_arg_inside_procarg0 = true end end @@ -726,7 +742,12 @@ def blockarg(amper_t, name_t) def procarg0(arg) if self.class.emit_procarg0 - arg.updated(:procarg0) + if arg.type == :arg && self.class.emit_arg_inside_procarg0 + n(:procarg0, [ arg ], + Source::Map::Collection.new(nil, nil, arg.location.expression)) + else + arg.updated(:procarg0) + end else arg end @@ -1232,18 +1253,18 @@ def check_duplicate_args(args, map={}) case this_arg.type when :arg, :optarg, :restarg, :blockarg, :kwarg, :kwoptarg, :kwrestarg, - :shadowarg, :procarg0 + :shadowarg - this_name, = *this_arg + check_duplicate_arg(this_arg, map) - that_arg = map[this_name] - that_name, = *that_arg + when :procarg0 - if that_arg.nil? - map[this_name] = this_arg - elsif arg_name_collides?(this_name, that_name) - diagnostic :error, :duplicate_argument, nil, - this_arg.loc.name, [ that_arg.loc.name ] + if this_arg.children[0].is_a?(Symbol) + # s(:procarg0, :a) + check_duplicate_arg(this_arg, map) + else + # s(:procarg0, s(:arg, :a), ...) + check_duplicate_args(this_arg.children, map) end when :mlhs @@ -1252,6 +1273,20 @@ def check_duplicate_args(args, map={}) end end + def check_duplicate_arg(this_arg, map={}) + this_name, = *this_arg + + that_arg = map[this_name] + that_name, = *that_arg + + if that_arg.nil? + map[this_name] = this_arg + elsif arg_name_collides?(this_name, that_name) + diagnostic :error, :duplicate_argument, nil, + this_arg.loc.name, [ that_arg.loc.name ] + end + end + def arg_name_collides?(this_name, that_name) case @parser.version when 18 diff --git a/test/test_parser.rb b/test/test_parser.rb index 5f3d8f45d..6ca191b19 100644 --- a/test/test_parser.rb +++ b/test/test_parser.rb @@ -2457,7 +2457,7 @@ def test_block_arg_combinations # f_arg opt_f_block_arg # f_arg tCOMMA assert_parses_blockargs( - s(:args, s(:procarg0, :a)), + s(:args, s(:procarg0, s(:arg, :a))), %q{|a|}, SINCE_1_9) @@ -2700,6 +2700,39 @@ def test_procarg0_legacy Parser::Builders::Default.emit_procarg0 = true end + def test_emit_arg_inside_procarg0_legacy + Parser::Builders::Default.emit_arg_inside_procarg0 = false + assert_parses_blockargs( + s(:args, + s(:procarg0, :a)), + %q{|a|}, + SINCE_1_9) + ensure + Parser::Builders::Default.emit_arg_inside_procarg0 = true + end + + def test_procarg0 + assert_parses( + s(:block, + s(:send, nil, :m), + s(:args, + s(:procarg0, s(:arg, :foo))), nil), + %q{m { |foo| } }, + %q{ ^^^ expression (args.procarg0)}, + SINCE_1_9) + + assert_parses( + s(:block, + s(:send, nil, :m), + s(:args, + s(:procarg0, s(:arg, :foo), s(:arg, :bar))), nil), + %q{m { |(foo, bar)| } }, + %q{ ^ begin (args.procarg0) + | ^ end (args.procarg0) + | ^^^^^^^^^^ expression (args.procarg0)}, + SINCE_1_9) + end + def test_block_kwarg_combinations # f_block_kwarg tCOMMA f_kwrest opt_f_block_arg assert_parses_blockargs( @@ -5675,7 +5708,7 @@ def test_ruby_bug_10653 s(:send, s(:int, 1), :tap), s(:args, - s(:procarg0, :n)), + s(:procarg0, s(:arg, :n))), s(:send, nil, :p, s(:lvar, :n))), s(:int, 0)), @@ -6286,7 +6319,7 @@ def test_parser_bug_272 s(:send, nil, :a, s(:ivar, :@b)), s(:args, - s(:procarg0, :c)), nil), + s(:procarg0, s(:arg, :c))), nil), %q{a @b do |c|;end}, %q{}, SINCE_1_9)