Add a logging layer to address common issues (#381)

.rubocop_todo.yml

@@ -18,7 +18,7 @@ Metrics/AbcSize:
 # Offense count: 2
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 179
+  Max: 191
 
 # Offense count: 6
 Metrics/CyclomaticComplexity:

CHANGELOG.md

@@ -12,6 +12,7 @@ scheme are considered to be bugs.
 
 ### Added
 
+* [#381](https://github.com/intridea/hashie/pull/381): Add a logging layer that lets us report potential issues to our users. As the first logged issue, report when a `Hashie::Mash` is attempting to overwrite a built-in method, since that is one of our number one questions - [@michaelherold](https://github.com/michaelherold).
 * Your contribution here.
 
 ### Changed

README.md

@@ -28,6 +28,15 @@ The library is broken up into a number of atomically includable Hash extension m
 
 Any of the extensions listed below can be mixed into a class by `include`-ing `Hashie::Extensions::ExtensionName`.
 
+## Logging
+
+Hashie has a built-in logger that you can override. By default, it logs to `STDOUT` but can be replaced by any `Logger` class. The logger is accessible on the Hashie module, as shown below:
+
+```ruby
+# Set the logger to the Rails logger
+Hashie.logger = Rails.logger
+```
+
 ### Coercion
 
 Coercions allow you to set up "coercion rules" based either on the key or the value type to massage data as it's being inserted into the Hash. Key coercions might be used, for example, in lightweight data modeling applications such as an API client:

lib/hashie.rb

@@ -1,6 +1,22 @@
+require 'logger'
 require 'hashie/version'
 
 module Hashie
+  # The logger that Hashie uses for reporting errors.
+  #
+  # @return [Logger]
+  def self.logger
+    @logger ||= Logger.new(STDOUT)
+  end
+
+  # Sets the logger that Hashie uses for reporting errors.
+  #
+  # @param logger [Logger] The logger to set as Hashie's logger.
+  # @return [void]
+  def self.logger=(logger)
+    @logger = logger
+  end
+
   autoload :Clash,              'hashie/clash'
   autoload :Dash,               'hashie/dash'
   autoload :Hash,               'hashie/hash'
@@ -8,6 +24,7 @@ module Hashie
   autoload :Trash,              'hashie/trash'
   autoload :Rash,               'hashie/rash'
   autoload :Array,              'hashie/array'
+  autoload :Utils,              'hashie/utils'
 
   module Extensions
     autoload :Coercion,          'hashie/extensions/coercion'

lib/hashie/mash.rb

@@ -109,6 +109,8 @@ module Hashie
     # a string before it is set, and Hashes will be converted
     # into Mashes for nesting purposes.
     def custom_writer(key, value, convert = true) #:nodoc:
+      key_as_symbol = key.to_sym
+      log_built_in_message(key_as_symbol) if methods.include?(key_as_symbol)
       regular_writer(convert_key(key), convert ? convert_value(value) : value)
     end
 
@@ -295,5 +297,18 @@ module Hashie
         val
       end
     end
+
+    private
+
+    def log_built_in_message(method_key)
+      method_information = Hashie::Utils.method_information(method(method_key))
+
+      Hashie.logger.warn(
+        'You are setting a key that conflicts with a built-in method ' \
+        "#{self.class}##{method_key} #{method_information}. " \
+        'This can cause unexpected behavior when accessing the key via as a ' \
+        'property. You can still access the key via the #[] method.'
+      )
+    end
   end
 end

lib/hashie/utils.rb

@@ -0,0 +1,16 @@
+module Hashie
+  # A collection of helper methods that can be used throughout the gem.
+  module Utils
+    # Describes a method by where it was defined.
+    #
+    # @param bound_method [Method] The method to describe.
+    # @return [String]
+    def self.method_information(bound_method)
+      if bound_method.source_location
+        "defined at #{bound_method.source_location.join(':')}"
+      else
+        "defined in #{bound_method.owner}"
+      end
+    end
+  end
+end

spec/hashie/mash_spec.rb

@@ -134,6 +134,14 @@ describe Hashie::Mash do
     expect(subject.type).to eq 'Steve'
   end
 
+  shared_context 'with a logger' do
+    it 'logs a warning when overriding built-in methods' do
+      Hashie::Mash.new('trust' => { 'two' => 2 })
+
+      expect(logger_output).to match('Hashie::Mash#trust')
+    end
+  end
+
   context 'updating' do
     subject do
       described_class.new(

spec/hashie/utils_spec.rb

@@ -0,0 +1,25 @@
+require 'spec_helper'
+
+def a_method_to_match_against
+  'Hello world!'
+end
+
+RSpec.describe Hashie::Utils do
+  describe '.method_information' do
+    it 'states the module or class that a native method was defined in' do
+      bound_method = method(:trust)
+
+      message = Hashie::Utils.method_information(bound_method)
+
+      expect(message).to match('Kernel')
+    end
+
+    it 'states the line a Ruby method was defined at' do
+      bound_method = method(:a_method_to_match_against)
+
+      message = Hashie::Utils.method_information(bound_method)
+
+      expect(message).to match('spec/hashie/utils_spec.rb')
+    end
+  end
+end

spec/hashie_spec.rb

@@ -0,0 +1,13 @@
+require 'spec_helper'
+
+RSpec.describe Hashie do
+  describe '.logger' do
+    shared_context 'with a logger' do
+      it 'is available via an accessor' do
+        Hashie.logger.info('Fee fi fo fum')
+
+        expect(logger_output).to match('Fee fi fo fum')
+      end
+    end
+  end
+end

spec/support/logger.rb

@@ -0,0 +1,22 @@
+# A shared context that allows you to check the output of Hashie's logger.
+#
+# @example
+#   shared_context 'with a logger' do
+#     Hashie.logger.info 'What is happening in here?!'
+#
+#     expect(logger_output).to match('What is happening in here?!')
+#   end
+RSpec.shared_context 'with a logger' do
+  # @private
+  let(:log) { StringIO.new }
+
+  # The output string from the logger
+  let(:logger_output) { log.rewind && log.string }
+
+  around(:each) do |example|
+    original_logger = Hashie.logger
+    Hashie.logger = Logger.new(log)
+    example.run
+    Hashie.logger = original_logger
+  end
+end