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

Add Object#in? support for open ranges #47316

Conversation

joiggama
Copy link
Contributor

@joiggama joiggama commented Feb 8, 2023

Motivation / Background

Fixes #47272

Detail

This Pull Request changes Object#in? behavior to properly handle open date ranges i.e. beginless and endless ranges.

e.g. Date.today.in?(Date.tomorrow..) => false

Additional information

This leverages Range#cover? to properly handle beginless and endless
date range inclusion checks.

Current behavior in Ruby 3.1 is that method never returns, whereas Ruby 3.2 raises a TypeError:

cannot determine inclusion in beginless/endless ranges (TypeError)

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

assert_not Date.today.in?(..Date.yesterday)

# This never returns in Ruby 3.1
Timeout.timeout(0.1) { assert_not Date.today.in?(Date.tomorrow..) }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this assertion. We don't need to test all the behavior of cover?. This will avoid the need for this test to wait at least 0.1s to finish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, also,is no longer necessary for 3.1, I had it before introducing cover in Object#in? change for the test to fail fast.

Copy link
Contributor Author

@joiggama joiggama Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the timeout from last assertion, kept that specific assertion as it was the one with worst outcome (not returning in Ruby 3.1), also reworded the method documentation a bit.

@joiggama joiggama force-pushed the activesupport-object-in-support-open-ranges branch 3 times, most recently from eb8931a to 264905b Compare February 8, 2023 17:24
This leverages Range#cover? to properly handle beginless and endless
range inclusion checks.
@joiggama joiggama force-pushed the activesupport-object-in-support-open-ranges branch from 264905b to 47b0aa0 Compare February 8, 2023 17:32
@rafaelfranca rafaelfranca merged commit b900564 into rails:main Feb 8, 2023
@joiggama joiggama deleted the activesupport-object-in-support-open-ranges branch February 8, 2023 21:49
carlosantoniodasilva added a commit to carlosantoniodasilva/rails that referenced this pull request Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checking if a date in an open date range loops forever when the range starts after the test date
2 participants