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

Regexp#match{?} with nil raises TypeError as String, Symbol #1506

Merged
merged 8 commits into from Oct 17, 2019

Conversation

@kachick
Copy link
Member

kachick commented Dec 28, 2016

@eregon

This comment has been minimized.

Copy link
Member

eregon commented Dec 28, 2017

@kachick Could you update based on matz's feedback in https://bugs.ruby-lang.org/issues/13083?
So Regexp#match and Regexp#match? should raise TypeError when passed nil.
BTW, you can change specs directly in this PR.

@kachick

This comment has been minimized.

Copy link
Member Author

kachick commented Dec 28, 2017

Sorry for 1 year pending to update.... 🙇 🙇

@kachick kachick changed the title {String|Symbol}#match{?} with nil returns falsy as Regexp#match{?} [WIP] {String|Symbol}#match{?} with nil returns falsy as Regexp#match{?} Dec 28, 2017
@matzbot matzbot force-pushed the ruby:trunk branch from 2677ddd to ce7ad3a Jan 18, 2018
@kachick kachick changed the title [WIP] {String|Symbol}#match{?} with nil returns falsy as Regexp#match{?} Regexp#match{?} with nil raises TypeError as String, Symbol Apr 22, 2018
@kachick kachick force-pushed the kachick:string-symbol-match-with-nil branch from c7a4e32 to 194b55a Apr 22, 2018
kachick added 3 commits Dec 28, 2016
To improve consistency with Regexp#match{?}

* String#match(nil) returns `nil` instead of TypeError
* String#match?(nil) returns `false` instead of TypeError
* Symbol#match(nil) returns `nil` instead of TypeError
* Symbol#match?(nil) returns `false` instead of TypeError
@kachick kachick force-pushed the kachick:string-symbol-match-with-nil branch from afc0fde to 9437b32 Apr 22, 2018
@eregon eregon self-requested a review Apr 22, 2018

/1/.match(nil)
$~.should be_nil
end

This comment has been minimized.

Copy link
@eregon

eregon Apr 22, 2018

Member

It would be better to add a ruby_version_is ""..."2.6" guard instead of removing, since the behavior continues to apply to older versions.
Could you also add specs for the new behavior?

This comment has been minimized.

Copy link
@kachick

kachick Aug 17, 2019

Author Member

Addresses in 1041650 ~

@k0kubun k0kubun changed the base branch from trunk to master Aug 15, 2019
@k0kubun

This comment has been minimized.

Copy link
Member

k0kubun commented Aug 17, 2019

It seems to have a conflict now. Could you rebase this from master?

@kachick

This comment has been minimized.

Copy link
Member Author

kachick commented Aug 17, 2019

I have resolved the conflict!

@znz

This comment has been minimized.

Copy link
Member

znz commented Oct 11, 2019

It seems to have a conflict again. Could you rebase this from master?

@kachick

This comment has been minimized.

Copy link
Member Author

kachick commented Oct 15, 2019

Updated!

@znz znz merged commit 2a22a6b into ruby:master Oct 17, 2019
12 checks passed
12 checks passed
latest (check)
Details
check_branch
Details
latest (check)
Details
latest (windows-2016, 2017, test)
Details
latest (test-bundler)
Details
latest (test-bundler)
Details
latest (windows-2019, 2019, test)
Details
latest (test-bundled-gems)
Details
latest (test-bundled-gems)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr Build is passing
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kachick

This comment has been minimized.

Copy link
Member Author

kachick commented Oct 17, 2019

Thank you!

@kachick kachick deleted the kachick:string-symbol-match-with-nil branch Oct 17, 2019
@rafaelfranca

This comment has been minimized.

Copy link
Contributor

rafaelfranca commented Oct 31, 2019

This is a backward incompatible change. Should it be released in Ruby 2.7?

matzbot pushed a commit that referenced this pull request Dec 3, 2019
…1506)"

This reverts commit 2a22a6b.
Revert [Feature #13083]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.