Fix Range#overlap? ignoring empty ranges

Previously, #overlap? would incorrectly return true when one of the
ranges is effectively "empty":

```ruby
(2...2).overlap? 1..2 # => true
(1..2).overlap? 2...2 # => true
```

This is fixed in the Ruby 3.3 implementation of Range#overlap?, so this
commit fixes it for Ruby < 3.3 as well.

The tests added are from the Ruby repository and the implementation is
effectively a Ruby version of the fix in C.

Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: Shouichi Kamiya <shouichi.kamiya@gmail.com>
This commit is contained in:
Hartley McGuire 2023-09-22 19:23:28 -04:00
parent e6f09150a8
commit 64184c6f50
No known key found for this signature in database
GPG Key ID: E823FC1403858A82
3 changed files with 92 additions and 1 deletions

@ -1,3 +1,7 @@
* Fix Range#overlap? not taking empty ranges into account on Ruby < 3.3
*Nobuyoshi Nakada*, *Shouichi Kamiya*, *Hartley McGuire*
* Use Ruby 3.3 Range#overlap? if available
*Yasuo Honda*

@ -6,7 +6,33 @@ class Range
# (1..5).overlap?(7..9) # => false
unless Range.method_defined?(:overlap?)
def overlap?(other)
other.begin == self.begin || cover?(other.begin) || other.cover?(self.begin)
raise TypeError unless other.is_a? Range
self_begin = self.begin
other_end = other.end
other_excl = other.exclude_end?
return false if _empty_range?(self_begin, other_end, other_excl)
other_begin = other.begin
self_end = self.end
self_excl = self.exclude_end?
return false if _empty_range?(other_begin, self_end, self_excl)
return true if self_begin == other_begin
return false if _empty_range?(self_begin, self_end, self_excl)
return false if _empty_range?(other_begin, other_end, other_excl)
true
end
private
def _empty_range?(b, e, excl)
return false if b.nil? || e.nil?
comp = b <=> e
comp.nil? || comp > 0 || (comp == 0 && excl)
end
end

@ -70,6 +70,67 @@ def test_overlaps_alias
assert_not (1...5).overlaps?(6..10)
end
def test_overlap_behaves_like_ruby
assert_not_operator(0..2, :overlap?, -2..-1)
assert_not_operator(0..2, :overlap?, -2...0)
assert_operator(0..2, :overlap?, -1..0)
assert_operator(0..2, :overlap?, 1..2)
assert_operator(0..2, :overlap?, 2..3)
assert_not_operator(0..2, :overlap?, 3..4)
assert_not_operator(0...2, :overlap?, 2..3)
assert_operator(..0, :overlap?, -1..0)
assert_operator(...0, :overlap?, -1..0)
assert_operator(..0, :overlap?, 0..1)
assert_operator(..0, :overlap?, ..1)
assert_not_operator(..0, :overlap?, 1..2)
assert_not_operator(...0, :overlap?, 0..1)
assert_not_operator(0.., :overlap?, -2..-1)
assert_not_operator(0.., :overlap?, ...0)
assert_operator(0.., :overlap?, -1..0)
assert_operator(0.., :overlap?, ..0)
assert_operator(0.., :overlap?, 0..1)
assert_operator(0.., :overlap?, 1..2)
assert_operator(0.., :overlap?, 1..)
assert_not_operator((1..3), :overlap?, ("a".."d"))
assert_raise(TypeError) { (0..).overlap?(1) }
assert_raise(TypeError) { (0..).overlap?(nil) }
assert_operator((1..3), :overlap?, (2..4))
assert_operator((1...3), :overlap?, (2..3))
assert_operator((2..3), :overlap?, (1..2))
assert_operator((..3), :overlap?, (3..))
assert_operator((nil..nil), :overlap?, (3..))
assert_operator((nil...nil), :overlap?, (nil..))
assert_raise(TypeError) { (1..3).overlap?(1) }
assert_not_operator((1..2), :overlap?, (2...2))
assert_not_operator((2...2), :overlap?, (1..2))
assert_not_operator((4..1), :overlap?, (2..3))
assert_not_operator((4..1), :overlap?, (..3))
assert_not_operator((4..1), :overlap?, (2..))
assert_not_operator((1..4), :overlap?, (3..2))
assert_not_operator((..4), :overlap?, (3..2))
assert_not_operator((1..), :overlap?, (3..2))
assert_not_operator((4..5), :overlap?, (2..3))
assert_not_operator((4..5), :overlap?, (2...4))
assert_not_operator((1..2), :overlap?, (3..4))
assert_not_operator((1...3), :overlap?, (3..4))
assert_not_operator((4..5), :overlap?, (2..3))
assert_not_operator((4..5), :overlap?, (2...4))
assert_not_operator((1..2), :overlap?, (3..4))
assert_not_operator((1...3), :overlap?, (3..4))
assert_not_operator((...3), :overlap?, (3..))
end
def test_should_include_identical_inclusive
assert((1..10).include?(1..10))
end