[PATCH/puppet 1/1] Fix 2841 - Puppetdoc/RDoc parses realize function

[Brice Figureau]

Puppetdoc wasn't parsing the realize function.
This patch let puppetdoc find realize and display in RDoc html
mode the list of realized resource per class or node.

Signed-off-by: Brice Figureau <brice-...@daysofwonder.com>
---
lib/puppet/util/rdoc/code_objects.rb | 7 ++-
.../util/rdoc/generators/puppet_generator.rb | 19 +++++++-
.../util/rdoc/generators/template/puppet/puppet.rb | 14 +++++-
lib/puppet/util/rdoc/parser.rb | 50 ++++++++++++++------
spec/unit/util/rdoc/parser.rb | 30 ++++++++++++
5 files changed, 102 insertions(+), 18 deletions(-)

diff --git a/lib/puppet/util/rdoc/code_objects.rb b/lib/puppet/util/rdoc/code_objects.rb
index be5e468..9958699 100644
--- a/lib/puppet/util/rdoc/code_objects.rb
+++ b/lib/puppet/util/rdoc/code_objects.rb
@@ -88,12 +88,13 @@ module RDoc
# It is mapped to a HTMLPuppetClass for display
# It leverages RDoc (ruby) Class
class PuppetClass < ClassModule
- attr_accessor :resource_list, :requires, :childs
+ attr_accessor :resource_list, :requires, :childs, :realizes

def initialize(name, superclass)
super(name,superclass)
@resource_list = []
@requires = []
+ @realizes = []
@childs = []
end

@@ -116,6 +117,10 @@ module RDoc
add_to(@requires, required)
end

+ def add_realize(realized)
+ add_to(@realizes, realized)
+ end
+
def add_child(child)
@childs << child
end
diff --git a/lib/puppet/util/rdoc/generators/puppet_generator.rb b/lib/puppet/util/rdoc/generators/puppet_generator.rb
index f06879f..bd64cc3 100644
--- a/lib/puppet/util/rdoc/generators/puppet_generator.rb
+++ b/lib/puppet/util/rdoc/generators/puppet_generator.rb
@@ -334,8 +334,9 @@ module Generators
def build_referenced_list(list)
res = []
list.each do |i|
- ref = @context.find_symbol(i.name)
- ref = ref.viewer if ref
+ ref = AllReferences[i.name]
+ ref = @context.find_symbol(i.name) unless ref
+ ref = ref.viewer if ref and ref.respond_to?(:viewer)
name = i.respond_to?(:full_name) ? i.full_name : i.name
h_name = CGI.escapeHTML(name)
if ref and ref.document_self
@@ -409,6 +410,9 @@ module Generators
rl = build_require_list(@context)
@values["requires"] = rl unless rl.empty?

+ rl = build_realize_list(@context)
+ @values["realizes"] = rl unless rl.empty?
+
cl = build_child_list(@context)
@values["childs"] = cl unless cl.empty?

@@ -419,6 +423,10 @@ module Generators
build_referenced_list(context.requires)
end

+ def build_realize_list(context)
+ build_referenced_list(context.realizes)
+ end
+
def build_child_list(context)
build_referenced_list(context.childs)
end
@@ -499,6 +507,9 @@ module Generators
rl = build_require_list(@context)
@values["requires"] = rl unless rl.empty?

+ rl = build_realize_list(@context)
+ @values["realizes"] = rl unless rel.empty?
+
cl = build_child_list(@context)
@values["childs"] = cl unless cl.empty?

@@ -608,6 +619,10 @@ module Generators
build_referenced_list(context.requires)
end

+ def build_realize_list(context)
+ build_referenced_list(context.realizes)
+ end
+
def build_child_list(context)
build_referenced_list(context.childs)
end
diff --git a/lib/puppet/util/rdoc/generators/template/puppet/puppet.rb b/lib/puppet/util/rdoc/generators/template/puppet/puppet.rb
index bae9fc7..e03381f 100644
--- a/lib/puppet/util/rdoc/generators/template/puppet/puppet.rb
+++ b/lib/puppet/util/rdoc/generators/template/puppet/puppet.rb
@@ -615,6 +615,19 @@ END:requires
</div>
ENDIF:requires

+ <!-- if realizes -->
+IF:realizes
+ <div id="realizes">
+ <h3 class="section-bar">Realized Resources</h3>
+
+ <div id="realizes-list">
+START:realizes
+ <span class="realizes-name">HREF:aref:name:</span>
+END:realizes
+ </div>
+ </div>
+ENDIF:realizes
+
START:sections
<div id="section">
IF:sectitle
@@ -626,7 +639,6 @@ IF:seccomment
ENDIF:seccomment
ENDIF:sectitle

-
<!-- if facts -->
IF:facts
<div id="class-list">
diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index 416711d..c60b950 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -161,6 +161,22 @@ class Parser
end
end

+ # create documentation for realize statements we can find in +code+
+ # and associate it with +container+
+ def scan_for_realize(container, code)
+ code = [code] unless code.is_a?(Array)
+ code.each do |stmt|
+ scan_for_realize(container,stmt.children) if stmt.is_a?(Puppet::Parser::AST::ASTArray)
+
+ if stmt.is_a?(Puppet::Parser::AST::Function) and stmt.name == 'realize'
+ stmt.arguments.each do |realized|
+ Puppet.debug "found #{stmt.name}: #{realized}"
+ container.add_realize(Include.new(realized.to_s, stmt.doc))
+ end
+ end
+ end
+ end
+
# create documentation for global variables assignements we can find in +code+
# and associate it with +container+
def scan_for_vardef(container, code)
@@ -184,20 +200,9 @@ class Parser

if stmt.is_a?(Puppet::Parser::AST::Resource) and !stmt.type.nil?
begin
- type = stmt.type.split("::").collect { |s| s.capitalize }.join("::")
- title = stmt.title.is_a?(Puppet::Parser::AST::ASTArray) ? stmt.title.to_s.gsub(/\[(.*)\]/,'\1') : stmt.title.to_s
- Puppet.debug "rdoc: found resource: %s[%s]" % [type,title]
-
- param = []
- stmt.params.children.each do |p|
- res = {}
- res["name"] = p.param
- res["value"] = "#{p.value.to_s}" unless p.value.nil?
-
- param << res
- end
-
- container.add_resource(PuppetResource.new(type, title, stmt.doc, param))
+ ref = resource_stmt_to_ref(stmt)
+ Puppet.debug "rdoc: found resource: %s[%s]" % [ref.type, ref.title]
+ container.add_resource(ref)
rescue => detail
raise Puppet::ParseError, "impossible to parse resource in #{stmt.file} at line #{stmt.line}: #{detail}"
end
@@ -205,6 +210,21 @@ class Parser
end
end

+ def resource_stmt_to_ref(stmt)
+ type = stmt.type.split("::").collect { |s| s.capitalize }.join("::")
+ title = stmt.title.is_a?(Puppet::Parser::AST::ASTArray) ? stmt.title.to_s.gsub(/\[(.*)\]/,'\1') : stmt.title.to_s
+
+ param = []
+ stmt.params.children.each do |p|
+ res = {}
+ res["name"] = p.param
+ res["value"] = "#{p.value.to_s}" unless p.value.nil?
+
+ param << res
+ end
+ PuppetResource.new(type, title, stmt.doc, param)
+ end
+
# create documentation for a class named +name+
def document_class(name, klass, container)
Puppet.debug "rdoc: found new class %s" % name
@@ -228,6 +248,7 @@ class Parser
code ||= klass.code
unless code.nil?
scan_for_include_or_require(cls, code)
+ scan_for_realize(cls, code)
scan_for_resource(cls, code) if Puppet.settings[:document_all]
end

@@ -251,6 +272,7 @@ class Parser
code ||= node.code
unless code.nil?
scan_for_include_or_require(n, code)
+ scan_for_realize(n, code)
scan_for_vardef(n, code)
scan_for_resource(n, code) if Puppet.settings[:document_all]
end
diff --git a/spec/unit/util/rdoc/parser.rb b/spec/unit/util/rdoc/parser.rb
index 85a62e7..593e6ac 100755
--- a/spec/unit/util/rdoc/parser.rb
+++ b/spec/unit/util/rdoc/parser.rb
@@ -372,6 +372,36 @@ describe RDoc::Parser do
end
end

+ describe "when scanning for realized virtual resources" do
+
+ def create_stmt
+ stmt_value = stub "resource_ref", :to_s => "File[\"/tmp/a\"]"
+ stmt = stub_everything 'stmt', :name => "realize", :arguments => [stmt_value], :doc => "mydoc"
+ stmt.stubs(:is_a?).with(Puppet::Parser::AST::ASTArray).returns(false)
+ stmt.stubs(:is_a?).with(Puppet::Parser::AST::Function).returns(true)
+ stmt
+ end
+
+ before(:each) do
+ @class = stub_everything 'class'
+ @code = stub_everything 'code'
+ @code.stubs(:is_a?).with(Puppet::Parser::AST::ASTArray).returns(true)
+ end
+
+ it "should also scan mono-instruction code" do
+ @class.expects(:add_realize).with { |i| i.is_a?(RDoc::Include) and i.name == "File[\"/tmp/a\"]" and i.comment == "mydoc" }
+
+ @parser.scan_for_realize(@class,create_stmt())
+ end
+
+ it "should register recursively includes to the current container" do
+ @code.stubs(:children).returns([ create_stmt() ])
+
+ @class.expects(:add_realize).with { |i| i.is_a?(RDoc::Include) and i.name == "File[\"/tmp/a\"]" and i.comment == "mydoc" }
+ @parser.scan_for_realize(@class, [@code])
+ end
+ end
+
describe "when scanning for variable definition" do
before :each do
@class = stub_everything 'class'
--
1.6.5.2

[Markus Roberts]

+                ref = AllReferences[i.name]
+                ref = @context.find_symbol(i.name) unless ref
+                ref = ref.viewer if ref and ref.respond_to?(:viewer)

Not worth worrying about, but just to note it, this could be simplified to:

                ref = AllReferences[i.name] || @context.find_symbol(i.name)
                ref = ref.viewer if ref.respond_to?(:viewer)

 

+            rl = build_realize_list(@context)
+            @values["realizes"] = rl unless rel.empty?
+

Is this supposed to be ("rl" vs. "rel")?:

            @values["realizes"] = rl unless rl.empty?
 

+    def resource_stmt_to_ref(stmt)

+        type = stmt.type.split("::").collect { |s| s.capitalize }.join("::")
+        title = stmt.title.is_a?(Puppet::Parser::AST::ASTArray) ? stmt.title.to_s.gsub(/\[(.*)\]/,'\1') : stmt.title.to_s
+
+        param = []
+        stmt.params.children.each do |p|
+            res = {}
+            res["name"] = p.param
+            res["value"] = "#{p.value.to_s}" unless p.value.nil?
+
+            param << res
+        end

Again, just to note it, this could be written:

        param = stmt.params.children.collect do |p|
            res = {"name" => p.param}
            res["value"] = p.value.to_s unless p.value.nil?
            res
        end

or possibly even:

        param = stmt.params.children.collect do |p|
            {"name" => p.param, "value" => p.value.to_s}
        end

 depending on the reason for skipping value if nil.

-- Markus

[Brice Figureau]

On Tue, 2009-11-24 at 16:14 -0800, Markus Roberts wrote:
>
> + ref = AllReferences[i.name]
> + ref = @context.find_symbol(i.name) unless ref
> + ref = ref.viewer if ref and
> ref.respond_to?(:viewer)
>
> Not worth worrying about, but just to note it, this could be
> simplified to:
>
> ref = AllReferences[i.name] ||
> @context.find_symbol(i.name)
> ref = ref.viewer if ref.respond_to?(:viewer)

Correct, that's better.

>
> + rl = build_realize_list(@context)
> + @values["realizes"] = rl unless rel.empty?
> +
>
> Is this supposed to be ("rl" vs. "rel")?:

Yes, I fixed it after sending the patch (it wasn't worth resending).

I'm not sure, I'll have a look to the use of this hash (this is code
modeled from what RDoc does for function argument, hence its, hmm,
quality).
Ruby question: does nil.to_s raises an exception or produces ""?
(I know I can check by myself, but I feel lazy this morning)

--
Brice Figureau
Follow the latest Puppet Community evolutions on www.planetpuppet.org!

[Markus Roberts]

> Ruby question: does nil.to_s raises an exception or produces ""?

(I know I can check by myself, but I feel lazy this morning)

You can call to_s on anything.  nil.to_s --> ""

[Brice Figureau]

Thanks, I modified the code based on your comments.

Do you +1 the patch?
--
Brice Figureau
My Blog: http://www.masterzen.fr/

[Markus Roberts]

> Do you +1 the patch?

Yes, I do +1 that patch.