Issue 4334: Because set(property, nil) gets turned into get(property), add a 'nil_set' parameter to get() to skip a spurious warning when that happens.cd/fix-spurious-warnings

3 files changed, 33 insertions, 5 deletions

diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb
index 598c6c3c23..b16df41c8e 100644
--- a/lib/chef/mixin/params_validate.rb
+++ b/lib/chef/mixin/params_validate.rb
@@ -466,7 +466,7 @@ class Chef
       # "value nil" and to keep default stickiness working exactly the same
       # @api private
       class SetOrReturnProperty < Chef::Property
-        def get(resource)
+        def get(resource, nil_set: false)
           value = super
           # All values are sticky, frozen or not
           if !is_set?(resource)
@@ -478,7 +478,7 @@ class Chef
         def call(resource, value = NOT_PASSED)
           # setting to nil does a get
           if value.nil? && !explicitly_accepts_nil?(resource)
-            get(resource)
+            get(resource, nil_set: true)
           else
             super
           end
diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 95acdeea47..c36daa821a 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -266,7 +266,7 @@ class Chef
         # In Chef 12, value(nil) does a *get* instead of a set, so we
         # warn if the value would have been changed. In Chef 13, it will be
         # equivalent to value = nil.
-        result = get(resource)
+        result = get(resource, nil_set: true)
 
         # Warn about this becoming a set in Chef 13.
         begin
@@ -311,7 +311,7 @@ class Chef
     # @raise Chef::Exceptions::ValidationFailed If the value is invalid for
     #   this property, or if the value is required and not set.
     #
-    def get(resource)
+    def get(resource, nil_set: false)
       # If it's set, return it (and evaluate any lazy values)
       if is_set?(resource)
         value = get_value(resource)
@@ -335,7 +335,8 @@ class Chef
         #
         # It won't do what they expect. This checks whether you try to *read*
         # `content` while we are compiling the resource.
-        if resource.respond_to?(:resource_initializing) &&
+        if !nil_set &&
+            resource.respond_to?(:resource_initializing) &&
             resource.resource_initializing &&
             resource.respond_to?(:enclosing_provider) &&
             resource.enclosing_provider &&
diff --git a/spec/integration/recipes/lwrp_inline_resources_spec.rb b/spec/integration/recipes/lwrp_inline_resources_spec.rb
index 0fc2c338fd..46fd83423d 100644
--- a/spec/integration/recipes/lwrp_inline_resources_spec.rb
+++ b/spec/integration/recipes/lwrp_inline_resources_spec.rb
@@ -45,6 +45,33 @@ describe "LWRPs with inline resources" do
     end
   end
 
+  context "with an inline resource with a property that shadows the enclosing provider's property" do
+    class LwrpShadowedPropertyTest < Chef::Resource::LWRPBase
+      PATH = ::File.join(Dir.tmpdir, "shadow-property.txt")
+      use_automatic_resource_name
+      actions :fiddle
+      property :content
+      action :fiddle do
+        file PATH do
+          content new_resource.content
+          action [:create, :delete]
+        end
+      end
+    end
+
+    after { File.delete(LwrpShadowedPropertyTest::PATH) if File.exists?(LwrpShadowedPropertyTest::PATH) }
+
+    # https://github.com/chef/chef/issues/4334
+    it "does not warn spuriously" do
+      expect(Chef::Log).to_not receive(:warn).with(/is declared in both/)
+      expect_recipe {
+        lwrp_shadowed_property_test "fnord" do
+          action :fiddle
+        end
+      }
+    end
+  end
+
   context "with an inline_resources provider with two actions, one calling the other" do
     class LwrpInlineResourcesTest2 < Chef::Resource::LWRPBase
       resource_name :lwrp_inline_resources_test2