Don't warn when setting most affixed keys (#500)

Due to how we have implemented the bang/underbang/query behavior within
Mash, setting keys that have those affixes in them actually allow
overwriting the behavior of those affixes. As such, we shouldn't warn
when setting a key that matches those patterns.

When it comes to setter-like keys, I believe we still _do_ want to warn
for two reasons:

1. Trying to access the key via method access is a syntax error. Ruby
   expects any method ending in `=` to be a 2+-arity method due to the
   infix notation of setter methods. This is unexpected behavior unless
   you're very familiar with Ruby parsing.
2. You can still retrieve the key via the normal `Hash#[]` reader, but
   it prevents setting a similar key without the equal sign. You can see
   this in the test about setters. I'd say that is unexpected and
   surprising behavior.

Because of these two gotchas, I think we should still warn in cases
where you try to set a key that looks like a setter.
This commit is contained in:
Michael Herold 2020-01-15 13:17:27 -06:00 committed by GitHub
parent 25fe2f747e
commit eb69c58b62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 1 deletions

View File

@ -34,6 +34,7 @@ scheme are considered to be bugs.
* [#507](https://github.com/hashie/hashie/pull/507): Suppress `Psych.safe_load` arg warn when using Psych 3.1.0+ - [@koic](https://github.com/koic).
* [#508](https://github.com/hashie/hashie/pull/508): Fixed `Mash.load` no longer uses Rails-only `#except` - [@bobbymcwho](https://github.com/bobbymcwho).
* [#508](https://github.com/hashie/hashie/pull/508): Fixed `Hashie::Extensions::DeepMerge` `#deep_merge` not correctly dup'ing sub-hashes if active_support hash extensions were not present - [@bobbymcwho](https://github.com/bobbymcwho).
* [#500](https://github.com/hashie/hashie/pull/500): Do not warn when setting Mash keys that look like underbang, bang, and query methods - [@michaelherold](https://github.com/michaelherold).
* [#510](https://github.com/hashie/hashie/pull/510): Ensure that `Hashie::Mash#compact` is only defined on Ruby version >= 2.4.0 - [@bobbymcwho](https://github.com/bobbymcwho).
* Your contribution here.

View File

@ -406,7 +406,12 @@ module Hashie
end
def log_collision?(method_key)
respond_to?(method_key) && !self.class.disable_warnings?(method_key) &&
return unless respond_to?(method_key)
_, suffix = method_name_and_suffix(method_key)
(!suffix || suffix == '='.freeze) &&
!self.class.disable_warnings?(method_key) &&
!(regular_key?(method_key) || regular_key?(method_key.to_s))
end
end

View File

@ -159,6 +159,28 @@ describe Hashie::Mash do
expect(logger_output).to be_empty
end
it 'does not write to the logger when setting most affixed keys' do
underbang = Hashie::Mash.new('foo_' => 'foo')
bang = Hashie::Mash.new('foo!' => 'foo')
query = Hashie::Mash.new('foo?' => 'foo')
expect(logger_output).to be_empty
expect(underbang.foo_).to eq 'foo'
expect(bang.foo!).to eq 'foo'
expect(query.foo?).to eq 'foo'
end
it 'warns when setting a key that looks like a setter' do
setter = Hashie::Mash.new('foo=' => 'foo')
expect(logger_output).to match 'Hashie::Mash#foo='
expect('setter.foo=').not_to parse_as_valid_ruby
setter.foo = 'bar'
expect(setter.to_h).to eq 'foo=' => 'foo'
end
it 'cannot disable logging on the base Mash' do
expected_error = Hashie::Extensions::KeyConflictWarning::CannotDisableMashWarnings

View File

@ -10,6 +10,7 @@ require 'hashie'
require 'rspec/pending_for'
require './spec/support/ruby_version_check'
require './spec/support/logger'
require './spec/support/matchers'
RSpec.configure do |config|
config.extend RubyVersionCheck

13
spec/support/matchers.rb Normal file
View File

@ -0,0 +1,13 @@
RSpec::Matchers.define :parse_as_valid_ruby do
require 'ripper'
match do |actual|
parsed = Ripper.sexp(actual)
!parsed.nil?
end
failure_message do |actual|
"expected that #{actual} would parse as valid Ruby"
end
end