Project

General

Profile

Actions

Bug #13312

closed

String#casecmp raises TypeError instead of returning nil

Added by stomar (Marcus Stollsteimer) about 7 years ago. Updated almost 7 years ago.

Status:
Closed
Target version:
-
[ruby-core:80145]

Description

String#casecmp and String#casecmp? behave differently from other comparison methods: for incomparable values they raise a TypeError, while Symbol#{casecmp,casecmp?} and the #<=> methods (also for other classes) return nil:

"abc" <=> 1       # => nil
"abc".casecmp 1   # TypeError: no implicit conversion of Integer into String
"abc".casecmp? 1  # TypeError: no implicit conversion of Integer into String

:abc <=> 1        # => nil
:abc.casecmp 1    # => nil
:abc.casecmp? 1   # => nil

1  <=> Time.now   # => nil
[] <=> :foo       # => nil

This is surprising, since String#casecmp is essentially a case-insensitive version of String#<=>, which seems to imply that they should behave in a similar way. Also, the different behavior for String and Symbol might be an indication that this is a bug and not intentional.


Files

patch.diff (2.5 KB) patch.diff shingo (shingo morita), 03/29/2017 09:57 AM
Actions #1

Updated by stomar (Marcus Stollsteimer) about 7 years ago

  • Description updated (diff)

Updated by shingo (shingo morita) about 7 years ago

I agree this is not expecting behavior. then I made a patch for this.
Added test codes below.

diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb
index 7dbf27e..3b72d25 100644
--- a/test/ruby/test_string.rb
+++ b/test/ruby/test_string.rb
@@ -2318,6 +2318,12 @@ def test_compare_different_encoding_string
 
   def test_casecmp
     assert_equal(1, "\u3042B".casecmp("\u3042a"))
+    assert_nil("Foo".casecmp(:Foo))
+    assert_nil("1".casecmp(1))
+
+    o = Object.new
+    def o.to_str; "abcde"; end
+    assert_equal(1, "abcdef".casecmp(o))
   end
 
   def test_casecmp?
@@ -2325,6 +2331,12 @@ def test_casecmp?
     assert_equal(false, 'FoO'.casecmp?('BaR'))
     assert_equal(false, 'baR'.casecmp?('FoO'))
     assert_equal(true, 'äöü'.casecmp?('ÄÖÜ'))
+    assert_nil("Foo".casecmp?(:Foo))
+    assert_nil("1".casecmp?(1))
+
+    o = Object.new
+    def o.to_str; "foo"; end
+    assert_equal(true, "Foo".casecmp?(o))
   end
 
   def test_upcase2

Updated by hsbt (Hiroshi SHIBATA) almost 7 years ago

  • Status changed from Open to Assigned
  • Assignee set to stomar (Marcus Stollsteimer)

@stomar (Marcus Stollsteimer)

It should return nil same as other comparison methods. Can you handle this?

Updated by stomar (Marcus Stollsteimer) almost 7 years ago

Can you handle this?

Ok.

Actions #5

Updated by stomar (Marcus Stollsteimer) almost 7 years ago

  • Status changed from Assigned to Closed

Applied in changeset trunk|r58837.


string.c: fix String#{casecmp,casecmp?} for non-string arguments

  • string.c: make String#{casecmp,casecmp?} return nil for
    non-string arguments instead of raising a TypeError.

  • test/ruby/test_string.rb: add tests.

Reported by Marcus Stollsteimer. Based on a patch by Shingo Morita.
[ruby-core:80145] [Bug #13312]

Updated by stomar (Marcus Stollsteimer) almost 7 years ago

I did not include the type check in str_casecmp, which seems superfluous to me. I also adjusted the tests, docs, and examples to those of the corresponding Symbol methods, and fixed the code formatting.

@hsbt (Hiroshi SHIBATA)

I also added a NEWS entry and updated the specs with a version guard for 2.5 (I do not know how this would be handled in case the change should be backported).

Updated by Eregon (Benoit Daloze) almost 7 years ago

stomar (Marcus Stollsteimer) wrote:

I also added a NEWS entry and updated the specs with a version guard for 2.5 (I do not know how this would be handled in case the change should be backported).

Thanks, r58839 looks great.

In case of backport we can adjust the version guard, but only after a release of 2.x as we need to test against releases in ruby/spec Travis and locally.

Updated by usa (Usaku NAKAMURA) almost 7 years ago

  • Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN to 2.2: WONTFIX, 2.3: WONTFIX, 2.4: UNKNOWN

I think this change may not cause any compatibility issue with real-world scripts, but it means that nobody hits this problem, too.
Then I decided not to backport to ruby_2_3.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0