From 165170c08b4f3f7678c4c463718aaba9bec0fde8 Mon Sep 17 00:00:00 2001 From: Will Hopper Date: Tue, 10 Jan 2017 11:06:04 -0800 Subject: [PATCH 1/2] (PDOC-155) Allow type documentation in Puppet 4 code Because Puppet 4 is typed, parameter type information can be automatically determined without any explicit documentation in @param tags. However, users may desire to do so anyway for consistency. This commit allows @param tags to include a [type] in Puppet 4 code. Strings will emit a warning if the documented type does not match the actual type. In such an event, the incorrect documented type will be ignored in favor of the real one. --- README.md | 6 ++--- .../yard/handlers/puppet/base.rb | 4 +-- .../handlers/puppet/class_handler_spec.rb | 27 +++++++++++++++++-- .../puppet/defined_type_handler_spec.rb | 27 +++++++++++++++++-- .../handlers/puppet/function_handler_spec.rb | 26 ++++++++++++++++-- 5 files changed, 79 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 596cfc1..a02f557 100644 --- a/README.md +++ b/README.md @@ -210,15 +210,15 @@ The Strings elements appearing in the above comment block are: * The `include` statement, which adds the usage example code. * Two `@param` tags, with the name of the parameter first, followed by a string describing the parameter's purpose. -Puppet 4 is a typed language, so Puppet Strings automatically documents the parameter types from code. With Puppet 3, however, include the parameter type with the `@param` tag: +Puppet 4 is a typed language, so Puppet Strings automatically documents the parameter types from code. You may optionally include a parameter type in the `@param` tag. Strings will emit a warning and ignore the documented type should it differ from the actual type. + +With Puppet 3 code, you must always include the parameter type with the `@param` tag: ``` # @param first [String] The first parameter for this class. # @param second [Integer] The second parameter for this class. ``` -Note that if you document a parameter's type, and that parameter already has a Puppet type specifier, Strings emits a warning. - Defined types are documented in exactly the same way as classes: ``` diff --git a/lib/puppet-strings/yard/handlers/puppet/base.rb b/lib/puppet-strings/yard/handlers/puppet/base.rb index 8bcd8d2..a7df226 100644 --- a/lib/puppet-strings/yard/handlers/puppet/base.rb +++ b/lib/puppet-strings/yard/handlers/puppet/base.rb @@ -31,8 +31,8 @@ class PuppetStrings::Yard::Handlers::Puppet::Base < YARD::Handlers::Base next end - # Warn if the parameter is typed and the tag also has a type - log.warn "The @param tag for parameter '#{parameter.name}' should not contain a type specification near #{statement.file}:#{statement.line}: ignoring in favor of parameter type information." if parameter.type && tag.types && !tag.types.empty? + # Warn if the parameter type and tag types don't match + log.warn "The type of the @param tag for parameter '#{parameter.name}' does not match the parameter type specification near #{statement.file}:#{statement.line}: ignoring in favor of parameter type information." if parameter.type && tag.types && !tag.types.empty? && parameter.type != tag.types[0] if parameter.type tag.types = [parameter.type] diff --git a/spec/unit/puppet-strings/yard/handlers/puppet/class_handler_spec.rb b/spec/unit/puppet-strings/yard/handlers/puppet/class_handler_spec.rb index cc5b7c4..88ee266 100644 --- a/spec/unit/puppet-strings/yard/handlers/puppet/class_handler_spec.rb +++ b/spec/unit/puppet-strings/yard/handlers/puppet/class_handler_spec.rb @@ -111,7 +111,30 @@ SOURCE end end - describe 'parsing a class with a typed parameter that also has a @param tag type' do + describe 'parsing a class with a typed parameter that also has a @param tag type which matches' do + let(:source) { <<-SOURCE +# A simple foo class. +# @param [Integer] param1 First param. +# @param param2 Second param. +# @param param3 Third param. +class foo(Integer $param1, $param2, String $param3 = hi) inherits foo::bar { + file { '/tmp/foo': + ensure => present + } +} +SOURCE + } + + it 'should respect the type that was documented' do + expect{ subject }.to output('').to_stdout_from_any_process + expect(subject.size).to eq(1) + tags = subject.first.tags(:param) + expect(tags.size).to eq(3) + expect(tags[0].types).to eq(['Integer']) + end + end + + describe 'parsing a class with a typed parameter that also has a @param tag type which does not match' do let(:source) { <<-SOURCE # A simple foo class. # @param [Boolean] param1 First param. @@ -126,7 +149,7 @@ SOURCE } it 'should output a warning' do - expect{ subject }.to output(/\[warn\]: The @param tag for parameter 'param1' should not contain a type specification near \(stdin\):5: ignoring in favor of parameter type information\./).to_stdout_from_any_process + expect{ subject }.to output(/\[warn\]: The type of the @param tag for parameter 'param1' does not match the parameter type specification near \(stdin\):5: ignoring in favor of parameter type information./).to_stdout_from_any_process end end diff --git a/spec/unit/puppet-strings/yard/handlers/puppet/defined_type_handler_spec.rb b/spec/unit/puppet-strings/yard/handlers/puppet/defined_type_handler_spec.rb index 633f8f0..a6e46cc 100644 --- a/spec/unit/puppet-strings/yard/handlers/puppet/defined_type_handler_spec.rb +++ b/spec/unit/puppet-strings/yard/handlers/puppet/defined_type_handler_spec.rb @@ -111,7 +111,30 @@ SOURCE end end - describe 'parsing a defined type with a typed parameter that also has a @param tag type' do + describe 'parsing a defined type with a typed parameter that also has a @param tag type which matches' do + let(:source) { <<-SOURCE +# A simple foo defined type. +# @param [Integer] param1 First param. +# @param param2 Second param. +# @param param3 Third param. +define foo(Integer $param1, $param2, String $param3 = hi) { + file { '/tmp/foo': + ensure => present + } +} +SOURCE + } + + it 'should respect the type that was documented' do + expect{ subject }.to output('').to_stdout_from_any_process + expect(subject.size).to eq(1) + tags = subject.first.tags(:param) + expect(tags.size).to eq(3) + expect(tags[0].types).to eq(['Integer']) + end + end + + describe 'parsing a defined type with a typed parameter that also has a @param tag type which does not match' do let(:source) { <<-SOURCE # A simple foo defined type. # @param [Boolean] param1 First param. @@ -126,7 +149,7 @@ SOURCE } it 'should output a warning' do - expect{ subject }.to output(/\[warn\]: The @param tag for parameter 'param1' should not contain a type specification near \(stdin\):5: ignoring in favor of parameter type information\./).to_stdout_from_any_process + expect{ subject }.to output(/\[warn\]: The type of the @param tag for parameter 'param1' does not match the parameter type specification near \(stdin\):5: ignoring in favor of parameter type information./).to_stdout_from_any_process end end diff --git a/spec/unit/puppet-strings/yard/handlers/puppet/function_handler_spec.rb b/spec/unit/puppet-strings/yard/handlers/puppet/function_handler_spec.rb index 62b7fa0..b58d3ca 100644 --- a/spec/unit/puppet-strings/yard/handlers/puppet/function_handler_spec.rb +++ b/spec/unit/puppet-strings/yard/handlers/puppet/function_handler_spec.rb @@ -115,7 +115,29 @@ SOURCE end end - describe 'parsing a function with a typed parameter that also has a @param tag type' do + describe 'parsing a function with a typed parameter that also has a @param tag type which matches' do + let(:source) { <<-SOURCE +# A simple foo function. +# @param [Integer] param1 First param. +# @param param2 Second param. +# @param param3 Third param. +# @return [Undef] Returns nothing. +function foo(Integer $param1, $param2, String $param3 = hi) { + notice 'hello world' +} +SOURCE + } + + it 'should respect the type that was documented' do + expect{ subject }.to output('').to_stdout_from_any_process + expect(subject.size).to eq(1) + tags = subject.first.tags(:param) + expect(tags.size).to eq(3) + expect(tags[0].types).to eq(['Integer']) + end + end + + describe 'parsing a function with a typed parameter that also has a @param tag type which does not match' do let(:source) { <<-SOURCE # A simple foo function. # @param [Boolean] param1 First param. @@ -129,7 +151,7 @@ SOURCE } it 'should output a warning' do - expect{ subject }.to output(/\[warn\]: The @param tag for parameter 'param1' should not contain a type specification near \(stdin\):6: ignoring in favor of parameter type information\./).to_stdout_from_any_process + expect{ subject }.to output(/\[warn\]: The type of the @param tag for parameter 'param1' does not match the parameter type specification near \(stdin\):6: ignoring in favor of parameter type information./).to_stdout_from_any_process end end From 2d0a1f0c702c84d73d81eece4c01b8f2fc713d03 Mon Sep 17 00:00:00 2001 From: Will Hopper Date: Tue, 10 Jan 2017 12:37:06 -0800 Subject: [PATCH 2/2] (maint) Put provider command in quotes in JSON test --- spec/unit/puppet-strings/json_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/puppet-strings/json_spec.rb b/spec/unit/puppet-strings/json_spec.rb index feed48c..60f9cd5 100644 --- a/spec/unit/puppet-strings/json_spec.rb +++ b/spec/unit/puppet-strings/json_spec.rb @@ -83,7 +83,7 @@ Puppet::Type.type(:database).provide :linux do defaultfor :osfamily => 'RedHat', :operatingsystemmajrelease => '7' has_feature :implements_some_feature has_feature :some_other_feature - commands foo: /usr/bin/foo + commands foo: '/usr/bin/foo' end Puppet::Type.newtype(:database) do