Merge pull request #6818 from chef/tm/better_resource_docs

Implement resource enhancement RFCs

9 files changed, 154 insertions, 15 deletions

diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb
index d90e38b916..c955dd3b12 100644
--- a/lib/chef/mixin/params_validate.rb
+++ b/lib/chef/mixin/params_validate.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
-# Copyright:: Copyright 2008-2017, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -35,6 +35,8 @@ class Chef
       # map options are:
       #
       # @param opts [Hash<Symbol,Object>] Validation opts.
+      #   @option opts [String] :validation_message A custom message to return
+      #     should validation fail.
       #   @option opts [Object,Array] :is An object, or list of
       #     objects, that must match the value using Ruby's `===` operator
       #     (`opts[:is].any? { |v| v === value }`). (See #_pv_is.)
@@ -91,16 +93,20 @@ class Chef
         raise ArgumentError, "Options must be a hash" unless opts.kind_of?(Hash)
         raise ArgumentError, "Validation Map must be a hash" unless map.kind_of?(Hash)
 
+        @validation_message ||= {}
+
         map.each do |key, validation|
           unless key.kind_of?(Symbol) || key.kind_of?(String)
             raise ArgumentError, "Validation map keys must be symbols or strings!"
           end
+
           case validation
           when true
             _pv_required(opts, key)
           when false
             true
           when Hash
+            @validation_message[key] = validation.delete(:validation_message) if validation.has_key?(:validation_message)
             validation.each do |check, carg|
               check_method = "_pv_#{check}"
               if respond_to?(check_method, true)
@@ -129,6 +135,10 @@ class Chef
         validation.has_key?(:is) && _pv_is({ key => nil }, key, validation[:is], raise_error: false)
       end
 
+      def _validation_message(key, default)
+        @validation_message.has_key?(key) ? @validation_message[key] : default
+      end
+
       # Return the value of a parameter, or nil if it doesn't exist.
       def _pv_opts_lookup(opts, key)
         if opts.has_key?(key.to_s)
@@ -145,7 +155,7 @@ class Chef
         if is_required
           return true if opts.has_key?(key.to_s) && (explicitly_allows_nil || !opts[key.to_s].nil?)
           return true if opts.has_key?(key.to_sym) && (explicitly_allows_nil || !opts[key.to_sym].nil?)
-          raise Exceptions::ValidationFailed, "Required argument #{key.inspect} is missing!"
+          raise Exceptions::ValidationFailed, _validation_message(key, "Required argument #{key.inspect} is missing!")
         end
         true
       end
@@ -168,7 +178,7 @@ class Chef
           to_be.each do |tb|
             return true if value == tb
           end
-          raise Exceptions::ValidationFailed, "Option #{key} must be equal to one of: #{to_be.join(", ")}!  You passed #{value.inspect}."
+          raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} must be equal to one of: #{to_be.join(", ")}!  You passed #{value.inspect}.")
         end
       end
 
@@ -187,7 +197,7 @@ class Chef
           to_be.each do |tb|
             return true if value.kind_of?(tb)
           end
-          raise Exceptions::ValidationFailed, "Option #{key} must be a kind of #{to_be}!  You passed #{value.inspect}."
+          raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} must be a kind of #{to_be}!  You passed #{value.inspect}.")
         end
       end
 
@@ -202,7 +212,7 @@ class Chef
         unless value.nil?
           Array(method_name_list).each do |method_name|
             unless value.respond_to?(method_name)
-              raise Exceptions::ValidationFailed, "Option #{key} must have a #{method_name} method!"
+              raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} must have a #{method_name} method!")
             end
           end
         end
@@ -234,7 +244,7 @@ class Chef
 
             if value.respond_to?(predicate_method)
               if value.send(predicate_method)
-                raise Exceptions::ValidationFailed, "Option #{key} cannot be #{predicate_method_base_name}"
+                raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} cannot be #{predicate_method_base_name}")
               end
             end
           end
@@ -294,7 +304,7 @@ class Chef
           Array(regex).flatten.each do |r|
             return true if r.match(value.to_s)
           end
-          raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} does not match regular expression #{regex.inspect}"
+          raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key}'s value #{value} does not match regular expression #{regex.inspect}")
         end
       end
 
@@ -316,7 +326,7 @@ class Chef
         if !value.nil?
           callbacks.each do |message, zeproc|
             unless zeproc.call(value)
-              raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} #{message}!"
+              raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key}'s value #{value} #{message}!")
             end
           end
         end
@@ -428,7 +438,7 @@ class Chef
           unless errors.empty?
             message << " Errors:\n#{errors.map { |m| "- #{m}" }.join("\n")}"
           end
-          raise Exceptions::ValidationFailed, message
+          raise Exceptions::ValidationFailed, _validation_message(key, message)
         end
       end
 
diff --git a/lib/chef/mixin/properties.rb b/lib/chef/mixin/properties.rb
index 8ff2cc4501..6b95b87063 100644
--- a/lib/chef/mixin/properties.rb
+++ b/lib/chef/mixin/properties.rb
@@ -75,6 +75,8 @@ class Chef
         #     will return if the user does not set one. If this is `lazy`, it will
         #     be run in the context of the instance (and able to access other
         #     properties).
+        #   @option options [String] :description A description of the property.
+        #   @option options [String] :introduced The release that introduced this property
         #   @option options [Boolean] :desired_state `true` if this property is
         #     part of desired state. Defaults to `true`.
         #   @option options [Boolean] :identity `true` if this property
@@ -301,6 +303,17 @@ class Chef
         raise ArgumentError, "Property #{name} is not defined in class #{self}" if !property
         property.reset(self)
       end
+
+      #
+      # The description of the property
+      #
+      # @param name [Symbol] The name of the property.
+      # @return [String] The description of the property.
+      def property_description(name)
+        property = self.class.properties[name.to_sym]
+        raise ArgumentError, "Property #{name} is not defined in class #{self}" if !property
+        property.description
+      end
     end
   end
 end
diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 9d0957dcdf..be54c8bfa1 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -60,10 +60,12 @@ class Chef
     #   options).
     #   @option options [Symbol] :name The name of this property.
     #   @option options [Class] :declared_in The class this property comes from.
+    #   @option options [String] :description A description of the property.
     #   @option options [Symbol] :instance_variable_name The instance variable
     #     tied to this property. Must include a leading `@`. Defaults to `@<name>`.
     #     `nil` means the property is opaque and not tied to a specific instance
     #     variable.
+    #   @option options [String] :introduced The release that introduced this property
     #   @option options [Boolean] :desired_state `true` if this property is part of desired
     #     state. Defaults to `true`.
     #   @option options [Boolean] :identity `true` if this property is part of object
@@ -158,6 +160,24 @@ class Chef
     end
 
     #
+    # A description of this property.
+    #
+    # @return [String]
+    #
+    def description
+      options[:description]
+    end
+
+    #
+    # When this property was introduced
+    #
+    # @return [String]
+    #
+    def introduced
+      options[:introduced]
+    end
+
+    #
     # The instance variable associated with this property.
     #
     # Defaults to `@<name>`
@@ -252,7 +272,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].include?(k)
+        [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required, :nillable, :sensitive, :description, :introduced].include?(k)
       end
     end
 
diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index 327bf52a13..7cb2301772 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -197,6 +197,20 @@ class Chef
       @requirements ||= ResourceRequirements.new(@new_resource, run_context)
     end
 
+    def description(description = "NOT_PASSED")
+      if description != "NOT_PASSED"
+        @description = description
+      end
+      @description
+    end
+
+    def introduced(introduced = "NOT_PASSED")
+      if introduced != "NOT_PASSED"
+        @introduced = introduced
+      end
+      @introduced
+    end
+
     def converge_by(descriptions, &block)
       converge_actions.add_action(descriptions, &block)
     end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index ac1146a647..77e735bbce 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1174,8 +1174,8 @@ class Chef
     # Internal Resource Interface (for Chef)
     #
 
-    FORBIDDEN_IVARS = [:@run_context, :@not_if, :@only_if, :@enclosing_provider]
-    HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider]
+    FORBIDDEN_IVARS = [:@run_context, :@not_if, :@only_if, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message]
+    HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message]
 
     include Chef::Mixin::ConvertToClassName
     extend Chef::Mixin::ConvertToClassName
@@ -1374,6 +1374,27 @@ class Chef
       end
     end
 
+    def self.description(description = "NOT_PASSED")
+      if description != "NOT_PASSED"
+        @description = description
+      end
+      @description
+    end
+
+    def self.introduced(introduced = "NOT_PASSED")
+      if introduced != "NOT_PASSED"
+        @introduced = introduced
+      end
+      @introduced
+    end
+
+    def self.examples(examples = "NOT_PASSED")
+      if examples != "NOT_PASSED"
+        @examples = examples
+      end
+      @examples
+    end
+
     #
     # The cookbook in which this Resource was defined (if any).
     #
diff --git a/spec/unit/mixin/params_validate_spec.rb b/spec/unit/mixin/params_validate_spec.rb
index 0cafb925c8..7bc8a27398 100644
--- a/spec/unit/mixin/params_validate_spec.rb
+++ b/spec/unit/mixin/params_validate_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2018, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -342,6 +342,17 @@ describe Chef::Mixin::ParamsValidate do
     end.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
+  it "allows a custom validation message" do
+    expect do
+      @vo.validate({ :not_blank => "should pass" },
+                   { :not_blank => { :cannot_be => [ :nil, :empty ], validation_message: "my validation message" } })
+    end.not_to raise_error
+    expect do
+      @vo.validate({ :not_blank => "" },
+                   { :not_blank => { :cannot_be => [ :nil, :empty ], validation_message: "my validation message" } })
+    end.to raise_error(Chef::Exceptions::ValidationFailed, "my validation message")
+  end
+
   it "should set and return a value, then return the same value" do
     value = "meow"
     expect(@vo.set_or_return(:test, value, {}).object_id).to eq(value.object_id)
diff --git a/spec/unit/mixin/properties_spec.rb b/spec/unit/mixin/properties_spec.rb
index 1af0bc7abd..ee0c252381 100644
--- a/spec/unit/mixin/properties_spec.rb
+++ b/spec/unit/mixin/properties_spec.rb
@@ -11,6 +11,7 @@ module ChefMixinPropertiesSpec
         property :a, "a", default: "a"
         property :ab, %w{a b}, default: "a"
         property :ac, %w{a c}, default: "a"
+        property :d, "d", description: "The d property", introduced: "14.0"
       end
 
       context "and a module B with properties b, ab and bc" do
@@ -30,11 +31,20 @@ module ChefMixinPropertiesSpec
           end
 
           it "A.properties has a, ab, and ac with types 'a', ['a', 'b'], and ['b', 'c']" do
-            expect(A.properties.keys).to eq [ :a, :ab, :ac ]
+            expect(A.properties.keys).to eq [ :a, :ab, :ac, :d ]
             expect(A.properties[:a].validation_options[:is]).to eq "a"
             expect(A.properties[:ab].validation_options[:is]).to eq %w{a b}
             expect(A.properties[:ac].validation_options[:is]).to eq %w{a c}
           end
+
+          it "A.properties can get the description of `d`" do
+            expect(A.properties[:d].description).to eq "The d property"
+          end
+
+          it "A.properties can get the release that introduced `d`" do
+            expect(A.properties[:d].introduced).to eq "14.0"
+          end
+
           it "B.properties has b, ab, and bc with types 'b', nil and ['b', 'c']" do
             expect(B.properties.keys).to eq [ :b, :ab, :bc ]
             expect(B.properties[:b].validation_options[:is]).to eq "b"
@@ -42,7 +52,7 @@ module ChefMixinPropertiesSpec
             expect(B.properties[:bc].validation_options[:is]).to eq %w{b c}
           end
           it "C.properties has a, b, c, ac and bc with merged types" do
-            expect(C.properties.keys).to eq [ :a, :ab, :ac, :b, :bc, :c ]
+            expect(C.properties.keys).to eq [ :a, :ab, :ac, :d, :b, :bc, :c ]
             expect(C.properties[:a].validation_options[:is]).to eq "a"
             expect(C.properties[:b].validation_options[:is]).to eq "b"
             expect(C.properties[:c].validation_options[:is]).to eq "c"
diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb
index 13afcdfbc2..882ea3353b 100644
--- a/spec/unit/property/validation_spec.rb
+++ b/spec/unit/property/validation_spec.rb
@@ -699,4 +699,13 @@ describe "Chef::Resource.property validation" do
       end
     end
   end
+
+  context "custom validation messages" do
+    with_property ":x, String, validation_message: 'Must be a string, fool'" do
+      it "raise with the correct error message" do
+        expect { resource.x 1 }.to raise_error Chef::Exceptions::ValidationFailed,
+          "Must be a string, fool"
+      end
+    end
+  end
 end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 934a613eee..fe853922a1 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -387,6 +387,37 @@ describe Chef::Resource do
     end
   end
 
+  context "Documentation of resources" do
+    it "can have a description" do
+      c = Class.new(Chef::Resource) do
+        description "my description"
+      end
+      expect(c.description).to eq "my description"
+    end
+
+    it "can say when it was introduced" do
+      c = Class.new(Chef::Resource) do
+        introduced "14.0"
+      end
+      expect(c.introduced).to eq "14.0"
+    end
+
+    it "can have some examples" do
+      c = Class.new(Chef::Resource) do
+        examples <<-EOH
+resource "foo" do
+  foo foo
+end
+        EOH
+      end
+      expect(c.examples).to eq <<-EOH
+resource "foo" do
+  foo foo
+end
+        EOH
+    end
+  end
+
   describe "self.resource_name" do
     context "When resource_name is not set" do
       it "and there are no provides lines, resource_name is nil" do