Merge pull request #31 from iankronquist/warn-when-not-matching/PDOC-21

(PDOC-37) Warn when documented name does not match declared name

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

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
 

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

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
+

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

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')

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
+  it "should issue a warning if the parameter names do not match the docstring" do
-    parse <<-RUBY
+      expected_output_not_a_param = "[warn]: @param tag has unknown parameter" +
-        # The summary
+        " name: not_a_param \n    in file `(stdin)' near line 3"
-        This is not ruby code
+      expected_output_also_not_a_param = "[warn]: @param tag has unknown " +
-    RUBY
+        "parameter name: also_not_a_param \n    in file `(stdin)' near line 3"
-
+      expect {
-    expect(YARD::Registry.all).to be_empty
+        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

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