Bug #5536
String#start_with? and end_with? ignore arguments convertible to a String [PATCH]
| Status: | Closed | Start date: | 11/01/2011 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  matz (Yukihiro Matsumoto) |
% Done: | 100% |
|
| Category: | core | |||
| Target version: | 2.0.0 | |||
| ruby -v: | ruby 2.0.0dev (2011-11-01 trunk 33605) [x86_64-linux] |
Description
Hi,
Currently, String#start_with? and String#end_with? ignore arguments not convertible to String.
I believe it should instead raise an error,
as it may lead to false expectations from the user and
it is inconsistent with the rest of the API.
For example, if I try to use start_with? with a RegExp (which would be a nice feature BTW):
"str".start_with? /s/ # => false
I believe it should be:
"str".start_with? /s/ # => TypeError: can't convert Regexp into String
If you prefer the current behavior, could you explain me why?
P.S.: There is no test for String#{start,end}_with? in test/, should I add one or is it enough to change RubySpec (which I'll do when this gets accepted)?
Related issues
Associated revisions
* string.c (rb_str_start_with, rb_str_end_with): raise an error if
an argument is not convertible to a String.
[ruby-core:40623][Bug #5536]
History
Updated by nobu (Nobuyoshi Nakada) 7 months ago
- Status changed from Open to Assigned
- Assignee set to matz (Yukihiro Matsumoto)
(11/11/01 21:43), Benoit Daloze wrote:
> Currently, String#start_with? and String#end_with? ignore arguments not convertible to String.
>
> I believe it should instead raise an error,
> as it may lead to false expectations from the user and
> it is inconsistent with the rest of the API.
Indeed.
> If you prefer the current behavior, could you explain me why?
It hasn't changed from the origin, and I'm curious too.
> P.S.: There is no test for String#{start,end}_with? in test/, should I add one or is it enough to change RubySpec (which I'll do when this gets accepted)?
See test/ruby/test_m17n_comb.rb.
Updated by Eregon (Benoit Daloze) 7 months ago
> Issue #5536 has been updated by Nobuyoshi Nakada.
>
> (11/11/01 21:43), Benoit Daloze wrote:
>> P.S.: There is no test for String#{start,end}_with? in test/, should I add one or is it enough to change RubySpec (which I'll do when this gets accepted)?
>
> See test/ruby/test_m17n_comb.rb.
Sorry, I missed it, I only looked at test/ruby/test_string.rb.
But I'm not sure the error tests belong there, as test_m17n_comb.rb mainly focus on Encoding interactions.
Should I add in test_string.rb or test_m17n_comb.rb?
Here is the diff for test_m17n_comb.rb:
diff --git a/test/ruby/test_m17n_comb.rb b/test/ruby/test_m17n_comb.rb
index 79016af..9c6d5f3 100644
--- a/test/ruby/test_m17n_comb.rb
+++ b/test/ruby/test_m17n_comb.rb
@@ -1551,6 +1551,7 @@ class TestM17NComb < Test::Unit::TestCase
end
assert_equal(false, enccall(s1, :end_with?, s2), desc)
}
+ assert_raise(TypeError) { "str".end_with? :not_convertible_to_string }
end
def test_str_start_with?
@@ -1572,6 +1573,7 @@ class TestM17NComb < Test::Unit::TestCase
end
assert_equal(false, enccall(s1, :start_with?, s2), desc)
}
+ assert_raise(TypeError) { "str".start_with? :not_convertible_to_string }
end
def test_str_ord
Updated by nobu (Nobuyoshi Nakada) 7 months ago
> Should I add in test_string.rb or test_m17n_comb.rb?
test_string.rb, I think.
Updated by Eregon (Benoit Daloze) 7 months ago
Tests attached.
Updated by Eregon (Benoit Daloze) about 1 month ago
- File 0001-string.c-rb_str_start_with-rb_str_end_with-raise-an-.patch added
- File 0002-test-ruby-test_string.rb-add-test_start_with-and-tes.patch added
I have uploaded a new set of patchs to better respect the style in test/ruby/test_string.rb (use S(str) for strings).
Could we merge this?
Updated by nobu (Nobuyoshi Nakada) about 1 month ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
This issue was solved with changeset r35213.
Benoit, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.
----------
* string.c (rb_str_start_with, rb_str_end_with): raise an error if
an argument is not convertible to a String.
[ruby-core:40623][Bug #5536]