Fix inconsistencies with Dash defaults

The normal behavior of Dash with respect to property defaults differed
from the behavior of a Dash/Trash with IgnoreUndeclared mixed in. This
is because some situations called the defaults and some did not.

This change normalizes the behavior so that all situations where the
defaults should be used to override unset values behave consistently,
as well as all situations where the default should not override a `nil`
value.

8 files changed, 85 insertions, 21 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 33b5fe5..6deac4b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,7 @@ Any violations of this scheme are considered to be bugs.
 * [#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).
 * [#531](https://github.com/hashie/hashie/pull/531): Fixed [slice doesn't work using symbols](https://github.com/hashie/hashie/issues/529) using hash with `IndifferentAccess` extension - [@gnomex](https://github.com/gnomex).
 * [#533](https://github.com/hashie/hashie/pull/533): Fixed `NoMethodError: undefined method `to_json'` at `hashie/dash_spec` - [@gnomex](https://github.com/gnomex).
+* [#537](https://github.com/hashie/hashie/pull/537): Fixed inconsistencies with handling defaults in `Dash` with and without `IgnoreUnclared` mixed in - [@michaelherold](https://github.com/michaelherold).
 * Your contribution here.
 
 ### Security
diff --git a/README.md b/README.md
index 47608f6..3ad96b1 100644
--- a/README.md
+++ b/README.md
@@ -875,6 +875,27 @@ p = Tricky.new('trick' => 'two')
 p.trick # => NoMethodError
 ```
 
+If you would like to update a Dash and use any default values set in the case of a `nil` value, use `#update_attributes!`.
+
+```ruby
+class WithDefaults < Hashie::Dash
+  property :description, default: 'none'
+end
+
+dash = WithDefaults.new
+dash.description  #=> 'none'
+
+dash.description = 'You committed one of the classic blunders!'
+dash.description  #=> 'You committed one of the classic blunders!'
+
+dash.description = nil
+dash.description  #=> nil
+
+dash.description = 'Only slightly less known is ...'
+dash.update_attributes!(description: nil)
+dash.description  #=> 'none'
+```
+
 ### Potential Gotchas
 
 Because Dashes are subclasses of the built-in Ruby Hash class, the double-splat operator takes the Dash as-is without any conversion. This can lead to strange behavior when you use the double-splat operator on a Dash as the first part of a keyword list or built Hash. For example:
diff --git a/lib/hashie/dash.rb b/lib/hashie/dash.rb
index 460790b..785d94d 100644
--- a/lib/hashie/dash.rb
+++ b/lib/hashie/dash.rb
@@ -104,19 +104,6 @@ module Hashie
     def initialize(attributes = {}, &block)
       super(&block)
 
-      self.class.defaults.each_pair do |prop, value|
-        self[prop] = begin
-          val = value.dup
-          if val.is_a?(Proc)
-            val.arity == 1 ? val.call(self) : val.call
-          else
-            val
-          end
-        rescue TypeError
-          value
-        end
-      end
-
       initialize_attributes(attributes)
       assert_required_attributes_set!
     end
@@ -173,13 +160,19 @@ module Hashie
       update_attributes(attributes)
 
       self.class.defaults.each_pair do |prop, value|
-        next unless self[prop].nil?
+        next unless fetch(prop, nil).nil?
         self[prop] = begin
-          value.dup
+          val = value.dup
+          if val.is_a?(Proc)
+            val.arity == 1 ? val.call(self) : val.call
+          else
+            val
+          end
         rescue TypeError
           value
         end
       end
+
       assert_required_attributes_set!
     end
 
@@ -189,7 +182,7 @@ module Hashie
       return unless attributes
 
       cleaned_attributes = attributes.reject { |_attr, value| value.nil? }
-      update_attributes(cleaned_attributes)
+      update_attributes!(cleaned_attributes)
     end
 
     def update_attributes(attributes)
diff --git a/lib/hashie/extensions/ignore_undeclared.rb b/lib/hashie/extensions/ignore_undeclared.rb
index 9b506dd..64cf0b6 100644
--- a/lib/hashie/extensions/ignore_undeclared.rb
+++ b/lib/hashie/extensions/ignore_undeclared.rb
@@ -31,12 +31,11 @@ module Hashie
     module IgnoreUndeclared
       def initialize_attributes(attributes)
         return unless attributes
+
         klass = self.class
-        translations = klass.respond_to?(:translations) && klass.translations
-        attributes.each_pair do |att, value|
-          next unless klass.property?(att) || (translations && translations.include?(att))
-          self[att] = value
-        end
+        translations = klass.respond_to?(:translations) && klass.translations || []
+
+        super(attributes.select { |attr, _| klass.property?(attr) || translations.include?(attr) })
       end
 
       def property_exists?(property)
diff --git a/spec/hashie/dash_spec.rb b/spec/hashie/dash_spec.rb
index 5f3bfe3..3adfc8f 100644
--- a/spec/hashie/dash_spec.rb
+++ b/spec/hashie/dash_spec.rb
@@ -606,3 +606,13 @@ context 'with method access' do
     it { is_expected.to eq true }
   end
 end
+
+RSpec.describe Hashie::Dash do
+  let(:test) do
+    Class.new(Hashie::Dash) do
+      property :description, default: ''
+    end
+  end
+
+  include_examples 'Dash default handling', :description
+end
diff --git a/spec/hashie/extensions/ignore_undeclared_spec.rb b/spec/hashie/extensions/ignore_undeclared_spec.rb
index 50c4f31..6f2d818 100644
--- a/spec/hashie/extensions/ignore_undeclared_spec.rb
+++ b/spec/hashie/extensions/ignore_undeclared_spec.rb
@@ -27,6 +27,18 @@ describe Hashie::Extensions::IgnoreUndeclared do
       hash = subject.new(city: 'Toronto')
       expect { hash.country = 'Canada' }.to raise_error(NoMethodError)
     end
+
+    context 'with a default value' do
+      let(:test) do
+        Class.new(Hashie::Trash) do
+          include Hashie::Extensions::IgnoreUndeclared
+
+          property :description, from: :desc, default: ''
+        end
+      end
+
+      include_examples 'Dash default handling', :description, :desc
+    end
   end
 
   context 'combined with DeepMerge' do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 0bdf972..5ac288a 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -13,6 +13,8 @@ require './spec/support/ruby_version_check'
 require './spec/support/logger'
 require './spec/support/matchers'
 
+Dir[File.expand_path(File.join(__dir__, 'support', '**', '*'))].sort.each { |file| require file }
+
 RSpec.configure do |config|
   config.extend RubyVersionCheck
   config.expect_with :rspec do |expect|
diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb
new file mode 100644
index 0000000..df05592
--- /dev/null
+++ b/spec/support/shared_examples.rb
@@ -0,0 +1,26 @@
+RSpec.shared_examples 'Dash default handling' do |property, name = property|
+  it 'uses the default when initializing' do
+    expect(test.new(name => nil).public_send(property)).to eq ''
+  end
+
+  it 'allows you to set the value to nil with the hash writer' do
+    trash = test.new(name => 'foo')
+    trash[name] = nil
+
+    expect(trash.public_send(property)).to be_nil
+  end
+
+  it 'allows you to set the value to nil with the method writer' do
+    trash = test.new(name => 'foo')
+    trash[name] = nil
+
+    expect(trash.public_send(property)).to be_nil
+  end
+
+  it 'uses the default when updating with defaults' do
+    trash = test.new(name => 'foo')
+    trash.update_attributes!(name => nil)
+
+    expect(trash.public_send(property)).to eq ''
+  end
+end