Improve bang method defs, make slice! operate in-place. [#5028 state:resolved]

Signed-off-by: NJosé Valim <jose.valim@gmail.com>

activesupport/lib/active_support/core_ext/string/multibyte.rb

@@ -2,7 +2,7 @@
 require 'active_support/multibyte'
 
 class String
-  if '1.9'.respond_to?(:force_encoding)
+  if RUBY_VERSION >= "1.9"
     # == Multibyte proxy
     #
     # +mb_chars+ is a multibyte safe proxy for string methods.

activesupport/lib/active_support/multibyte/chars.rb

@@ -325,18 +325,6 @@ def slice(*args)
       end
       alias_method :[], :slice
 
-      # Like <tt>String#slice!</tt>, except instead of byte offsets you specify character offsets.
-      #
-      # Example:
-      #   s = 'こんにちは'
-      #   s.mb_chars.slice!(2..3).to_s #=> "にち"
-      #   s #=> "こんは"
-      def slice!(*args)
-        slice = self[*args]
-        self[*args] = ''
-        slice
-      end
-
       # Limit the byte size of the string to a number of bytes without breaking characters. Usable
       # when the storage for a string is limited for some reason.
       #
@@ -425,14 +413,14 @@ def tidy_bytes(force = false)
         chars(Unicode.tidy_bytes(@wrapped_string, force))
       end
 
-      %w(lstrip rstrip strip reverse upcase downcase tidy_bytes capitalize).each do |method|
-        define_method("#{method}!") do |*args|
-          unless args.nil?
-            @wrapped_string = send(method, *args).to_s
-          else
-            @wrapped_string = send(method).to_s
+       %w(capitalize downcase lstrip reverse rstrip slice strip tidy_bytes upcase).each do |method|
+        # Only define a corresponding bang method for methods defined in the proxy; On 1.9 the proxy will
+        # exclude lstrip!, rstrip! and strip! because they are already work as expected on multibyte strings.
+        if public_method_defined?(method)
+          define_method("#{method}!") do |*args|
+            @wrapped_string = send(args.nil? ? method : method, *args).to_s
+            self
           end
-          self
         end
       end
 

activesupport/test/multibyte_chars_test.rb

@@ -123,22 +123,30 @@ def test_indexed_insert_accepts_fixnums
     assert_equal 'こに わ', @chars
   end
 
-  def test_overridden_bang_methods_return_self
-    [:rstrip!, :lstrip!, :strip!, :reverse!, :upcase!, :downcase!, :capitalize!].each do |method|
-      assert_equal @chars.object_id, @chars.send(method).object_id
-    end
+  %w{capitalize downcase lstrip reverse rstrip strip upcase}.each do |method|
+    class_eval(<<-EOTESTS)
+      def test_#{method}_bang_should_return_self
+        assert_equal @chars.object_id, @chars.send("#{method}!").object_id
+      end
+
+      def test_#{method}_bang_should_change_wrapped_string
+        original = ' él piDió Un bUen café '
+        proxy = chars(original.dup)
+        proxy.send("#{method}!")
+        assert_not_equal original, proxy.to_s
+      end
+    EOTESTS
   end
 
-  def test_overridden_bang_methods_change_wrapped_string
-    [:rstrip!, :lstrip!, :strip!, :reverse!, :upcase!, :downcase!].each do |method|
-      original = ' Café '
-      proxy = chars(original.dup)
-      proxy.send(method)
-      assert_not_equal original, proxy.to_s
-    end
-    proxy = chars('òu')
-    proxy.capitalize!
-    assert_equal 'Òu', proxy.to_s
+  def test_tidy_bytes_bang_should_return_self
+    assert_equal @chars.object_id, @chars.tidy_bytes!.object_id
+  end
+
+  def test_tidy_bytes_bang_should_change_wrapped_string
+    original = " Un bUen café \x92"
+    proxy = chars(original.dup)
+    proxy.tidy_bytes!
+    assert_not_equal original, proxy.to_s
   end
 
   if RUBY_VERSION >= '1.9'
@@ -417,8 +425,9 @@ def test_slice_bang_returns_sliced_out_substring
   end
 
   def test_slice_bang_removes_the_slice_from_the_receiver
-    @chars.slice!(1..2)
-    assert_equal 'こわ', @chars
+    chars = 'úüù'.mb_chars
+    chars.slice!(0,2)
+    assert_equal 'úü', chars
   end
 
   def test_slice_should_throw_exceptions_on_invalid_arguments