From 11e016e81e555feca6652feea1e9570c09765792 Mon Sep 17 00:00:00 2001 From: Ian Kronquist Date: Thu, 20 Aug 2015 15:09:52 -0700 Subject: [PATCH] (PDOC-38) Prevent warnings from being issued twice Since we threw away all of Yard's warnings we are no longer checking that the parameter names match for Ruby methods. Thus we need to override Yard's method_details template with our own to trigger our warning function. However, there's a catch. If this ruby method is in a Puppet 4x function, we don't want our warning function to trigger because the user has already been warned. Look in the registry to see if there is already a Puppet 4x function with the same name registered. Also, print errors to stderr instead of using log.warn. --- .../default/method_details/html/header.erb | 17 +++ .../templates/default/method_details/setup.rb | 22 +++ .../default/method_details/text/header.erb | 2 + .../yard/templates/default/template_helper.rb | 2 +- .../lib/test.rb | 18 +++ .../lib/test.rb | 7 + .../test-param-names-differ/lib/test.rb | 7 + .../lib/test.rb | 11 ++ .../lib/test.rb | 7 + .../test-param-names-match/lib/test.rb | 7 + .../strings/yard/host_class_handler_spec.rb | 8 +- .../yard/puppet_4x_function_handler_spec.rb | 135 +++++++----------- .../strings/yard/template_helper_spec.rb | 8 +- 13 files changed, 156 insertions(+), 95 deletions(-) create mode 100644 lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/html/header.erb create mode 100644 lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/setup.rb create mode 100644 lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/text/header.erb create mode 100644 spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-dispatch/lib/test.rb create mode 100644 spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-types/lib/test.rb create mode 100644 spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ/lib/test.rb create mode 100644 spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-dispatch/lib/test.rb create mode 100644 spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-types/lib/test.rb create mode 100644 spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match/lib/test.rb diff --git a/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/html/header.erb b/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/html/header.erb new file mode 100644 index 0000000..1ca030b --- /dev/null +++ b/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/html/header.erb @@ -0,0 +1,17 @@ +

Method: <%= object.path %>

+
+
+
Defined in:
+
+ <%= object.file %><% if object.files.size > 1 %>,
+ <%= object.files[1..-1].map {|f| f.first }.join(",
") %>
+ <% end %> + + + + +
+
+ <%= yieldall :index => 0 %> +
+
\ No newline at end of file diff --git a/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/setup.rb b/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/setup.rb new file mode 100644 index 0000000..99cdecb --- /dev/null +++ b/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/setup.rb @@ -0,0 +1,22 @@ +include T('default/module') +require File.join(File.dirname(__FILE__),'../html_helper') +require File.join(File.dirname(__FILE__),'../template_helper') + +def init + sections :header, [:method_signature, T('docstring'), :source] + parents = YARD::Registry.all(:method).reject do |item| + item.name == object.name and item.namespace === PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject + end + if parents.length == 0 +require 'pry'; binding.pry + @template_helper = TemplateHelper.new + @template_helper.check_parameters_match_docs object + end +end + +def source + return if owner != object.namespace + return if Tags::OverloadTag === object + return if object.source.nil? + erb(:source) +end diff --git a/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/text/header.erb b/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/text/header.erb new file mode 100644 index 0000000..e0429b1 --- /dev/null +++ b/lib/puppet_x/puppetlabs/strings/yard/templates/default/method_details/text/header.erb @@ -0,0 +1,2 @@ +<%= yieldall %> + diff --git a/lib/puppet_x/puppetlabs/strings/yard/templates/default/template_helper.rb b/lib/puppet_x/puppetlabs/strings/yard/templates/default/template_helper.rb index ac4a920..dca57e9 100644 --- a/lib/puppet_x/puppetlabs/strings/yard/templates/default/template_helper.rb +++ b/lib/puppet_x/puppetlabs/strings/yard/templates/default/template_helper.rb @@ -153,7 +153,7 @@ class TemplateHelper "#{actual_types.inspect} Sorry, the file and line number could" + "not be determined." end - log.warn warning + $stderr.puts warning end end end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-dispatch/lib/test.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-dispatch/lib/test.rb new file mode 100644 index 0000000..4fc372c --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-dispatch/lib/test.rb @@ -0,0 +1,18 @@ +# @param not_a_param [Integer] the first number to be compared +# @param also_not_a_param [Integer] the second number to be compared +Puppet::Functions.create_function(:max) do + dispatch max_1 do + param 'Integer[1,2]', :num_a + param 'Integer', :num_b + end + dispatch max_2 { + param 'String', :num_c + param 'String[1,2]', :num_d + } + def max_1(num_a, num_b) + num_a >= num_b ? num_a : num_b + end + def max_2(num_a, num_b) + num_a >= num_b ? num_a : num_b + end +end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-types/lib/test.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-types/lib/test.rb new file mode 100644 index 0000000..22cb89c --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ-with-types/lib/test.rb @@ -0,0 +1,7 @@ +# @param not_a_param [Integer[1,2]] the first number to be compared +# @param also_not_a_param [Integer[1,2]] the second number to be compared +Puppet::Functions.create_function(:max) do + def max(num_a, num_b) + num_a >= num_b ? num_a : num_b + end +end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ/lib/test.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ/lib/test.rb new file mode 100644 index 0000000..cf46559 --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-differ/lib/test.rb @@ -0,0 +1,7 @@ +# @param not_a_param [Integer] the first number to be compared +# @param also_not_a_param [Integer] the second number to be compared +Puppet::Functions.create_function(:max) do + def max(num_a, num_b) + num_a >= num_b ? num_a : num_b + end +end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-dispatch/lib/test.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-dispatch/lib/test.rb new file mode 100644 index 0000000..6e7342c --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-dispatch/lib/test.rb @@ -0,0 +1,11 @@ +# @param [Integer] num_a the first number to be compared +# @param num_b [Integer] the second number to be compared +Puppet::Functions.create_function(:max) do + dispatch max_1 do + param 'Integer', :num_a + param 'Integer', :num_b + end + def max_1(num_a, num_b) + num_a >= num_b ? num_a : num_b + end +end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-types/lib/test.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-types/lib/test.rb new file mode 100644 index 0000000..fc5e92a --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match-with-types/lib/test.rb @@ -0,0 +1,7 @@ +# @param num_a [Integer[1,2]] the first number to be compared +# @param num_b [Integer[1,2]] the second number to be compared +Puppet::Functions.create_function(:max)do + def max(num_a, num_b) + num_a >= num_b ? num_a : num_b + end +end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match/lib/test.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match/lib/test.rb new file mode 100644 index 0000000..1497270 --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/examples/test-param-names-match/lib/test.rb @@ -0,0 +1,7 @@ +# @param num_a [Integer] the first number to be compared +# @param num_b [Integer] the second number to be compared +Puppet::Functions.create_function(:max)do + def max(num_a, num_b) + num_a >= num_b ? num_a : num_b + end +end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/host_class_handler_spec.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/host_class_handler_spec.rb index 4939b6e..20694ab 100644 --- a/spec/unit/puppet_x/puppetlabs/strings/yard/host_class_handler_spec.rb +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/host_class_handler_spec.rb @@ -67,8 +67,7 @@ describe PuppetX::PuppetLabs::Strings::YARD::Handlers::HostClassHandler do # puppet. `expected` is the output expected from the stable branch. The # output from the master branch will use this instead: # "...specifies the types [String] in file..." - expected = <<-output -[warn]: @param tag types do not match the code. The ident parameter is declared as types [\"Float\"] in the docstring, but the code specifies the types [Puppet::Pops::Types::PStringType] in file manifests/init.pp near line 2 + expected_stout = <<-output Files: 1 Modules: 0 ( 0 undocumented) Classes: 0 ( 0 undocumented) @@ -78,12 +77,15 @@ Puppet Classes: 1 ( 0 undocumented) Puppet Types: 0 ( 0 undocumented) 100.00% documented output + expected_stderr = "@param tag types do not match the code. The ident parameter is declared as types [\"Float\"] in the docstring, but the code specifies the types [Puppet::Pops::Types::PStringType] in file manifests/init.pp near line 2\n" expect { + expect { PuppetModuleHelper.using_module(File.dirname(__FILE__),'test') do |tmp| Dir.chdir('test') Puppet::Face[:strings, :current].yardoc end - }.to output(expected).to_stdout_from_any_process + }.to output(expected_stderr).to_stderr_from_any_process + }.to output(expected_stout).to_stdout_from_any_process end end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/puppet_4x_function_handler_spec.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/puppet_4x_function_handler_spec.rb index 51f5f5f..8f5b110 100644 --- a/spec/unit/puppet_x/puppetlabs/strings/yard/puppet_4x_function_handler_spec.rb +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/puppet_4x_function_handler_spec.rb @@ -1,5 +1,7 @@ require 'spec_helper' +require 'lib/strings_spec/module_helper' require 'puppet_x/puppetlabs/strings/yard/handlers/puppet_4x_function_handler' +require 'puppet/face/strings' require 'strings_spec/parsing' describe PuppetX::PuppetLabs::Strings::YARD::Handlers::Puppet4xFunctionHandler do @@ -55,122 +57,81 @@ describe PuppetX::PuppetLabs::Strings::YARD::Handlers::Puppet4xFunctionHandler d end it "should issue a warning if the parameter names do not match the docstring" do - expected_output_not_a_param = "[warn]: @param tag has unknown parameter" + - " name: not_a_param \n in file `(stdin)' near line 3" - expected_output_also_not_a_param = "[warn]: @param tag has unknown " + - "parameter name: also_not_a_param \n in file `(stdin)' near line 3" + expected_output_not_a_param = "[warn]: The parameter not_a_param is documented, but doesn't exist in your code, in file lib/test.rb near line 3" + expected_output_also_not_a_param = "[warn]: The parameter also_not_a_param is documented, but doesn't exist in your code, in file lib/test.rb near line 3" expect { - parse <<-RUBY - # @param not_a_param [Integer] the first number to be compared - # @param also_not_a_param [Integer] the second number to be compared - Puppet::Functions.create_function(:max) do - def max(num_a, num_b) - num_a >= num_b ? num_a : num_b - end + expect { + PuppetModuleHelper.using_module(File.dirname(__FILE__),'test-param-names-differ') do |tmp| + Dir.chdir('test-param-names-differ') + Puppet::Face[:strings, :current].yardoc end - RUBY - }.to output("#{expected_output_not_a_param}\n#{expected_output_also_not_a_param}\n").to_stdout_from_any_process + }.to output(/documented/).to_stdout_from_any_process + }.to output("#{expected_output_not_a_param}\n#{expected_output_also_not_a_param}\n").to_stderr_from_any_process end it "should not issue a warning when the parameter names match the docstring" do + expected = "" expect { - parse <<-RUBY - # @param num_a [Integer] the first number to be compared - # @param num_b [Integer] the second number to be compared - Puppet::Functions.create_function(:max) do - def max(num_a, num_b) - num_a >= num_b ? num_a : num_b - end + expect { + PuppetModuleHelper.using_module(File.dirname(__FILE__),'test-param-names-match') do |tmp| + Dir.chdir('test-param-names-match') + Puppet::Face[:strings, :current].yardoc end - RUBY - }.to output("").to_stdout_from_any_process - end + }.to output(/documented/).to_stdout_from_any_process + }.to output(expected).to_stderr_from_any_process + end it "should not issue a warning when there are parametarized types and parameter names are the same" do + expected = "" expect { - parse <<-RUBY - # @param num_a Integer[1,2] the first number to be compared - # @param num_b Integer[1,2] the second number to be compared - Puppet::Functions.create_function(:max) do - def max(num_a, num_b) - num_a >= num_b ? num_a : num_b - end + expect { + PuppetModuleHelper.using_module(File.dirname(__FILE__),'test-param-names-match-with-types') do |tmp| + Dir.chdir('test-param-names-match-with-types') + Puppet::Face[:strings, :current].yardoc end - RUBY - }.to output("").to_stdout_from_any_process + }.to output(/documented/).to_stdout_from_any_process + }.to output(expected).to_stderr_from_any_process end it "should issue a warning when there are parametarized types and parameter names differ" do expected_output_not_num_a = "[warn]: @param tag has unknown parameter" + " name: not_num_a \n in file `(stdin)' near line 3" + expected_output_not_a_param = "[warn]: The parameter not_a_param is documented, but doesn't exist in your code, in file lib/test.rb near line 3" + expected_output_also_not_a_param = "[warn]: The parameter also_not_a_param is documented, but doesn't exist in your code, in file lib/test.rb near line 3" expect { - parse <<-RUBY - # @param not_num_a Integer[1,2] the first number to be compared - # @param num_b Integer[1,2] the second number to be compared - Puppet::Functions.create_function(:max) do - dispatch max_1 do - param 'Integer[1,2]', :num_a - param 'Integer[1,2]', :num_b - end - - def max_1(num_a, num_b) - num_a >= num_b ? num_a : num_b - end + expect { + PuppetModuleHelper.using_module(File.dirname(__FILE__),'test-param-names-differ-with-types') do |tmp| + Dir.chdir('test-param-names-differ-with-types') + Puppet::Face[:strings, :current].yardoc end - RUBY - }.to output("#{expected_output_not_num_a}\n").to_stdout_from_any_process + }.to output(/documented/).to_stdout_from_any_process + }.to output("#{expected_output_not_a_param}\n#{expected_output_also_not_a_param}\n").to_stderr_from_any_process end it "should issue a warning if the parameter names do not match the docstring in dispatch method" do - expected_output_not_a_param = "[warn]: @param tag has unknown parameter" + - " name: not_a_param \n in file `(stdin)' near line 3" - expected_output_also_not_a_param = "[warn]: @param tag has unknown " + - "parameter name: also_not_a_param \n in file `(stdin)' near line 3" + expected_output_not_a_param = "[warn]: The parameter not_a_param is documented, but doesn't exist in your code, in file lib/test.rb near line 3" + expected_output_also_not_a_param = "[warn]: The parameter also_not_a_param is documented, but doesn't exist in your code, in file lib/test.rb near line 3" expect { - parse <<-RUBY - # @param not_a_param [Integer] the first number to be compared - # @param also_not_a_param [Integer] the second number to be compared - Puppet::Functions.create_function(:max) do - dispatch max_1 do - param 'Integer[1,2]', :num_a - param 'Integer', :num_b - end - - dispatch max_2 { - param 'String', :num_c - param 'String[1,2]', :num_d - } - - def max_1(num_a, num_b) - num_a >= num_b ? num_a : num_b - end - - def max_2(num_a, num_b) - num_a >= num_b ? num_a : num_b - end + expect { + PuppetModuleHelper.using_module(File.dirname(__FILE__),'test-param-names-differ-with-dispatch') do |tmp| + Dir.chdir('test-param-names-differ-with-dispatch') + Puppet::Face[:strings, :current].yardoc end - RUBY - }.to output("#{expected_output_not_a_param}\n#{expected_output_also_not_a_param}\n").to_stdout_from_any_process + }.to output(/documented/).to_stdout_from_any_process + }.to output("#{expected_output_not_a_param}\n#{expected_output_also_not_a_param}\n").to_stderr_from_any_process end it "should not issue a warning if the parameter names do match the " + "docstring in dispatch method" do + expected = "" expect { - parse <<-RUBY - # @param [Integer] num_a the first number to be compared - # @param num_b [Integer] the second number to be compared - Puppet::Functions.create_function(:max) do - dispatch max_1 do - param 'Integer', :num_a - param 'Integer', :num_b - end - - def max_1(num_a, num_b) - num_a >= num_b ? num_a : num_b - end + expect { + PuppetModuleHelper.using_module(File.dirname(__FILE__),'test-param-names-match-with-dispatch') do |tmp| + Dir.chdir('test-param-names-match-with-dispatch') + Puppet::Face[:strings, :current].yardoc end - RUBY - }.to output("").to_stdout_from_any_process + }.to output(/documented/).to_stdout_from_any_process + }.to output(expected).to_stderr_from_any_process end end diff --git a/spec/unit/puppet_x/puppetlabs/strings/yard/template_helper_spec.rb b/spec/unit/puppet_x/puppetlabs/strings/yard/template_helper_spec.rb index db02849..ed7e68d 100644 --- a/spec/unit/puppet_x/puppetlabs/strings/yard/template_helper_spec.rb +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/template_helper_spec.rb @@ -57,7 +57,7 @@ describe TemplateHelper do end it "should issue a warning if the parameter types do not match the docstring in dispatch method" do - expected_output_not_a_param = "[warn]: @param tag types do not match the" + + expected_output_not_a_param = "@param tag types do not match the" + " code. The arg1 parameter is declared as types [\"Integer\"] in the " + "docstring, but the code specifies the types [\"Optional[String]\"] " + "in file test near line 0\n" @@ -78,7 +78,7 @@ describe TemplateHelper do template_helper = TemplateHelper.new expect { template_helper.check_types_match_docs(object, param_details) - }.to output(expected_output_not_a_param).to_stdout_from_any_process + }.to output(expected_output_not_a_param).to_stderr_from_any_process end it "should not issue a warning if the parameter types do match the docstring in dispatch method" do @@ -99,7 +99,7 @@ describe TemplateHelper do template_helper = TemplateHelper.new expect { template_helper.check_types_match_docs(object, param_details) - }.to output("").to_stdout_from_any_process + }.to output("").to_stderr_from_any_process end it "should not issue a warning if the types in the docstring in dispatch method are assignable to parameter types" do @@ -120,6 +120,6 @@ describe TemplateHelper do template_helper = TemplateHelper.new expect { template_helper.check_types_match_docs(object, param_details) - }.to output("").to_stdout_from_any_process + }.to output("").to_stderr_from_any_process end end