[GR-18163] Fix Array unsafe methods when error is raised in a block

PullRequest: truffleruby/3631

Author: andrykonchin

CHANGELOG.md

@@ -71,6 +71,8 @@ Compatibility:
 * Fix `Array` methods `reject`, `reject!`, `inject`, `map`, `select`, `each_index` and handle a case when array is modified by a passed block like CRuby does (#2822, andrykonchin, @eregon).
 * Fix `EncodingError` exception message when Symbol has invalid encoding (#2850, @andrykonchin).
 * Raise `EncodingError` at parse time when Hash literal contains a Symbol key with invalid encoding (#2848, @andrykonchin).
+* Fix `Array` methods `reject`, `reject!`, `inject`, `map`, `select`, `each_index` and handle a case when array is modified by a passed block like CRuby does (#2822, @andrykonchin, @eregon).
+* Fix `Array` methods `select!` and `keep_if` and handle a case when exception is raised in a passed block properly (@andrykonchin).
 
 Performance:
 

spec/ruby/core/array/all_spec.rb

@@ -4,4 +4,10 @@
 describe "Array#all?" do
   @value_to_return = -> (_) { true }
   it_behaves_like :array_iterable_and_tolerating_size_increasing, :all?
+
+  it "ignores the block if there is an argument" do
+    -> {
+      ['bar', 'foobar'].all?(/bar/) { false }.should == true
+    }.should complain(/given block not used/)
+  end
 end

spec/ruby/core/array/any_spec.rb

@@ -38,4 +38,12 @@
       array_with_members.any? {|v| v == 42 }.should == false
     end
   end
+
+  describe 'when given a pattern argument' do
+    it "ignores the block if there is an argument" do
+      -> {
+        ['bar', 'foobar'].any?(/bar/) { false }.should == true
+      }.should complain(/given block not used/)
+    end
+  end
 end

spec/ruby/core/array/count_spec.rb

@@ -14,6 +14,12 @@
     [:a, :b, :c].count { |s| s != :b }.should == 2
   end
 
+  it "ignores the block if there is an argument" do
+    -> {
+      [:a, :b, :b, :c].count(:b) { |e| e.size > 10 }.should == 2
+    }.should complain(/given block not used/)
+  end
+
   context "when a block argument given" do
     it_behaves_like :array_iterable_and_tolerating_size_increasing, :count
   end

spec/ruby/core/array/delete_if_spec.rb

@@ -48,6 +48,32 @@
     -> { ArraySpecs.empty_frozen_array.delete_if {} }.should raise_error(FrozenError)
   end
 
+  it "does not truncate the array is the block raises an exception" do
+    a = [1, 2, 3]
+    begin
+      a.delete_if { raise StandardError, 'Oops' }
+    rescue
+    end
+
+    a.should == [1, 2, 3]
+  end
+
+  it "only removes elements for which the block returns true, keeping the element which raised an error." do
+    a = [1, 2, 3, 4]
+    begin
+      a.delete_if do |e|
+        case e
+        when 2 then true
+        when 3 then raise StandardError, 'Oops'
+        else false
+        end
+      end
+    rescue StandardError
+    end
+
+    a.should == [1, 3, 4]
+  end
+
   it_behaves_like :enumeratorized_with_origin_size, :delete_if, [1,2,3]
   it_behaves_like :delete_if, :delete_if
 

spec/ruby/core/array/fill_spec.rb

@@ -73,6 +73,33 @@
     -> { [].fill(1, 2, true) {|i|} }.should raise_error(ArgumentError)
   end
 
+  it "does not truncate the array is the block raises an exception" do
+    a = [1, 2, 3]
+    begin
+      a.fill { raise StandardError, 'Oops' }
+    rescue
+    end
+
+    a.should == [1, 2, 3]
+  end
+
+  it "only changes elements before error is raised, keeping the element which raised an error." do
+    a = [1, 2, 3, 4]
+    begin
+      a.fill do |i|
+        case i
+        when 0 then -1
+        when 1 then -2
+        when 2 then raise StandardError, 'Oops'
+        else 0
+        end
+      end
+    rescue StandardError
+    end
+
+    a.should == [-1, -2, 3, 4]
+  end
+
   it "tolerates increasing an array size during iteration" do
     array = [:a, :b, :c]
     ScratchPad.record []

spec/ruby/core/array/initialize_spec.rb

@@ -53,7 +53,9 @@
   end
 
   it "does not use the given block" do
-    ->{ [1, 2, 3].send(:initialize) { raise } }.should_not raise_error
+    -> {
+      -> { [1, 2, 3].send(:initialize) { raise } }.should_not raise_error
+    }.should complain(/#{__FILE__}:#{__LINE__-1}: warning: given block not used/, verbose: true)
   end
 end
 

spec/ruby/core/array/new_spec.rb

@@ -26,7 +26,9 @@
   end
 
   it "does not use the given block" do
-    ->{ Array.new { raise } }.should_not raise_error
+    -> {
+      -> { Array.new { raise } }.should_not raise_error
+    }.should complain(/warning: given block not used/, verbose: true)
   end
 end
 

spec/ruby/core/array/none_spec.rb

@@ -4,4 +4,10 @@
 describe "Array#none?" do
   @value_to_return = -> (_) { false }
   it_behaves_like :array_iterable_and_tolerating_size_increasing, :none?
+
+  it "ignores the block if there is an argument" do
+    -> {
+      ['bar', 'foobar'].none?(/baz/) { true }.should == true
+    }.should complain(/given block not used/)
+  end
 end

spec/ruby/core/array/one_spec.rb

@@ -4,4 +4,10 @@
 describe "Array#one?" do
   @value_to_return = -> (_) { false }
   it_behaves_like :array_iterable_and_tolerating_size_increasing, :one?
+
+  it "ignores the block if there is an argument" do
+    -> {
+      ['bar', 'foobar'].one?(/foo/) { false }.should == true
+    }.should complain(/given block not used/)
+  end
 end