Changes to `Mash` initialization key string conversion. (#521)

12 files changed, 127 insertions, 41 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index aecc4ac..616726c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ scheme are considered to be bugs.
 
 ### Changed
 
+* [#521](https://github.com/hashie/hashie/pull/499): Do not convert keys that cannot be represented as symbols to `String` in `Mash` initialization - [@carolineartz](https://github.com/carolineartz).
 * Your contribution here.
 
 ### Deprecated
@@ -28,6 +29,7 @@ scheme are considered to be bugs.
 
 ### Fixed
 
+* [#516](https://github.com/hashie/hashie/issues/516): Fixed `NoMethodError` raised when including `Hashie::Extensions::Mash::SymbolizeKeys` and `Hashie::Extensions::SymbolizeKeys` in mashes/hashes with non string or symbol keys - [@carolineartz](https://github.com/carolineartz).
 * Your contribution here.
 
 ### Security
diff --git a/README.md b/README.md
index 7d0f8ca..d417f5d 100644
--- a/README.md
+++ b/README.md
@@ -191,13 +191,13 @@ end
 
 ### KeyConversion
 
-The KeyConversion extension gives you the convenience methods of `symbolize_keys` and `stringify_keys` along with their bang counterparts. You can also include just stringify or just symbolize with `Hashie::Extensions::StringifyKeys` or [`Hashie::Extensions::SymbolizeKeys`](#mash-extension-symbolizekeys).
+The KeyConversion extension gives you the convenience methods of `symbolize_keys` and `stringify_keys` along with their bang counterparts. You can also include just stringify or just symbolize with `Hashie::Extensions::StringifyKeys` or `Hashie::Extensions::SymbolizeKeys`.
 
 Hashie also has a utility method for converting keys on a Hash without a mixin:
 
 ```ruby
-Hashie.symbolize_keys! hash # => Symbolizes keys of hash.
-Hashie.symbolize_keys hash # => Returns a copy of hash with keys symbolized.
+Hashie.symbolize_keys! hash # => Symbolizes all string keys of hash.
+Hashie.symbolize_keys hash # => Returns a copy of hash with string keys symbolized.
 Hashie.stringify_keys! hash # => Stringifies keys of hash.
 Hashie.stringify_keys hash # => Returns a copy of hash with keys stringified.
 ```
@@ -580,6 +580,19 @@ my_gem = MyGem.new(name: "Hashie", dependencies: { rake: "< 11", rspec: "~> 3.0"
 my_gem.dependencies.class #=> MyGem
 ```
 
+### How does Mash handle key types which cannot be symbolized?
+
+Mash preserves keys which cannot be converted *directly* to both a string and a symbol, such as numeric keys. Since Mash is conceived to provide psuedo-object functionality, handling keys which cannot represent a method call falls outside its scope of value.
+
+#### Example
+
+```ruby
+Hashie::Mash.new('1' => 'one string', :'1' => 'one sym', 1 => 'one num')
+# => {"1"=>"one sym", 1=>"one num"}
+```
+
+The symbol key `:'1'` is converted the string `'1'` to support indifferent access and consequently its value `'one sym'` will override the previously set `'one string'`. However, the subsequent key of `1` cannot directly convert to a symbol and therefore **not** converted to the string `'1'` that would otherwise override the previously set value of `'one sym'`.
+
 ### What else can Mash do?
 
 Mash allows you also to transform any files into a Mash objects.
@@ -642,7 +655,7 @@ Mash.load('data/user.csv', permitted_classes: [Symbol], permitted_symbols: [], a
 
 ### Mash Extension: KeepOriginalKeys
 
-This extension can be mixed into a Mash to keep the form of any keys passed directly into the Mash. By default, Mash converts keys to strings to give indifferent access. This extension still allows indifferent access, but keeps the form of the keys to eliminate confusion when you're not expecting the keys to change.
+This extension can be mixed into a Mash to keep the form of any keys passed directly into the Mash. By default, Mash converts symbol keys to strings to give indifferent access. This extension still allows indifferent access, but keeps the form of the keys to eliminate confusion when you're not expecting the keys to change.
 
 ```ruby
 class KeepingMash < ::Hashie::Mash
@@ -701,7 +714,7 @@ safe_mash[:zip] = 'test' # => still ArgumentError
 
 ### Mash Extension: SymbolizeKeys
 
-This extension can be mixed into a Mash to change the default behavior of converting keys to strings. After mixing this extension into a Mash, the Mash will convert all keys to symbols. It can be useful to use with keywords argument, which required symbol keys.
+This extension can be mixed into a Mash to change the default behavior of converting keys to strings. After mixing this extension into a Mash, the Mash will convert all string keys to symbols. It can be useful to use with keywords argument, which required symbol keys.
 
 ```ruby
 class SymbolizedMash < ::Hashie::Mash
diff --git a/UPGRADING.md b/UPGRADING.md
index d516c97..b044cf1 100644
--- a/UPGRADING.md
+++ b/UPGRADING.md
@@ -1,6 +1,44 @@
 Upgrading Hashie
 ================
 
+### Upgrading to 5.0.0
+
+#### Mash initialization key conversion
+
+Mash initialization now only converts to string keys which can be represented as symbols.
+
+```ruby
+Hashie::Mash.new(
+  {foo: "bar"} => "baz",
+  "1" => "one string",
+  :"1" => "one sym",
+  1 => "one num"
+)
+
+# Before
+{"{:foo=>\"bar\"}"=>"baz", "1"=>"one num"}
+
+# After
+{{:foo=>"bar"}=>"baz", "1"=>"one sym", 1=>"one num"}
+```
+
+#### Mash#dig with numeric keys
+
+`Hashie::Mash#dig` no longer considers numeric keys for indifferent access.
+
+```ruby
+my_mash = Hashie::Mash.new("1" => "a") # => {"1"=>"a"}
+
+my_mash.dig("1") # => "a"
+my_mash.dig(:"1") # => "a"
+
+# Before
+my_mash.dig(1) # => "a"
+
+# After
+my_mash.dig(1) # => nil
+```
+
 ### Upgrading to 4.0.0
 
 #### Non-destructive Hash methods called on Mash
diff --git a/lib/hashie/extensions/mash/symbolize_keys.rb b/lib/hashie/extensions/mash/symbolize_keys.rb
index 91a6b14..c0f2a4d 100644
--- a/lib/hashie/extensions/mash/symbolize_keys.rb
+++ b/lib/hashie/extensions/mash/symbolize_keys.rb
@@ -5,7 +5,7 @@ module Hashie
       #
       # @example
       #   class LazyResponse < Hashie::Mash
-      #     include Hashie::Extensions::Mash::SymbolizedKeys
+      #     include Hashie::Extensions::Mash::SymbolizeKeys
       #   end
       #
       #   response = LazyResponse.new("id" => 123, "name" => "Rey").to_h
@@ -24,13 +24,13 @@ module Hashie
 
         private
 
-        # Converts a key to a symbol
+        # Converts a key to a symbol, if possible
         #
         # @api private
-        # @param [String, Symbol] key the key to convert to a symbol
-        # @return [void]
+        # @param [<K>] key the key to attempt convert to a symbol
+        # @return [Symbol, K]
         def convert_key(key)
-          key.to_sym
+          key.respond_to?(:to_sym) ? key.to_sym : key
         end
       end
     end
diff --git a/lib/hashie/extensions/symbolize_keys.rb b/lib/hashie/extensions/symbolize_keys.rb
index e4315bd..d1da9cb 100644
--- a/lib/hashie/extensions/symbolize_keys.rb
+++ b/lib/hashie/extensions/symbolize_keys.rb
@@ -46,7 +46,7 @@ module Hashie
           hash.extend(Hashie::Extensions::SymbolizeKeys) unless hash.respond_to?(:symbolize_keys!)
           hash.keys.each do |k| # rubocop:disable Performance/HashEachMethods
             symbolize_keys_recursively!(hash[k])
-            hash[k.to_sym] = hash.delete(k)
+            hash[convert_key(k)] = hash.delete(k)
           end
           hash
         end
@@ -61,6 +61,17 @@ module Hashie
             symbolize_keys!(new_hash)
           end
         end
+
+        private
+
+        # Converts a key to a symbol, if possible
+        #
+        # @api private
+        # @param [<K>] key the key to attempt convert to a symbol
+        # @return [Symbol, K]
+        def convert_key(key)
+          key.respond_to?(:to_sym) ? key.to_sym : key
+        end
       end
 
       class << self
diff --git a/lib/hashie/hash.rb b/lib/hashie/hash.rb
index 7a97ef6..a2bd7ab 100644
--- a/lib/hashie/hash.rb
+++ b/lib/hashie/hash.rb
@@ -21,8 +21,8 @@ module Hashie
         assignment_key =
           if options[:stringify_keys]
             k.to_s
-          elsif options[:symbolize_keys]
-            k.to_s.to_sym
+          elsif options[:symbolize_keys] && k.respond_to?(:to_sym)
+            k.to_sym
           else
             k
           end
diff --git a/lib/hashie/mash.rb b/lib/hashie/mash.rb
index 96b24c9..6ab6e9a 100644
--- a/lib/hashie/mash.rb
+++ b/lib/hashie/mash.rb
@@ -121,8 +121,8 @@ module Hashie
     alias regular_reader []
     alias regular_writer []=
 
-    # Retrieves an attribute set in the Mash. Will convert
-    # any key passed in to a string before retrieving.
+    # Retrieves an attribute set in the Mash. Will convert a key passed in
+    # as a symbol to a string before retrieving.
     def custom_reader(key)
       default_proc.call(self, key) if default_proc && !key?(key)
       value = regular_reader(convert_key(key))
@@ -130,14 +130,12 @@ module Hashie
       value
     end
 
-    # Sets an attribute in the Mash. Key will be converted to
-    # a string before it is set, and Hashes will be converted
-    # into Mashes for nesting purposes.
+    # Sets an attribute in the Mash. Symbol keys will be converted to
+    # strings before being set, and Hashes will be converted into Mashes
+    # for nesting purposes.
     def custom_writer(key, value, convert = true) #:nodoc:
-      key_as_symbol = (key = convert_key(key)).to_sym
-
-      log_built_in_message(key_as_symbol) if log_collision?(key_as_symbol)
-      regular_writer(key, convert ? convert_value(value) : value)
+      log_built_in_message(key) if key.respond_to?(:to_sym) && log_collision?(key.to_sym)
+      regular_writer(convert_key(key), convert ? convert_value(value) : value)
     end
 
     alias [] custom_reader
@@ -371,7 +369,7 @@ module Hashie
     end
 
     def convert_key(key) #:nodoc:
-      key.to_s
+      key.respond_to?(:to_sym) ? key.to_s : key
     end
 
     def convert_value(val, duping = false) #:nodoc:
@@ -406,6 +404,7 @@ module Hashie
     end
 
     def log_collision?(method_key)
+      return unless method_key.is_a?(String) || method_key.is_a?(Symbol)
       return unless respond_to?(method_key)
 
       _, suffix = method_name_and_suffix(method_key)
diff --git a/lib/hashie/version.rb b/lib/hashie/version.rb
index b97bf9b..e4ffc63 100644
--- a/lib/hashie/version.rb
+++ b/lib/hashie/version.rb
@@ -1,3 +1,3 @@
 module Hashie
-  VERSION = '4.1.0'.freeze
+  VERSION = '5.0.0'.freeze
 end
diff --git a/spec/hashie/extensions/mash/symbolize_keys_spec.rb b/spec/hashie/extensions/mash/symbolize_keys_spec.rb
index 9846737..bf58ea1 100644
--- a/spec/hashie/extensions/mash/symbolize_keys_spec.rb
+++ b/spec/hashie/extensions/mash/symbolize_keys_spec.rb
@@ -9,12 +9,30 @@ RSpec.describe Hashie::Extensions::Mash::SymbolizeKeys do
     end.to raise_error(ArgumentError)
   end
 
-  it 'symbolizes all keys in the Mash' do
-    my_mash = Class.new(Hashie::Mash) do
+  context 'when included in a Mash' do
+    class SymbolizedMash < Hashie::Mash
       include Hashie::Extensions::Mash::SymbolizeKeys
     end
 
-    expect(my_mash.new('test' => 'value').to_h).to eq(test: 'value')
+    it 'symbolizes string keys in the Mash' do
+      my_mash = SymbolizedMash.new('test' => 'value')
+      expect(my_mash.to_h).to eq(test: 'value')
+    end
+
+    it 'preserves keys which cannot be symbolized' do
+      my_mash = SymbolizedMash.new(
+        '1' => 'symbolizable one',
+        1 => 'one',
+        [1, 2, 3] => 'testing',
+        { 'test' => 'value' } => 'value'
+      )
+      expect(my_mash.to_h).to eq(
+        :'1' => 'symbolizable one',
+        1 => 'one',
+        [1, 2, 3] => 'testing',
+        { 'test' => 'value' } => 'value'
+      )
+    end
   end
 
   context 'implicit to_hash on double splat' do
diff --git a/spec/hashie/extensions/symbolize_keys_spec.rb b/spec/hashie/extensions/symbolize_keys_spec.rb
index be345ea..34ac8d8 100644
--- a/spec/hashie/extensions/symbolize_keys_spec.rb
+++ b/spec/hashie/extensions/symbolize_keys_spec.rb
@@ -89,9 +89,10 @@ describe Hashie::Extensions::SymbolizeKeys do
   context 'singleton methods' do
     subject { Hash }
     let(:object) do
-      subject.new.merge('a' => 1, 'b' => { 'c' => 2 }).extend(Hashie::Extensions::SymbolizeKeys)
+      subject.new.merge('a' => 1, 'b' => { 'c' => 2 }, 1 => 'numeric key')
+             .extend(Hashie::Extensions::SymbolizeKeys)
     end
-    let(:expected_hash) { { a: 1, b: { c: 2 } } }
+    let(:expected_hash) { { a: 1, b: { c: 2 }, 1 => 'numeric key' } }
 
     describe '.symbolize_keys' do
       it 'does not raise error' do
diff --git a/spec/hashie/hash_spec.rb b/spec/hashie/hash_spec.rb
index 0282258..9e2b4e0 100644
--- a/spec/hashie/hash_spec.rb
+++ b/spec/hashie/hash_spec.rb
@@ -41,7 +41,7 @@ describe Hash do
   it '#to_hash with symbolize_keys set to true returns a hash with symbolized keys' do
     hash = Hashie::Hash['a' => 'hey', 123 => 'bob', 'array' => [1, 2, 3]]
     symbolized_hash = hash.to_hash(symbolize_keys: true)
-    expect(symbolized_hash).to eq(a: 'hey', :"123" => 'bob', array: [1, 2, 3])
+    expect(symbolized_hash).to eq(a: 'hey', 123 => 'bob', array: [1, 2, 3])
   end
 
   it "#to_hash should not blow up when #to_hash doesn't accept arguments" do
@@ -112,9 +112,9 @@ describe Hash do
 
       expected = {
         a: 'hey',
-        :"123" => 'bob',
+        123 => 'bob',
         array: [1, 2, 3],
-        subhash: { a: 'hey', b: 'bar', :'123' => 'bob', array: [1, 2, 3] }
+        subhash: { a: 'hey', b: 'bar', 123 => 'bob', array: [1, 2, 3] }
       }
 
       expect(symbolized_hash).to eq(expected)
diff --git a/spec/hashie/mash_spec.rb b/spec/hashie/mash_spec.rb
index b0aef57..4731985 100644
--- a/spec/hashie/mash_spec.rb
+++ b/spec/hashie/mash_spec.rb
@@ -613,6 +613,18 @@ describe Hashie::Mash do
       expect(converted.to_hash['a'].first.is_a?(Hash)).to be_truthy
       expect(converted.to_hash['a'].first['c'].first.is_a?(Hashie::Mash)).to be_falsy
     end
+
+    it 'only stringifies keys which can be converted to symbols' do
+      initial_hash = { 1 => 'a', ['b'] => 2, 'c' => 3, d: 4 }
+      converted = Hashie::Mash.new(initial_hash)
+      expect(converted).to eq(1 => 'a', ['b'] => 2, 'c' => 3, 'd' => 4)
+    end
+
+    it 'preserves keys which cannot be converted to symbols' do
+      initial_hash = { 1 => 'a', '1' => 'b', :'1' => 'c' }
+      converted = Hashie::Mash.new(initial_hash)
+      expect(converted).to eq(1 => 'a', '1' => 'c')
+    end
   end
 
   describe '#fetch' do
@@ -900,7 +912,7 @@ describe Hashie::Mash do
     end
 
     it 'returns a mash with the keys and values inverted' do
-      expect(mash.invert).to eq('apple' => 'a', '4' => 'b')
+      expect(mash.invert).to eq('apple' => 'a', 4 => 'b')
     end
 
     context 'when using with subclass' do
@@ -977,14 +989,6 @@ describe Hashie::Mash do
         expect(subject.dig('a', 'b')).to eq(1)
       end
 
-      context 'with numeric key' do
-        subject { described_class.new('1' => { b: 1 }) }
-        it 'accepts a numeric value as key' do
-          expect(subject.dig(1, :b)).to eq(1)
-          expect(subject.dig('1', :b)).to eq(1)
-        end
-      end
-
       context 'when the Mash wraps a Hashie::Array' do
         it 'handles digging into an array' do
           mash = described_class.new(alphabet: { first_three: Hashie::Array['a', 'b', 'c'] })