[GR-18163] Struct#values_at: fix validation and raise IndexError/Rang…

…eError when passed index argument is out of range

PullRequest: truffleruby/3549

CHANGELOG.md

@@ -27,6 +27,7 @@ Compatibility:
 * Add `#public?`, `#private?` and `#protected?` methods for `Method` and `UnboundMethod` classes (@andrykonchin).
 * Add optional argument to `Thread::Queue.new` (@andrykonchin).
 * Support a module as the second argument of `Kernel#load` (@andrykonchin).
+* Improve argument validation in `Struct#valies_at` - raise `IndexError` or `RangeError` when arguments are out of range (#2773, @andrykonchin).
 
 Performance:
 

spec/ruby/core/array/values_at_spec.rb

@@ -1,6 +1,7 @@
 require_relative '../../spec_helper'
 require_relative 'fixtures/classes'
 
+# Should be synchronized with core/struct/values_at_spec.rb
 describe "Array#values_at" do
   it "returns an array of elements at the indexes when passed indexes" do
     [1, 2, 3, 4, 5].values_at().should == []

spec/ruby/core/struct/values_at_spec.rb

@@ -1,16 +1,59 @@
 require_relative '../../spec_helper'
 require_relative 'fixtures/classes'
 
+# Should be synchronized with core/array/values_at_spec.rb
 describe "Struct#values_at" do
-  it "returns an array of values" do
+  before do
     clazz = Struct.new(:name, :director, :year)
-    movie = clazz.new('Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002)
-    movie.values_at(0, 1).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park']
-    movie.values_at(0..2).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002]
+    @movie = clazz.new('Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002)
+  end
+
+  context "when passed a list of Integers" do
+    it "returns an array containing each value given by one of integers" do
+      @movie.values_at(0, 1).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park']
+    end
+
+    it "raises IndexError if any of integers is out of range" do
+      -> { @movie.values_at(3) }.should raise_error(IndexError, "offset 3 too large for struct(size:3)")
+      -> { @movie.values_at(-4) }.should raise_error(IndexError, "offset -4 too small for struct(size:3)")
+    end
+  end
+
+  context "when passed an integer Range" do
+    it "returns an array containing each value given by the elements of the range" do
+      @movie.values_at(0..2).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002]
+    end
+
+    it "fills with nil values for range elements larger than the structure" do
+      @movie.values_at(0..3).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002, nil]
+    end
+
+    it "raises RangeError if any element of the range is negative and out of range" do
+      -> { @movie.values_at(-4..3) }.should raise_error(RangeError, "-4..3 out of range")
+    end
+
+    it "supports endless Range" do
+      @movie.values_at(0..).should == ["Sympathy for Mr. Vengeance", "Chan-wook Park", 2002]
+    end
+
+    it "supports beginningless Range" do
+      @movie.values_at(..2).should == ["Sympathy for Mr. Vengeance", "Chan-wook Park", 2002]
+    end
+  end
+
+  it "supports multiple integer Ranges" do
+    @movie.values_at(0..2, 1..2).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002, 'Chan-wook Park', 2002]
+  end
+
+  it "supports mixing integer Ranges and Integers" do
+    @movie.values_at(0..2, 2).should == ['Sympathy for Mr. Vengeance', 'Chan-wook Park', 2002, 2002]
+  end
+
+  it "returns a new empty Array if no arguments given" do
+    @movie.values_at().should == []
   end
 
   it "fails when passed unsupported types" do
-    car = StructClasses::Car.new('Ford', 'Ranger')
-    -> { car.values_at('make') }.should raise_error(TypeError)
+    -> { @movie.values_at('make') }.should raise_error(TypeError, "no implicit conversion of String into Integer")
   end
 end

src/main/ruby/truffleruby/core/struct.rb

@@ -373,7 +373,31 @@ def deconstruct_keys(keys)
   end
 
   def values_at(*args)
-    to_a.values_at(*args)
+    out = []
+
+    args.each do |elem|
+      if Primitive.object_kind_of?(elem, Range)
+        start, length = Primitive.range_normalized_start_length(elem, size)
+        finish = start + length - 1
+
+        raise RangeError, "#{elem} out of range" if start < 0
+        next if finish < start
+
+        finish_in_bounds = [finish, _attrs.length - 1].min
+        start.upto(finish_in_bounds) do |index|
+          name = check_index_var(index)
+          out << Primitive.object_hidden_var_get(self, name)
+        end
+
+        (finish_in_bounds + 1).upto(finish) { out << nil }
+      else
+        index = Primitive.rb_num2int(elem)
+        name = check_index_var(index)
+        out << Primitive.object_hidden_var_get(self, name)
+      end
+    end
+
+    out
   end
 
   private def polyglot_read_member(name)