Nested keys trigger conflict warning

[rpdillon]

Given the following class:

class Foo < Hashie::Mash
  def bar
    self.baz
  end
end

I'm finding that

• Foo.new(baz: 1) generates no warnings, as expected
• Foo.new(bar: 1) generates warnings, as expected
• Foo.new(baz: { bar: 1 }) generates warnings, which is not expected

The final example gives this warning:

W, [2018-02-01T12:07:40.621060 #22182]  WARN -- : You are setting a key that conflicts with a built-in method Foo#bar defined at (irb):2. This can cause unexpected behavior when accessing the key via as a property. You can still access the key via the #[] method.

Is this behavior intended?

[dblock]

That definitely looks like a bug. Care to write a spec and maybe even a fix?

[michaelherold]

This isn't a bug, surprisingly, but it is definitely unexpected behavior. The way Mashes propagate themselves inside of nested hashes involves wrapping the included hashes inside the Mash class. When you subclass a Mash, instead of wrapping the included hashes in a Mash, it wraps it in itself (in this case a Foo).

Since Foo has a #bar method, the warning is raised on the inside hash, as it is wrapped in a Foo.

I'm not sure what would be more surprising - if the inner hash was a Mash, or if the inner hash is a Foo. What do you think?

[dblock]

Mash keeps on giving. Maybe someone can update http://code.dblock.org/2017/02/24/the-demonic-possession-of-hashie-mash.html for me? No but really, please.

I think Mash should propagate all behavior except when otherwise. Frankly this is an amazing side effect - sometimes we want functions to propagate, sometimes not. I want it both ways :)

[dblock]

I would see if wrapping the inside of the Mash in a Hashie::Mash instead of itself breaks any specs. If not I would call that a fix, document it, and add a spec for this exact scenario. Otherwise just document it?

[michaelherold]

This change:

diff --git a/lib/hashie/mash.rb b/lib/hashie/mash.rb
index a6d1347..a5d3900 100644
--- a/lib/hashie/mash.rb
+++ b/lib/hashie/mash.rb
@@ -324,7 +324,7 @@ module Hashie
         duping ? val.dup : val
       when ::Hash
         val = val.dup if duping
-        self.class.new(val)
+        Mash.new(val)
       when Array
         val.map { |e| convert_value(e) }
       when ::Array

causes these failures:

Failures:

  1) Hashie::Extensions::Mash::SymbolizeKeys implicit to_hash on double splat is converted on method calls
     Failure/Error: expect(subject).to eq(outer: { inner: 42 }, testing: [1, 2, 3])

       expected: {:outer=>{:inner=>42}, :testing=>[1, 2, 3]}
            got: {:outer=>#<Hashie::Mash inner=42>, :testing=>#<Hashie::Array [1, 2, 3]>}

       (compared using ==)

       Diff:
       @@ -1,3 +1,3 @@
       -:outer => {:inner=>42},
       +:outer => {"inner"=>42},
        :testing => [1, 2, 3],

     # ./spec/hashie/extensions/mash/symbolize_keys_spec.rb:32:in `block (3 levels) in <top (required)>'

  2) Hashie::Extensions::Mash::SymbolizeKeys implicit to_hash on double splat is converted on explicit operator call
     Failure/Error: expect(**instance).to eq(outer: { inner: 42 }, testing: [1, 2, 3])

       expected: {:outer=>{:inner=>42}, :testing=>[1, 2, 3]}
            got: #<#<Class:0x00007fe753cae648> outer=#<Hashie::Mash inner=42> testing=[1, 2, 3]>

       (compared using ==)

       Diff:
       @@ -1,3 +1,3 @@
       -:outer => {:inner=>42},
       +:outer => {"inner"=>42},
        :testing => [1, 2, 3],

     # ./spec/hashie/extensions/mash/symbolize_keys_spec.rb:36:in `block (3 levels) in <top (required)>'

Finished in 0.18218 seconds (files took 0.83603 seconds to load)
648 examples, 2 failures

Failed examples:

rspec ./spec/hashie/extensions/mash/symbolize_keys_spec.rb:31 # Hashie::Extensions::Mash::SymbolizeKeys implicit to_hash on double splat is converted on method calls
rspec ./spec/hashie/extensions/mash/symbolize_keys_spec.rb:35 # Hashie::Extensions::Mash::SymbolizeKeys implicit to_hash on double splat is converted on explicit operator call

This is because the class that uses the Mash extension loses the extension when we cast it back to a Mash via the change.

Since that's the case, making the change would require a major version bump. I'm not sure I want to do that right now - I would rather save the bump to 4.0 for more changes than this little one. What do you think, dB?

If this is a change we want to make, we could start a 4.0 branch and make it on there ... and if we make the change, I think we need to work out a way to not drop any Mash extensions that are included on the class.

The more I think about it, the more I think that this is probably intended behavior and not a bug, in particular because of the existence of Mash extensions.

---

At any rate, @rpdillon, if you would like to suppress the warning, you can do this:

class Foo < Hashie::Mash
  disable_warnings

  def bar
    self.baz
  end
end

[dblock]

I think we should leave things as is and document the behavior of inheriting the class itself for nested mashes. We can also create a class called MashMash that does something else :)