Comparable#clamp with a range [Feature #14784]

5 files changed, 103 insertions, 9 deletions

diff --git a/NEWS b/NEWS
index a3283a66b1..36660defba 100644
--- a/NEWS
+++ b/NEWS
@@ -142,6 +142,12 @@ Array::
 
     * Added Array#intersection. [Feature #16155]
 
+Comparable::
+
+  Modified method::
+
+    * Comparable#clamp now accepts a Range argument. [Feature #14784]
+
 Complex::
 
   New method::
diff --git a/common.mk b/common.mk
index ff56a0c93f..19aa723807 100644
--- a/common.mk
+++ b/common.mk
@@ -1636,6 +1636,7 @@ class.$(OBJEXT): {$(VPATH)}thread_native.h
 class.$(OBJEXT): {$(VPATH)}vm_core.h
 class.$(OBJEXT): {$(VPATH)}vm_debug.h
 class.$(OBJEXT): {$(VPATH)}vm_opts.h
+compar.$(OBJEXT): $(hdrdir)/ruby.h
 compar.$(OBJEXT): $(hdrdir)/ruby/ruby.h
 compar.$(OBJEXT): {$(VPATH)}assert.h
 compar.$(OBJEXT): {$(VPATH)}compar.c
@@ -1643,6 +1644,7 @@ compar.$(OBJEXT): {$(VPATH)}config.h
 compar.$(OBJEXT): {$(VPATH)}defines.h
 compar.$(OBJEXT): {$(VPATH)}id.h
 compar.$(OBJEXT): {$(VPATH)}intern.h
+compar.$(OBJEXT): {$(VPATH)}internal.h
 compar.$(OBJEXT): {$(VPATH)}missing.h
 compar.$(OBJEXT): {$(VPATH)}st.h
 compar.$(OBJEXT): {$(VPATH)}subst.h
diff --git a/compar.c b/compar.c
index 027328d6d3..fe4cc42833 100644
--- a/compar.c
+++ b/compar.c
@@ -11,6 +11,7 @@
 
 #include "ruby/ruby.h"
 #include "id.h"
+#include "internal.h"
 
 VALUE rb_mComparable;
 
@@ -176,24 +177,46 @@ cmp_between(VALUE x, VALUE min, VALUE max)
 /*
  *  call-seq:
  *     obj.clamp(min, max) ->  obj
+ *     obj.clamp(range)    ->  obj
  *
- * Returns _min_ if _obj_ <code><=></code> _min_ is less than zero,
- * _max_ if _obj_ <code><=></code> _max_ is greater than zero and
- * _obj_ otherwise.
+ * In the first form, returns _min_ if _obj_ <code><=></code> _min_ is
+ * less than zero, _max_ if _obj_ <code><=></code> _max_ is greater
+ * than zero and _obj_ otherwise.  In the second form, clamps by
+ * _range.min_ and _range.max_.  If _range_ is an exclusive range,
+ * raises an ArgumentError.
  *
  *     12.clamp(0, 100)         #=> 12
  *     523.clamp(0, 100)        #=> 100
  *     -3.123.clamp(0, 100)     #=> 0
+ *     12.clamp(0..100)         #=> 12
+ *     523.clamp(0..100)        #=> 100
+ *     -3.123.clamp(0..100)     #=> 0
  *
  *     'd'.clamp('a', 'f')      #=> 'd'
  *     'z'.clamp('a', 'f')      #=> 'f'
+ *     'd'.clamp('a'..'f')      #=> 'd'
+ *     'z'.clamp('a'..'f')      #=> 'f'
+ *
+ *     12.clamp(0...100)        #=> ArgumentError
  */
 
 static VALUE
-cmp_clamp(VALUE x, VALUE min, VALUE max)
+cmp_clamp(int argc, VALUE *argv, VALUE x)
 {
+    VALUE min, max;
     int c;
 
+    if (rb_scan_args(argc, argv, "11", &min, &max) == 1) {
+        VALUE range = min;
+        int excl;
+        if (!rb_range_values(range, &min, &max, &excl)) {
+            rb_raise(rb_eTypeError, "wrong argument type %s (expected Range)",
+                     rb_builtin_class_name(range));
+        }
+        if (excl || NIL_P(min) || NIL_P(max)) {
+            rb_raise(rb_eArgError, "cannot clamp with an exclusive range");
+        }
+    }
     if (cmpint(min, max) > 0) {
 	rb_raise(rb_eArgError, "min argument must be smaller than max argument");
     }
@@ -259,5 +282,5 @@ Init_Comparable(void)
     rb_define_method(rb_mComparable, "<", cmp_lt, 1);
     rb_define_method(rb_mComparable, "<=", cmp_le, 1);
     rb_define_method(rb_mComparable, "between?", cmp_between, 2);
-    rb_define_method(rb_mComparable, "clamp", cmp_clamp, 2);
+    rb_define_method(rb_mComparable, "clamp", cmp_clamp, -1);
 }
diff --git a/spec/ruby/core/comparable/clamp_spec.rb b/spec/ruby/core/comparable/clamp_spec.rb
index 6d216220d4..393496fc76 100644
--- a/spec/ruby/core/comparable/clamp_spec.rb
+++ b/spec/ruby/core/comparable/clamp_spec.rb
@@ -2,10 +2,12 @@ require_relative '../../spec_helper'
 require_relative 'fixtures/classes'
 
 describe 'Comparable#clamp' do
-  it 'raises an Argument error unless given 2 parameters' do
-    c = ComparableSpecs::Weird.new(0)
-    -> { c.clamp(c) }.should raise_error(ArgumentError)
-    -> { c.clamp(c, c, c) }.should raise_error(ArgumentError)
+  ruby_version_is ""..."2.7" do
+    it 'raises an Argument error unless given 2 parameters' do
+      c = ComparableSpecs::Weird.new(0)
+      -> { c.clamp(c) }.should raise_error(ArgumentError)
+      -> { c.clamp(c, c, c) }.should raise_error(ArgumentError)
+    end
   end
 
   it 'raises an Argument error unless the 2 parameters are correctly ordered' do
@@ -45,4 +47,42 @@ describe 'Comparable#clamp' do
 
     c.clamp(one, two).should equal(two)
   end
+
+  ruby_version_is "2.7" do
+    it 'returns self if within the given range parameters' do
+      one = ComparableSpecs::WithOnlyCompareDefined.new(1)
+      two = ComparableSpecs::WithOnlyCompareDefined.new(2)
+      three = ComparableSpecs::WithOnlyCompareDefined.new(3)
+      c = ComparableSpecs::Weird.new(2)
+
+      c.clamp(one..two).should equal(c)
+      c.clamp(two..two).should equal(c)
+      c.clamp(one..three).should equal(c)
+      c.clamp(two..three).should equal(c)
+    end
+
+    it 'returns the minimum value of the range parameters if smaller than it' do
+      one = ComparableSpecs::WithOnlyCompareDefined.new(1)
+      two = ComparableSpecs::WithOnlyCompareDefined.new(2)
+      c = ComparableSpecs::Weird.new(0)
+
+      c.clamp(one..two).should equal(one)
+    end
+
+    it 'returns the maximum value of the range parameters if greater than it' do
+      one = ComparableSpecs::WithOnlyCompareDefined.new(1)
+      two = ComparableSpecs::WithOnlyCompareDefined.new(2)
+      c = ComparableSpecs::Weird.new(3)
+
+      c.clamp(one..two).should equal(two)
+    end
+
+    it 'raises an Argument error if the range parameter is exclusive' do
+      one = ComparableSpecs::WithOnlyCompareDefined.new(1)
+      two = ComparableSpecs::WithOnlyCompareDefined.new(2)
+      c = ComparableSpecs::Weird.new(3)
+
+      -> { c.clamp(one...two) }.should raise_error(ArgumentError)
+    end
+  end
 end
diff --git a/test/ruby/test_comparable.rb b/test/ruby/test_comparable.rb
index 4f1d38a7e6..c363a03a64 100644
--- a/test/ruby/test_comparable.rb
+++ b/test/ruby/test_comparable.rb
@@ -90,6 +90,29 @@ class TestComparable < Test::Unit::TestCase
     }
   end
 
+  def test_clamp_with_range
+    cmp->(x) do 0 <=> x end
+    assert_equal(1, @o.clamp(1..2))
+    assert_equal(-1, @o.clamp(-2..-1))
+    assert_equal(@o, @o.clamp(-1..3))
+
+    assert_equal(1, @o.clamp(1..1))
+    assert_equal(@o, @o.clamp(0..0))
+
+    assert_raise_with_message(ArgumentError, 'cannot clamp with an exclusive range') {
+      @o.clamp(1...2)
+    }
+    assert_raise_with_message(ArgumentError, 'cannot clamp with an exclusive range') {
+      @o.clamp(1...)
+    }
+    assert_raise_with_message(ArgumentError, 'cannot clamp with an exclusive range') {
+      @o.clamp(...2)
+    }
+    assert_raise_with_message(ArgumentError, 'min argument must be smaller than max argument') {
+      @o.clamp(2..1)
+    }
+  end
+
   def test_err
     assert_raise(ArgumentError) { 1.0 < nil }
     assert_raise(ArgumentError) { 1.0 < Object.new }