Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Calling unsafe methods which don't return a string shouldn't fail #1750

Merged
merged 1 commit into from

2 participants

@dmathieu
Collaborator

Unsafe methods are always calling #to_str when called so the object returned is a new string object.

If the original method doesn't return a string, like gsub with only one argument though, it'll fail as there's no to_str method.

@dmathieu
Collaborator

I have just changed my commit, to simplify the logic of the improvement.

@fxn
Owner

send accepts strings, no need to create a symbol from them.

@dmathieu
Collaborator

Actually, as we're in a class_eval and the method name is a variable, I must either create a string or a symbol.
For micro-performances reasons, symbols are then better as it wouldn't create a new object instance if the same symbol has already been called.

@dmathieu
Collaborator

@fxn : what I mean is, anyway, we must either create a string or a symbol.
Creating a symbol avoids creating a new object every time the method is called.

@fxn
Owner

Would it be better to interpolate unsafe_method as an ordinary method call?

@dmathieu
Collaborator

Oh ok, right, I definitely can do that.

@dmathieu
Collaborator

I have updated my commit.

@fxn
Owner

Excellent, thank you :).

@fxn fxn merged commit 5654f68 into rails:master
@jake3030 jake3030 referenced this pull request from a commit in jake3030/rails
@al2o3cr al2o3cr break out of initializer early if gems aren't loaded [#1750 state:res…
…olved]

Signed-off-by: Joshua Peek <josh@joshpeek.com>
01c818e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
4 activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -137,8 +137,8 @@ def to_yaml(*args)
UNSAFE_STRING_METHODS.each do |unsafe_method|
class_eval <<-EOT, __FILE__, __LINE__
- def #{unsafe_method}(*args)
- super.to_str
+ def #{unsafe_method}(*args, &block)
+ to_str.#{unsafe_method}(*args, &block)
end
def #{unsafe_method}!(*args)
View
4 activesupport/test/safe_buffer_test.rb
@@ -104,4 +104,8 @@ def setup
@buffer.safe_concat "BUSTED"
end
end
+
+ test "should not fail if the returned object is not a string" do
+ assert_kind_of Enumerator, @buffer.gsub(/.*/)
+ end
end
Something went wrong with that request. Please try again.