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

• Status: Closed
• Assignee: matz (Yukihiro Matsumoto)
• Target version: 2.0.0
• ruby -v: ruby 2.0.0dev (2011-11-01 trunk 33605) [x86_64-linux]

[Eregon (Benoit Daloze)]

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)?

[nobu (Nobuyoshi Nakada)]

• 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.

[Eregon (Benoit Daloze)]

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

[nobu (Nobuyoshi Nakada)]

Should I add in test_string.rb or test_m17n_comb.rb?

test_string.rb, I think.

[Eregon (Benoit Daloze)]

• File 0002-test-ruby-test_string.rb-add-test_start_with-and-tes.patch 0002-test-ruby-test_string.rb-add-test_start_with-and-tes.patch added

Tests attached.

[Eregon (Benoit Daloze)]

• File 0001-string.c-rb_str_start_with-rb_str_end_with-raise-an-.patch 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 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?

[nobu (Nobuyoshi Nakada)]

• 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]

[prijutme4ty (Ilya Vorontsov)]

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

[Eregon (Benoit Daloze)]

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 #start_with?, there is a C-level API for matching only at start, but there is no corresponding API for #end_with?.