Merge pull request #36185 from jonathanhefner/optimize-string-first-and-last

Improve String#first and #last performance

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

@@ -76,17 +76,7 @@ class String
   #   str.first(0) # => ""
   #   str.first(6) # => "hello"
   def first(limit = 1)
-    ActiveSupport::Deprecation.warn(
-      "Calling String#first with a negative integer limit " \
-      "will raise an ArgumentError in Rails 6.1."
-    ) if limit < 0
-    if limit == 0
-      ""
-    elsif limit >= size
-      dup
-    else
-      to(limit - 1)
-    end
+    self[0, limit] || (raise ArgumentError, "negative limit")
   end
 
   # Returns the last character of the string. If a limit is supplied, returns a substring
@@ -100,16 +90,6 @@ class String
   #   str.last(0) # => ""
   #   str.last(6) # => "hello"
   def last(limit = 1)
-    ActiveSupport::Deprecation.warn(
-      "Calling String#last with a negative integer limit " \
-      "will raise an ArgumentError in Rails 6.1."
-    ) if limit < 0
-    if limit == 0
-      ""
-    elsif limit >= size
-      dup
-    else
-      from(-limit)
-    end
+    self[[length - limit, 0].max, limit] || (raise ArgumentError, "negative limit")
   end
 end

activesupport/test/core_ext/string_ext_test.rb

@@ -482,12 +482,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
 
@@ -509,12 +513,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