RFC-102: Deprecation warning in resourcestm/deprecated_properties

* `deprecated_property_alias` allows the resource author to provide
   transition from old properties to new ones with a deprecation
   warning.
* The `deprecated` option on a property emits a deprecation warning.
* The `deprecated` method on a resource takes a message, but does not
  yet emit a deprecation warning.

Signed-off-by: Thom May <thom@chef.io>

5 files changed, 74 insertions, 3 deletions

diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb
index ac4440f6db..792df69f71 100644
--- a/lib/chef/deprecated.rb
+++ b/lib/chef/deprecated.rb
@@ -280,6 +280,13 @@ class Chef
 
     # id 3694 was deleted
 
+    # Returned when using the deprecated option on a property
+    class Property < Base
+      def inspect
+        "#{message}\n#{location}"
+      end
+    end
+
     class Generic < Base
       def url
         "https://docs.chef.io/chef_deprecations_client.html"
diff --git a/lib/chef/mixin/properties.rb b/lib/chef/mixin/properties.rb
index 6b95b87063..fb765fbb3a 100644
--- a/lib/chef/mixin/properties.rb
+++ b/lib/chef/mixin/properties.rb
@@ -149,6 +149,10 @@ class Chef
           Property.derive(**options)
         end
 
+        def deprecated_property_alias(from, to, message)
+          Property.emit_deprecated_alias(from, to, message, self)
+        end
+
         #
         # Create a lazy value for assignment to a default value.
         #
diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 942fff0ee9..b38ec24de6 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -51,6 +51,27 @@ class Chef
       new(**options)
     end
 
+    # This is to support #deprecated_property_alias, by emitting an alias and a
+    # deprecatation warning when called.
+    #
+    # @param from [String] Name of the deprecated property
+    # @param to [String] Name of the correct property
+    # @param message [String] Deprecation message to show to the cookbook author
+    # @param declared_in [Class] Class this property comes from
+    #
+    def self.emit_deprecated_alias(from, to, message, declared_in)
+      declared_in.class_eval <<-EOM, __FILE__, __LINE__ + 1
+        def #{from}(value=NOT_PASSED)
+          Chef.deprecated(:property, "#{message}")
+          #{to}(value)
+        end
+        def #{from}=(value)
+          Chef.deprecated(:property, "#{message}")
+          #{to} = value
+        end
+      EOM
+    end
+
     #
     # Create a new property.
     #
@@ -90,6 +111,8 @@ class Chef
     #   @option options [Boolean] :required `true` if this property
     #     must be present; `false` otherwise. This is checked after the resource
     #     is fully initialized.
+    #   @option options [String] :deprecated If set, this property is deprecated and
+    #     will create a deprecation warning.
     #
     def initialize(**options)
       options = options.inject({}) { |memo, (key, value)| memo[key.to_sym] = value; memo }
@@ -272,7 +295,7 @@ class Chef
     #
     def validation_options
       @validation_options ||= options.reject do |k, v|
-        [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required, :nillable, :sensitive, :description, :introduced].include?(k)
+        [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required, :nillable, :sensitive, :description, :introduced, :deprecated].include?(k)
       end
     end
 
@@ -380,6 +403,10 @@ class Chef
     def set(resource, value)
       value = set_value(resource, input_to_stored_value(resource, value))
 
+      if options.has_key?(:deprecated)
+        Chef.deprecated(:property, options[:deprecated])
+      end
+
       if value.nil? && required?
         raise Chef::Exceptions::ValidationFailed, "#{name} is a required property"
       else
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 26298e5b1d..46bc25b713 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -148,6 +148,7 @@ class Chef
       @not_if = []
       @only_if = []
       @source_line = nil
+      @deprecated = false
       # We would like to raise an error when the user gives us a guard
       # interpreter and a ruby_block to the guard. In order to achieve this
       # we need to understand when the user overrides the default guard
@@ -1181,8 +1182,8 @@ class Chef
     # Internal Resource Interface (for Chef)
     #
 
-    FORBIDDEN_IVARS = [:@run_context, :@logger, :@not_if, :@only_if, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message]
-    HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@logger, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message]
+    FORBIDDEN_IVARS = [:@run_context, :@logger, :@not_if, :@only_if, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message, :@deprecated]
+    HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@logger, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message, :@deprecated]
 
     include Chef::Mixin::ConvertToClassName
     extend Chef::Mixin::ConvertToClassName
@@ -1406,6 +1407,14 @@ class Chef
       @examples
     end
 
+    def self.deprecated(deprecated = "NOT_PASSED")
+      if deprecated != "NOT_PASSED"
+        @deprecated = true
+        @deprecated_message = deprecated
+      end
+      @deprecated
+    end
+
     #
     # The cookbook in which this Resource was defined (if any).
     #
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index 996585ab98..79c4baa89f 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -118,6 +118,19 @@ describe "Chef::Resource.property" do
     end
   end
 
+  context "deprecated properties" do
+    it "does not create a deprecation warning on definition" do
+      expect { resource_class.class_eval { property :x, String, deprecated: 10 } }.not_to raise_error Chef::Exceptions::DeprecatedFeatureError
+    end
+
+    with_property ":x, deprecated: 'a deprecated property'" do
+      it "deprecated properties emit a deprecation warning" do
+        expect(Chef).to receive(:deprecated).with(:property, "a deprecated property")
+        expect(resource.x 10).to eq 10
+      end
+    end
+  end
+
   with_property ":x, name_property: true" do
     context "and subclass" do
       let(:subresource_class) do
@@ -1143,6 +1156,17 @@ describe "Chef::Resource.property" do
 
   end
 
+  context "with aliased properties" do
+    with_property ":real, Integer" do
+      it "should set the real property and emit a deprecation message" do
+        expect(Chef).to receive(:deprecated).with(:property, "we don't like the deprecated property no more")
+        resource_class.class_eval { deprecated_property_alias :deprecated, :real, "we don't like the deprecated property no more" }
+        resource.deprecated 10
+        expect(resource.real).to eq 10
+      end
+    end
+  end
+
   context "redefining Object methods" do
     it "disallows redefining Object methods" do
       expect { resource_class.class_eval { property :hash } }.to raise_error(ArgumentError)