Skip to content

Commit

Permalink
Copy options before delegating in with_options
Browse files Browse the repository at this point in the history
cd31e11 switched to passing options as
keyword arguments, which always creates a new hash.

9e4ff29 removed a now-unnecessary call
to `dup`, since the options could no longer be accidentally mutated.

a55620f switched back to passing
options as a positional argument for Ruby < 2.7, but didn't restore the
call to `dup`, which meant that the same options hash was now passed
with every method call and mutations leaked from one call to another.
  • Loading branch information
eugeneius committed May 23, 2020
1 parent 277d637 commit 537b071
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
2 changes: 1 addition & 1 deletion activesupport/lib/active_support/option_merger.rb
Expand Up @@ -38,7 +38,7 @@ def invoke_method(method, arguments, options, &block)
end
else
def invoke_method(method, arguments, options, &block)
arguments << options if options
arguments << options.dup if options
@context.__send__(method, *arguments, &block)
end
end
Expand Down
6 changes: 6 additions & 0 deletions activesupport/test/option_merger_test.rb
Expand Up @@ -37,6 +37,12 @@ def test_method_with_options_appends_options_when_options_are_missing
end
end

def test_method_with_options_copies_options_when_options_are_missing
with_options(@options) do |o|
assert_not_same @options, o.method_with_options
end
end

def test_method_with_options_allows_to_overwrite_options
local_options = { hello: "moon" }
assert_equal @options.keys, local_options.keys
Expand Down

0 comments on commit 537b071

Please sign in to comment.