Add selective key-conflict warnings for Mash subclasses (#478)

In some cases, you want to be able to ignore Mash warnings for keys
that you know you aren't going to access via a method accessor, yet be
warned for other keys that you know you might want to access. This
change adds the ability to selectively ignore warnings for specific keys
instead of globally ignoring the warnings.

The change retains the original behavior as well, so if you call
`Mash.disable_warnings` without a value it will still globally ignore
the warnings.

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-03-22 11:21:24 -0400 using RuboCop version 0.52.1.
+# on 2019-07-17 09:23:49 -0400 using RuboCop version 0.52.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -13,7 +13,7 @@ Metrics/AbcSize:
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 212
+  Max: 221
 
 # Offense count: 7
 Metrics/CyclomaticComplexity:

CHANGELOG.md

@@ -14,6 +14,7 @@ scheme are considered to be bugs.
 
 * [#323](https://github.com/intridea/hashie/pull/323): Added `Hashie::Extensions::Mash::DefineAccessors` - [@marshall-lee](https://github.com/marshall-lee).
 * [#474](https://github.com/intridea/hashie/pull/474): Expose `YAML#safe_load` options in `Mash#load` - [@riouruma](https://github.com/riouruma), [@dblock](https://github.com/dblock).
+* [#478](https://github.com/intridea/hashie/pull/478): Added optional array parameter to `Hashie::Mash.disable_warnings` - [@bobbymcwho](https://github.com/bobbymcwho).
 
 ### Changed
 

README.md

@@ -515,6 +515,48 @@ class Response < Hashie::Mash
 end
 ```
 
+The default is to disable logging for all methods that conflict. If you would like to only disable the logging for specific methods, you can include an array of method keys:
+
+```ruby
+class Response < Hashie::Mash
+  disable_warnings :zip, :zap
+end
+```
+
+This behavior is cumulative. The examples above and below behave identically.
+
+```ruby
+class Response < Hashie::Mash
+  disable_warnings :zip
+  disable_warnings :zap
+end
+```
+
+Disable warnings will honor the last `disable_warnings` call. Calling without parameters will override the blacklist, and calling with parameters will create a new blacklist. This includes child classes that inherit from a class that disables warnings.
+
+```ruby
+class Message < Hashie::Mash
+  disable_warnings :zip, :zap
+  disable_warnings
+end
+
+# No errors will be logged
+Message.new(merge: 'true', compact: true)
+```
+
+```ruby
+class Message < Hashie::Mash
+  disable_warnings
+end
+
+class Response < Message
+  disable_warnings :zip, :zap
+end
+
+# 2 errors will be logged
+Response.new(merge: 'true', compact: true, zip: '90210', zap: 'electric')
+```
+
 ### How does the wrapping of Mash sub-Hashes work?
 
 Mash duplicates any sub-Hashes that you add to it and wraps them in a Mash. This allows for infinite chaining of nested Hashes within a Mash without modifying the object(s) that are passed into the Mash. When you subclass Mash, the subclass wraps any sub-Hashes in its own class. This preserves any extensions that you mixed into the Mash subclass and allows them to work within the sub-Hashes, in addition to the main containing Mash.

lib/hashie/mash.rb

@@ -78,8 +78,14 @@ module Hashie
     #
     # @api semipublic
     # @return [void]
-    def self.disable_warnings
+    def self.disable_warnings(*method_keys)
       raise CannotDisableMashWarnings if self == Hashie::Mash
+      if method_keys.any?
+        disable_warnings_blacklist.concat(method_keys).tap(&:flatten!).uniq!
+      else
+        disable_warnings_blacklist.clear
+      end
+
       @disable_warnings = true
     end
 
@@ -87,17 +93,26 @@ module Hashie
     #
     # @api semipublic
     # @return [Boolean]
-    def self.disable_warnings?
+    def self.disable_warnings?(method_key = nil)
+      return disable_warnings_blacklist.include?(method_key) if disable_warnings_blacklist.any? && method_key
       @disable_warnings ||= false
     end
 
+    # Returns an array of blacklisted methods that this class disables warnings for.
+    #
+    # @api semipublic
+    # @return [Boolean]
+    def self.disable_warnings_blacklist
+      @_disable_warnings_blacklist ||= []
+    end
+
     # Inheritance hook that sets class configuration when inherited.
     #
     # @api semipublic
     # @return [void]
     def self.inherited(subclass)
       super
-      subclass.disable_warnings if disable_warnings?
+      subclass.disable_warnings(disable_warnings_blacklist) if disable_warnings?
     end
 
     def self.load(path, options = {})
@@ -346,7 +361,7 @@ module Hashie
     private
 
     def log_built_in_message(method_key)
-      return if self.class.disable_warnings?
+      return if self.class.disable_warnings?(method_key)
 
       method_information = Hashie::Utils.method_information(method(method_key))
 
@@ -359,7 +374,7 @@ module Hashie
     end
 
     def log_collision?(method_key)
-      respond_to?(method_key) && !self.class.disable_warnings? &&
+      respond_to?(method_key) && !self.class.disable_warnings?(method_key) &&
         !(regular_key?(method_key) || regular_key?(method_key.to_s))
     end
   end

spec/hashie/mash_spec.rb

@@ -154,7 +154,6 @@ describe Hashie::Mash do
       mash_class = Class.new(Hashie::Mash) do
         disable_warnings
       end
-
       mash_class.new('trust' => { 'two' => 2 })
 
       expect(logger_output).to be_blank
@@ -174,6 +173,78 @@ describe Hashie::Mash do
 
       expect(logger_output).to be_blank
     end
+
+    it 'writes to logger when a key is overridden that is not blacklisted' do
+      mash_class = Class.new(Hashie::Mash) do
+        disable_warnings :merge
+      end
+
+      mash_class.new('address' => { 'zip' => '90210' })
+      expect(logger_output).not_to be_blank
+    end
+
+    it 'does not write to logger when a key is overridden that is blacklisted' do
+      mash_class = Class.new(Hashie::Mash) do
+        disable_warnings :zip
+      end
+
+      mash_class.new('address' => { 'zip' => '90210' })
+      expect(logger_output).to be_blank
+    end
+
+    it 'carries over the disabled blacklist for warnings on grandchild classes' do
+      child_class = Class.new(Hashie::Mash) do
+        disable_warnings :zip, :merge
+      end
+      grandchild_class = Class.new(child_class)
+
+      grandchild_class.new('address' => { 'zip' => '90210' }, 'merge' => true)
+
+      expect(grandchild_class.disable_warnings_blacklist).to eq(%i[zip merge])
+      expect(logger_output).to be_blank
+    end
+
+    context 'multiple disable_warnings calls' do
+      context 'calling disable_warnings multiple times with parameters' do
+        it 'appends each new parameter to the blacklist' do
+          child_class = Class.new(Hashie::Mash) do
+            disable_warnings :zip
+            disable_warnings :merge
+            disable_warnings :cycle
+          end
+
+          expect(child_class.disable_warnings_blacklist).to eq(%i[zip merge cycle])
+        end
+      end
+
+      context 'calling disable_warnings without keys after calling with keys' do
+        it 'uses the last call to ignore the blacklist' do
+          child_class = Class.new(Hashie::Mash) do
+            disable_warnings :zip
+            disable_warnings
+          end
+
+          child_class.new('address' => { 'zip' => '90210' }, 'merge' => true, 'cycle' => 'bi')
+
+          expect(child_class.disable_warnings_blacklist).to eq([])
+          expect(logger_output).to be_blank
+        end
+      end
+
+      context 'calling disable_parameters with keys after calling without keys' do
+        it 'only ignores logging for the blacklisted methods' do
+          child_class = Class.new(Hashie::Mash) do
+            disable_warnings
+            disable_warnings :zip
+          end
+
+          child_class.new('address' => { 'zip' => '90210' }, 'merge' => true)
+
+          expect(logger_output).to match(/#{child_class}#merge/)
+          expect(logger_output).not_to match(/#{child_class}#zip/)
+        end
+      end
+    end
   end
 
   context 'updating' do