Warn if parameters that don't exist are documented

Fixes voxpupuli#18

lib/puppet-lint/plugins/check_parameter_documentation.rb

@@ -1,8 +1,6 @@
 PuppetLint.new_check(:parameter_documentation) do
   def check
     class_indexes.concat(defined_type_indexes).each do |idx|
-      next if idx[:param_tokens].nil?
-
       doc_params = {}
       doc_params_duplicates = Hash.new { |hash, key| hash[key] = [doc_params[key]] }
       is_private = false
@@ -25,51 +23,75 @@ def check
         end
       end
 
-      next if is_private
-
-      params = []
-      e = idx[:param_tokens].each
-      begin
-        while (ptok = e.next)
-          if ptok.type == :VARIABLE
-            params << ptok
-            nesting = 0
-            # skip to the next parameter to avoid finding default values of variables
-            while true
-              ptok = e.next
-              case ptok.type
-              when :LPAREN
-                nesting += 1
-              when :RPAREN
-                nesting -= 1
-              when :COMMA
-                break unless nesting > 0
-              end
-            end
-          end
-        end
-      rescue StopIteration; end
+      params = extract_params(idx)
 
       # warn about duplicates
       doc_params_duplicates.each do |parameter, tokens|
         tokens.each do |token|
            notify :warning, {
-             :message => "Duplicate parameter documentation for #{parameter}",
+             :message => "Duplicate #{type_str(idx)} parameter documentation for #{parameter}",
              :line    => token.line,
              :column  => token.column + token.value.match(/\A\s*(@param\s*)?/)[0].length + 1 # `+ 1` is to account for length of the `#` COMMENT token.
            }
         end
       end
 
-      params.each do |p|
-        next if doc_params.has_key? p.value
-        idx_type = idx[:type] == :CLASS ? "class" : "defined type"
+      # warn about documentation for parameters that don't exist
+      doc_params.each do |parameter, token|
+        next if params.find { |p| p.value == parameter }
+
         notify :warning, {
-          :message => "missing documentation for #{idx_type} parameter #{idx[:name_token].value}::#{p.value}",
-          :line    => p.line,
-          :column  => p.column,
+          :message => "No matching #{type_str(idx)} parameter for documentation of #{parameter}",
+          :line    => token.line,
+          :column  => token.column + token.value.match(/\A\s*(@param\s*)?/)[0].length + 1
         }
       end
+
+      unless is_private
+        params.each do |p|
+          next if doc_params.has_key? p.value
+          notify :warning, {
+            :message => "missing documentation for #{type_str(idx)} parameter #{idx[:name_token].value}::#{p.value}",
+            :line    => p.line,
+            :column  => p.column,
+          }
+        end
+      end
     end
   end
+
+  private
+
+  def type_str(idx)
+    idx[:type] == :CLASS ? "class" : "defined type"
+  end
+
+  def extract_params(idx)
+    params = []
+    return params if idx[:param_tokens].nil?
+
+    e = idx[:param_tokens].each
+    begin
+      while (ptok = e.next)
+        if ptok.type == :VARIABLE
+          params << ptok
+          nesting = 0
+          # skip to the next parameter to avoid finding default values of variables
+          while true
+            ptok = e.next
+            case ptok.type
+            when :LPAREN
+              nesting += 1
+            when :RPAREN
+              nesting -= 1
+            when :COMMA
+              break unless nesting > 0
+            end
+          end
+        end
+      end
+    rescue StopIteration; end
+
+    params
+  end
 end

spec/puppet-lint/plugins/check_parameter_documentation_spec.rb

@@ -564,11 +564,11 @@ class example($foo, $bar) { }
     end
 
     it 'should create a warning on line 3' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(3).in_column(10)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(3).in_column(10)
     end
 
     it 'should create a warning on line 6' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(6).in_column(10)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(6).in_column(10)
     end
   end
 
@@ -594,15 +594,41 @@ class example($foo, $bar) { }
     end
 
     it 'should create a warning on line 3' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(3).in_column(10)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(3).in_column(10)
     end
 
     it 'should create a warning on line 6' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(6).in_column(10)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(6).in_column(10)
     end
 
     it 'should create a warning on line 8' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(8).in_column(10)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(8).in_column(10)
+    end
+  end
+
+  context 'private class with parameters documented twice (@param)' do
+    let(:code) do
+      <<-EOS.gsub(/^\s+/, '')
+      # @summary Example class
+      #
+      # @param bar docs
+      # @param bar Duplicate/conflicting docs
+      #
+      # @api private
+      class example($bar) { }
+    EOS
+    end
+
+    it 'should detect two problems' do
+      expect(problems).to have(2).problem
+    end
+
+    it 'should create a warning on line 3' do
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(3).in_column(10)
+    end
+
+    it 'should create a warning on line 4' do
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(4).in_column(10)
     end
   end
 
@@ -625,11 +651,75 @@ class example($foo, $bar) { }
     end
 
     it 'should create a warning on line 3' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(3).in_column(10)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(3).in_column(10)
     end
 
     it 'should create a warning on line 6' do
-      expect(problems).to contain_warning('Duplicate parameter documentation for bar').on_line(6).in_column(3)
+      expect(problems).to contain_warning('Duplicate class parameter documentation for bar').on_line(6).in_column(3)
+    end
+  end
+
+  context 'class with documentation for parameters that don\'t exist' do
+    let(:code) do
+      <<-EOS.gsub(/^\s+/, '')
+      # @summary Example class
+      #
+      # @param foo
+      class example { }
+    EOS
+    end
+
+    it 'should detect a single problem' do
+      expect(problems).to have(1).problem
+    end
+
+    it 'should create a warning on line 3' do
+      expect(problems).to contain_warning('No matching class parameter for documentation of foo').on_line(3).in_column(10)
+    end
+  end
+
+  context 'private class with documentation for parameters that don\'t exist' do
+    let(:code) do
+      <<-EOS.gsub(/^\s+/, '')
+      # @summary Example class
+      #
+      # @param foo
+      #   Example docs
+      #
+      # @api private
+      class example { }
+    EOS
+    end
+
+    it 'should detect a single problem' do
+      expect(problems).to have(1).problem
+    end
+
+    it 'should create a warning on line 3' do
+      expect(problems).to contain_warning('No matching class parameter for documentation of foo').on_line(3).in_column(10)
+    end
+  end
+
+  context 'define with documentation for parameters that don\'t exist' do
+    let(:code) do
+      <<-EOS.gsub(/^\s+/, '')
+      # @summary Example define
+      #
+      # @param bar Docs for bar
+      # @param foo
+      #   Docs for foo
+      #
+      # @api private
+      define example::example(String[1] $bar) { }
+    EOS
+    end
+
+    it 'should detect a single problem' do
+      expect(problems).to have(1).problem
+    end
+
+    it 'should create a warning on line 4' do
+      expect(problems).to contain_warning('No matching defined type parameter for documentation of foo').on_line(4).in_column(10)
     end
   end
 end