Skip to content

Commit

Permalink
Fix up else in implicit begin for ripper translation
Browse files Browse the repository at this point in the history
  • Loading branch information
@kddnewton
kddnewton committed Mar 6, 2024
1 parent c00902a commit 4344b70
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 133 deletions.
160 changes: 37 additions & 123 deletions lib/prism/translation/ripper.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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)))
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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.
#
Expand Down
10 changes: 0 additions & 10 deletions test/prism/ripper_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ class RipperTest < TestCase

skips = %w[
arrays.txt
begin_rescue.txt
blocks.txt
case.txt
classes.txt
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4344b70

Please sign in to comment.