From ec879e7eac89dd7ef4f67eb737b3a633a96e280f Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Mon, 1 Apr 2024 13:48:09 -0400 Subject: [ruby/prism] Use RubyVM::InstructionSequence instead of Ripper for validity check https://github.com/ruby/prism/commit/ddec1c163d --- test/prism/errors_test.rb | 138 ++++++++++++++++++++++++++++------------------ 1 file changed, 83 insertions(+), 55 deletions(-) diff --git a/test/prism/errors_test.rb b/test/prism/errors_test.rb index d9dedc88e1..856e46fb76 100644 --- a/test/prism/errors_test.rb +++ b/test/prism/errors_test.rb @@ -1130,28 +1130,24 @@ module Prism end def test_duplicated_parameter_names - # For some reason, Ripper reports no error for Ruby 3.0 when you have - # duplicated parameter names for positional parameters. - unless RUBY_VERSION < "3.1.0" - expected = DefNode( - :foo, - Location(), - nil, - ParametersNode([RequiredParameterNode(0, :a), RequiredParameterNode(0, :b), RequiredParameterNode(ParameterFlags::REPEATED_PARAMETER, :a)], [], nil, [], [], nil, nil), - nil, - [:a, :b], - Location(), - nil, - Location(), - Location(), - nil, - Location() - ) + expected = DefNode( + :foo, + Location(), + nil, + ParametersNode([RequiredParameterNode(0, :a), RequiredParameterNode(0, :b), RequiredParameterNode(ParameterFlags::REPEATED_PARAMETER, :a)], [], nil, [], [], nil, nil), + nil, + [:a, :b], + Location(), + nil, + Location(), + Location(), + nil, + Location() + ) - assert_errors expected, "def foo(a,b,a);end", [ - ["duplicated argument name", 12..13] - ] - end + assert_errors expected, "def foo(a,b,a);end", [ + ["duplicated argument name", 12..13] + ] expected = DefNode( :foo, @@ -1370,7 +1366,7 @@ module Prism source = "-> { _1 + -> { _2 } }" errors = [["numbered parameter is already used in outer scope", 15..17]] - assert_errors expression(source), source, errors, compare_ripper: false + assert_errors expression(source), source, errors end def test_invalid_number_underscores @@ -1436,7 +1432,7 @@ module Prism ["unexpected name for a parameter", 8..10], ["unexpected name for a parameter", 11..13] ] - assert_errors expression(source), source, errors, compare_ripper: false + assert_errors expression(source), source, errors end def test_class_name @@ -1488,11 +1484,10 @@ module Prism def test_repeated_parameter_name_in_destructured_params source = "def f(a, (b, (a))); end" - # In Ruby 3.0.x, `Ripper.sexp_raw` does not return `nil` for this case. - compare_ripper = RUBY_ENGINE == "ruby" && (RUBY_VERSION.split('.').map { |x| x.to_i } <=> [3, 1]) >= 1 + assert_errors expression(source), source, [ ["duplicated argument name", 14..15], - ], compare_ripper: compare_ripper + ] end def test_assign_to_numbered_parameter @@ -1574,6 +1569,7 @@ module Prism 1 => ^(if 1; (return) else (return) end) 1 => ^(unless 1; (return) else (return) end) RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 7..13], @@ -1584,7 +1580,7 @@ module Prism [message, 97..103], [message, 123..129], [message, 168..174], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_statement @@ -1607,6 +1603,7 @@ module Prism for x in (return) end RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 4..10], @@ -1617,7 +1614,7 @@ module Prism [message, 110..116], [message, 132..138], [message, 154..160], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_def @@ -1629,12 +1626,13 @@ module Prism def x(a: return) end RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 5..11], [message, 29..35], [message, 50..56], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_assignment @@ -1644,13 +1642,14 @@ module Prism a, b = return, 1 a, b = 1, *return RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 4..10], [message, 18..24], [message, 32..38], [message, 53..59], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_modifier @@ -1662,6 +1661,7 @@ module Prism (return) => a (return) in a RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 6..12], @@ -1670,7 +1670,7 @@ module Prism [message, 58..64], [message, 67..73], [message, 81..87] - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_expression @@ -1685,6 +1685,7 @@ module Prism ((return)..) ((return)...) RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 1..7], @@ -1696,7 +1697,7 @@ module Prism [message, 85..91], [message, 96..102], [message, 109..115] - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_array @@ -1709,6 +1710,7 @@ module Prism [ *return ] [ **return ] RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 1..7], @@ -1718,7 +1720,7 @@ module Prism [message, 58..64], [message, 70..76], [message, 83..89], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_hash @@ -1728,13 +1730,14 @@ module Prism { a: return } { **return } RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 2..8], [message, 23..29], [message, 37..43], [message, 50..56], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_call @@ -1745,6 +1748,7 @@ module Prism (return)[1] = 2 (return)::foo RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 1..7], @@ -1752,7 +1756,7 @@ module Prism [message, 27..33], [message, 39..45], [message, 55..61], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_constant_path @@ -1760,11 +1764,12 @@ module Prism (return)::A class (return)::A; end RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 1..7], [message, 19..25], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_arguments @@ -1778,6 +1783,7 @@ module Prism foo(:a => return) foo(a: return) RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 4..10], @@ -1788,7 +1794,7 @@ module Prism [message, 71..77], [message, 94..100], [message, 109..115], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_unary_call @@ -1796,11 +1802,12 @@ module Prism +(return) not return RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 2..8], [message, 14..20], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_void_value_expression_in_binary_call @@ -1812,13 +1819,14 @@ module Prism 1 or (return) (return) or 1 RUBY + message = 'unexpected void value expression' assert_errors expression(source), source, [ [message, 5..11], [message, 14..20], [message, 42..48], [message, 71..77], - ], compare_ripper: false # Ripper does not check 'void value expression'. + ] end def test_trailing_comma_in_calls @@ -1934,13 +1942,14 @@ module Prism proc { |foo = foo| } proc { |foo: foo| } RUBY + message = 'parameter default value references itself' assert_errors expression(source), source, [ [message, 14..17], [message, 37..40], [message, 61..64], [message, 81..84], - ], compare_ripper: false # Ripper does not check 'circular reference'. + ] end def test_command_calls @@ -1976,8 +1985,9 @@ module Prism begin; rescue a b; end begin; rescue a b => c; end RUBY + sources.each do |source| - assert_nil Ripper.sexp_raw(source) + refute_valid_syntax(source) assert_false(Prism.parse(source).success?) end end @@ -1993,8 +2003,9 @@ module Prism 1.. % 2 1.. ** 2 RUBY + sources.each do |source| - assert_nil Ripper.sexp_raw(source) + refute_valid_syntax(source) assert_false(Prism.parse(source).success?) end end @@ -2050,21 +2061,21 @@ module Prism source = 'foo(&1) { }' assert_errors expression(source), source, [ ['multiple block arguments; only one block is allowed', 8..11] - ], compare_ripper: false # Ripper does not check 'both block arg and actual block given'. + ] end def test_forwarding_arg_and_block source = 'def foo(...) = foo(...) { }' assert_errors expression(source), source, [ ['both a block argument and a forwarding argument; only one block is allowed', 24..27] - ], compare_ripper: false # Ripper does not check 'both block arg and actual block given'. + ] end def test_it_with_ordinary_parameter source = "proc { || it }" errors = [["`it` is not allowed when an ordinary parameter is defined", 10..12]] - assert_errors expression(source), source, errors, compare_ripper: false + assert_errors expression(source), source, errors end def test_regular_expression_with_unknown_regexp_options @@ -2124,7 +2135,7 @@ module Prism ["cannot define singleton method for literals", 380..388], ["cannot define singleton method for literals", 404..407] ] - assert_errors expression(source), source, errors, compare_ripper: false + assert_errors expression(source), source, errors end def test_assignment_to_literal_in_conditionals @@ -2169,20 +2180,37 @@ module Prism () => [A => a, {a: b => a}] RUBY - assert_error_messages source, Array.new(source.lines.length, "duplicated variable name"), compare_ripper: false + assert_error_messages source, Array.new(source.lines.length, "duplicated variable name") end def test_duplicate_pattern_hash_key assert_error_messages "() => {a:, a:}", ["duplicated key name", "duplicated variable name"] assert_error_messages "() => {a:1, a:2}", ["duplicated key name"] - refute_error_messages "() => [{a:1}, {a:2}]" + refute_error_messages "case (); in [{a:1}, {a:2}]; end" end private - def assert_errors(expected, source, errors, compare_ripper: RUBY_ENGINE == "ruby") - # Ripper behaves differently on JRuby/TruffleRuby, so only check this on CRuby - assert_nil Ripper.sexp_raw(source) if compare_ripper + def check_syntax(source) + $VERBOSE, previous = nil, $VERBOSE + + begin + RubyVM::InstructionSequence.compile(source) + ensure + $VERBOSE = previous + end + end + + def assert_valid_syntax(source) + check_syntax(source) + end + + def refute_valid_syntax(source) + assert_raise(SyntaxError) { check_syntax(source) } + end + + def assert_errors(expected, source, errors) + refute_valid_syntax(source) if RUBY_ENGINE == "ruby" result = Prism.parse(source) node = result.value.statements.body.last @@ -2191,14 +2219,14 @@ module Prism assert_equal(errors, result.errors.map { |e| [e.message, e.location.start_offset..e.location.end_offset] }) end - def assert_error_messages(source, errors, compare_ripper: RUBY_ENGINE == "ruby") - assert_nil Ripper.sexp_raw(source) if compare_ripper + def assert_error_messages(source, errors) + refute_valid_syntax(source) if RUBY_ENGINE == "ruby" result = Prism.parse(source) assert_equal(errors, result.errors.map(&:message)) end - def refute_error_messages(source, compare_ripper: RUBY_ENGINE == "ruby") - refute_nil Ripper.sexp_raw(source) if compare_ripper + def refute_error_messages(source) + assert_valid_syntax(source) if RUBY_ENGINE == "ruby" assert Prism.parse_success?(source) end -- cgit v1.2.3