Repository: ruby/yarp
Pull request #1218 (2023-08-09 13:16:50)

These should not use YP_NODE_TYPE_P since they are checking operators


Patch:
diff --git a/Gemfile b/Gemfile
index 80c6ddbe..a3b38c74 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,6 +2,8 @@
 
 source "https://rubygems.org"
 
+ruby ">= 3.1.0"
+
 gemspec
 
 gem "rake"
diff --git a/Gemfile.lock b/Gemfile.lock
index a48fbf23..b5bea076 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    yarp (0.6.0)
+    yarp (0.4.0)
 
 GEM
   remote: https://rubygems.org/
@@ -30,5 +30,8 @@ DEPENDENCIES
   test-unit
   yarp!
 
+RUBY VERSION
+   ruby 3.1.0p0
+
 BUNDLED WITH
    2.3.6
diff --git a/ext/yarp/extension.h b/ext/yarp/extension.h
index fe9530d6..c8f950c2 100644
--- a/ext/yarp/extension.h
+++ b/ext/yarp/extension.h
@@ -5,7 +5,7 @@
 #include <ruby/encoding.h>
 #include "yarp.h"
 
-#define EXPECTED_YARP_VERSION "0.6.0"
+#define EXPECTED_YARP_VERSION "0.4.0"
 
 VALUE yp_source_new(yp_parser_t *parser);
 VALUE yp_token_new(yp_parser_t *parser, yp_token_t *token, rb_encoding *encoding, VALUE source);
diff --git a/include/yarp/version.h b/include/yarp/version.h
index 8ecde686..d8d9b024 100644
--- a/include/yarp/version.h
+++ b/include/yarp/version.h
@@ -1,5 +1,5 @@
 #define YP_VERSION_MAJOR 0
-#define YP_VERSION_MINOR 6
+#define YP_VERSION_MINOR 4
 #define YP_VERSION_PATCH 0
 
-#define YP_VERSION "0.6.0"
+#define YP_VERSION "0.4.0"
diff --git a/src/yarp.c b/src/yarp.c
index 0fa60351..ba909759 100644
--- a/src/yarp.c
+++ b/src/yarp.c
@@ -1419,7 +1419,7 @@ yp_class_node_create(yp_parser_t *parser, yp_constant_id_list_t *locals, const y
 static yp_class_variable_operator_and_write_node_t *
 yp_class_variable_operator_and_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_CLASS_VARIABLE_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL));
+    assert(operator->type == YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL);
     yp_class_variable_operator_and_write_node_t *node = YP_ALLOC_NODE(parser, yp_class_variable_operator_and_write_node_t);
 
     *node = (yp_class_variable_operator_and_write_node_t) {
@@ -1464,7 +1464,7 @@ yp_class_variable_operator_write_node_create(yp_parser_t *parser, yp_node_t *tar
 static yp_class_variable_operator_or_write_node_t *
 yp_class_variable_operator_or_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_CLASS_VARIABLE_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_PIPE_PIPE_EQUAL));
+    assert(operator->type == YP_TOKEN_PIPE_PIPE_EQUAL);
     yp_class_variable_operator_or_write_node_t *node = YP_ALLOC_NODE(parser, yp_class_variable_operator_or_write_node_t);
 
     *node = (yp_class_variable_operator_or_write_node_t) {
@@ -1625,7 +1625,7 @@ yp_constant_path_write_node_create(yp_parser_t *parser, yp_constant_path_node_t
 static yp_constant_operator_and_write_node_t *
 yp_constant_operator_and_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_CONSTANT_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL));
+    assert(operator->type == YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL);
     yp_constant_operator_and_write_node_t *node = YP_ALLOC_NODE(parser, yp_constant_operator_and_write_node_t);
 
     *node = (yp_constant_operator_and_write_node_t) {
@@ -1670,7 +1670,7 @@ yp_constant_operator_write_node_create(yp_parser_t *parser, yp_node_t *target, c
 static yp_constant_operator_or_write_node_t *
 yp_constant_operator_or_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_CONSTANT_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_PIPE_PIPE_EQUAL));
+    assert(operator->type == YP_TOKEN_PIPE_PIPE_EQUAL);
     yp_constant_operator_or_write_node_t *node = YP_ALLOC_NODE(parser, yp_constant_operator_or_write_node_t);
 
     *node = (yp_constant_operator_or_write_node_t) {
@@ -2127,7 +2127,7 @@ yp_hash_pattern_node_node_list_create(yp_parser_t *parser, yp_node_list_t *assoc
 static yp_global_variable_operator_and_write_node_t *
 yp_global_variable_operator_and_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_GLOBAL_VARIABLE_READ_NODE) || YP_NODE_TYPE_P(target, YP_NODE_BACK_REFERENCE_READ_NODE) || YP_NODE_TYPE_P(target, YP_NODE_NUMBERED_REFERENCE_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL));
+    assert(operator->type == YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL);
     yp_global_variable_operator_and_write_node_t *node = YP_ALLOC_NODE(parser, yp_global_variable_operator_and_write_node_t);
 
     *node = (yp_global_variable_operator_and_write_node_t) {
@@ -2172,7 +2172,7 @@ yp_global_variable_operator_write_node_create(yp_parser_t *parser, yp_node_t *ta
 static yp_global_variable_operator_or_write_node_t *
 yp_global_variable_operator_or_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_GLOBAL_VARIABLE_READ_NODE) || YP_NODE_TYPE_P(target, YP_NODE_BACK_REFERENCE_READ_NODE) || YP_NODE_TYPE_P(target, YP_NODE_NUMBERED_REFERENCE_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_PIPE_PIPE_EQUAL));
+    assert(operator->type == YP_TOKEN_PIPE_PIPE_EQUAL);
     yp_global_variable_operator_or_write_node_t *node = YP_ALLOC_NODE(parser, yp_global_variable_operator_or_write_node_t);
 
     *node = (yp_global_variable_operator_or_write_node_t) {
@@ -2478,7 +2478,7 @@ yp_in_node_create(yp_parser_t *parser, yp_node_t *pattern, yp_statements_node_t
 static yp_instance_variable_operator_and_write_node_t *
 yp_instance_variable_operator_and_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_INSTANCE_VARIABLE_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL));
+    assert(operator->type == YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL);
     yp_instance_variable_operator_and_write_node_t *node = YP_ALLOC_NODE(parser, yp_instance_variable_operator_and_write_node_t);
 
     *node = (yp_instance_variable_operator_and_write_node_t) {
@@ -2523,7 +2523,7 @@ yp_instance_variable_operator_write_node_create(yp_parser_t *parser, yp_node_t *
 static yp_instance_variable_operator_or_write_node_t *
 yp_instance_variable_operator_or_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_INSTANCE_VARIABLE_READ_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_PIPE_PIPE_EQUAL));
+    assert(operator->type == YP_TOKEN_PIPE_PIPE_EQUAL);
     yp_instance_variable_operator_or_write_node_t *node = YP_ALLOC_NODE(parser, yp_instance_variable_operator_or_write_node_t);
 
     *node = (yp_instance_variable_operator_or_write_node_t) {
@@ -2816,7 +2816,7 @@ yp_lambda_node_create(
 static yp_local_variable_operator_and_write_node_t *
 yp_local_variable_operator_and_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value, yp_constant_id_t constant_id) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_LOCAL_VARIABLE_READ_NODE) || YP_NODE_TYPE_P(target, YP_NODE_CALL_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL));
+    assert(operator->type == YP_TOKEN_AMPERSAND_AMPERSAND_EQUAL);
     yp_local_variable_operator_and_write_node_t *node = YP_ALLOC_NODE(parser, yp_local_variable_operator_and_write_node_t);
 
     *node = (yp_local_variable_operator_and_write_node_t) {
@@ -2863,7 +2863,7 @@ yp_local_variable_operator_write_node_create(yp_parser_t *parser, yp_node_t *tar
 static yp_local_variable_operator_or_write_node_t *
 yp_local_variable_operator_or_write_node_create(yp_parser_t *parser, yp_node_t *target, const yp_token_t *operator, yp_node_t *value, yp_constant_id_t constant_id) {
     assert(YP_NODE_TYPE_P(target, YP_NODE_LOCAL_VARIABLE_READ_NODE) || YP_NODE_TYPE_P(target, YP_NODE_CALL_NODE));
-    assert(YP_NODE_TYPE_P(operator, YP_TOKEN_PIPE_PIPE_EQUAL));
+    assert(operator->type == YP_TOKEN_PIPE_PIPE_EQUAL);
     yp_local_variable_operator_or_write_node_t *node = YP_ALLOC_NODE(parser, yp_local_variable_operator_or_write_node_t);
 
     *node = (yp_local_variable_operator_or_write_node_t) {
diff --git a/templates/java/org/yarp/Loader.java.erb b/templates/java/org/yarp/Loader.java.erb
index c6a437d6..3776f335 100644
--- a/templates/java/org/yarp/Loader.java.erb
+++ b/templates/java/org/yarp/Loader.java.erb
@@ -57,7 +57,7 @@ public class Loader {
         expect((byte) 'P');
 
         expect((byte) 0);
-        expect((byte) 6);
+        expect((byte) 4);
         expect((byte) 0);
 
         // This loads the name of the encoding. We don't actually do anything
diff --git a/templates/lib/yarp/serialize.rb.erb b/templates/lib/yarp/serialize.rb.erb
index fcdf131b..e3a173cb 100644
--- a/templates/lib/yarp/serialize.rb.erb
+++ b/templates/lib/yarp/serialize.rb.erb
@@ -1,16 +1,5 @@
 require "stringio"
 
-# Polyfill for String#unpack1 with the offset parameter.
-if String.instance_method(:unpack1).parameters.none? { |_, name| name == :offset }
-  String.prepend(
-    Module.new {
-      def unpack1(format, offset: 0)
-        offset == 0 ? super(format) : self[offset..].unpack1(format)
-      end
-    }
-  )
-end
-
 module YARP
   module Serialize
     def self.load(input, serialized)
@@ -40,8 +29,8 @@ module YARP
       end
 
       def load
-        raise "Invalid serialization" if io.read(4) != "YARP"
-        raise "Invalid serialization" if io.read(3).unpack("C3") != [0, 6, 0]
+        io.read(4) => "YARP"
+        io.read(3).unpack("C3") => [0, 4, 0]
 
         @encoding = Encoding.find(io.read(load_varint))
         @input = input.force_encoding(@encoding).freeze
diff --git a/yarp.gemspec b/yarp.gemspec
index 1e198372..65e95eaa 100644
--- a/yarp.gemspec
+++ b/yarp.gemspec
@@ -2,7 +2,7 @@
 
 Gem::Specification.new do |spec|
   spec.name = "yarp"
-  spec.version = "0.6.0"
+  spec.version = "0.4.0"
   spec.authors = ["Shopify"]
   spec.email = ["ruby@shopify.com"]
 
@@ -10,8 +10,6 @@ Gem::Specification.new do |spec|
   spec.homepage = "https://github.com/ruby/yarp"
   spec.license = "MIT"
 
-  spec.required_ruby_version = ">= 3.0.0"
-
   spec.require_paths = ["lib"]
   spec.files = [
     "CODE_OF_CONDUCT.md",


Test patch:
diff --git a/test/errors_test.rb b/test/errors_test.rb
index 8a443d43..9c282a2d 100644
--- a/test/errors_test.rb
+++ b/test/errors_test.rb
@@ -984,27 +984,23 @@ class ErrorsTest < Test::Unit::TestCase
   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(
-        Location(),
-        nil,
-        ParametersNode([RequiredParameterNode(:a), RequiredParameterNode(:b), RequiredParameterNode(:a)], [], [], nil, [], nil, nil),
-        nil,
-        [:a, :b],
-        Location(),
-        nil,
-        Location(),
-        Location(),
-        nil,
-        Location()
-      )
+    expected = DefNode(
+      Location(),
+      nil,
+      ParametersNode([RequiredParameterNode(:a), RequiredParameterNode(:b), RequiredParameterNode(:a)], [], [], nil, [], nil, nil),
+      nil,
+      [:a, :b],
+      Location(),
+      nil,
+      Location(),
+      Location(),
+      nil,
+      Location()
+    )
 
-      assert_errors expected, "def foo(a,b,a);end", [
-        ["Duplicated parameter name.", 12..13]
-      ]
-    end
+    assert_errors expected, "def foo(a,b,a);end", [
+      ["Duplicated parameter name.", 12..13]
+    ]
 
     expected = DefNode(
       Location(),
diff --git a/test/newline_test.rb b/test/newline_test.rb
index 48898a69..8078b9ba 100644
--- a/test/newline_test.rb
+++ b/test/newline_test.rb
@@ -63,9 +63,6 @@ class NewlineTest < Test::Unit::TestCase
           line.include?('(token[2].start_with?("\#$") || token[2].start_with?("\#@"))')
         end
         actual.delete(index + 1)
-      elsif relative == "test/parse_test.rb"
-        line_number = source.lines.index { |line| line.include?("while (node = queue.shift)") } + 1
-        expected.delete_at(expected.index(line_number))
       end
 
       assert_equal expected, actual
diff --git a/test/parse_test.rb b/test/parse_test.rb
index c9e1420a..40e8b2a7 100644
--- a/test/parse_test.rb
+++ b/test/parse_test.rb
@@ -2,10 +2,6 @@
 
 require "yarp_test_helper"
 
-# To accurately compare against the snapshots, we need to make sure that we're
-# running on Ruby 3.2+.
-return if RUBY_VERSION < "3.2.0"
-
 # It is useful to have a diff even if the strings to compare are big
 # However, ruby/ruby does not have a version of Test::Unit with access to
 # max_diff_target_string_size
@@ -29,18 +25,38 @@ class ParseTest < Test::Unit::TestCase
     ignore_warnings { Encoding.default_external = @previous_default_external }
   end
 
+  def test_Ruby_3_2_plus
+    assert_operator RUBY_VERSION, :>=, "3.2.0", "ParseTest requires Ruby 3.2+"
+  end
+
   def test_empty_string
     YARP.parse("") => YARP::ParseResult[value: YARP::ProgramNode[statements: YARP::StatementsNode[body: []]]]
   end
 
-  known_failures = %w[seattlerb/heredoc_nested.txt]
-  known_failures << "seattlerb/pct_w_heredoc_interp_nested.txt" if RUBY_VERSION < "3.3.0"
+  known_failures = %w[
+    seattlerb/heredoc_nested.txt
+  ]
+
+  if RUBY_VERSION < "3.3.0"
+    known_failures << "seattlerb/pct_w_heredoc_interp_nested.txt"
+  end
+
+  def find_source_file_node(node)
+    if node.is_a?(YARP::SourceFileNode)
+      node
+    else
+      node && node.child_nodes.each do |child_node|
+        source_file_node = find_source_file_node(child_node)
+        return source_file_node if source_file_node
+      end
+    end
+  end
 
   def test_parse_takes_file_path
     filepath = "filepath.rb"
-    result = YARP.parse("def foo; __FILE__; end", filepath)
+    parsed_result = YARP.parse("def foo; __FILE__; end", filepath)
 
-    assert_equal filepath, find_source_file_node(result.value).filepath
+    assert_equal filepath, find_source_file_node(parsed_result.value).filepath
   end
 
   base = File.join(__dir__, "fixtures")
@@ -132,14 +148,6 @@ class ParseTest < Test::Unit::TestCase
 
   private
 
-  def find_source_file_node(program)
-    queue = [program]
-    while (node = queue.shift)
-      return node if node.is_a?(YARP::SourceFileNode)
-      queue.concat(node.child_nodes.compact)
-    end
-  end
-
   def ignore_warnings
     previous_verbosity = $VERBOSE
     $VERBOSE = nil