From f754b422855131111092c0c147d744775cc4793f Mon Sep 17 00:00:00 2001 From: Kenta Murata Date: Fri, 23 Oct 2020 15:26:51 +0900 Subject: 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 | 9 ++- range.c | 7 ++ .../enumerator/arithmetic_sequence/step_spec.rb | 2 - spec/ruby/core/numeric/shared/step.rb | 6 +- spec/ruby/core/numeric/step_spec.rb | 90 ++++++++++++++-------- spec/ruby/core/range/step_spec.rb | 43 ++++++++--- test/ruby/test_arithmetic_sequence.rb | 8 -- test/ruby/test_numeric.rb | 36 ++++----- test/ruby/test_range.rb | 3 + 9 files changed, 124 insertions(+), 80 deletions(-) diff --git a/numeric.c b/numeric.c index fb9b28bcb3..7495b11095 100644 --- a/numeric.c +++ b/numeric.c @@ -2695,9 +2695,9 @@ 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 (!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, diff --git a/range.c b/range.c index c59662da0d..15b3d573f5 100644 --- a/range.c +++ b/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)) { diff --git a/spec/ruby/core/enumerator/arithmetic_sequence/step_spec.rb b/spec/ruby/core/enumerator/arithmetic_sequence/step_spec.rb index 20a5cb6e7b..8b00fd4309 100644 --- a/spec/ruby/core/enumerator/arithmetic_sequence/step_spec.rb +++ b/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 diff --git a/spec/ruby/core/numeric/shared/step.rb b/spec/ruby/core/numeric/shared/step.rb index fac79b3e63..6c47cdeecb 100644 --- a/spec/ruby/core/numeric/shared/step.rb +++ b/spec/ruby/core/numeric/shared/step.rb @@ -261,8 +261,10 @@ describe :numeric_step, :shared => true do step_enum_class = Enumerator::ArithmeticSequence end - it "returns an #{step_enum_class} when step is 0" do - @step.call(1, 2, 0).should be_an_instance_of(step_enum_class) + 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 diff --git a/spec/ruby/core/numeric/step_spec.rb b/spec/ruby/core/numeric/step_spec.rb index e9067864c8..2773000229 100644 --- a/spec/ruby/core/numeric/step_spec.rb +++ b/spec/ruby/core/numeric/step_spec.rb @@ -26,12 +26,14 @@ describe "Numeric#step" do step_enum_class = Enumerator::ArithmeticSequence end - it "returns an #{step_enum_class} when step is 0" do - 1.step(5, 0).should be_an_instance_of(step_enum_class) - 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 - 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) + 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 @@ -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,18 +87,20 @@ describe "Numeric#step" do end describe 'with keyword arguments' do - it "doesn't raise an error when step is 0" do - -> { 1.step(to: 5, by: 0) { break } }.should_not raise_error - end + 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 - it "doesn't raise an error when step is 0.0" do - -> { 1.step(to: 2, by: 0.0) { break } }.should_not raise_error - end + it "doesn't raise an error when step is 0.0" do + -> { 1.step(to: 2, by: 0.0) { break } }.should_not raise_error + end - it "should loop over self when step is 0 or 0.0" do - 1.step(to: 2, by: 0.0).take(5).should eql [1.0, 1.0, 1.0, 1.0, 1.0] - 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] + it "should loop over self when step is 0 or 0.0" do + 1.step(to: 2, by: 0.0).take(5).should eql [1.0, 1.0, 1.0, 1.0, 1.0] + 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 @@ -106,12 +110,14 @@ describe "Numeric#step" do 1.step(by: 42).size.should == infinity_value end - it "should return infinity_value when step is 0" do - 1.step(to: 5, by: 0).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 - it "should return infinity_value when step is 0.0" do - 1.step(to: 2, by: 0.0).size.should == infinity_value + 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 @@ -146,12 +152,24 @@ describe "Numeric#step" do end describe 'with mixed arguments' do - it "doesn't raise an error when step is 0" do - -> { 1.step(5, by: 0) { break } }.should_not raise_error + 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 + + 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 - it "doesn't raise an error when step is 0.0" do - -> { 1.step(2, by: 0.0) { break } }.should_not raise_error + 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 @@ -162,21 +180,25 @@ describe "Numeric#step" do -> { 1.step(5, 1, by: 5) { break } }.should raise_error(ArgumentError) end - 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] + 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 - it "should return infinity_value when step is 0" do - 1.step(5, by: 0).size.should == infinity_value - end + 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 - it "should return infinity_value when step is 0.0" do - 1.step(2, by: 0.0).size.should == infinity_value + it "should return infinity_value when step is 0.0" do + 1.step(2, by: 0.0).size.should == infinity_value + end end end end diff --git a/spec/ruby/core/range/step_spec.rb b/spec/ruby/core/range/step_spec.rb index d83868a0fb..e284551ab3 100644 --- a/spec/ruby/core/range/step_spec.rb +++ b/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,20 +371,41 @@ 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 - it "raises a TypeError if step does not respond to #to_int" do - obj = mock("Range#step non-integer") - enum = (1..2).step(obj) - -> { enum.size }.should raise_error(TypeError) + 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) + -> { enum.size }.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") + enum = (1..2).step(obj) + + -> { enum.size }.should raise_error(TypeError) + end 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") - enum = (1..2).step(obj) + 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 - -> { enum.size }.should raise_error(TypeError) + 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 @@ -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 diff --git a/test/ruby/test_arithmetic_sequence.rb b/test/ruby/test_arithmetic_sequence.rb index 45a0ab9222..70ec113e61 100644 --- a/test/ruby/test_arithmetic_sequence.rb +++ b/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 diff --git a/test/ruby/test_numeric.rb b/test/ruby/test_numeric.rb index 0fcf385c0d..c8751fbc57 100644 --- a/test/ruby/test_numeric.rb +++ b/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 diff --git a/test/ruby/test_range.rb b/test/ruby/test_range.rb index ba9b81ecc7..8ac1930be6 100644 --- a/test/ruby/test_range.rb +++ b/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 } -- cgit v1.2.3