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

Revert nil error and adding deprecation message #2637

Merged
merged 2 commits into from Nov 3, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

8 re.c
@@ -3323,7 +3323,9 @@ rb_reg_match_m(int argc, VALUE *argv, VALUE re)
pos = 0;
}

str = SYMBOL_P(str) ? rb_sym2str(str) : StringValue(str);
if (NIL_P(str)) {
rb_warn("given argument is nil; this will raise a TypeError in the next release");
}
pos = reg_match_pos(re, &str, pos);
if (pos < 0) {
rb_backref_set(Qnil);
@@ -3369,6 +3371,10 @@ rb_reg_match_p(VALUE re, VALUE str, long pos)
const UChar *start, *end;
int tmpreg;

if (NIL_P(str)) {
rb_warn("given argument is nil; this will raise a TypeError in the next release");
return Qfalse;
}
str = SYMBOL_P(str) ? rb_sym2str(str) : StringValue(str);
if (pos) {
if (pos < 0) {
@@ -87,7 +87,7 @@
end
end

ruby_version_is ""..."2.7" do
ruby_version_is ""..."3.0" do
it "resets $~ if passed nil" do
# set $~
/./.match("a")
@@ -98,7 +98,13 @@
end
end

ruby_version_is "2.7" do
ruby_version_is "2.7"..."3.0" do
it "warns the deprecation when the given argument is nil" do
-> { /foo/.match(nil) }.should complain(/given argument is nil; this will raise a TypeError in the next release/)
end
end

ruby_version_is "3.0" do
it "raises TypeError when the given argument is nil" do
-> { /foo/.match(nil) }.should raise_error(TypeError)
end
@@ -137,13 +143,19 @@
/str/i.match?('string', 1).should be_false
end

ruby_version_is ""..."2.7" do
ruby_version_is ""..."3.0" do
it "returns false when given nil" do
/./.match?(nil).should be_false
end
end

ruby_version_is "2.7" do
ruby_version_is "2.7"..."3.0" do
it "warns the deprecation" do
-> { /./.match?(nil) }.should complain(/given argument is nil; this will raise a TypeError in the next release/)
end
end

ruby_version_is "3.0" do
it "raises TypeError when given nil" do
-> { /./.match?(nil) }.should raise_error(TypeError)
end
@@ -539,7 +539,7 @@ def test_equal
end

def test_match
assert_raise(TypeError) { //.match(nil) }
assert_nil(//.match(nil))
assert_equal("abc", /.../.match(:abc)[0])
assert_raise(TypeError) { /.../.match(Object.new)[0] }
assert_equal("bc", /../.match('abc', 1)[0])
@@ -561,10 +561,10 @@ def test_match_p
/backref/ =~ 'backref'
# must match here, but not in a separate method, e.g., assert_send,
# to check if $~ is affected or not.
assert_equal(false, //.match?(nil))

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Nov 1, 2019

Contributor

Should we assert the warning?

This comment has been minimized.

Copy link
@eregon

eregon Nov 2, 2019

Member

It's already checked in specs above.

assert_equal(true, //.match?(""))
assert_equal(true, /.../.match?(:abc))
assert_raise(TypeError) { /.../.match?(Object.new) }
assert_raise(TypeError) { //.match?(nil) }
assert_equal(true, /b/.match?('abc'))
assert_equal(true, /b/.match?('abc', 1))
assert_equal(true, /../.match?('abc', 1))
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.