Bug #13312
closedString#casecmp raises TypeError instead of returning nil
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
Updated by stomar (Marcus Stollsteimer) over 7 years ago
- Description updated (diff)
Updated by shingo (shingo morita) over 7 years ago
- File patch.diff patch.diff added
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) over 7 years ago
- Status changed from Open to Assigned
- Assignee set to stomar (Marcus Stollsteimer)
It should return nil same as other comparison methods. Can you handle this?
Updated by stomar (Marcus Stollsteimer) over 7 years ago
Can you handle this?
Ok.
Updated by stomar (Marcus Stollsteimer) over 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) over 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.
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) over 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) over 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.