Merge pull request #6717 from chef/better_resource_errors

Improve property warnings in resources

3 files changed, 22 insertions, 22 deletions

diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index a72e41a61e..9d0957dcdf 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -99,7 +99,7 @@ class Chef
       if options.has_key?(:name_attribute)
         # If we have both name_attribute and name_property and they differ, raise an error
         if options.has_key?(:name_property)
-          raise ArgumentError, "Cannot specify both name_property and name_attribute together on property #{self}."
+          raise ArgumentError, "name_attribute and name_property are functionally identical and both cannot be specified on a property at once. Use just one on property #{self}"
         end
         # replace name_property with name_attribute in place
         options = Hash[options.map { |k, v| k == :name_attribute ? [ :name_property, v ] : [ k, v ] }]
@@ -107,7 +107,7 @@ class Chef
       end
 
       if options.has_key?(:default) && options.has_key?(:name_property)
-        raise ArgumentError, "Cannot specify both default and name_property/name_attribute together on property #{self}"
+        raise ArgumentError, "A property cannot be both a name_property/name_attribute and have a default value. Use one or the other on property #{self}"
       end
 
       # Recursively freeze the default if it isn't a lazy value.
@@ -361,7 +361,7 @@ class Chef
       end
 
       if value.nil? && required?
-        raise Chef::Exceptions::ValidationFailed, "#{name} is required"
+        raise Chef::Exceptions::ValidationFailed, "#{name} is a required property"
       else
         value
       end
@@ -386,7 +386,7 @@ class Chef
       value = set_value(resource, input_to_stored_value(resource, value))
 
       if value.nil? && required?
-        raise Chef::Exceptions::ValidationFailed, "#{name} is required"
+        raise Chef::Exceptions::ValidationFailed, "#{name} is a required property"
       else
         value
       end
@@ -457,7 +457,7 @@ class Chef
     # options.
     #
     # @param resource [Chef::Resource] The resource we're validating against
-    #   (to provide context for the validate).
+    #   (to provide context for the validation).
     # @param value The value to validate.
     #
     # @raise Chef::Exceptions::ValidationFailed If the value is invalid for
@@ -514,18 +514,18 @@ class Chef
       # very confusing results.
       if property_redefines_method?
         resource_name = declared_in.respond_to?(:resource_name) ? declared_in.resource_name : declared_in
-        raise ArgumentError, "Property `#{name}` of resource `#{resource_name}` overwrites an existing method."
+        raise ArgumentError, "Property `#{name}` of resource `#{resource_name}` overwrites an existing method. A different name should be used for this property."
       end
 
       # We prefer this form because the property name won't show up in the
       # stack trace if you use `define_method`.
       declared_in.class_eval <<-EOM, __FILE__, __LINE__ + 1
         def #{name}(value=NOT_PASSED)
-          raise "Property `#{name}` of `\#{self}` was incorrectly passed a block.  Possible property-resource collision.  To call a resource named `#{name}` either rename the property or else use `declare_resource(:#{name}, ...)`" if block_given?
+          raise "Property `#{name}` of `\#{self}` was incorrectly passed a block. Possible property-resource collision. To call a resource named `#{name}` either rename the property or else use `declare_resource(:#{name}, ...)`" if block_given?
           self.class.properties[#{name.inspect}].call(self, value)
         end
         def #{name}=(value)
-          raise "Property `#{name}` of `\#{self}` was incorrectly passed a block.  Possible property-resource collision.  To call a resource named `#{name}` either rename the property or else use `declare_resource(:#{name}, ...)`" if block_given?
+          raise "Property `#{name}` of `\#{self}` was incorrectly passed a block. Possible property-resource collision. To call a resource named `#{name}` either rename the property or else use `declare_resource(:#{name}, ...)`" if block_given?
           self.class.properties[#{name.inspect}].set(self, value)
         end
       EOM
diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb
index 53f25cb35b..f698a65771 100644
--- a/spec/integration/recipes/resource_action_spec.rb
+++ b/spec/integration/recipes/resource_action_spec.rb
@@ -509,7 +509,7 @@ module ResourceActionSpec
         Chef::Config[:treat_deprecation_warnings_as_errors] = false
         expect_converge do
           has_property_named_template "hi"
-        end.to raise_error(/Property `template` of `has_property_named_template\[hi\]` was incorrectly passed a block.  Possible property-resource collision.  To call a resource named `template` either rename the property or else use `declare_resource\(:template, ...\)`/)
+        end.to raise_error(/Property `template` of `has_property_named_template\[hi\]` was incorrectly passed a block. Possible property-resource collision. To call a resource named `template` either rename the property or else use `declare_resource\(:template, ...\)`/)
       end
     end
 
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index b8cf7f5d1b..996585ab98 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -93,7 +93,7 @@ describe "Chef::Resource.property" do
       expect(resource.bare_property).to eq 20
     end
     it "can be set with =" do
-      expect(resource.bare_property 10).to eq 10
+      expect(resource.bare_property = 10).to eq 10
       expect(resource.bare_property).to eq 10
     end
     it "can be set to nil with =" do
@@ -1037,20 +1037,20 @@ describe "Chef::Resource.property" do
 
       context "default ordering deprecation warnings" do
         it "emits an error for property :x, default: 10, #{name}: true" do
-          expect { resource_class.property :x, :default => 10, name.to_sym => true }.to raise_error Chef::Exceptions::ArgumentError,
-            /Cannot specify both default and name_property\/name_attribute together on property x of resource chef_resource_property_spec_(\d+)/
+          expect { resource_class.property :x, :default => 10, name.to_sym => true }.to raise_error ArgumentError,
+            /A property cannot be both a name_property\/name_attribute and have a default value. Use one or the other on property x of resource chef_resource_property_spec_(\d+)/
         end
         it "emits an error for property :x, default: nil, #{name}: true" do
-          expect { resource_class.property :x, :default => nil, name.to_sym => true }.to raise_error Chef::Exceptions::ArgumentError,
-            /Cannot specify both default and name_property\/name_attribute together on property x of resource chef_resource_property_spec_(\d+)/
+          expect { resource_class.property :x, :default => nil, name.to_sym => true }.to raise_error ArgumentError,
+            /A property cannot be both a name_property\/name_attribute and have a default value. Use one or the other on property x of resource chef_resource_property_spec_(\d+)/
         end
         it "emits an error for property :x, #{name}: true, default: 10" do
-          expect { resource_class.property :x, name.to_sym => true, :default => 10 }.to raise_error Chef::Exceptions::ArgumentError,
-            /Cannot specify both default and name_property\/name_attribute together on property x of resource chef_resource_property_spec_(\d+)/
+          expect { resource_class.property :x, name.to_sym => true, :default => 10 }.to raise_error ArgumentError,
+            /A property cannot be both a name_property\/name_attribute and have a default value. Use one or the other on property x of resource chef_resource_property_spec_(\d+)/
         end
         it "emits an error for property :x, #{name}: true, default: nil" do
-          expect { resource_class.property :x, name.to_sym => true, :default => nil }.to raise_error Chef::Exceptions::ArgumentError,
-            /Cannot specify both default and name_property\/name_attribute together on property x of resource chef_resource_property_spec_(\d+)/
+          expect { resource_class.property :x, name.to_sym => true, :default => nil }.to raise_error ArgumentError,
+            /A property cannot be both a name_property\/name_attribute and have a default value. Use one or the other on property x of resource chef_resource_property_spec_(\d+)/
         end
       end
     end
@@ -1058,13 +1058,13 @@ describe "Chef::Resource.property" do
 
   it "raises an error if both name_property and name_attribute are specified" do
     expect { resource_class.property :x, :name_property => false, :name_attribute => 1 }.to raise_error ArgumentError,
-      /Cannot specify both name_property and name_attribute together on property x of resource chef_resource_property_spec_(\d+)./
+      /name_attribute and name_property are functionally identical and both cannot be specified on a property at once. Use just one on property x of resource chef_resource_property_spec_(\d+)./
     expect { resource_class.property :x, :name_property => false, :name_attribute => nil }.to raise_error ArgumentError,
-      /Cannot specify both name_property and name_attribute together on property x of resource chef_resource_property_spec_(\d+)./
+      /name_attribute and name_property are functionally identical and both cannot be specified on a property at once. Use just one on property x of resource chef_resource_property_spec_(\d+)./
     expect { resource_class.property :x, :name_property => false, :name_attribute => false }.to raise_error ArgumentError,
-      /Cannot specify both name_property and name_attribute together on property x of resource chef_resource_property_spec_(\d+)./
+      /name_attribute and name_property are functionally identical and both cannot be specified on a property at once. Use just one on property x of resource chef_resource_property_spec_(\d+)./
     expect { resource_class.property :x, :name_property => true, :name_attribute => true }.to raise_error ArgumentError,
-      /Cannot specify both name_property and name_attribute together on property x of resource chef_resource_property_spec_(\d+)./
+      /name_attribute and name_property are functionally identical and both cannot be specified on a property at once. Use just one on property x of resource chef_resource_property_spec_(\d+)./
   end
 
   context "property_type" do