Mash existence methods (?) return false for false values

[jmanian]

I'm not sure if this is expected behavior or not, but it seems unexpected based on the comment here:

hashie/lib/hashie/mash.rb

Line 19 in d5f2539

# * Existence (<tt>?</tt>): Returns true or false depending on whether that key has been set.

Here is an example:

[31] pry(main)> m = Hashie::Mash.new(a: 1, b: true, c: false)
=> {"a"=>1, "b"=>true, "c"=>false}
[32] pry(main)> m.a?
=> true
[33] pry(main)> m.b?
=> true
[34] pry(main)> m.c?  # I expect this to return true because the key has been set
=> false              # However it returns false
[35] pry(main)> m.d?
=> false
[36] pry(main)> m.a
=> 1
[37] pry(main)> m.b
=> true
[38] pry(main)> m.c
=> false
[39] pry(main)> m.d
=> nil

In other words, I expect the existence ? methods to behave the same as Hash#has_key?

If this is the expected behavior then I can submit a PR to update the comment.
If this is unexpected behavior then I can submit a PR to change the behavior.

One solution might be to replace !!self[name] with self.has_key?(name) here:

hashie/lib/hashie/mash.rb

Line 312 in d5f2539

!!self[name]

Edit: I should add that the same applies to nil values.

[dblock]

Oh my. I bet the original intent was presence, but this is obviously not what we have implemented.

Could you see what specs we break if the behavior actually does what the comment says? I am going to assume it's a lot, in which case let's change the comment and document in README.

[michaelherold]

It breaks a single test that is explicitly testing that behavior:

hashie/spec/hashie/mash_spec.rb

Lines 42 to 47 in d5f2539

	it 'returns false on a ? method if a value has been set to nil or false' do
	subject.test = nil
	expect(subject).not_to be_test
	subject.test = false
	expect(subject).not_to be_test
	end

I believe query methods are for truthiness checks against the value at the key.

[jmanian]

I did a little digging and it turns out that it used to do what the comment says, and it was even implemented with key? at that time. It was changed to the current behavior intentionally way back in 2010 with 5a53de5, in response to #4.

5a53de5 also added a note to the readme to explain the behavior, and the note is still there today. But it left the original comment in place, which described the old behavior.

So I'll submit a PR to change the comment to something like this:

 # * Truthiness (<tt>?</tt>): Returns true or false depending on the truthiness of
 #   the attribute, or false if the key is not set.

[dblock]

The moral of the story is to not document your code.