Don't treat name_property/attribute as defaults if they are false

2 files changed, 46 insertions, 1 deletions

diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 011ff93aef..d699f16a9d 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -89,7 +89,11 @@ class Chef
 
       # Only pick the first of :default, :name_property and :name_attribute if
       # more than one is specified.
-      found_defaults = options.keys.select { |k| [ :default, :name_attribute, :name_property ].include?(k) }
+      found_defaults = options.keys.select do |k|
+         # Only treat name_property or name_attribute as a default if they are `true`
+        k == :default ||
+        ((k == :name_property || k == :name_attribute) && options[k])
+      end
       if found_defaults.size > 1
         preferred_default = found_defaults[0]
         # We do *not* prefer `default: nil` even if it's first, because earlier
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index 6415286397..de8d493026 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -946,6 +946,19 @@ describe "Chef::Resource.property" do
           expect(resource.x).to eq 'blah'
         end
       end
+
+      with_property ":x, #{name}: false" do
+        it "defaults to nil" do
+          expect(resource.x).to be_nil
+        end
+      end
+
+      with_property ":x, #{name}: nil" do
+        it "defaults to nil" do
+          expect(resource.x).to be_nil
+        end
+      end
+
       context "default ordering deprecation warnings" do
         it "emits a deprecation warning for property :x, default: 10, #{name}: true" do
           expect { resource_class.property :x, :default => 10, name.to_sym => true }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
@@ -964,6 +977,7 @@ describe "Chef::Resource.property" do
             /Cannot specify keys #{name}, default together on property x. Only one \(#{name}\) will be obeyed./
         end
       end
+
       context "default ordering" do
         before { Chef::Config[:treat_deprecation_warnings_as_errors] = false }
         with_property ":x, default: 10, #{name}: true" do
@@ -987,6 +1001,33 @@ describe "Chef::Resource.property" do
           end
         end
       end
+
+      context "default ordering when #{name} is nil" do
+        with_property ":x, #{name}: nil, default: 10" do
+          it "chooses default" do
+            expect(resource.x).to eq 10
+          end
+        end
+        with_property ":x, default: 10, #{name}: nil" do
+          it "chooses default" do
+            expect(resource.x).to eq 10
+          end
+        end
+      end
+
+      context "default ordering when #{name} is false" do
+        with_property ":x, #{name}: false, default: 10" do
+          it "chooses default" do
+            expect(resource.x).to eq 10
+          end
+        end
+        with_property ":x, default: 10, #{name}: nil" do
+          it "chooses default" do
+            expect(resource.x).to eq 10
+          end
+        end
+      end
+
       with_property ":x, #{name}: true, required: true" do
         it "defaults x to resource.name" do
           expect(resource.x).to eq 'blah'