Bug #5536

String#start_with? and end_with? ignore arguments convertible to a String [PATCH]

Added by Benoit Daloze over 2 years ago. Updated about 1 year ago.

[ruby-core:40623]
Status:Closed
Priority:Normal
Assignee:Yukihiro Matsumoto
Category:core
Target version:2.0.0
ruby -v:ruby 2.0.0dev (2011-11-01 trunk 33605) [x86_64-linux] Backport:

Description

Hi,

Currently, String#startwith? and String#endwith? 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 startwith? 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)?

0001-string.c-rb_str_start_with-rb_str_end_with-raise-an-.patch Magnifier (1.17 KB) Benoit Daloze, 11/01/2011 09:43 PM

0002-test-ruby-test_string.rb-add-test_start_with-and-tes.patch Magnifier (1.58 KB) Benoit Daloze, 11/02/2011 12:03 AM

0001-string.c-rb_str_start_with-rb_str_end_with-raise-an-.patch Magnifier - second set of patchs (fix) (1.17 KB) Benoit Daloze, 03/25/2012 09:11 PM

0002-test-ruby-test_string.rb-add-test_start_with-and-tes.patch Magnifier - second set of patchs (tests) (1.63 KB) Benoit Daloze, 03/25/2012 09:11 PM


Related issues

Duplicates ruby-trunk - Feature #3388: regexp support for start_with? and end_with? Feedback 06/04/2010

Associated revisions

Revision 35213
Added by Nobuyoshi Nakada about 2 years ago

  • string.c (rbstrstartwith, rbstrendwith): raise an error if an argument is not convertible to a String. [Bug #5536]

History

#1 Updated by Nobuyoshi Nakada over 2 years ago

  • Status changed from Open to Assigned
  • Assignee set to Yukihiro Matsumoto

(11/11/01 21:43), Benoit Daloze wrote:

Currently, String#startwith? and String#endwith? 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/testm17ncomb.rb.

#2 Updated by Benoit Daloze over 2 years 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/testm17ncomb.rb.

Sorry, I missed it, I only looked at test/ruby/teststring.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 teststring.rb or testm17n_comb.rb?

Here is the diff for testm17ncomb.rb:

diff --git a/test/ruby/testm17ncomb.rb b/test/ruby/testm17ncomb.rb
index 79016af..9c6d5f3 100644
--- a/test/ruby/testm17ncomb.rb
+++ b/test/ruby/testm17ncomb.rb
@@ -1551,6 +1551,7 @@ class TestM17NComb < Test::Unit::TestCase
end
assertequal(false, enccall(s1, :endwith?, s2), desc)
}
+ assertraise(TypeError) { "str".endwith? :notconvertibleto_string }
end

def teststrstartwith?
@@ -1572,6 +1573,7 @@ class TestM17NComb < Test::Unit::TestCase
end
assert
equal(false, enccall(s1, :startwith?, s2), desc)
}
+ assert
raise(TypeError) { "str".startwith? :notconvertibletostring }
end

def teststrord

#3 Updated by Nobuyoshi Nakada over 2 years ago

Should I add in teststring.rb or testm17n_comb.rb?

test_string.rb, I think.

#5 Updated by Benoit Daloze about 2 years ago

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?

#6 Updated by Nobuyoshi Nakada about 2 years 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 (rbstrstartwith, rbstrendwith): raise an error if an argument is not convertible to a String. [Bug #5536]

#7 Updated by Ilya Vorontsov about 1 year ago

=begin
Can you please explain, why not just make (({"str".start_with(/s/)})) behave as (({"str".match(/s/)})) ?
=end

#8 Updated by Benoit Daloze about 1 year ago

prijutme4ty (Ilya Vorontsov) wrote:

=begin
Can you please explain, why not just make (({"str".start_with(/s/)})) behave as (({"str".match(/s/)})) ?
=end

See #3388 for this feature (this issue is closed).
It is the approach I initially took, but building and compiling a Regexp is expensive, and so it should be cached or another way be used. For #startwith?, there is a C-level API for matching only at start, but there is no corresponding API for #endwith?.

Also available in: Atom PDF