aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJonathan Hefner <jonathan@hefner.pro>2019-05-05 13:19:09 -0500
committerJonathan Hefner <jonathan@hefner.pro>2019-05-05 13:48:05 -0500
commit0e2de0e3fdc9a1fc763531b74e9fc49666022ff9 (patch)
tree283b765f796f0c2fe34715b400aa3e41ebc34929
parent9b0e632def6a66bbd0c0aba8531b2a173f7c3064 (diff)
downloadrails-0e2de0e3fdc9a1fc763531b74e9fc49666022ff9.tar.gz
rails-0e2de0e3fdc9a1fc763531b74e9fc49666022ff9.tar.bz2
rails-0e2de0e3fdc9a1fc763531b74e9fc49666022ff9.zip
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.)
-rw-r--r--activesupport/lib/active_support/core_ext/string/access.rb24
-rw-r--r--activesupport/test/core_ext/string_ext_test.rb32
2 files changed, 22 insertions, 34 deletions
diff --git a/activesupport/lib/active_support/core_ext/string/access.rb b/activesupport/lib/active_support/core_ext/string/access.rb
index 4ca24028b0..a0a7c54410 100644
--- a/activesupport/lib/active_support/core_ext/string/access.rb
+++ b/activesupport/lib/active_support/core_ext/string/access.rb
@@ -75,17 +75,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
@@ -99,16 +89,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
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