Options argument hash for deep_clone is modified

[invke]

I recently noticed that the options hash / kwargs given to deep_clone get modified during its execution. It's probably not that common of an issue, as most examples I've seen and usages I've done pass a literal hash as arguments to the method. However recently I had a situation where I moved the options to be constant and froze them, then when I passed them to deep_clone I received something like:

FrozenError: can't modify frozen Hash: { :include => [:expense] }
from ~/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/deep_cloneable-3.0.0/lib/deep_cloneable/deep_clone.rb:8:in `delete'

It made me think that if I forgot to freeze the provided options, then it would have modified it. Future calls would then have been missing the options that get deleted during the execution of deep_clone, a possibly quite hard to trace bug would have occurred. It would also seemingly only be certain options, based on the usage of Hash#delete. It might be good to deep_dup the options hash at the start of deep_clone, I noticed the gem depends upon ActiveSupport core extensions.

# This is just a made-up example loosely related to what I did
class BuildSessionMethodObject
  CARRIED_FORWARD_SESSION_ATTRIBUTES = {
    include: %i[worksheets],
    exclude: [
      :duration,
      :approved_at,
      :approved_by,
      { worksheets: %i[notes] }
    ]
  }.freeze

  def build(session)
    new_session = instance.deep_clone(CARRIED_FORWARD_SESSION_ATTRIBUTES)
    # ... do more things with session
  end
end

Although it's not that big of an issue for me in this situation (I simply .deep_dup the arguments before passing), it could cause really confusing behaviour in other situations, e.g. where the arguments where calculated programmatically and stored in a variable.

If you guys consider this something worth changing I'd be happy to submit a PR, I don't have time currently but I may be able to come Friday.

[moiristo]

Hi, thanks for reaching out! I totally agree that it would be better if the passed options hash would not be modified. Shouldn't be too hard to fix. I'll have some time this weekend. A PR is always welcome of course :)

[moiristo]

Regarding the solution, I think the best solution would be to remove the hash#delete calls and just read from the hash instead as I wonder if those deletes are really necessary.