Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align Range#cover? extension behavior with plain Ruby behavior for "backwards" ranges #38837

Merged
merged 1 commit into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions activesupport/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
* Align `Range#cover?` extension behavior with Ruby behavior for backwards ranges.

`(1..10).cover?(5..3)` now returns `false`, as it does in plain Ruby.

Also update `#include?` and `#===` behavior to match.

*Michael Groeneman*

* Update to TZInfo v2.0.0.

This changes the output of `ActiveSupport::TimeZone.utc_to_local`, but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ module CompareWithRange
# 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.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? ? :< : :<=
Expand All @@ -38,6 +40,8 @@ def ===(value)
# 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.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? ? :< : :<=
Expand All @@ -61,6 +65,8 @@ def include?(value)
# The given range must be fully bounded, with both start and end.
def cover?(value)
if value.is_a?(::Range)
is_backwards_op = value.exclude_end? ? :>= : :>
return false if value.begin && value.end && value.begin.send(is_backwards_op, value.end)
# 1...10 covers 1..9 but it does not cover 1..10.
# 1..10 covers 1...11 but it does not cover 1...12.
operator = exclude_end? && !value.exclude_end? ? :< : :<=
Expand Down
27 changes: 27 additions & 0 deletions activesupport/test/core_ext/range_ext_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,15 @@ def test_should_include_other_with_exclusive_end
assert((1..10).include?(1...11))
end

def test_include_returns_false_for_backwards
assert_not((1..10).include?(5..3))
end

# Match quirky plain-Ruby behavior
def test_include_returns_false_for_empty_exclusive_end
assert_not((1..5).include?(3...3))
end

if RUBY_VERSION >= "2.6"
def test_include_with_endless_range
assert(eval("1..").include?(2))
Expand Down Expand Up @@ -100,6 +109,15 @@ def test_should_compare_other_with_exclusive_end
assert((1..10) === (1...11))
end

def test_compare_returns_false_for_backwards
assert_not((1..10) === (5..3))
end

# Match quirky plain-Ruby behavior
def test_compare_returns_false_for_empty_exclusive_end
assert_not((1..5) === (3...3))
end

if RUBY_VERSION >= "2.6"
def test_should_compare_range_with_endless_range
assert(eval("1..") === (2..4))
Expand Down Expand Up @@ -145,6 +163,15 @@ def test_should_cover_other_with_exclusive_end
assert((1..10).cover?(1...11))
end

def test_cover_returns_false_for_backwards
assert_not((1..10).cover?(5..3))
end

# Match quirky plain-Ruby behavior
def test_cover_returns_false_for_empty_exclusive_end
assert_not((1..5).cover?(3...3))
end

if RUBY_VERSION >= "2.6"
def test_should_cover_range_with_endless_range
assert(eval("1..").cover?(2..4))
Expand Down