From 4344b70ed588576222fa253fc1701a9413440def Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 5 Mar 2024 21:22:45 -0500 Subject: [PATCH] Fix up else in implicit begin for ripper translation --- lib/prism/translation/ripper.rb | 160 ++++++++------------------------ test/prism/ripper_test.rb | 10 -- 2 files changed, 37 insertions(+), 133 deletions(-) diff --git a/lib/prism/translation/ripper.rb b/lib/prism/translation/ripper.rb index f4f73ac1bd6..a64160c51f5 100644 --- a/lib/prism/translation/ripper.rb +++ b/lib/prism/translation/ripper.rb @@ -176,22 +176,13 @@ def self.sexp(source) # alias foo bar # ^^^^^^^^^^^^^ def visit_alias_method_node(node) - new_name = visit_alias_method_node_value(node.new_name) - old_name = visit_alias_method_node_value(node.old_name) + new_name = visit(node.new_name) + old_name = visit(node.old_name) bounds(node.location) on_alias(new_name, old_name) end - # Visit one side of an alias method node. - private def visit_alias_method_node_value(node) - if node.is_a?(SymbolNode) && node.opening_loc.nil? - visit_symbol_literal_node(node, no_symbol_wrapper: true) - else - visit(node) - end - end - # alias $foo $bar # ^^^^^^^^^^^^^^^ def visit_alias_global_variable_node(node) @@ -408,7 +399,20 @@ def visit_begin_node(node) end rescue_clause = visit(node.rescue_clause) - else_clause = visit(node.else_clause) + else_clause = + unless (else_clause_node = node.else_clause).nil? + else_statements = + if else_clause_node.statements.nil? + [nil] + else + body = else_clause_node.statements.body + body.unshift(nil) if source.byteslice(else_clause_node.else_keyword_loc.end_offset...else_clause_node.statements.body[0].location.start_offset).include?(";") + body + end + + bounds(else_clause_node.location) + visit_statements_node_body(else_statements) + end ensure_clause = visit(node.ensure_clause) bounds(node.location) @@ -535,10 +539,10 @@ def visit_break_node(node) bounds(node.location) on_break(on_args_new) else - arguments = visit_arguments(node.arguments.arguments) + arguments = visit(node.arguments) bounds(node.location) - on_break(on_args_add_block(arguments, false)) + on_break(arguments) end end @@ -1171,20 +1175,19 @@ def visit_embedded_variable_node(node) # Visit an EnsureNode node. def visit_ensure_node(node) - if node.statements - # If there are any statements, we need to see if there's a semicolon - # between the ensure and the start of the first statement. - - stmts_val = visit(node.statements) - if node_has_semicolon?(node) - # If there's a semicolon, we need to replace [:stmts_new] with - # [:stmts_add, [:stmts_new], [:void_stmt]]. - stmts_val[1] = on_stmts_add(on_stmts_new, on_void_stmt) + statements = + if node.statements.nil? + [nil] + else + body = node.statements.body + body.unshift(nil) if source.byteslice(node.ensure_keyword_loc.end_offset...body[0].location.start_offset).include?(";") + body end - else - stmts_val = on_stmts_add(on_stmts_new, on_void_stmt) - end - on_ensure(stmts_val) + + statements = visit_statements_node_body(statements) + + bounds(node.location) + on_ensure(statements) end # false @@ -1863,7 +1866,7 @@ def visit_match_write_node(node) # A node that is missing from the syntax tree. This is only used in the # case of a syntax error. def visit_missing_node(node) - raise NoMethodError, __method__ + raise "Cannot visit missing nodes directly." end # module Foo; end @@ -2346,7 +2349,7 @@ def visit_super_node(node) # :foo # ^^^^ def visit_symbol_node(node) - if (opening = node.opening) && opening.match?(/^%s|['"]$/) + if (opening = node.opening)&.match?(/^%s|['"]$/) bounds(node.value_loc) content = on_string_content @@ -2355,6 +2358,9 @@ def visit_symbol_node(node) end on_dyna_symbol(content) + elsif opening.nil? && node.closing_loc.nil? + bounds(node.value_loc) + on_symbol_literal(visit_token(node.value)) else bounds(node.value_loc) on_symbol_literal(on_symbol(visit_token(node.value))) @@ -2371,24 +2377,7 @@ def visit_true_node(node) # undef foo # ^^^^^^^^^ def visit_undef_node(node) - names = - node.names.map do |name| - case name - when SymbolNode - bounds(name.value_loc) - token = visit_token(name.unescaped) - - if name.opening_loc.nil? - on_symbol_literal(token) - else - on_symbol_literal(on_symbol(token)) - end - when InterpolatedSymbolNode - visit(name) - else - raise - end - end + names = visit_all(node.names) bounds(node.location) on_undef(names) @@ -2551,39 +2540,6 @@ def visit_token(token) end end - # Ripper has several methods of emitting a symbol literal. Inside an alias - # sometimes it suppresses the [:symbol] wrapper around ident. If the symbol - # is also the name of a keyword (e.g. :if) it will emit a :@kw wrapper, not - # an :@ident wrapper, with similar treatment for constants and operators. - def visit_symbol_literal_node(node, no_symbol_wrapper: false) - if (opening = node.opening) && (['"', "'"].include?(opening[-1]) || opening.start_with?("%s")) - bounds(node.value_loc) - str_val = node.value.to_s - if str_val == "" - return on_dyna_symbol(on_string_content) - else - tstring_val = on_tstring_content(str_val) - return on_dyna_symbol(on_string_add(on_string_content, tstring_val)) - end - end - - bounds(node.value_loc) - node_name = node.value.to_s - if RUBY_KEYWORDS.include?(node_name) - token_val = on_kw(node_name) - elsif node_name.length == 0 - raise NoMethodError, __method__ - elsif /[[:upper:]]/.match(node_name[0]) - token_val = on_const(node_name) - elsif /[[:punct:]]/.match(node_name[0]) - token_val = on_op(node_name) - else - token_val = on_ident(node_name) - end - sym_val = no_symbol_wrapper ? token_val : on_symbol(token_val) - on_symbol_literal(sym_val) - end - # Visit a node that represents a number. We need to explicitly handle the # unary - operator. def visit_number_node(node) @@ -2595,55 +2551,13 @@ def visit_number_node(node) value = yield slice[1..-1] bounds(node.location) - on_unary(visit_unary_operator(:-@), value) + on_unary(:-@, value) else bounds(location) yield slice end end - if RUBY_ENGINE == "jruby" && Gem::Version.new(JRUBY_VERSION) < Gem::Version.new("9.4.6.0") - # JRuby before 9.4.6.0 uses :- for unary minus instead of :-@ - def visit_unary_operator(value) - value == :-@ ? :- : value - end - else - # For most Rubies and JRuby after 9.4.6.0 this is a no-op. - def visit_unary_operator(value) - value - end - end - - # Some nodes, such as `begin`, `ensure` and `do` may have a semicolon - # after the keyword and before the first statement. This affects - # Ripper's return values. - def node_has_semicolon?(node) - first_field, second_field = case node - when BeginNode - [:begin_keyword_loc, :statements] - when EnsureNode - [:ensure_keyword_loc, :statements] - when BlockNode - [:opening_loc, :body] - else - raise NoMethodError, __method__ - end - first_offs, second_offs = delimiter_offsets_for(node, first_field, second_field) - - # We need to know if there's a semicolon after the keyword, but before - # the start of the first statement in the ensure. - source.byteslice(first_offs..second_offs).include?(";") - end - - # For a given node, grab the offsets for the end of the first field - # and the beginning of the second field. - def delimiter_offsets_for(node, first, second) - first_field = node.send(first) - first_end_loc = first_field.start_offset + first_field.length - second_begin_loc = node.send(second).body[0].location.start_offset - 1 - [first_end_loc, second_begin_loc] - end - # This method is responsible for updating lineno and column information # to reflect the current node. # diff --git a/test/prism/ripper_test.rb b/test/prism/ripper_test.rb index 153844e37a8..a7589faeedd 100644 --- a/test/prism/ripper_test.rb +++ b/test/prism/ripper_test.rb @@ -12,7 +12,6 @@ class RipperTest < TestCase skips = %w[ arrays.txt - begin_rescue.txt blocks.txt case.txt classes.txt @@ -36,14 +35,11 @@ class RipperTest < TestCase multi_write.txt non_alphanumeric_methods.txt patterns.txt - procs.txt regex.txt regex_char_width.txt repeat_parameters.txt rescue.txt seattlerb/TestRubyParserShared.txt - seattlerb/begin_rescue_else_ensure_bodies.txt - seattlerb/begin_rescue_else_ensure_no_bodies.txt seattlerb/block_break.txt seattlerb/block_call_dot_op2_brace_block.txt seattlerb/block_command_operation_colon.txt @@ -57,7 +53,6 @@ class RipperTest < TestCase seattlerb/block_return.txt seattlerb/bug190.txt seattlerb/bug191.txt - seattlerb/bug_215.txt seattlerb/bug_comma.txt seattlerb/bug_hash_args_trailing_comma.txt seattlerb/bug_hash_interp_array.txt @@ -183,7 +178,6 @@ class RipperTest < TestCase symbols.txt ternary_operator.txt tilde_heredocs.txt - undef.txt unescaping.txt unless.txt unparser/corpus/literal/assignment.txt @@ -208,7 +202,6 @@ class RipperTest < TestCase unparser/corpus/literal/while.txt unparser/corpus/literal/yield.txt unparser/corpus/semantic/dstr.txt - unparser/corpus/semantic/kwbegin.txt unparser/corpus/semantic/literal.txt unparser/corpus/semantic/while.txt until.txt @@ -233,7 +226,6 @@ class RipperTest < TestCase whitequark/bug_ascii_8bit_in_literal.txt whitequark/bug_do_block_in_hash_brace.txt whitequark/bug_heredoc_do.txt - whitequark/bug_rescue_empty_else.txt whitequark/case_cond_else.txt whitequark/case_expr_else.txt whitequark/character.txt @@ -269,8 +261,6 @@ class RipperTest < TestCase whitequark/parser_slash_slash_n_escaping_in_literals.txt whitequark/pattern_matching_blank_else.txt whitequark/pattern_matching_else.txt - whitequark/rescue_else.txt - whitequark/rescue_else_ensure.txt whitequark/rescue_without_begin_end.txt whitequark/return_block.txt whitequark/ruby_bug_10653.txt