kotovalexarian-likes-github/ruby--ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00
Code Releases Activity

Make Array methods return Array instances instead of subclass instances

Browse source 
This changes the following methods to return Array instances instead
of subclass instances:

* Array#drop
* Array#drop_while
* Array#flatten
* Array#slice!
* Array#slice/#[]
* Array#take
* Array#take_while
* Array#uniq
* Array#*

Fixes [Bug #6087]
This commit is contained in:
Jeremy Evans 2020-11-03 14:01:38 -08:00 • committed by GitHub
parent 7d6c72dc06
commit 2a294d499b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
Notes: git 2020-11-04 07:02:06 +09:00 
Merged: https://github.com/ruby/ruby/pull/3690

Merged-By: jeremyevans <code@jeremyevans.net>
6 changed files with 171 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
array.c
View file

@ -1189,7 +1189,7 @@ ary_make_partial_step(VALUE ary, VALUE klass, long offset, long len, long step)
static VALUE
ary_make_shared_copy(VALUE ary)
{
return ary_make_partial(ary, rb_obj_class(ary), 0, RARRAY_LEN(ary));
return ary_make_partial(ary, rb_cArray, 0, RARRAY_LEN(ary));
}
enum ary_take_pos_flags
@ -1628,7 +1628,7 @@ rb_ary_subseq_step(VALUE ary, long beg, long len, long step)
if (alen < len || alen < beg + len) {
len = alen - beg;
}
klass = rb_obj_class(ary);
klass = rb_cArray;
if (len == 0) return ary_new(klass, 0);
if (step == 0)
rb_raise(rb_eArgError, "slice step cannot be zero");
@ -4010,7 +4010,6 @@ ary_slice_bang_by_rb_ary_splice(VALUE ary, long pos, long len)
}
else {
VALUE arg2 = rb_ary_new4(len, RARRAY_CONST_PTR_TRANSIENT(ary)+pos);
RBASIC_SET_CLASS(arg2, rb_obj_class(ary));
rb_ary_splice(ary, pos, len, 0, 0);
return arg2;
}
@ -4820,7 +4819,7 @@ rb_ary_times(VALUE ary, VALUE times)
len = NUM2LONG(times);
if (len == 0) {
ary2 = ary_new(rb_obj_class(ary), 0);
ary2 = ary_new(rb_cArray, 0);
goto out;
}
if (len < 0) {
@ -4831,7 +4830,7 @@ rb_ary_times(VALUE ary, VALUE times)
}
len *= RARRAY_LEN(ary);
ary2 = ary_new(rb_obj_class(ary), len);
ary2 = ary_new(rb_cArray, len);
ARY_SET_LEN(ary2, len);
ptr = RARRAY_CONST_PTR_TRANSIENT(ary);
@ -5947,7 +5946,6 @@ rb_ary_uniq(VALUE ary)
hash = ary_make_hash(ary);
uniq = rb_hash_values(hash);
}
RBASIC_SET_CLASS(uniq, rb_obj_class(ary));
if (hash) {
ary_recycle_hash(hash);
}
@ -6146,7 +6144,7 @@ flatten(VALUE ary, int level)
st_clear(memo);
}
RBASIC_SET_CLASS(result, rb_obj_class(ary));
RBASIC_SET_CLASS(result, rb_cArray);
return result;
}

24
spec/ruby/core/array/flatten_spec.rb
View file

@ -75,12 +75,24 @@ describe "Array#flatten" do
[[obj]].flatten(1)
end
it "returns subclass instance for Array subclasses" do
ArraySpecs::MyArray[].flatten.should be_an_instance_of(ArraySpecs::MyArray)
ArraySpecs::MyArray[1, 2, 3].flatten.should be_an_instance_of(ArraySpecs::MyArray)
ArraySpecs::MyArray[1, [2], 3].flatten.should be_an_instance_of(ArraySpecs::MyArray)
ArraySpecs::MyArray[1, [2, 3], 4].flatten.should == ArraySpecs::MyArray[1, 2, 3, 4]
[ArraySpecs::MyArray[1, 2, 3]].flatten.should be_an_instance_of(Array)
ruby_version_is ''...'3.0' do
it "returns subclass instance for Array subclasses" do
ArraySpecs::MyArray[].flatten.should be_an_instance_of(ArraySpecs::MyArray)
ArraySpecs::MyArray[1, 2, 3].flatten.should be_an_instance_of(ArraySpecs::MyArray)
ArraySpecs::MyArray[1, [2], 3].flatten.should be_an_instance_of(ArraySpecs::MyArray)
ArraySpecs::MyArray[1, [2, 3], 4].flatten.should == ArraySpecs::MyArray[1, 2, 3, 4]
[ArraySpecs::MyArray[1, 2, 3]].flatten.should be_an_instance_of(Array)
end
end
ruby_version_is '3.0' do
it "returns Array instance for Array subclasses" do
ArraySpecs::MyArray[].flatten.should be_an_instance_of(Array)
ArraySpecs::MyArray[1, 2, 3].flatten.should be_an_instance_of(Array)
ArraySpecs::MyArray[1, [2], 3].flatten.should be_an_instance_of(Array)
ArraySpecs::MyArray[1, [2, 3], 4].flatten.should == Array[1, 2, 3, 4]
[ArraySpecs::MyArray[1, 2, 3]].flatten.should be_an_instance_of(Array)
end
end
it "is not destructive" do

18
spec/ruby/core/array/multiply_spec.rb
View file

@ -76,10 +76,20 @@ describe "Array#* with an integer" do
@array = ArraySpecs::MyArray[1, 2, 3, 4, 5]
end
it "returns a subclass instance" do
(@array * 0).should be_an_instance_of(ArraySpecs::MyArray)
(@array * 1).should be_an_instance_of(ArraySpecs::MyArray)
(@array * 2).should be_an_instance_of(ArraySpecs::MyArray)
ruby_version_is ''...'3.0' do
it "returns a subclass instance" do
(@array * 0).should be_an_instance_of(ArraySpecs::MyArray)
(@array * 1).should be_an_instance_of(ArraySpecs::MyArray)
(@array * 2).should be_an_instance_of(ArraySpecs::MyArray)
end
end
ruby_version_is '3.0' do
it "returns an Array instance" do
(@array * 0).should be_an_instance_of(Array)
(@array * 1).should be_an_instance_of(Array)
(@array * 2).should be_an_instance_of(Array)
end
end
it "does not call #initialize on the subclass instance" do

60
spec/ruby/core/array/shared/slice.rb
View file

@ -397,28 +397,56 @@ describe :array_slice, shared: true do
@array = ArraySpecs::MyArray[1, 2, 3, 4, 5]
end
it "returns a subclass instance with [n, m]" do
@array.send(@method, 0, 2).should be_an_instance_of(ArraySpecs::MyArray)
ruby_version_is ''...'3.0' do
it "returns a subclass instance with [n, m]" do
@array.send(@method, 0, 2).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a subclass instance with [-n, m]" do
@array.send(@method, -3, 2).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a subclass instance with [n..m]" do
@array.send(@method, 1..3).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a subclass instance with [n...m]" do
@array.send(@method, 1...3).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a subclass instance with [-n..-m]" do
@array.send(@method, -3..-1).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a subclass instance with [-n...-m]" do
@array.send(@method, -3...-1).should be_an_instance_of(ArraySpecs::MyArray)
end
end
it "returns a subclass instance with [-n, m]" do
@array.send(@method, -3, 2).should be_an_instance_of(ArraySpecs::MyArray)
end
ruby_version_is '3.0' do
it "returns a Array instance with [n, m]" do
@array.send(@method, 0, 2).should be_an_instance_of(Array)
end
it "returns a subclass instance with [n..m]" do
@array.send(@method, 1..3).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a Array instance with [-n, m]" do
@array.send(@method, -3, 2).should be_an_instance_of(Array)
end
it "returns a subclass instance with [n...m]" do
@array.send(@method, 1...3).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a Array instance with [n..m]" do
@array.send(@method, 1..3).should be_an_instance_of(Array)
end
it "returns a subclass instance with [-n..-m]" do
@array.send(@method, -3..-1).should be_an_instance_of(ArraySpecs::MyArray)
end
it "returns a Array instance with [n...m]" do
@array.send(@method, 1...3).should be_an_instance_of(Array)
end
it "returns a subclass instance with [-n...-m]" do
@array.send(@method, -3...-1).should be_an_instance_of(ArraySpecs::MyArray)
it "returns a Array instance with [-n..-m]" do
@array.send(@method, -3..-1).should be_an_instance_of(Array)
end
it "returns a Array instance with [-n...-m]" do
@array.send(@method, -3...-1).should be_an_instance_of(Array)
end
end
it "returns an empty array when m == n with [m...n]" do

12
spec/ruby/core/array/uniq_spec.rb
View file

@ -128,8 +128,16 @@ describe "Array#uniq" do
[false, nil, 42].uniq { :bar }.should == [false]
end
it "returns subclass instance on Array subclasses" do
ArraySpecs::MyArray[1, 2, 3].uniq.should be_an_instance_of(ArraySpecs::MyArray)
ruby_version_is ''...'3.0' do
it "returns subclass instance on Array subclasses" do
ArraySpecs::MyArray[1, 2, 3].uniq.should be_an_instance_of(ArraySpecs::MyArray)
end
end
ruby_version_is '3.0' do
it "returns Array instance on Array subclasses" do
ArraySpecs::MyArray[1, 2, 3].uniq.should be_an_instance_of(Array)
end
end
it "properly handles an identical item even when its #eql? isn't reflexive" do

140
test/ruby/test_array.rb
View file

@ -15,6 +15,11 @@ class TestArray < Test::Unit::TestCase
$VERBOSE = @verbose
end
def assert_equal_instance(x, y, *msg)
assert_equal(x, y, *msg)
assert_instance_of(x.class, y)
end
def test_percent_i
assert_equal([:foo, :bar], %i[foo bar])
assert_equal([:"\"foo"], %i["foo])
@ -114,6 +119,9 @@ class TestArray < Test::Unit::TestCase
assert_equal('1', (x * 1).join(":"))
assert_equal('', (x * 0).join(":"))
assert_instance_of(Array, (@cls[] * 5))
assert_instance_of(Array, (@cls[1] * 5))
*x = *(1..7).to_a
assert_equal(7, x.size)
assert_equal([1, 2, 3, 4, 5, 6, 7], x)
@ -842,14 +850,14 @@ class TestArray < Test::Unit::TestCase
a2 = @cls[ 5, 6 ]
a3 = @cls[ 4, a2 ]
a4 = @cls[ a1, a3 ]
assert_equal(@cls[1, 2, 3, 4, 5, 6], a4.flatten)
assert_equal(@cls[ a1, a3], a4)
assert_equal_instance([1, 2, 3, 4, 5, 6], a4.flatten)
assert_equal_instance(@cls[ a1, a3], a4)
a5 = @cls[ a1, @cls[], a3 ]
assert_equal(@cls[1, 2, 3, 4, 5, 6], a5.flatten)
assert_equal(@cls[1, 2, 3, 4, [5, 6]], a5.flatten(1))
assert_equal(@cls[], @cls[].flatten)
assert_equal(@cls[],
assert_equal_instance([1, 2, 3, 4, 5, 6], a5.flatten)
assert_equal_instance([1, 2, 3, 4, [5, 6]], a5.flatten(1))
assert_equal_instance([], @cls[].flatten)
assert_equal_instance([],
@cls[@cls[@cls[@cls[],@cls[]],@cls[@cls[]],@cls[]],@cls[@cls[@cls[]]]].flatten)
end
@ -1472,59 +1480,59 @@ class TestArray < Test::Unit::TestCase
assert_equal(1, a.slice(-100))
assert_nil(a.slice(-101))
assert_equal(@cls[1], a.slice(0,1))
assert_equal(@cls[100], a.slice(99,1))
assert_equal(@cls[], a.slice(100,1))
assert_equal(@cls[100], a.slice(99,100))
assert_equal(@cls[100], a.slice(-1,1))
assert_equal(@cls[99], a.slice(-2,1))
assert_equal_instance([1], a.slice(0,1))
assert_equal_instance([100], a.slice(99,1))
assert_equal_instance([], a.slice(100,1))
assert_equal_instance([100], a.slice(99,100))
assert_equal_instance([100], a.slice(-1,1))
assert_equal_instance([99], a.slice(-2,1))
assert_equal(@cls[10, 11, 12], a.slice(9, 3))
assert_equal(@cls[10, 11, 12], a.slice(-91, 3))
assert_equal_instance([10, 11, 12], a.slice(9, 3))
assert_equal_instance([10, 11, 12], a.slice(-91, 3))
assert_nil(a.slice(-101, 2))
assert_equal(@cls[1], a.slice(0..0))
assert_equal(@cls[100], a.slice(99..99))
assert_equal(@cls[], a.slice(100..100))
assert_equal(@cls[100], a.slice(99..200))
assert_equal(@cls[100], a.slice(-1..-1))
assert_equal(@cls[99], a.slice(-2..-2))
assert_equal_instance([1], a.slice(0..0))
assert_equal_instance([100], a.slice(99..99))
assert_equal_instance([], a.slice(100..100))
assert_equal_instance([100], a.slice(99..200))
assert_equal_instance([100], a.slice(-1..-1))
assert_equal_instance([99], a.slice(-2..-2))
assert_equal(@cls[10, 11, 12], a.slice(9..11))
assert_equal(@cls[98, 99, 100], a.slice(97..))
assert_equal(@cls[10, 11, 12], a.slice(-91..-89))
assert_equal(@cls[10, 11, 12], a.slice(-91..-89))
assert_equal_instance([10, 11, 12], a.slice(9..11))
assert_equal_instance([98, 99, 100], a.slice(97..))
assert_equal_instance([10, 11, 12], a.slice(-91..-89))
assert_equal_instance([10, 11, 12], a.slice(-91..-89))
assert_equal(@cls[5, 8, 11], a.slice((4..12)%3))
assert_equal(@cls[95, 97, 99], a.slice((94..)%2))
assert_equal_instance([5, 8, 11], a.slice((4..12)%3))
assert_equal_instance([95, 97, 99], a.slice((94..)%2))
# [0] [1] [2] [3] [4] [5] [6] [7]
# ary = [ 1 2 3 4 5 6 7 8 ... ]
# (0) (1) (2) <- (..7) % 3
# (2) (1) (0) <- (7..) % -3
assert_equal(@cls[1, 4, 7], a.slice((..7)%3))
assert_equal(@cls[8, 5, 2], a.slice((7..)% -3))
assert_equal_instance([1, 4, 7], a.slice((..7)%3))
assert_equal_instance([8, 5, 2], a.slice((7..)% -3))
# [-98] [-97] [-96] [-95] [-94] [-93] [-92] [-91] [-90]
# ary = [ ... 3 4 5 6 7 8 9 10 11 ... ]
# (0) (1) (2) <- (-98..-90) % 3
# (2) (1) (0) <- (-90..-98) % -3
assert_equal(@cls[3, 6, 9], a.slice((-98..-90)%3))
assert_equal(@cls[11, 8, 5], a.slice((-90..-98)% -3))
assert_equal_instance([3, 6, 9], a.slice((-98..-90)%3))
assert_equal_instance([11, 8, 5], a.slice((-90..-98)% -3))
# [ 48] [ 49] [ 50] [ 51] [ 52] [ 53]
# [-52] [-51] [-50] [-49] [-48] [-47]
# ary = [ ... 49 50 51 52 53 54 ... ]
# (0) (1) (2) <- (48..-47) % 2
# (2) (1) (0) <- (-47..48) % -2
assert_equal(@cls[49, 51, 53], a.slice((48..-47)%2))
assert_equal(@cls[54, 52, 50], a.slice((-47..48)% -2))
assert_equal_instance([49, 51, 53], a.slice((48..-47)%2))
assert_equal_instance([54, 52, 50], a.slice((-47..48)% -2))
idx = ((3..90) % 2).to_a
assert_equal(@cls[*a.values_at(*idx)], a.slice((3..90)%2))
assert_equal_instance(a.values_at(*idx), a.slice((3..90)%2))
idx = 90.step(3, -2).to_a
assert_equal(@cls[*a.values_at(*idx)], a.slice((90 .. 3)% -2))
assert_equal_instance(a.values_at(*idx), a.slice((90 .. 3)% -2))
end
def test_slice_out_of_range
@ -1550,15 +1558,18 @@ class TestArray < Test::Unit::TestCase
assert_equal(@cls[1, 2, 3, 5], a)
a = @cls[1, 2, 3, 4, 5]
assert_equal(@cls[3,4], a.slice!(2,2))
s = a.slice!(2,2)
assert_equal_instance([3,4], s)
assert_equal(@cls[1, 2, 5], a)
a = @cls[1, 2, 3, 4, 5]
assert_equal(@cls[4,5], a.slice!(-2,2))
s = a.slice!(-2,2)
assert_equal_instance([4,5], s)
assert_equal(@cls[1, 2, 3], a)
a = @cls[1, 2, 3, 4, 5]
assert_equal(@cls[3,4], a.slice!(2..3))
s = a.slice!(2..3)
assert_equal_instance([3,4], s)
assert_equal(@cls[1, 2, 5], a)
a = @cls[1, 2, 3, 4, 5]
@ -1921,26 +1932,22 @@ class TestArray < Test::Unit::TestCase
sc = Class.new(@cls)
a = sc[]
b = a.dup
assert_instance_of(sc, a.uniq)
assert_equal(sc[], a.uniq)
assert_equal_instance([], a.uniq)
assert_equal(b, a)
a = sc[1]
b = a.dup
assert_instance_of(sc, a.uniq)
assert_equal(sc[1], a.uniq)
assert_equal_instance([1], a.uniq)
assert_equal(b, a)
a = sc[1, 1]
b = a.dup
assert_instance_of(sc, a.uniq)
assert_equal(sc[1], a.uniq)
assert_equal_instance([1], a.uniq)
assert_equal(b, a)
a = sc[1, 1]
b = a.dup
assert_instance_of(sc, a.uniq{|x| x})
assert_equal(sc[1], a.uniq{|x| x})
assert_equal_instance([1], a.uniq{|x| x})
assert_equal(b, a)
end
@ -2355,23 +2362,23 @@ class TestArray < Test::Unit::TestCase
end
def test_take
assert_equal([1,2,3], [1,2,3,4,5,0].take(3))
assert_equal_instance([1,2,3], @cls[1,2,3,4,5,0].take(3))
assert_raise(ArgumentError, '[ruby-dev:34123]') { [1,2].take(-1) }
assert_equal([1,2], [1,2].take(1000000000), '[ruby-dev:34123]')
assert_equal_instance([1,2], @cls[1,2].take(1000000000), '[ruby-dev:34123]')
end
def test_take_while
assert_equal([1,2], [1,2,3,4,5,0].take_while {|i| i < 3 })
assert_equal_instance([1,2], @cls[1,2,3,4,5,0].take_while {|i| i < 3 })
end
def test_drop
assert_equal([4,5,0], [1,2,3,4,5,0].drop(3))
assert_equal_instance([4,5,0], @cls[1,2,3,4,5,0].drop(3))
assert_raise(ArgumentError, '[ruby-dev:34123]') { [1,2].drop(-1) }
assert_equal([], [1,2].drop(1000000000), '[ruby-dev:34123]')
assert_equal_instance([], @cls[1,2].drop(1000000000), '[ruby-dev:34123]')
end
def test_drop_while
assert_equal([3,4,5,0], [1,2,3,4,5,0].drop_while {|i| i < 3 })
assert_equal_instance([3,4,5,0], @cls[1,2,3,4,5,0].drop_while {|i| i < 3 })
end
LONGP = [127, 63, 31, 15, 7].map {|x| 2**x-1 }.find do |x|
@ -2451,6 +2458,7 @@ class TestArray < Test::Unit::TestCase
def test_aref
assert_raise(ArgumentError) { [][0, 0, 0] }
assert_raise(ArgumentError) { @cls[][0, 0, 0] }
end
def test_fetch
@ -2954,15 +2962,6 @@ class TestArray < Test::Unit::TestCase
end
end
class Array2 < Array
end
def test_array_subclass
assert_equal(Array2, Array2[1,2,3].uniq.class, "[ruby-dev:34581]")
assert_equal(Array2, Array2[1,2][0,1].class) # embedded
assert_equal(Array2, Array2[*(1..100)][1..99].class) #not embedded
end
def test_initialize2
a = [1] * 1000
a.instance_eval { initialize }
@ -3302,3 +3301,24 @@ class TestArray < Test::Unit::TestCase
end
end
end
class TestArraySubclass < TestArray
def setup
@verbose = $VERBOSE
$VERBOSE = nil
@cls = Class.new(Array)
end
def test_to_a
a = @cls[ 1, 2, 3 ]
a_id = a.__id__
assert_equal_instance([1, 2, 3], a.to_a)
assert_not_equal(a_id, a.to_a.__id__)
end
def test_array_subclass
assert_equal(Array, @cls[1,2,3].uniq.class, "[ruby-dev:34581]")
assert_equal(Array, @cls[1,2][0,1].class) # embedded
assert_equal(Array, @cls[*(1..100)][1..99].class) #not embedded
end
end