diff --git a/CHANGELOG.md b/CHANGELOG.md index 32488601a974..d57c2f5013de 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/spec/ruby/core/array/values_at_spec.rb b/spec/ruby/core/array/values_at_spec.rb index 2c6fd16947cb..e85bbee400bf 100644 --- a/spec/ruby/core/array/values_at_spec.rb +++ b/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 == [] diff --git a/spec/ruby/core/struct/values_at_spec.rb b/spec/ruby/core/struct/values_at_spec.rb index e7d287cba22a..5e5a496600f7 100644 --- a/spec/ruby/core/struct/values_at_spec.rb +++ b/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 diff --git a/src/main/ruby/truffleruby/core/struct.rb b/src/main/ruby/truffleruby/core/struct.rb index c55752d3f410..1ab529fe46d7 100644 --- a/src/main/ruby/truffleruby/core/struct.rb +++ b/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)