Merge pull request #35 from iankronquist/type-checking/PDOC-19

Type checking/pdoc 21

lib/puppet_x/puppetlabs/strings/yard/code_objects/defined_type_object.rb

@@ -2,4 +2,6 @@ class PuppetX::PuppetLabs::Strings::YARD::CodeObjects::DefinedTypeObject < Puppe
   # A list of parameters attached to this class.
   # @return [Array<Array(String, String)>]
   attr_accessor :parameters
+  attr_accessor :type_info
+
 end

lib/puppet_x/puppetlabs/strings/yard/code_objects/host_class_object.rb

@@ -2,6 +2,8 @@ class PuppetX::PuppetLabs::Strings::YARD::CodeObjects::HostClassObject < PuppetX
   # The {HostClassObject} that this class inherits from, if any.
   # @return [HostClassObject, Proxy, nil]
   attr_accessor :parent_class
+  attr_accessor :type_info
+
 
   # NOTE: `include_mods` is never used as it makes no sense for Puppet, but
   # this is called by `YARD::Registry` and it will pass a parameter.

lib/puppet_x/puppetlabs/strings/yard/code_objects/puppet_namespace_object.rb

@@ -5,6 +5,8 @@ class PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject < Y
     [self]
   end
 
+  attr_accessor :type_info
+
   # FIXME: We used to override `self.new` to ensure no YARD proxies were
   # created for namespaces segments that did not map to a host class or
   # defined type. Fighting the system in this way turned out to be

lib/puppet_x/puppetlabs/strings/yard/handlers/host_class_handler.rb

@@ -8,6 +8,23 @@ class PuppetX::PuppetLabs::Strings::YARD::Handlers::HostClassHandler < PuppetX::
         param_tuple << ( a[1].nil? ? nil : a[1].source )
       end
     end
+    tp = Puppet::Pops::Types::TypeParser.new
+    param_type_info = {}
+    statement.pops_obj.parameters.each do |pop_param|
+      # If the parameter's type expression is nil, default to Any
+      if pop_param.type_expr == nil
+        param_type_info[pop_param.name] = Puppet::Pops::Types::TypeFactory.any() 
+      else
+        begin
+          param_type_info[pop_param.name] =  tp.interpret_any(pop_param.type_expr)
+        rescue Puppet::ParseError
+          # If the type could not be interpreted, default to type Any
+          param_type_info[pop_param.name] = "TypeError - " +
+            "#{pop_param.type_expr} isn't a valid type."
+        end
+      end
+    end
+    obj.type_info = [param_type_info]
 
     statement.pops_obj.tap do |o|
       if o.parent_class

lib/puppet_x/puppetlabs/strings/yard/handlers/puppet_4x_function_handler.rb

@@ -52,19 +52,24 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base
     obj = MethodObject.new(function_namespace, name) do |o|
     end
 
-    # overload_signatures is a array of arrays of tuples or empty:
+    # The data structure for overload_signatures is an array of hashes. Each
+    # hash represents the arguments a single function dispatch (aka overload)
+    # can take.
     # overload_signatures = [
-    #   [ # First function dispatch arguments
+    #   { # First function dispatch arguments
     #     # argument name, argument type
-    #     ['arg0', 'Variant[String,Array[String]]'
-    #     ['arg1', 'Optional[Type]']
-    #   ],
-    #   [ # Second function dispatch arguments
-    #     ['arg0', 'Variant[String,Array[String]]'
-    #     ['arg1', 'Optional[Type]']
-    #     ['arg2', 'Any']
-    #   ]
+    #     'arg0': 'Variant[String,Array[String]]',
+    #     'arg1': 'Optional[Type]'
+    #   },
+    #   { # Second function dispatch arguments
+    #     'arg0': 'Variant[String,Array[String]]',
+    #     'arg1': 'Optional[Type]',
+    #     'arg2': 'Any'
+    #   }
     # ]
+    # Note that the order for arguments to a function doesn't actually matter
+    # because we allow users flexibility when listing their arguments in the
+    # comments.
     overload_signatures = []
     statement.traverse do |node|
       # Find all of the dispatch methods
@@ -75,16 +80,23 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base
         if do_block == command
           next
         end
+        signature = {}
         # Iterate through each of the children of the do block and build
         # tuples of parameter names and parameter type signatures
         do_block.children.first.children.each do |child|
-          overload_signatures <<= [extract_type_from_command(child)]
+          name, type = extract_type_from_command(child)
+          # This can happen if there is a function or something we aren't
+          # expecting.
+          if name != nil and type != nil
+            signature[name] = type
           end
         end
+        overload_signatures <<= signature
+      end
     end
 
-    # If the overload_signatures array is empty because we couldn't find any
-    # dispatch blocks, then there must be one method named the same as the
+    # If the overload_signatures list is empty because we couldn't find any
+    # dispatch blocks, then there must be one function named the same as the
     # name of the function being created.
     if overload_signatures.length == 0
       statement.traverse do |node|
@@ -92,7 +104,7 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base
         # function being created.
         if (node.type == :def and node.children.first.type == :ident and
             node.children.first.source == obj.name.to_s)
-          signature = []
+          signature = {}
           # Find its parameters. If they don't exist, fine
           params = node.jump :params
           break if params == node
@@ -100,7 +112,7 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base
             if param.type == :ident
               # The parameters of Puppet functions with no defined dispatch are
               # as though they are Any type.
-              signature << [param[0], 'Any']
+              signature[param[0]] =  'Any'
             end
           end
           overload_signatures <<= signature
@@ -114,9 +126,9 @@ class Puppet4xFunctionHandler < YARD::Handlers::Ruby::Base
     # at the docstring.
     obj.type_info = overload_signatures
 
-    # The yard docstring parser expects a array of arrays, not a array of
-    # arrays of arrays.
-    obj.parameters = overload_signatures.flatten(1)
+    # The yard docstring parser expects a list of lists, not a list of lists of
+    # lists.
+    obj.parameters = overload_signatures.map { |sig| sig.to_a }.flatten(1)
 
     obj['puppet_4x_function'] = true
 

lib/puppet_x/puppetlabs/strings/yard/templates/default/hostclass/setup.rb

@@ -6,6 +6,10 @@ def init
 
   @template_helper = TemplateHelper.new
   @template_helper.check_parameters_match_docs object
+  params = object.parameters.map { |param| param.first }
+  param_tags = object.tags.find_all{ |tag| tag.tag_name == "param"}
+  param_details = @template_helper.extract_param_details(params, param_tags) unless params.nil?
+  @template_helper.check_types_match_docs object, param_details
 end
 
 def subclasses

lib/puppet_x/puppetlabs/strings/yard/templates/default/puppetnamespace/setup.rb

@@ -76,6 +76,7 @@ def method_details_list
       params = parameters.nil? ? nil :  parameters[1].split(/\s*,\s*/)
 
       param_details = @template_helper.extract_param_details(params, param_tags) unless params.nil?
+      @template_helper.check_types_match_docs object, param_details
     else
       param_details = @template_helper.comment_only_param_details(param_tags)
     end

lib/puppet_x/puppetlabs/strings/yard/templates/default/template_helper.rb

@@ -105,6 +105,63 @@ class TemplateHelper
     parameter_info
   end
 
+  # Check that any types specified in the docstrings match the actual method
+  # types. This is used by puppet 4x functions and defined types.
+  # @param object the code object to examine for parameters names
+  def check_types_match_docs(object, params_hash)
+    # We'll need this to extract type info from the type specified by the
+    # docstring.
+    type_parser = Puppet::Pops::Types::TypeParser.new
+    type_calculator = Puppet::Pops::Types::TypeCalculator.new
+
+    object.type_info.each do |function|
+      function.keys.each do |key|
+        if function[key].class == String
+          begin
+            instantiated = type_parser.parse function[key].gsub(/'/, '').gsub(/"/, "")
+          rescue Puppet::ParseError
+            # Likely the result of a malformed type
+            next
+          end
+        else
+          instantiated = function[key]
+        end
+        params_hash.each do |param|
+          if param[:name] == key and param[:types] != nil
+            param[:types].each do |type|
+              param_instantiated = type_parser.parse type
+              if not type_calculator.assignable? instantiated, param_instantiated
+                actual_types = object.type_info.map do |sig|
+                  sig[key]
+                end
+                # Get the locations where the object can be found. We only care about
+                # the first one.
+                locations = object.files
+                # If the locations aren't in the shape we expect then report that
+                # the file number couldn't be determined.
+                if locations.length >= 1 and locations[0].length == 2
+                  file = locations[0][0]
+                  line = locations[0][1]
+                  warning = "@param tag types do not match the code. The " +
+                    "#{param[:name]} parameter is declared as types #{param[:types]} in " +
+                    "the docstring, but the code specifies the types " +
+                    "#{actual_types.inspect} in file #{file} near line #{line}"
+                else
+                  warning = "@param tag types do not match the code. The " +
+                    "#{param[:name]} parameter is declared as types #{param[:types]} in " +
+                    "the docstring, but the code specifies the types " +
+                    "#{actual_types.inspect} Sorry, the file and line number could" +
+                    "not be determined."
+                end
+                log.warn warning
+              end
+            end
+          end
+        end
+      end
+    end
+  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

spec/unit/puppet_x/puppetlabs/strings/yard/puppet_4x_function_handler_spec.rb

@@ -129,8 +129,8 @@ describe PuppetX::PuppetLabs::Strings::YARD::Handlers::Puppet4xFunctionHandler d
         "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
+          # @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
@@ -158,8 +158,8 @@ describe PuppetX::PuppetLabs::Strings::YARD::Handlers::Puppet4xFunctionHandler d
         "docstring in dispatch method" do
       expect {
         parse <<-RUBY
-          # @param num_a Integer the first number to be compared
-          # @param num_b Integer the second number to be compared
+          # @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

spec/unit/puppet_x/puppetlabs/strings/yard/template_helper_spec.rb

@@ -56,4 +56,70 @@ describe TemplateHelper do
     obj2.docstring.instance_variable_set("@tags", [])
   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" +
+      " 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"
+    object = PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject.new(:root, 'Puppet4xFunctions')
+    object.files = [['test', 0]]
+    object.type_info = [{
+      'arg0' => 'Variant[String,Array[String]]',
+      'arg1' => 'Optional[String]'
+    }]
+    param_details = [{
+      :name => 'arg0',
+      :types => ['Variant[String,Array[String]]']
+    },
+    {
+      :name => 'arg1',
+      :types => ['Integer']
+    }]
+    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
+  end
+
+  it "should not issue a warning if the parameter types do match the docstring in dispatch method" do
+    object = PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject.new(:root, 'Puppet4xFunctions')
+    object.files = [['test', 0]]
+    object.type_info = [{
+      'arg0' => 'Variant[String,Array[String]]',
+      'arg1' => 'Optional[String]'
+    }]
+    param_details = [{
+      :name => 'arg0',
+      :types => ['Variant[String,Array[String]]']
+    },
+    {
+      :name => 'arg1',
+      :types => ['Optional[String]']
+    }]
+    template_helper = TemplateHelper.new
+    expect {
+      template_helper.check_types_match_docs(object, param_details)
+    }.to output("").to_stdout_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
+    object = PuppetX::PuppetLabs::Strings::YARD::CodeObjects::PuppetNamespaceObject.new(:root, 'Puppet4xFunctions')
+    object.files = [['test', 0]]
+    object.type_info = [{
+      'arg0' => 'Variant[String,Array[String]]',
+      'arg1' => 'Optional[String]'
+    }]
+    param_details = [{
+      :name => 'arg0',
+      :types => ['Variant[String,Array[String]]']
+    },
+    {
+      :name => 'arg1',
+      :types => ['String']
+    }]
+    template_helper = TemplateHelper.new
+    expect {
+      template_helper.check_types_match_docs(object, param_details)
+    }.to output("").to_stdout_from_any_process
+  end
 end