(maint) Mergeup 1.6.x: FM-7839, desc/docs cleanup

Gemfile

@@ -12,7 +12,7 @@ group :tests do
   # rubocop 0.58 throws when testing against ruby 2.1, so pin to the latest,
   # unless we're dealing with jruby...
   if RUBY_PLATFORM == 'java'
-    # load any rubocop version that works on java for the Rakefile
+    # load a rubocop version that works on java for the Rakefile
     gem 'parser', '2.3.3.1'
     gem 'rubocop', '0.41.2'
   elsif Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.0')

lib/puppet/resource_api.rb

@@ -35,7 +35,7 @@ def register_type(definition)
     end
 
     Puppet::Type.newtype(definition[:name].to_sym) do
-      @docs = definition[:docs]
+      @docs = definition[:desc]
       @type_definition = type_def
 
       # Keeps a copy of the provider around. Weird naming to avoid clashes with puppet's own `provider` member

lib/puppet/resource_api/type_definition.rb

@@ -101,10 +101,20 @@ def validate_schema(definition, attr_key)
         }
       end
 
+      # fixup desc/docs backwards compatibility
+      if definition.key? :docs
+        if definition[:desc]
+          raise Puppet::DevError, '`%{name}` has both `desc` and `docs`, prefer using `desc`' % { name: definition[:name] }
+        end
+        definition[:desc] = definition[:docs]
+        definition.delete(:docs)
+      end
+      Puppet.warning('`%{name}` has no documentation, add it using a `desc` key' % { name: definition[:name] }) unless definition.key? :desc
+
       attributes.each do |key, attr|
         raise Puppet::DevError, "`#{definition[:name]}.#{key}` must be a Hash, not a #{attr.class}" unless attr.is_a? Hash
         raise Puppet::DevError, "`#{definition[:name]}.#{key}` has no type" unless attr.key? :type
-        Puppet.warning("`#{definition[:name]}.#{key}` has no docs") unless attr.key? :desc
+        Puppet.warning('`%{name}.%{key}` has no documentation, add it using a `desc` key' % { name: definition[:name], key: key }) unless attr.key? :desc
 
         # validate the type by attempting to parse into a puppet type
         @data_type_cache[attributes[key][:type]] ||=

spec/puppet/resource_api/base_context_spec.rb

@@ -13,7 +13,16 @@ def send_log(log, msg)
     TestContext.new(definition)
   end
 
-  let(:definition_hash) { { name: 'some_resource', attributes: { name: { type: 'String', desc: 'message' } }, features: feature_support } }
+  let(:definition_hash) do
+    {
+      name: 'some_resource',
+      desc: 'a test resource',
+      attributes: {
+        name: { type: 'String', desc: 'message' },
+      },
+      features: feature_support,
+    }
+  end
   let(:definition) { Puppet::ResourceApi::TypeDefinition.new(definition_hash) }
   let(:feature_support) { [] }
 

spec/puppet/resource_api/base_type_definition_spec.rb

@@ -4,22 +4,27 @@
   subject(:type) { described_class.new(definition, :attributes) }
 
   let(:definition) do
-    { name: 'some_resource', attributes: {
-      ensure:      {
-        type:    'Enum[present, absent]',
-        desc:    'Whether this resource should be present or absent on the target system.',
-        default: 'present',
+    {
+      name: 'some_resource',
+      desc: 'some desc',
+      attributes: {
+        ensure:      {
+          type:    'Enum[present, absent]',
+          desc:    'Whether this resource should be present or absent on the target system.',
+          default: 'present',
+        },
+        name:        {
+          type:      'String',
+          desc:      'The name of the resource you want to manage.',
+          behaviour: :namevar,
+        },
+        prop:        {
+          type:      'Integer',
+          desc:      'A mandatory property, that MUST NOT be validated on deleting.',
+        },
       },
-      name:        {
-        type:      'String',
-        desc:      'The name of the resource you want to manage.',
-        behaviour: :namevar,
-      },
-      prop:        {
-        type:      'Integer',
-        desc:      'A mandatory property, that MUST NOT be validated on deleting.',
-      },
-    }, features: feature_support }
+      features: feature_support,
+    }
   end
   let(:feature_support) { [] }
 
@@ -76,6 +81,7 @@
       let(:definition) do
         {
           name: 'some_transport',
+          desc: 'some desc',
           connection_info: {
             username:        {
               type:      'String',
@@ -215,17 +221,17 @@
       it { expect { type }.to raise_error Puppet::DevError, %r{has no type} }
     end
 
-    context 'when an attribute has no descrption' do
-      let(:definition) { { name: 'some_resource', attributes: { name: { type: 'String' } } } }
+    context 'when an attribute has no description' do
+      let(:definition) { { name: 'some_resource', desc: 'some desc', attributes: { name: { type: 'String' } } } }
 
       it 'Raises a warning message' do
-        expect(Puppet).to receive(:warning).with('`some_resource.name` has no docs')
+        expect(Puppet).to receive(:warning).with('`some_resource.name` has no documentation, add it using a `desc` key')
         type
       end
     end
 
     context 'when an attribute has an unsupported type' do
-      let(:definition) { { name: 'some_resource', attributes: { name: { type: 'basic' } } } }
+      let(:definition) { { name: 'some_resource', desc: 'some desc', attributes: { name: { type: 'basic' } } } }
 
       it { expect { type }.to raise_error %r{<basic> is not a valid type specification} }
     end

spec/puppet/resource_api/puppet_context_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe Puppet::ResourceApi::PuppetContext do
   subject(:context) { described_class.new(definition) }
 
-  let(:definition) { { name: 'some_resource', attributes: {} } }
+  let(:definition) { { name: 'some_resource', desc: 'a test resource', attributes: {} } }
 
   describe '#device' do
     context 'when a NetworkDevice is configured' do

spec/puppet/resource_api_spec.rb

@@ -25,7 +25,10 @@
   context 'when registering a minimal type' do
     let(:definition) { { name: 'minimal', attributes: {} } }
 
-    it { expect { described_class.register_type(definition) }.not_to raise_error }
+    it {
+      expect(Puppet).to receive(:warning).with('`minimal` has no documentation, add it using a `desc` key')
+      described_class.register_type(definition)
+    }
 
     describe 'the registered type' do
       subject(:type) { Puppet::Type.type(:minimal) }
@@ -40,10 +43,33 @@
     end
   end
 
+  context 'when registering a type with both desc and docs key' do
+    let(:definition) { { name: 'both', desc: 'the desc', docs: 'the docs', attributes: {} } }
+
+    it {
+      expect { described_class.register_type(definition) }.to raise_error Puppet::DevError, '`both` has both `desc` and `docs`, prefer using `desc`'
+    }
+  end
+
+  context 'when registering a type with a docs key' do
+    let(:definition) { { name: 'both', docs: 'the docs', attributes: {} } }
+
+    it { expect { described_class.register_type(definition) }.not_to raise_error }
+
+    describe 'the registered type' do
+      subject(:type) { Puppet::Type.type(:both) }
+
+      it { is_expected.not_to be_nil }
+      it { is_expected.to be_respond_to :instances }
+      it { expect(type.instance_variable_get(:@docs)).to eq 'the docs' }
+    end
+  end
+
   context 'when registering a type with multiple attributes' do
     let(:definition) do
       {
         name: type_name,
+        desc: 'a test resource',
         attributes: {
           name: {
             type: 'String',
@@ -747,6 +773,7 @@ def set(_context, _changes); end
     let(:definition) do
       {
         name: 'init_behaviour',
+        desc: 'a test resource',
         attributes: {
           ensure: {
             type: 'Enum[present, absent]',
@@ -1172,7 +1199,7 @@ def set(_context, _changes); end
     context 'when loading a provider that doesn\'t create the correct class' do
       let(:definition) { { name: 'no_class', attributes: {} } }
 
-      it { expect { described_class.load_provider('no_class') }.to raise_error Puppet::DevError, %r{Puppet::Provider::NoClass::NoClass} }
+      it { expect { described_class.load_provider('no_class') }.to raise_error Puppet::DevError, %r{provider class Puppet::Provider::NoClass::NoClass not found} }
     end
 
     context 'when loading a provider that creates the correct class' do
@@ -1253,6 +1280,7 @@ class OtherDevice; end
     let(:definition) do
       {
         name: 'canonicalizer',
+        desc: 'some desc',
         attributes: {
           name: {
             type: 'String',
@@ -1797,6 +1825,7 @@ def set(_context, changes) end
     let(:definition) do
       {
         name: 'test_noop_support',
+        desc: 'a test resource',
         features: ['no such feature'],
         attributes: {},
       }
@@ -1813,6 +1842,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             id: {
               type: 'String',
@@ -1829,6 +1859,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             param: {
               type: 'String',
@@ -1845,6 +1876,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             param_ro: {
               type: 'String',
@@ -1861,6 +1893,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             param_ro: {
               type: 'String',
@@ -1877,6 +1910,7 @@ def set(_context, changes) end
       let(:definition) do
         {
           name: 'test_behaviour',
+          desc: 'a test resource',
           attributes: {
             source: {
               type: 'String',