From 0e2de0e3fdc9a1fc763531b74e9fc49666022ff9 Mon Sep 17 00:00:00 2001
From: Jonathan Hefner <jonathan@hefner.pro>
Date: Sun, 5 May 2019 13:19:09 -0500
Subject: Improve String#first and #last performance

Removes unnecessary conditional and method call for significant
performance improvement.  As a side effect, this fixes an unexpected
behavior where passing a limit of 0 would return a frozen string.

This also implements the Rails 6.1 intended behavior with regards to
negative limits, and removes the previous deprecation warnings.

String#first Comparison:
  new:   3056515.0 i/s
  old:   1943310.2 i/s - 1.57x slower

String#last Comparison:
  new:   2691919.0 i/s
  old:   1924256.6 i/s - 1.40x slower

(Note: "old" benchmarks have deprecation warnings commented out, for a
more fair comparison.)
---
 activesupport/test/core_ext/string_ext_test.rb | 32 ++++++++++++++++----------
 1 file changed, 20 insertions(+), 12 deletions(-)

(limited to 'activesupport/test')

diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
index c5a000b67a..b83381bdb2 100644
--- a/activesupport/test/core_ext/string_ext_test.rb
+++ b/activesupport/test/core_ext/string_ext_test.rb
@@ -480,12 +480,16 @@ class StringAccessTest < ActiveSupport::TestCase
     assert_not_same different_string, string
   end
 
-  test "#first with negative Integer is deprecated" do
-    string = "hello"
-    message = "Calling String#first with a negative integer limit " \
-              "will raise an ArgumentError in Rails 6.1."
-    assert_deprecated(message) do
-      string.first(-1)
+  test "#first with Integer returns a non-frozen string" do
+    string = "he"
+    (0..string.length + 1).each do |limit|
+      assert_not string.first(limit).frozen?
+    end
+  end
+
+  test "#first with negative Integer raises ArgumentError" do
+    assert_raise ArgumentError do
+      "hello".first(-1)
     end
   end
 
@@ -507,12 +511,16 @@ class StringAccessTest < ActiveSupport::TestCase
     assert_not_same different_string, string
   end
 
-  test "#last with negative Integer is deprecated" do
-    string = "hello"
-    message = "Calling String#last with a negative integer limit " \
-              "will raise an ArgumentError in Rails 6.1."
-    assert_deprecated(message) do
-      string.last(-1)
+  test "#last with Integer returns a non-frozen string" do
+    string = "he"
+    (0..string.length + 1).each do |limit|
+      assert_not string.last(limit).frozen?
+    end
+  end
+
+  test "#last with negative Integer raises ArgumentError" do
+    assert_raise ArgumentError do
+      "hello".last(-1)
     end
   end
 
-- 
cgit v1.2.3