Make String methods return String instances when called on a subclass instance #3701
Conversation
1f9340c to
a94ff5a
Compare
|
Big PR 💪 |
|
I haven't tested with Rails, but I think it would need some adjusting for this. Rails appears to mark many of these methods as unsafe (they are in UNSAFE_STRING_METHODS), but some methods it appears to assume it will return the same class, such as |
|
We don't have to worry about Rails that much in this case. I suppose you're talking about this thing in Rails. https://github.com/rails/rails/blob/cf4fa5ca/activesupport/lib/active_support/core_ext/string/output_safety.rb#L133-L134 ActiveSupport::SafeBuffer.new('hello').reverse.class
#=> Stringand so this change won't bring that much breakage to SafeBuffer. In fact I could make all active_support and action_view tests pass on this branch with the following small patch for the SafeBuffer class. diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb
index 635f9cf457..c04b8baa30 100644
--- a/activesupport/lib/active_support/core_ext/string/output_safety.rb
+++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb
@@ -152,12 +152,12 @@ def initialize
def [](*args)
if html_safe?
- new_safe_buffer = super
+ new_string = super
- if new_safe_buffer
- new_safe_buffer.instance_variable_set :@html_safe, true
- end
+ return unless new_string
+ new_safe_buffer = self.class.new(new_string)
+ new_safe_buffer.instance_variable_set :@html_safe, true
new_safe_buffer
else
to_str[*args]
@@ -213,7 +213,7 @@ def +(other)
end
def *(*)
- new_safe_buffer = super
+ new_safe_buffer = self.class.new(super)
new_safe_buffer.instance_variable_set(:@html_safe, @html_safe)
new_safe_buffer
end |
… instance This modifies the following String methods to return String instances instead of subclass instances: * String#* * String#capitalize * String#center * String#chomp * String#chop * String#delete * String#delete_prefix * String#delete_suffix * String#downcase * String#dump * String#each/#each_line * String#gsub * String#ljust * String#lstrip * String#partition * String#reverse * String#rjust * String#rpartition * String#rstrip * String#scrub * String#slice! * String#slice/#[] * String#split * String#squeeze * String#strip * String#sub * String#succ/#next * String#swapcase * String#tr * String#tr_s * String#upcase This also fixes a bug in String#swapcase where it would return the receiver instead of a copy of the receiver if the receiver was the empty string. Some string methods were left to return subclass instances: * String#+@ * String#-@ Both of these methods will return the receiver (subclass instance) in some cases, so it is best to keep the returned class consistent. Fixes [ruby#10845]
a94ff5a to
2cf092f
Compare
… in Ruby 3 Ruby 3 introduces an incompatibility that String methods return String instances when called on a subclass instance. https://bugs.ruby-lang.org/issues/10845 ruby/ruby#3701
This modifies the following String methods to return String instances
instead of subclass instances:
This also fixes a bug in String#swapcase where it would return the
receiver instead of a copy of the receiver if the receiver was the
empty string.
Some string methods were left to return subclass instances:
Both of these methods will return the receiver (subclass instance)
in some cases, so it is best to keep the returned class consistent.
Fixes [#10845]