Add blacklisted warnings

[BobbyMcWho]

Adds an optional array parameter to the disable_warnings method, so that you can pick and choose which warnings to ignore.

Usage:

class Response < Hashie::Mash
  disable_warnings :zip
end

Response.new('zip' => '90210') # does not log a warning
Response.new('merge' => 'yes') # does log a warning

This preserves existing usage.

[dblock]

Why do we need the only keyword? Is there going to be some other exclusion here in the future?

I feel like disable_warnings: [:zip] and disable_warning :zip is the most straightforward interface.

[BobbyMcWho]

@dblock I suppose we don't explicitly need the only keyword. I can refactor if you think your proposal is more straightforward. Is it worth adding the disable_warning singular case vs just using the plural disable_warnings with an array of 1 (or checking in the function if a symbol was passed vs an array)

[dblock]

For rubocop just run rubocop -a ; rubocop --auto-gen-config. Or just disable Metrics/ClassLength altogether in .rubocop_config.yml.

[dblock]

I think the behavior when both variations are used is broken here. I'd expect these to pass:

    it 'does not write to the logger when warnings are disabled' do
      mash_class = Class.new(Hashie::Mash) do
        disable_warnings :zip
        disable_warnings
      end

      mash_class.new('trust' => { 'two' => 2 })

      expect(logger_output).to be_blank
    end

and this one when using an array

    it 'has cumulative behavior' do
      mash_class = Class.new(Hashie::Mash) do
        disable_warnings :two, :zip
      end

      mash_class.new('trust' => { 'two' => 2 })
      mash_class.new('address' => { 'zip' => '90210' })

      expect(logger_output).to be_blank
    end

[BobbyMcWho]

  it 'has cumulative behavior' do
      mash_class = Class.new(Hashie::Mash) do
        disable_warnings :two, :zip
      end

      mash_class.new('trust' => { 'two' => 2 })
      mash_class.new('address' => { 'zip' => '90210' })

      expect(logger_output).to be_blank
    end

This scenario works for the given keys, the error being thrown logged is due to #trust being defined from Kernal

I'll give some thoughts to the other scenario you mentioned.

[BobbyMcWho]

After looking at my test, the case where a child calls disable_warnings was incorrect, I resolved that and your first case should be resolved as well.

Now when disable_warnings is called without params after it has been called with them, we will clear the blacklist and ignore all conflicting method keys.

[michaelherold]

I like the implementation but I think we can get away with fewer lines of added code by changing the implementation of the disable_warnings? predicate.

[dblock]

I am happy with this with the minor fixes in my last comments. @BobbyMcWho you should squash your commits, leaving the final review/comments/merge to @michaelherold.

[michaelherold]

This is looking really good! I'd like to see a little more documentation in the readme and dB's typo fix, otherwise we're good-to-go.

[BobbyMcWho]

Thanks for the documentation suggestion @dblock, for the sake of clarity I also added:

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.

class Message < Hashie::Mash
  disable_warnings :zip, :zap
  disable_warnings
end
# No errors will be logged
Message.new(merge: 'true', compact: true)

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')

[michaelherold]

I'm really happy with this now. I fixed up some spacing issues in the readme that I'll squash out once the tests pass.

Thanks a bunch for the contribution!

[BobbyMcWho]

Thanks for all the input from both of you, it's been a pleasure!

[dblock]

Nice work @BobbyMcWho, thanks for hanging in there with us!