From cc8a312bbc3659241b2f2f61d4bc49a148e4929e Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 15 Nov 2022 18:30:55 +0200 Subject: [PATCH] Fix MatchData#values_at when passed index that is out of range --- CHANGELOG.md | 1 + spec/ruby/core/matchdata/values_at_spec.rb | 73 +++++++++++++++++++--- src/main/ruby/truffleruby/core/regexp.rb | 31 ++++++++- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db928a624012..78dc2df3a268 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Compatibility: * Fix `Kernel#Complex` and raise exception when an argument is formatted incorrectly (#2765, @andrykonchin). * Add `#public?`, `#private?` and `#protected?` methods for `Method` and `UnboundMethod` classes (@andrykonchin). * Add optional argument to `Thread::Queue.new` (@andrykonchin). +* Fix `MatchData#values_at` and handling indices that are out of range (#2783, @andrykonchin). Performance: diff --git a/spec/ruby/core/matchdata/values_at_spec.rb b/spec/ruby/core/matchdata/values_at_spec.rb index 8f7fdf557cb5..4fd0bfc42ab3 100644 --- a/spec/ruby/core/matchdata/values_at_spec.rb +++ b/spec/ruby/core/matchdata/values_at_spec.rb @@ -1,21 +1,76 @@ require_relative '../../spec_helper' -describe "MatchData#values_at" do - it "returns an array of the matching value" do - /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(0, 2, -2).should == ["HX1138", "X", "113"] +describe "Struct#values_at" do + # Should be synchronized with core/array/values_at_spec.rb and core/struct/values_at_spec.rb + # + # /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").to_a # => ["HX1138", "H", "X", "113", "8"] + + context "when passed a list of Integers" do + it "returns an array containing each value given by one of integers" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(0, 2, -2).should == ["HX1138", "X", "113"] + end + + it "returns nil value for any integer that is out of range" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(5).should == [nil] + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(-6).should == [nil] + end end - describe "when passed a Range" do - it "returns an array of the matching value" do - /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(2..4, 0..1).should == ["X", "113", "8", "HX1138", "H"] + context "when passed an integer Range" do + it "returns an array containing each value given by the elements of the range" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(0..2).should == ["HX1138", "H", "X"] + end + + it "fills with nil values for range elements larger than the captured values number" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(0..5).should == ["HX1138", "H", "X", "113", "8", nil] + end + + it "raises RangeError if any element of the range is negative and out of range" do + -> { /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(-6..3) }.should raise_error(RangeError, "-6..3 out of range") + end + + it "supports endless Range" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(0..).should == ["HX1138", "H", "X", "113", "8"] + end + + it "supports beginningless Range" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(0..2).should == ["HX1138", "H", "X"] + end + + it "returns an empty Array when Range is empty" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(2..0).should == [] + end + end + + context "when passed names" do + it 'slices captures with the given names' do + /(?.)(?.)(?.)/.match('012').values_at(:c, :a).should == ['2', '0'] + end + + it 'slices captures with the given String names' do + /(?.)(?.)(?.)/.match('012').values_at('c', 'a').should == ['2', '0'] end end - it 'slices captures with the given names' do - /(?.)(?.)(?.)/.match('012').values_at(:c, :a).should == ['2', '0'] + it "supports multiple integer Ranges" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(1..2, 2..3).should == ["H", "X", "X", "113"] end - it 'takes names and indices' do + it "supports mixing integer Ranges and Integers" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(1..2, 4).should == ["H", "X", "8"] + end + + it 'supports mixing of names and indices' do /\A(?.)(?.)\z/.match('01').values_at(0, 1, 2, :a, :b).should == ['01', '0', '1', '0', '1'] end + + it "returns a new empty Array if no arguments given" do + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at().should == [] + end + + it "fails when passed arguments of unsupported types" do + -> { + /(.)(.)(\d+)(\d)/.match("THX1138: The Movie").values_at(Object.new) + }.should raise_error(TypeError, "no implicit conversion of Object into Integer") + end end diff --git a/src/main/ruby/truffleruby/core/regexp.rb b/src/main/ruby/truffleruby/core/regexp.rb index 737d01c1714c..41faab2b7eda 100644 --- a/src/main/ruby/truffleruby/core/regexp.rb +++ b/src/main/ruby/truffleruby/core/regexp.rb @@ -359,7 +359,36 @@ def inspect end def values_at(*indexes) - indexes.map { |i| self[i] }.flatten(1) + out = [] + size = self.size + + indexes.each do |elem| + if Primitive.object_kind_of?(elem, String) || Primitive.object_kind_of?(elem, Symbol) + out << self[elem] + elsif 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 # ignore empty ranges + + finish_in_bounds = [finish, size - 1].min + start.upto(finish_in_bounds) do |index| + out << self[index] + end + + (finish_in_bounds + 1).upto(finish) { out << nil } + else + index = Primitive.rb_num2int(elem) + if index >= size || index < -size + out << nil + else + out << self[index] + end + end + end + + out end def to_s