tweak 3694 warnings

- clean up "ZenMaster" resource
- clean up 3694 detection to use properties
- unlazy the resource_name in the trivial resource check
- fixes an issue with resources-as-definitions pattern emitting
  3694 errors for trivial resources

4 files changed, 41 insertions, 23 deletions

diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 45ab4dd522..0589cb4c54 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -531,8 +531,6 @@ class Chef
       end
     end
 
-    protected
-
     #
     # The options this Property will use for get/set behavior and validation.
     #
@@ -583,6 +581,7 @@ class Chef
         (options.has_key?(:is) && resource.send(:_pv_is, { name => nil }, name, options[:is], raise_error: false))
     end
 
+    # @api private
     def get_value(resource)
       if instance_variable_name
         resource.instance_variable_get(instance_variable_name)
@@ -591,6 +590,7 @@ class Chef
       end
     end
 
+    # @api private
     def set_value(resource, value)
       if instance_variable_name
         resource.instance_variable_set(instance_variable_name, value)
@@ -599,6 +599,7 @@ class Chef
       end
     end
 
+    # @api private
     def value_is_set?(resource)
       if instance_variable_name
         resource.instance_variable_defined?(instance_variable_name)
@@ -607,6 +608,7 @@ class Chef
       end
     end
 
+    # @api private
     def reset_value(resource)
       if instance_variable_name
         if value_is_set?(resource)
@@ -617,6 +619,8 @@ class Chef
       end
     end
 
+    private
+
     def exec_in_resource(resource, proc, *args)
       if resource
         if proc.arity > args.size
diff --git a/lib/chef/resource_builder.rb b/lib/chef/resource_builder.rb
index d1f5c2e022..ee44b6714c 100644
--- a/lib/chef/resource_builder.rb
+++ b/lib/chef/resource_builder.rb
@@ -104,23 +104,26 @@ class Chef
     end
 
     def is_trivial_resource?(resource)
-      identicalish_resources?(resource_class.new(name, run_context), resource)
+      trivial_resource = resource_class.new(name, run_context)
+      # force un-lazy the name property on the created trivial resource
+      name_property = resource_class.properties.find { |sym, p| p.name_property? }
+      trivial_resource.send(name_property[0]) unless name_property.nil?
+      identicalish_resources?(trivial_resource, resource)
     end
 
     # this is an equality test specific to checking for 3694 cloning warnings
     def identicalish_resources?(first, second)
-      skipped_ivars = [ :@source_line, :@cookbook_name, :@recipe_name, :@params, :@elapsed_time, :@declared_type ]
-      checked_ivars = ( first.instance_variables | second.instance_variables ) - skipped_ivars
-      non_matching_ivars = checked_ivars.reject do |iv|
-        if iv == :@action && ( [first.instance_variable_get(iv)].flatten == [:nothing] || [second.instance_variable_get(iv)].flatten == [:nothing] )
-          # :nothing action on either side of the comparison always matches
+      non_matching_properties = first.class.properties.reject do |sym, property|
+        if !property.is_set?(first) && !property.is_set?(second)
           true
+        elsif property.is_set?(first) && property.is_set?(second)
+          property.send(:get_value, first) == property.send(:get_value, second)
         else
-          first.instance_variable_get(iv) == second.instance_variable_get(iv)
+          false
         end
       end
-      Chef::Log.debug("ivars which did not match with the prior resource: #{non_matching_ivars}")
-      non_matching_ivars.empty?
+      Chef::Log.debug("ivars which did not match with the prior resource: #{non_matching_properties.keys}")
+      non_matching_properties.empty?
     end
 
     def emit_cloned_resource_warning
diff --git a/spec/support/lib/chef/resource/zen_master.rb b/spec/support/lib/chef/resource/zen_master.rb
index f5137c18ec..4b94e7415c 100644
--- a/spec/support/lib/chef/resource/zen_master.rb
+++ b/spec/support/lib/chef/resource/zen_master.rb
@@ -24,18 +24,8 @@ class Chef
     class ZenMaster < Chef::Resource
       allowed_actions :win, :score
 
-      attr_reader :peace
-
-      def peace(tf)
-        @peace = tf
-      end
-
-      def something(arg = nil)
-        if !arg.nil?
-          @something = arg
-        end
-        @something
-      end
+      property :peace
+      property :something
     end
   end
 end
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index 70164f0a9b..3c30f96b20 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -266,6 +266,27 @@ describe Chef::Recipe do
         end
       end
 
+      class Coerced < Chef::Resource
+        resource_name :coerced
+        provides :coerced
+        default_action :whatever
+        property :package_name, [String, Array], coerce: proc { |x| [x].flatten }, name_property: true
+        def after_created
+          Array(action).each do |action|
+            run_action(action)
+          end
+        end
+        action :whatever do
+          package_name # unlazy the package_name
+        end
+      end
+
+      it "does not emit 3694 when the name_property is unlazied by running it at compile_time" do
+        recipe.coerced "string"
+        expect(Chef).to_not receive(:log_deprecation)
+        recipe.coerced "string"
+      end
+
       it "validating resources via build_resource" do
         expect {recipe.build_resource(:remote_file, "klopp") do
           source Chef::DelayedEvaluator.new { "http://chef.io" }