Fix beginless and endless range comparisons

activesupport/CHANGELOG.md

@@ -1,3 +1,18 @@
+*   Fix `RangeError` in `Range#===` and `Range#include?` for endless and beginless ranges
+
+    ```ruby
+    (1..) === (2..)
+    => true
+    (2..) === (1..)
+    => false
+    (..1) === (..2)
+    => false
+    (..2) === (..1)
+    => true
+    ```
+
+    *Alex Mooney*
+
 *   Add filename support for `ActiveSupport::Logger.logger_outputs_to?`
 
     ```ruby

activesupport/lib/active_support/core_ext/range/compare_range.rb

@@ -11,20 +11,8 @@ module CompareWithRange
     # The native Range#=== behavior is untouched.
     #  ('a'..'f') === ('c') # => true
     #  (5..9) === (11) # => false
-    #
-    # The given range must be fully bounded, with both start and end.
-    def ===(value)
-      if value.is_a?(::Range)
-        is_backwards_op = value.exclude_end? ? :>= : :>
-        return false if value.begin && value.end && value.begin.public_send(is_backwards_op, value.end)
-        # 1...10 includes 1..9 but it does not include 1..10.
-        # 1..10 includes 1...11 but it does not include 1...12.
-        operator = exclude_end? && !value.exclude_end? ? :< : :<=
-        value_max = !exclude_end? && value.exclude_end? ? value.max : value.last
-        super(value.first) && (self.end.nil? || value_max.public_send(operator, last))
-      else
-        super
-      end
+    def ===(other)
+      compare(other) { |arg_for_comparison| super(arg_for_comparison) }
     end
 
     # Extends the default Range#include? to support range comparisons.
@@ -36,21 +24,48 @@ def ===(value)
     # The native Range#include? behavior is untouched.
     #  ('a'..'f').include?('c') # => true
     #  (5..9).include?(11) # => false
-    #
-    # The given range must be fully bounded, with both start and end.
-    def include?(value)
-      if value.is_a?(::Range)
-        is_backwards_op = value.exclude_end? ? :>= : :>
-        return false if value.begin && value.end && value.begin.public_send(is_backwards_op, value.end)
+    def include?(other)
+      compare(other) { |arg_for_comparison| super(arg_for_comparison) }
+    end
+
+    private
+      def compare(other, &block)
+        if other.is_a?(::Range)
+          compare_with_range(other, &block)
+        else
+          block.call(other)
+        end
+      end
+
+      def compare_with_range(other, &block)
+        return false if other_is_backwards?(other)
+
+        begin_includes_other_begin?(other, &block) && end_includes_other_end?(other)
+      end
+
+      def other_is_backwards?(other)
+        is_backwards_op = other.exclude_end? ? :>= : :>
+        other.begin && other.end && other.begin.public_send(is_backwards_op, other.end)
+      end
+
+      def begin_includes_other_begin?(other, &block)
+        # A beginless range always includes the other range's begin.
+        self.begin.nil? || block.call(other.begin)
+      end
+
+      def end_includes_other_end?(other)
+        # An endless range always includes the other range's end.
+        return true if self.end.nil?
+        # If the other range is endless, it's not included.
+        return false if other.end.nil?
+
         # 1...10 includes 1..9 but it does not include 1..10.
         # 1..10 includes 1...11 but it does not include 1...12.
-        operator = exclude_end? && !value.exclude_end? ? :< : :<=
-        value_max = !exclude_end? && value.exclude_end? ? value.max : value.last
-        super(value.first) && (self.end.nil? || value_max.public_send(operator, last))
-      else
-        super
+        # We can't simplify to `max >= other.max` since it raises TypeError for exclusive ranges with float end points.
+        operator = exclude_end? && !other.exclude_end? ? :< : :<=
+        value_max = !exclude_end? && other.exclude_end? ? other.max : other.last
+        value_max.public_send(operator, last)
       end
-    end
   end
 end
 

activesupport/test/core_ext/range_ext_test.rb

@@ -131,6 +131,13 @@ def test_overlap_behaves_like_ruby
     assert_not_operator((...3), :overlap?, (3..))
   end
 
+  def test_include_uses_ruby_include
+    assert((1..3).include?(1.5))
+    assert(("a".."d").include?("c"))
+
+    assert_not(("a".."d").include?("cc")) # As opposed to `=== 'cc'` => `true`
+  end
+
   def test_should_include_identical_inclusive
     assert((1..10).include?(1..10))
   end
@@ -143,8 +150,10 @@ def test_should_include_other_with_exclusive_end
     assert((1..10).include?(1...11))
   end
 
-  def test_include_returns_false_for_backwards
+  def test_include_returns_false_for_backwards_ranges
     assert_not((1..10).include?(5..3))
+    assert_not((10..1).include?(3..5))
+    assert_not((10..1).include?(5..3))
   end
 
   # Match quirky plain-Ruby behavior
@@ -158,10 +167,12 @@ def test_include_with_endless_range
 
   def test_should_include_range_with_endless_range
     assert((1..).include?(2..4))
+    assert_not((2..4).include?(1..))
   end
 
   def test_should_not_include_range_with_endless_range
     assert_not((1..).include?(0..4))
+    assert_not((0..4).include?(1..))
   end
 
   def test_include_with_beginless_range
@@ -170,10 +181,20 @@ def test_include_with_beginless_range
 
   def test_should_include_range_with_beginless_range
     assert((..2).include?(-1..1))
+    assert((..2).include?(..1))
   end
 
   def test_should_not_include_range_with_beginless_range
     assert_not((..2).include?(-1..3))
+    assert_not((..1).include?(..2))
+    assert_not((-1..1).include?(..2))
+  end
+
+  def test_case_equality_uses_ruby_case_equality
+    assert((1..3) === 1.5)
+    assert(("a".."d") === "c")
+
+    assert(("a".."d") === "cc") # As opposed to `.include?('cc')` => `false`
   end
 
   def test_should_compare_identical_inclusive
@@ -199,18 +220,25 @@ def test_compare_returns_false_for_empty_exclusive_end
 
   def test_should_compare_range_with_endless_range
     assert((1..) === (2..4))
+    assert((1..) === (1..))
+    assert((1..) === (2..))
   end
 
   def test_should_not_compare_range_with_endless_range
     assert_not((1..) === (0..4))
+    assert_not((0..4) === (1..))
+    assert_not((2..) === (1..))
   end
 
   def test_should_compare_range_with_beginless_range
     assert((..2) === (-1..1))
+    assert((..2) === (..1))
   end
 
   def test_should_not_compare_range_with_beginless_range
     assert_not((..2) === (-1..3))
+    assert_not((..1) === (..2))
+    assert_not((-1..3) === (..2))
   end
 
   def test_exclusive_end_should_not_include_identical_with_inclusive_end