Add support for provider action description

Often actions are defined on the provider and not on the resource.  This
change updates Provider to support a description opt when declaring the
action, which lets us improve our generated documentation.

This also updates ResourceInspector to look at the provider if the
resource itself does not offer an action description for a given action.

Example:

action :foo, description: "bar" do
...
end

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

7 files changed, 98 insertions, 33 deletions

diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index 81ed530fc7..e7d7ca84ff 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -57,10 +57,12 @@ class Chef
     #
     # @since 13.0
     # @param name [String, Symbol] Name of the action to define.
+    # @param description [String] description of the action
     # @param block [Proc] Body of the action.
     #
     # @return [void]
-    def self.action(name, &block)
+    def self.action(name, description: nil, &block)
+      action_descriptions[name.to_sym] = description unless description.nil?
       # We need the block directly in a method so that `return` works.
       define_method("compile_action_#{name}", &block)
       class_eval <<-EOM
@@ -70,6 +72,29 @@ class Chef
       EOM
     end
 
+    # Return the hash of action descriptions defined for
+    # the provider class.
+    #
+    # @return [Hash] hash of [Symbol] => [String] containing
+    # any provided action descriptions.
+    def self.action_descriptions
+      @action_descriptions ||= {}
+    end
+
+    # Retrieve the description for a provider's action, if
+    # any description has been included in the definition.
+    #
+    # @param action [Symbol,String] the action name
+    # @return [String] the description of the action provided, or nil if no description
+    # was defined
+    def self.action_description(action)
+      description = action_descriptions[action.to_sym]
+      if description.nil? && superclass.respond_to?(:action_description)
+        description = superclass.action_description(action)
+      end
+      description
+    end
+
     # Deprecation stub for the old use_inline_resources mode.
     #
     # @return [void]
diff --git a/lib/chef/provider_resolver.rb b/lib/chef/provider_resolver.rb
index 94727a1043..9fafbe5f31 100644
--- a/lib/chef/provider_resolver.rb
+++ b/lib/chef/provider_resolver.rb
@@ -57,10 +57,16 @@ class Chef
     end
 
     def resolve
-      maybe_explicit_provider(resource) ||
+      resolved = maybe_explicit_provider(resource) ||
         maybe_custom_resource(resource) ||
-        maybe_dynamic_provider_resolution(resource, action) ||
+        maybe_dynamic_provider_resolution(resource, action)
+
+      if resolved.nil?
+        raise(Chef::Exceptions::ProviderNotFound, "Cannot find a provider for #{resource}") if node.nil?
+
         raise(Chef::Exceptions::ProviderNotFound, "Cannot find a provider for #{resource} on #{node["platform"]} version #{node["platform_version"]}")
+      end
+      resolved
     end
 
     # Does NOT call provides? on the resource (it is assumed this is being
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 2442b9a050..3707606dd9 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1063,7 +1063,8 @@ class Chef
     # action for the resource.
     #
     # @param name [Symbol] The action name to define.
-    # @param description [String] optional description for the action
+    # @param description [String] optional description for the action. Used for
+    #   documentation generation.
     # @param recipe_block The recipe to run when the action is taken. This block
     #   takes no parameters, and will be evaluated in a new context containing:
     #
@@ -1076,11 +1077,8 @@ class Chef
     def self.action(action, description: nil, &recipe_block)
       action = action.to_sym
       declare_action_class
-      action_class.action(action, &recipe_block)
+      action_class.action(action, description: description, &recipe_block)
       self.allowed_actions += [ action ]
-      # Accept any non-nil description, which will correctly override
-      # any specific inherited description.
-      action_descriptions[action] = description unless description.nil?
       default_action action if Array(default_action) == [:nothing]
     end
 
@@ -1090,18 +1088,15 @@ class Chef
     # @param action [Symbol,String] the action name
     # @return the description of the action provided, or nil if no description
     # was defined
-    def self.action_description(action)
-      action_descriptions[action.to_sym]
-    end
-
-    # @api private
-    #
-    # @return existing action description hash, or newly-initialized
-    # hash containing action descriptions inherited from parent Resource,
-    # if any.
-    def self.action_descriptions
-      @action_descriptions ||=
-        superclass.respond_to?(:action_descriptions) ? superclass.action_descriptions.dup : { nothing: nil }
+    def action_description(action)
+      provider_for_action(action).class.action_description(action)
+    rescue Chef::Exceptions::ProviderNotFound
+      # If a provider can't be found, there can be no description defined on the provider.
+      nil
+    rescue NameError => e
+      # This can happen when attempting to load a provider in a platform-specific
+      # environment where we have not required the necessary files yet
+      raise unless e.message =~ /uninitialized constant/
     end
 
     # Define a method to load up this resource's properties with the current
@@ -1188,9 +1183,10 @@ class Chef
         begin
           is_custom_resource!
           base_provider =
-            if superclass.custom_resource?
+            if superclass.custom_resource? || superclass != Chef::Resource
               superclass.action_class
             else
+
               ActionClass
             end
 
diff --git a/lib/chef/resource_inspector.rb b/lib/chef/resource_inspector.rb
index 95ae110170..094e355ad6 100644
--- a/lib/chef/resource_inspector.rb
+++ b/lib/chef/resource_inspector.rb
@@ -23,6 +23,11 @@ require_relative "node"
 require_relative "resources"
 require_relative "json_compat"
 
+# We need to require providers  so that we can resolve
+# action documentation that may have been defined on the providers
+# instead of the resources.
+require_relative "providers"
+
 class Chef
   module ResourceInspector
     def self.get_default(default)
@@ -39,11 +44,10 @@ class Chef
     def self.extract_resource(resource, complete = false)
       data = {}
       data[:description] = resource.description
-      # data[:deprecated] = resource.deprecated || false
       data[:default_action] = resource.default_action
       data[:actions] = {}
       resource.allowed_actions.each do |action|
-        data[:actions][action] = resource.action_description(action)
+        data[:actions][action] = resource.new(resource.to_s, nil).action_description(action)
       end
 
       data[:examples] = resource.examples
diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb
index 009a78a24a..fc22a3c9d2 100644
--- a/spec/integration/recipes/resource_action_spec.rb
+++ b/spec/integration/recipes/resource_action_spec.rb
@@ -354,8 +354,8 @@ module ResourceActionSpec
         end
 
         it "allows overridden action to have a description separate from the action defined in the base resource" do
-          expect(ActionJackson.action_description(:test1)).to eql "Original description"
-          expect(ActionJackalope.action_description(:test1)).to eql "An old action with a new description"
+          expect(ActionJackson.new("ActionJackson", nil).action_description(:test1)).to eql "Original description"
+          expect(ActionJackalope.new("ActionJackalope", nil).action_description(:test1)).to eql "An old action with a new description"
         end
 
         it "non-overridden actions run and can access overridden and non-overridden variables (but not necessarily new ones)" do
diff --git a/spec/unit/provider_spec.rb b/spec/unit/provider_spec.rb
index 28874bc0f3..88262dd33a 100644
--- a/spec/unit/provider_spec.rb
+++ b/spec/unit/provider_spec.rb
@@ -32,6 +32,21 @@ class NoWhyrunDemonstrator < Chef::Provider
   end
 end
 
+class ActionDescriptionDemonstrator < Chef::Provider
+  def load_current_resource; end
+
+  action :foo, description: "foo described" do
+    true
+  end
+
+  action :foo2 do
+    true
+  end
+
+end
+
+context "blah" do
+end
 class ConvergeActionDemonstrator < Chef::Provider
   attr_reader :system_state_altered
 
@@ -98,6 +113,14 @@ describe Chef::Provider do
     expect(@provider.action_nothing).to eql(true)
   end
 
+  it "should return an action description for action_description when one is available" do
+    expect(ActionDescriptionDemonstrator.action_description(:foo)).to eq "foo described"
+  end
+
+  it "should return nil for action_description when no description is available" do
+    expect(ActionDescriptionDemonstrator.action_description(:none)).to eq nil
+  end
+
   it "evals embedded recipes with a pristine resource collection" do
     @provider.run_context.instance_variable_set(:@resource_collection, "doesn't matter what this is")
     temporary_collection = nil
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index f7109cc680..5f662dea60 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -1172,21 +1172,23 @@ describe Chef::Resource do
       action :base_action3, description: "unmodified base action 3 desc" do; end
     end
 
+    let(:resource_inst) { TestResource.new("TestResource", nil) }
+
     it "returns nil when no description was provided for the action" do
-      expect(TestResource.action_description(:base_action0)).to eql(nil)
+      expect(resource_inst.action_description(:base_action0)).to eql(nil)
     end
 
     context "when action definition is a string" do
       it "returns the description whether a symbol or string is used to look it up" do
-        expect(TestResource.action_description("string_action")).to eql("a string test")
-        expect(TestResource.action_description(:string_action)).to eql("a string test")
+        expect(resource_inst.action_description("string_action")).to eql("a string test")
+        expect(resource_inst.action_description(:string_action)).to eql("a string test")
       end
     end
 
     context "when action definition is a symbol" do
       it "returns the description whether a symbol or string is used to look up" do
-        expect(TestResource.action_description("symbol_action")).to eql("a symbol test")
-        expect(TestResource.action_description(:symbol_action)).to eql("a symbol test")
+        expect(resource_inst.action_description("symbol_action")).to eql("a symbol test")
+        expect(resource_inst.action_description(:symbol_action)).to eql("a symbol test")
       end
     end
 
@@ -1196,14 +1198,23 @@ describe Chef::Resource do
         action :base_action3 do; end
       end
 
+      class TestResourceChild2 < TestResource
+        # We should never see this description
+        action :base_action2, description: "if you see this in an error, TestResourceChild was polluted with this description" do; end
+      end
+      let(:resource_inst) { TestResourceChild.new("TestResource", nil) }
+
       it "returns original description when a described action is not overridden in child resource" do
-        expect(TestResourceChild.action_description(:base_action1)).to eq "unmodified base action 1 desc"
+        expect(resource_inst.action_description(:base_action1)).to eq "unmodified base action 1 desc"
       end
       it "returns original description when the child resource overrides an inherited action but NOT its description" do
-        expect(TestResourceChild.action_description(:base_action3)).to eq "unmodified base action 3 desc"
+        expect(resource_inst.action_description(:base_action3)).to eq "unmodified base action 3 desc"
+      end
+      it "returns new description when the child resource overrides an inherited action and its description" do
+        expect(resource_inst.action_description(:base_action2)).to eq "modified base action 2 desc"
       end
       it "returns new description when the child resource overrides an inherited action and its description" do
-        expect(TestResourceChild.action_description(:base_action2)).to eq "modified base action 2 desc"
+        expect(resource_inst.action_description(:base_action2)).to eq "modified base action 2 desc"
       end
     end
   end