numeric.c, range.c: prohibit zero step

* numeric.c: prohibit zero step in Numeric#step

* range.c: prohibit zero step in Range#step

* Fix ruby-spec

[Feature #15573]

numeric.c

@@ -2695,10 +2695,10 @@ num_step_check_fix_args(int argc, VALUE *to, VALUE *step, VALUE by, int fix_nil,
         if (argc > 1 && NIL_P(*step)) {
             rb_raise(rb_eTypeError, "step must be numeric");
         }
+    }
     if (!allow_zero_step && rb_equal(*step, INT2FIX(0))) {
         rb_raise(rb_eArgError, "step can't be 0");
     }
-    }
     if (NIL_P(*step)) {
 	*step = INT2FIX(1);
     }
@@ -2801,6 +2801,9 @@ num_step(int argc, VALUE *argv, VALUE from)
         if (NIL_P(step)) {
             step = INT2FIX(1);
         }
+        else if (rb_equal(step, INT2FIX(0))) {
+            rb_raise(rb_eArgError, "step can't be 0");
+        }
         if ((NIL_P(to) || rb_obj_is_kind_of(to, rb_cNumeric)) &&
             rb_obj_is_kind_of(step, rb_cNumeric)) {
             return rb_arith_seq_new(from, ID2SYM(rb_frame_this_func()), argc, argv,

range.c

@@ -415,6 +415,13 @@ range_step(int argc, VALUE *argv, VALUE range)
     step = (!rb_check_arity(argc, 0, 1) ? INT2FIX(1) : argv[0]);
 
     if (!rb_block_given_p()) {
+        if (!rb_obj_is_kind_of(step, rb_cNumeric)) {
+            step = rb_to_int(step);
+        }
+        if (rb_equal(step, INT2FIX(0))) {
+            rb_raise(rb_eArgError, "step can't be 0");
+        }
+
         const VALUE b_num_p = rb_obj_is_kind_of(b, rb_cNumeric);
         const VALUE e_num_p = rb_obj_is_kind_of(e, rb_cNumeric);
         if ((b_num_p && (NIL_P(e) || e_num_p)) || (NIL_P(b) && e_num_p)) {

spec/ruby/core/enumerator/arithmetic_sequence/step_spec.rb

@@ -5,11 +5,9 @@ ruby_version_is "2.6" do
     it "returns the original value given to step method" do
       (1..10).step.step.should == 1
       (1..10).step(3).step.should == 3
-      (1..10).step(0).step.should == 0
 
       1.step(10).step.should == 1
       1.step(10, 3).step.should == 3
-      1.step(10, 0).step.should == 0
     end
   end
 end

spec/ruby/core/numeric/shared/step.rb

@@ -261,9 +261,11 @@ describe :numeric_step, :shared => true do
       step_enum_class = Enumerator::ArithmeticSequence
     end
 
+    ruby_version_is ""..."3.0" do
       it "returns an #{step_enum_class} when step is 0" do
         @step.call(1, 2, 0).should be_an_instance_of(step_enum_class)
       end
+    end
 
     it "returns an #{step_enum_class} when not passed a block and self > stop" do
       @step.call(1, 0, 2).should be_an_instance_of(step_enum_class)

spec/ruby/core/numeric/step_spec.rb

@@ -26,6 +26,7 @@ describe "Numeric#step" do
         step_enum_class = Enumerator::ArithmeticSequence
       end
 
+      ruby_version_is ""..."3.0" do
         it "returns an #{step_enum_class} when step is 0" do
           1.step(5, 0).should be_an_instance_of(step_enum_class)
         end
@@ -33,6 +34,7 @@ describe "Numeric#step" do
         it "returns an #{step_enum_class} when step is 0.0" do
           1.step(2, 0.0).should be_an_instance_of(step_enum_class)
         end
+      end
 
       describe "returned #{step_enum_class}" do
         describe "size" do
@@ -48,7 +50,7 @@ describe "Numeric#step" do
             end
           end
 
-          ruby_version_is "2.6" do
+          ruby_version_is "2.6"..."3.0" do
             it "is infinity when step is 0" do
               enum = 1.step(5, 0)
               enum.size.should == Float::INFINITY
@@ -85,6 +87,7 @@ describe "Numeric#step" do
   end
 
   describe 'with keyword arguments' do
+    ruby_version_is ""..."3.0" do
       it "doesn't raise an error when step is 0" do
         -> { 1.step(to: 5, by: 0) { break } }.should_not raise_error
       end
@@ -98,6 +101,7 @@ describe "Numeric#step" do
         1.step(to: 2, by: 0).take(5).should eql [1, 1, 1, 1, 1]
         1.1.step(to: 2, by: 0).take(5).should eql [1.1, 1.1, 1.1, 1.1, 1.1]
       end
+    end
 
     describe "when no block is given" do
       describe "returned Enumerator" do
@@ -106,6 +110,7 @@ describe "Numeric#step" do
             1.step(by: 42).size.should == infinity_value
           end
 
+          ruby_version_is ""..."3.0" do
             it "should return infinity_value when step is 0" do
               1.step(to: 5, by: 0).size.should == infinity_value
             end
@@ -113,6 +118,7 @@ describe "Numeric#step" do
             it "should return infinity_value when step is 0.0" do
               1.step(to: 2, by: 0.0).size.should == infinity_value
             end
+          end
 
           it "should return infinity_value when ascending towards a limit of Float::INFINITY" do
             1.step(to: Float::INFINITY, by: 42).size.should == infinity_value
@@ -146,6 +152,7 @@ describe "Numeric#step" do
   end
 
   describe 'with mixed arguments' do
+    ruby_version_is ""..."3.0" do
       it "doesn't raise an error when step is 0" do
         -> { 1.step(5, by: 0) { break } }.should_not raise_error
       end
@@ -153,6 +160,17 @@ describe "Numeric#step" do
       it "doesn't raise an error when step is 0.0" do
         -> { 1.step(2, by: 0.0) { break } }.should_not raise_error
       end
+    end
+
+    ruby_version_is "3.0" do
+      it " raises an ArgumentError when step is 0" do
+        -> { 1.step(5, by: 0) { break } }.should raise_error(ArgumentError)
+      end
+
+      it "raises an ArgumentError when step is 0.0" do
+        -> { 1.step(2, by: 0.0) { break } }.should raise_error(ArgumentError)
+      end
+    end
 
     it "raises a ArgumentError when limit and to are defined" do
       -> { 1.step(5, 1, to: 5) { break } }.should raise_error(ArgumentError)
@@ -162,15 +180,18 @@ describe "Numeric#step" do
       -> { 1.step(5, 1, by: 5) { break } }.should raise_error(ArgumentError)
     end
 
+    ruby_version_is ""..."3.0" do
       it "should loop over self when step is 0 or 0.0" do
         1.step(2, by: 0.0).take(5).should eql [1.0, 1.0, 1.0, 1.0, 1.0]
         1.step(2, by: 0).take(5).should eql [1, 1, 1, 1, 1]
         1.1.step(2, by: 0).take(5).should eql [1.1, 1.1, 1.1, 1.1, 1.1]
       end
+    end
 
     describe "when no block is given" do
       describe "returned Enumerator" do
         describe "size" do
+          ruby_version_is ""..."3.0" do
             it "should return infinity_value when step is 0" do
               1.step(5, by: 0).size.should == infinity_value
             end
@@ -181,6 +202,7 @@ describe "Numeric#step" do
           end
         end
       end
+    end
 
     before :all do
       # This lambda definition transforms a positional step method args into

spec/ruby/core/range/step_spec.rb

@@ -297,7 +297,7 @@ describe "Range#step" do
 
         it "yields Float values incremented by a Float step" do
           eval("(-2..)").step(1.5) { |x| break if x > 1.0; ScratchPad << x }
-          ScratchPad.recorded.should eql([-2.0, -0.5, 1.0])\
+          ScratchPad.recorded.should eql([-2.0, -0.5, 1.0])
 
           ScratchPad.record []
           eval("(-2..)").step(1.5) { |x| break if x > 1.0; ScratchPad << x }
@@ -371,8 +371,15 @@ describe "Range#step" do
   end
 
   describe "when no block is given" do
+    ruby_version_is "3.0" do
+      it "raises an ArgumentError if step is 0" do
+        -> { (-1..1).step(0) }.should raise_error(ArgumentError)
+      end
+    end
+
     describe "returned Enumerator" do
       describe "size" do
+        ruby_version_is ""..."3.0" do
           it "raises a TypeError if step does not respond to #to_int" do
             obj = mock("Range#step non-integer")
             enum = (1..2).step(obj)
@@ -386,6 +393,20 @@ describe "Range#step" do
 
             -> { enum.size }.should raise_error(TypeError)
           end
+        end
+
+        ruby_version_is "3.0" do
+          it "raises a TypeError if step does not respond to #to_int" do
+            obj = mock("Range#step non-integer")
+            -> { (1..2).step(obj) }.should raise_error(TypeError)
+          end
+
+          it "raises a TypeError if #to_int does not return an Integer" do
+            obj = mock("Range#step non-integer")
+            obj.should_receive(:to_int).and_return("1")
+            -> { (1..2).step(obj) }.should raise_error(TypeError)
+          end
+        end
 
         ruby_version_is ""..."2.6" do
           it "raises an ArgumentError if step is 0" do
@@ -404,7 +425,7 @@ describe "Range#step" do
           end
         end
 
-        ruby_version_is "2.6" do
+        ruby_version_is "2.6"..."3.0" do
           it "returns Float::INFINITY for zero step" do
             (-1..1).step(0).size.should == Float::INFINITY
             (-1..1).step(0.0).size.should == Float::INFINITY

test/ruby/test_arithmetic_sequence.rb

@@ -162,11 +162,6 @@ class TestArithmeticSequence < Test::Unit::TestCase
     assert_equal([], seq.first(1))
     assert_equal([], seq.first(3))
 
-    seq = 1.step(10, by: 0)
-    assert_equal(1, seq.first)
-    assert_equal([1], seq.first(1))
-    assert_equal([1, 1, 1], seq.first(3))
-
     seq = 10.0.step(-1.0, by: -2.0)
     assert_equal(10.0, seq.first)
     assert_equal([10.0], seq.first(1))
@@ -340,9 +335,6 @@ class TestArithmeticSequence < Test::Unit::TestCase
     [10, 8, 6, 4, 2].each do |i|
       assert_equal(i, seq.next)
     end
-
-    seq = ((1/10r)..(1/2r)).step(0)
-    assert_equal(1/10r, seq.next)
   end
 
   def test_next_bug15444

test/ruby/test_numeric.rb

@@ -267,12 +267,7 @@ class TestNumeric < Test::Unit::TestCase
     assert_raise(ArgumentError) { 1.step(10, "1") { } }
     assert_raise(ArgumentError) { 1.step(10, "1").size }
     assert_raise(TypeError) { 1.step(10, nil) { } }
-    assert_nothing_raised { 1.step(10, 0).size }
     assert_nothing_raised { 1.step(10, nil).size }
-    assert_nothing_raised { 1.step(by: 0, to: nil) }
-    assert_nothing_raised { 1.step(by: 0, to: nil).size }
-    assert_nothing_raised { 1.step(by: 0) }
-    assert_nothing_raised { 1.step(by: 0).size }
     assert_nothing_raised { 1.step(by: nil) }
     assert_nothing_raised { 1.step(by: nil).size }
 
@@ -292,6 +287,22 @@ class TestNumeric < Test::Unit::TestCase
     assert_raise(ArgumentError, bug9811) { 1.step(10, 1, by: 11) {} }
     assert_raise(ArgumentError, bug9811) { 1.step(10, 1, by: 11).size }
 
+    feature15573 = "[ruby-core:91324] [Feature #15573]"
+    assert_raise(ArgumentError, feature15573) { 1.step(10, 0) }
+    assert_raise(ArgumentError, feature15573) { 1.step(10, by: 0) }
+    assert_raise(ArgumentError, feature15573) { 1.step(10, 0) { break } }
+    assert_raise(ArgumentError, feature15573) { 1.step(10, by: 0) { break } }
+    assert_raise(ArgumentError, feature15573) { 42.step(by: 0, to: -Float::INFINITY) }
+    assert_raise(ArgumentError, feature15573) { 42.step(by: 0, to: 42.5) }
+    assert_raise(ArgumentError, feature15573) { 4.2.step(by: 0.0) }
+    assert_raise(ArgumentError, feature15573) { 4.2.step(by: -0.0) }
+    assert_raise(ArgumentError, feature15573) { 42.step(by: 0.0, to: 44) }
+    assert_raise(ArgumentError, feature15573) { 42.step(by: 0.0, to: 0) }
+    assert_raise(ArgumentError, feature15573) { 42.step(by: -0.0, to: 44) }
+    assert_raise(ArgumentError, feature15573) { bignum.step(by: 0) }
+    assert_raise(ArgumentError, feature15573) { bignum.step(by: 0.0) }
+    assert_raise(ArgumentError, feature15573) { bignum.step(by: 0, to: bignum+1) }
+    assert_raise(ArgumentError, feature15573) { bignum.step(by: 0, to: 0) }
 
     e = 1.step(10, {by: "1"})
     assert_raise(TypeError) {e.next}
@@ -322,8 +333,6 @@ class TestNumeric < Test::Unit::TestCase
 
     assert_step [], [2, 1, 3]
     assert_step [], [-2, -1, -3]
-    assert_step [3, 3, 3, 3], [3, by: 0], inf: true
-    assert_step [3, 3, 3, 3], [3, by: 0, to: 42], inf: true
     assert_step [10], [10, 1, -bignum]
 
     assert_step [], [1, 0, Float::INFINITY]
@@ -333,19 +342,6 @@ class TestNumeric < Test::Unit::TestCase
     assert_step [10, 11, 12, 13], [10], inf: true
     assert_step [10, 9, 8, 7], [10, by: -1], inf: true
     assert_step [10, 9, 8, 7], [10, by: -1, to: nil], inf: true
-
-    assert_step [42, 42, 42, 42], [42, by: 0, to: -Float::INFINITY], inf: true
-    assert_step [42, 42, 42, 42], [42, by: 0, to: 42.5], inf: true
-    assert_step [4.2, 4.2, 4.2, 4.2], [4.2, by: 0.0], inf: true
-    assert_step [4.2, 4.2, 4.2, 4.2], [4.2, by: -0.0], inf: true
-    assert_step [42.0, 42.0, 42.0, 42.0], [42, by: 0.0, to: 44], inf: true
-    assert_step [42.0, 42.0, 42.0, 42.0], [42, by: 0.0, to: 0], inf: true
-    assert_step [42.0, 42.0, 42.0, 42.0], [42, by: -0.0, to: 44], inf: true
-
-    assert_step [bignum]*4, [bignum, by: 0], inf: true
-    assert_step [bignum]*4, [bignum, by: 0.0], inf: true
-    assert_step [bignum]*4, [bignum, by: 0, to: bignum+1], inf: true
-    assert_step [bignum]*4, [bignum, by: 0, to: 0], inf: true
   end
 
   def test_step_bug15537

test/ruby/test_range.rb

@@ -271,8 +271,10 @@ class TestRange < Test::Unit::TestCase
     assert_kind_of(Enumerator::ArithmeticSequence, (1..).step(2))
 
     assert_raise(ArgumentError) { (0..10).step(-1) { } }
+    assert_raise(ArgumentError) { (0..10).step(0) }
     assert_raise(ArgumentError) { (0..10).step(0) { } }
     assert_raise(ArgumentError) { (0..).step(-1) { } }
+    assert_raise(ArgumentError) { (0..).step(0) }
     assert_raise(ArgumentError) { (0..).step(0) { } }
 
     a = []
@@ -303,6 +305,7 @@ class TestRange < Test::Unit::TestCase
     (2**32-1 .. 2**32+1).step(2) {|x| a << x }
     assert_equal([4294967295, 4294967297], a)
     zero = (2**32).coerce(0).first
+    assert_raise(ArgumentError) { (2**32-1 .. 2**32+1).step(zero) }
     assert_raise(ArgumentError) { (2**32-1 .. 2**32+1).step(zero) { } }
     a = []
     (2**32-1 .. ).step(2) {|x| a << x; break if a.size == 2 }