diff --git a/lib/puppet/face/strings.rb b/lib/puppet/face/strings.rb index e0221a5..26953bb 100644 --- a/lib/puppet/face/strings.rb +++ b/lib/puppet/face/strings.rb @@ -51,6 +51,11 @@ Puppet::Face.define(:strings, '0.0.1') do # and ruby files to parse. yard_args = (args.empty? ? MODULE_SOURCEFILES : args) + # This line monkeypatches yard's progress indicator so it doesn't write + # all over the terminal. This should definitely not be in real code, but + # it's very handy for debugging with pry + #class YARD::Logger; def progress(*args); end; end + yardoc_actions.generate_documentation(*yard_args) end end diff --git a/lib/puppet_x/puppetlabs/strings/yard/handlers/puppet_4x_function_handler.rb b/lib/puppet_x/puppetlabs/strings/yard/handlers/puppet_4x_function_handler.rb index 29b7b22..27d6ebf 100644 --- a/lib/puppet_x/puppetlabs/strings/yard/handlers/puppet_4x_function_handler.rb +++ b/lib/puppet_x/puppetlabs/strings/yard/handlers/puppet_4x_function_handler.rb @@ -26,7 +26,30 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base process do name = process_parameters - obj = MethodObject.new(function_namespace, name) + method_arguments = [] + + # To attach the method parameters to the new code object, traverse the + # ruby AST until a node is found which defines a list of parameters. + # Then, traverse the children of the parameters, storing each identifier + # in the list of method arguments. + obj = MethodObject.new(function_namespace, name) do |o| + statement.traverse do |node| + if node.type == :params + node.traverse do |params| + if params.type == :ident + # FIXME: Replace nil with parameter types + method_arguments << [params[0], nil] + end + end + + # Now that the parameters have been found, break out of the traversal + break + end + end + end + + obj.parameters = method_arguments + obj['puppet_4x_function'] = true register obj @@ -78,7 +101,7 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base # # @return [(String, Hash{String => String})] def process_parameters - # Passing `false` to prameters excludes the block param from the returned + # Passing `false` to parameters excludes the block param from the returned # list. name, _ = statement.parameters(false).compact diff --git a/lib/puppet_x/puppetlabs/strings/yard/templates/default/definedtype/setup.rb b/lib/puppet_x/puppetlabs/strings/yard/templates/default/definedtype/setup.rb index 39ae076..b11f1fa 100644 --- a/lib/puppet_x/puppetlabs/strings/yard/templates/default/definedtype/setup.rb +++ b/lib/puppet_x/puppetlabs/strings/yard/templates/default/definedtype/setup.rb @@ -19,6 +19,7 @@ def parameter_details @param_details = [] @param_details = @template_helper.extract_param_details(params, param_tags, true) + @template_helper.check_parameters_match_docs object erb(:parameter_details) end diff --git a/lib/puppet_x/puppetlabs/strings/yard/templates/default/hostclass/setup.rb b/lib/puppet_x/puppetlabs/strings/yard/templates/default/hostclass/setup.rb index 081d237..f640d5b 100644 --- a/lib/puppet_x/puppetlabs/strings/yard/templates/default/hostclass/setup.rb +++ b/lib/puppet_x/puppetlabs/strings/yard/templates/default/hostclass/setup.rb @@ -3,6 +3,9 @@ include T('default/definedtype') def init super sections.push :subclasses + + @template_helper = TemplateHelper.new + @template_helper.check_parameters_match_docs object end def subclasses @@ -18,3 +21,4 @@ def subclasses return if @subclasses.nil? || @subclasses.empty? erb(:subclasses) end + 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 40cf94e..64b236c 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 @@ -1,7 +1,8 @@ +require "puppet" + # A class containing helper methods to aid in the extraction of relevant data # from comments and YARD tags class TemplateHelper - # Extracts data from comments which include the supported YARD tags def extract_tag_data(object) examples = Hash.new @@ -38,10 +39,9 @@ class TemplateHelper def extract_param_details(parameters, tags_hash, fq_name = false) parameter_info = [] - # Extract the information for parameters that actually exist + # Extract the information for parameters that exist # as opposed to parameters that are defined only in the comments parameters.each do |param| - if fq_name param_name = param[0] fully_qualified_name = param[1] @@ -104,4 +104,26 @@ class TemplateHelper parameter_info end + + # Check that the actual function parameters match what is stated in the docs. + # If there is a mismatch, print a warning to stderr. + # This is necessary for puppet classes and defined types. This type of + # warning will be issued for ruby code by the ruby docstring parser. + # @param object the code object to examine for parameters names + def check_parameters_match_docs(object) + param_tags = object.tags.find_all{ |tag| tag.tag_name == "param"} + names = object.parameters.map {|l| l.first.gsub(/\W/, '') } + locations = object.files + param_tags.each do |tag| + if not names.include?(tag.name) + if locations.length >= 1 and locations[0].length == 2 + file_name = locations[0][0] + line_number = locations[0][1] + $stderr.puts "[warn]: The parameter #{tag.name} is documented, but doesn't exist in your code, in file #{file_name} near line #{line_number}" + else + $stderr.puts "[warn]: The parameter #{tag.name} is documented, but doesn't exist in your code. Sorry, the file and line number could not be determined." + end + end + end + end end diff --git a/spec/unit/puppet/face_spec.rb b/spec/unit/puppet/face_spec.rb index 03aad0d..5119c41 100644 --- a/spec/unit/puppet/face_spec.rb +++ b/spec/unit/puppet/face_spec.rb @@ -31,9 +31,25 @@ describe Puppet::Face do end describe "when generating HTML for documentation" do + + # HACK: In these tests we would like to suppress all output from the yard + # logger so we don't clutter up stdout. + # However, we do want the yard logger for other tests so we can + # assert that the right things are logged. To accomplish this, for + # this block of tests we monkeypatch the yard logger to be a generic + # stringio instance which does nothing and then we restore the + # original afterwards. + before(:all) do + @tmp = YARD::Logger.instance.io + YARD::Logger.instance.io = StringIO.new + end + + after(:all) do + YARD::Logger.instance.io = @tmp + end + it "should properly generate HTML for manifest comments" do - YARD::Logger.instance.io = StringIO.new using_module('test') do |tmp| Dir.chdir('test') 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 14720e3..c59821c 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 @@ -54,12 +54,21 @@ describe PuppetX::PuppetLabs::Strings::YARD::Handlers::Puppet4xFunctionHandler d expect(the_namespace).to document_a(:type => :puppetnamespace) end - it "should not add anything to the Registry if incorrect ruby code is present" do - parse <<-RUBY - # The summary - This is not ruby code - RUBY - - expect(YARD::Registry.all).to be_empty + 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" + 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 + end + RUBY + }.to output("#{expected_output_not_a_param}\n#{expected_output_also_not_a_param}\n").to_stdout_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 new file mode 100644 index 0000000..1f3a4b4 --- /dev/null +++ b/spec/unit/puppet_x/puppetlabs/strings/yard/template_helper_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require 'puppet_x/puppetlabs/strings/yard/templates/default/template_helper' +require 'puppet_x/puppetlabs/strings/yard/code_objects/puppet_namespace_object' +require 'strings_spec/parsing' + +describe TemplateHelper do + it "should not print any warning if the tags and parameters match" do + th = TemplateHelper.new + + # Case 0: If the documented tags do include the parameter, + # nothing is printed + tag0 = YARD::Tags::Tag.new(:param, 'a_parameter') + tag0.name = 'a_parameter' + obj0 = PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject.new(:root, 'Puppet3xFunctions') + obj0.add_tag tag0 + obj0.parameters = [['a_parameter']] + expect { th.check_parameters_match_docs obj0 }.to output("").to_stderr_from_any_process + + # The docstring is still alive between tests. Clear the tags registered with + # it so the tags won't persist between tests. + obj0.docstring.instance_variable_set("@tags", []) + end + + it "should print the warning with no location data if the tags and " + + "parameters differ and the location data is not properly formed" do + th = TemplateHelper.new + # Case 1: If the parameter and tag differ and the location is not properly + # formed, print out the warning with no location data + tag1 = YARD::Tags::Tag.new(:param, 'aa_parameter') + tag1.name = 'aa_parameter' + obj1 = PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject.new(:root, 'Puppet3xFunctions') + obj1.add_tag tag1 + obj1.parameters = [['b_parameter']] + expect { th.check_parameters_match_docs obj1 }.to output("[warn]: The parameter aa_parameter is documented, but doesn't exist in your code. Sorry, the file and line number could not be determined.\n").to_stderr_from_any_process + + # The docstring is still alive between tests. Clear the tags registered with + # it so the tags won't persist between tests. + obj1.docstring.instance_variable_set("@tags", []) + end + + it "should print the warning with location data if the tags and parameters " + + "differ and the location data is properly formed" do + th = TemplateHelper.new + # Case 2: If the parameter and tag differ and the location is properly + # formed, print out the warning with no location data + tag2 = YARD::Tags::Tag.new(:param, 'aaa_parameter') + tag2.name = 'aaa_parameter' + obj2 = PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject.new(:root, 'Puppet3xFunctions') + obj2.files = [['some_file.pp', 42]] + obj2.add_tag tag2 + obj2.parameters = [['b_parameter']] + expect { th.check_parameters_match_docs obj2 }.to output("[warn]: The parameter aaa_parameter is documented, but doesn't exist in your code, in file some_file.pp near line 42\n").to_stderr_from_any_process + + # The docstring is still alive between tests. Clear the tags registered with + # it so the tags won't persist between tests. + obj2.docstring.instance_variable_set("@tags", []) + end + +end