Only warn about potentially duplicate properties during the resource initializerjk/reduce-property-dup-warning

4 files changed, 53 insertions, 12 deletions

diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 742eba42fd..89e4ffe0e6 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -18,6 +18,8 @@
 
 require 'chef/exceptions'
 require 'chef/delayed_evaluator'
+require 'chef/chef_class'
+require 'chef/log'
 
 class Chef
   #
@@ -314,9 +316,10 @@ 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?(:enclosing_provider) && resource.enclosing_provider &&
-           !resource.currently_running_action &&
-           !name_property? &&
+        if resource.respond_to?(:resource_initializing) &&
+           resource.resource_initializing &&
+           resource.respond_to?(:enclosing_provider) &&
+           resource.enclosing_provider &&
            resource.enclosing_provider.respond_to?(name)
            Chef::Log.warn("#{Chef::Log.caller_location}: property #{name} is declared in both #{resource} and #{resource.enclosing_provider}. Use new_resource.#{name} instead. At #{Chef::Log.caller_location}")
         end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index a7b1f63463..4aec8cf1f6 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -569,8 +569,6 @@ class Chef
     #
     def run_action(action, notification_type=nil, notifying_resource=nil)
       # reset state in case of multiple actions on the same resource.
-      old_currently_running_action = @currently_running_action
-      @currently_running_action = action
       @elapsed_time = 0
       start_time = Time.now
       events.resource_action_start(self, action, notification_type, notifying_resource)
@@ -612,7 +610,6 @@ class Chef
         end
       end
     ensure
-      @currently_running_action = old_currently_running_action
       @elapsed_time = Time.now - start_time
       # Reporting endpoint doesn't accept a negative resource duration so set it to 0.
       # A negative value can occur when a resource changes the system time backwards
@@ -621,12 +618,18 @@ class Chef
     end
 
     #
-    # If we are currently running an action, this shows the action we are running.
-    # If the resource is running multiple actions at once, this will show the most recent.
+    # If we are currently initializing the resource, this will be true.
     #
     # Do NOT use this. It may be removed. It is for internal purposes only.
     # @api private
-    attr_reader :currently_running_action
+    attr_reader :resource_initializing
+    def resource_initializing=(value)
+      if value
+        @resource_initializing = true
+      else
+        remove_instance_variable(:@resource_initializing)
+      end
+    end
 
     #
     # Generic Ruby and Data Structure Stuff (for user)
diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb
index 72b3788cf0..5b2ed4bb3f 100644
--- a/lib/chef/resource_builder.rb
+++ b/lib/chef/resource_builder.rb
@@ -70,7 +70,14 @@ class Chef
       resource.params = params
 
       # Evaluate resource attribute DSL
-      resource.instance_eval(&block) if block_given?
+      if block_given?
+        resource.resource_initializing = true
+        begin
+          resource.instance_eval(&block)
+        ensure
+          resource.resource_initializing = false
+        end
+      end
 
       # emit a cloned resource warning if it is warranted
       if prior_resource
@@ -130,7 +137,7 @@ class Chef
       @prior_resource ||=
         begin
           key = "#{type}[#{name}]"
-          prior_resource = run_context.resource_collection.lookup(key)
+          run_context.resource_collection.lookup(key)
         rescue Chef::Exceptions::ResourceNotFound
           nil
         end
diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb
index 0ea67ea5f2..17a1eb9acd 100644
--- a/spec/integration/recipes/resource_action_spec.rb
+++ b/spec/integration/recipes/resource_action_spec.rb
@@ -377,6 +377,15 @@ describe "Resource.action" do
             x x
           end
         end
+        def self.x_warning_line
+          __LINE__-4
+        end
+        action :set_x_to_x_in_non_initializer do
+          r = resource_action_spec_with_x 'hi' do
+            x 10
+          end
+          x_times_2 = r.x*2
+        end
         action :set_x_to_10 do
           resource_action_spec_with_x 'hi' do
             x 10
@@ -384,6 +393,8 @@ describe "Resource.action" do
         end
       end
 
+      attr_reader :x_warning_line
+
       it "Using the enclosing resource to set x to x emits a warning that you're using the wrong x" do
         recipe = converge {
           resource_action_spec_also_with_x 'hi' do
@@ -393,7 +404,16 @@ describe "Resource.action" do
         }
         warnings = recipe.logs.lines.select { |l| l =~ /warn/i }
         expect(warnings.size).to eq 1
-        expect(warnings[0]).to match(/property x is declared in both resource_action_spec_with_x\[hi\] and resource_action_spec_also_with_x\[hi\] action :set_x_to_x. Use new_resource.x instead. At #{__FILE__}:#{__LINE__-19}/)
+        expect(warnings[0]).to match(/property x is declared in both resource_action_spec_with_x\[hi\] and resource_action_spec_also_with_x\[hi\] action :set_x_to_x. Use new_resource.x instead. At #{__FILE__}:#{ResourceActionSpecAlsoWithX.x_warning_line}/)
+      end
+
+      it "Using the enclosing resource to set x to x outside the initializer emits no warning" do
+        expect_recipe {
+          resource_action_spec_also_with_x 'hi' do
+            x 1
+            action :set_x_to_x_in_non_initializer
+          end
+        }.to emit_no_warnings_or_errors
       end
 
       it "Using the enclosing resource to set x to 10 emits no warning" do
@@ -404,6 +424,14 @@ describe "Resource.action" do
           end
         }.to emit_no_warnings_or_errors
       end
+
+      it "Using the enclosing resource to set x to 10 emits no warning" do
+        expect_recipe {
+          r = resource_action_spec_also_with_x 'hi'
+          r.x 1
+          r.action :set_x_to_10
+        }.to emit_no_warnings_or_errors
+      end
     end
 
   end