Fix #2422 & #2433 - make sure puppetdoc transform AST::Leaf boolean correctly

AST nodes don't have a valid to_s that is producing a correct
representation of said node.
This patch adds some of the AST node to_s to produce correct
values that can be used verbatim by puppetdoc to render
the documentation.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

9 files changed, 84 insertions, 25 deletions

diff --git a/lib/puppet/parser/ast/astarray.rb b/lib/puppet/parser/ast/astarray.rb
index 9b59d196a..dfd2bcd09 100644
--- a/lib/puppet/parser/ast/astarray.rb
+++ b/lib/puppet/parser/ast/astarray.rb
@@ -47,6 +47,10 @@ class Puppet::Parser::AST
 
             return self
         end
+
+        def to_s
+            "[" + @children.collect { |c| c.to_s }.join(', ') + "]"
+        end
     end
 
     # A simple container class, containing the parameters for an object.
diff --git a/lib/puppet/parser/ast/function.rb b/lib/puppet/parser/ast/function.rb
index 2f768ebdb..4c3a5dc05 100644
--- a/lib/puppet/parser/ast/function.rb
+++ b/lib/puppet/parser/ast/function.rb
@@ -51,5 +51,10 @@ class Puppet::Parser::AST
              @fname = Puppet::Parser::Functions.function(@name)
             # Lastly, check the parity
         end
+
+        def to_s
+            args = arguments.is_a?(ASTArray) ? arguments.to_s.gsub(/\[(.*)\]/,'\1') : arguments
+            "#{name}(#{args})"
+        end
     end
 end
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index dba9812a7..d08938011 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -11,7 +11,7 @@ class Puppet::Parser::AST
         end
 
         def to_s
-            return @value
+            return @value.to_s unless @value.nil?
         end
     end
 
@@ -29,6 +29,10 @@ class Puppet::Parser::AST
             end
             @value
         end
+
+        def to_s
+            @value ? "true" : "false"
+        end
     end
 
     # The base string class.
@@ -38,6 +42,10 @@ class Puppet::Parser::AST
         def evaluate(scope)
             return scope.strinterp(@value, file, line)
         end
+
+        def to_s
+            "\"#{@value}\""
+        end
     end
 
     # An uninterpreted string.
@@ -45,6 +53,10 @@ class Puppet::Parser::AST
         def evaluate(scope)
             return @value
         end
+
+        def to_s
+            "\"#{@value}\""
+        end
     end
 
     # The 'default' option on case statements and selectors.
diff --git a/lib/puppet/parser/ast/resource_reference.rb b/lib/puppet/parser/ast/resource_reference.rb
index 6189ab8de..117bc88af 100644
--- a/lib/puppet/parser/ast/resource_reference.rb
+++ b/lib/puppet/parser/ast/resource_reference.rb
@@ -64,5 +64,13 @@ class Puppet::Parser::AST
             end
             return objtype
         end
+
+        def to_s
+            if title.is_a?(ASTArray)
+                "#{type.to_s.capitalize}#{title}"
+            else
+                "#{type.to_s.capitalize}[#{title}]"
+            end
+        end
     end
 end
diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index 7954865f8..324b6b771 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -155,8 +155,8 @@ class Parser
             scan_for_vardef(container,stmt.children) if stmt.is_a?(Puppet::Parser::AST::ASTArray)
 
             if stmt.is_a?(Puppet::Parser::AST::VarDef)
-                Puppet.debug "rdoc: found constant: %s = %s" % [stmt.name.to_s, value_to_s(stmt.value)]
-                container.add_constant(Constant.new(stmt.name.to_s, value_to_s(stmt.value), stmt.doc))
+                Puppet.debug "rdoc: found constant: %s = %s" % [stmt.name.to_s, stmt.value.to_s]
+                container.add_constant(Constant.new(stmt.name.to_s, stmt.value.to_s, stmt.doc))
             end
         end
     end
@@ -169,20 +169,15 @@ class Parser
 
             if stmt.is_a?(Puppet::Parser::AST::Resource) and !stmt.type.nil?
                 type = stmt.type.split("::").collect { |s| s.capitalize }.join("::")
-                title = value_to_s(stmt.title)
+                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
-                    if !p.value.nil?
-                        if !p.value.is_a?(Puppet::Parser::AST::ASTArray)
-                            res["value"] = "'#{p.value}'"
-                        else
-                            res["value"] = "[%s]" % p.value.children.collect { |v| "'#{v}'" }.join(", ")
-                        end
-                    end
+                    res["value"] = "#{p.value.to_s}" unless p.value.nil?
+
                     param << res
                 end
 
@@ -420,19 +415,5 @@ class Parser
         comment.gsub!(/^#--.*?^#\+\+/m, '')
         comment.sub!(/^#--.*/m, '')
     end
-
-    # convert an AST value to a string
-    def value_to_s(value)
-        value = value.children if value.is_a?(Puppet::Parser::AST::ASTArray)
-        if value.is_a?(Array)
-            "['#{value.join(", ")}']"
-        elsif [:true, true, "true"].include?(value)
-            "true"
-        elsif [:false, false, "false"].include?(value)
-            "false"
-        else
-            value.to_s
-        end
-    end
 end
 end
\ No newline at end of file
diff --git a/spec/unit/parser/ast/astarray.rb b/spec/unit/parser/ast/astarray.rb
index 212c3fd14..1791c711c 100755
--- a/spec/unit/parser/ast/astarray.rb
+++ b/spec/unit/parser/ast/astarray.rb
@@ -62,5 +62,11 @@ describe Puppet::Parser::AST::ASTArray do
         operator.evaluate(@scope).should == [[123]]
     end
 
+    it "should return a valid string with to_s" do
+        a = stub 'a', :is_a? => true, :to_s => "a"
+        b = stub 'b', :is_a? => true, :to_s => "b"
+        array = Puppet::Parser::AST::ASTArray.new :children => [a,b]
 
+        array.to_s.should == "[a, b]"
+    end
 end
diff --git a/spec/unit/parser/ast/function.rb b/spec/unit/parser/ast/function.rb
index 15420132f..bb687eac0 100644
--- a/spec/unit/parser/ast/function.rb
+++ b/spec/unit/parser/ast/function.rb
@@ -16,6 +16,12 @@ describe Puppet::Parser::AST::Function do
         end
     end
 
+    it "should return its representation with to_s" do
+        args = stub 'args', :is_a? => true, :to_s => "[a, b]"
+
+        Puppet::Parser::AST::Function.new(:name => "func", :arguments => args).to_s.should == "func(a, b)"
+    end
+
     describe "when evaluating" do
 
         it "should fail if the function doesn't exist" do
diff --git a/spec/unit/parser/ast/leaf.rb b/spec/unit/parser/ast/leaf.rb
new file mode 100755
index 000000000..5ca7bc675
--- /dev/null
+++ b/spec/unit/parser/ast/leaf.rb
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+describe Puppet::Parser::AST::Leaf do
+    describe "when converting to string" do
+        it "should transform its value to string" do
+            value = stub 'value', :is_a? => true
+            value.expects(:to_s)
+            Puppet::Parser::AST::Leaf.new( :value => value ).to_s
+        end
+    end
+end
+
+describe Puppet::Parser::AST::FlatString do
+    describe "when converting to string" do
+        it "should transform its value to a quoted string" do
+            value = stub 'value', :is_a? => true, :to_s => "ab"
+            Puppet::Parser::AST::FlatString.new( :value => value ).to_s.should == "\"ab\""
+        end
+    end
+end
+
+describe Puppet::Parser::AST::FlatString do
+    describe "when converting to string" do
+        it "should transform its value to a quoted string" do
+            value = stub 'value', :is_a? => true, :to_s => "ab"
+            Puppet::Parser::AST::String.new( :value => value ).to_s.should == "\"ab\""
+        end
+    end
+end
diff --git a/spec/unit/parser/ast/resource_reference.rb b/spec/unit/parser/ast/resource_reference.rb
index 3a759c550..24865e846 100755
--- a/spec/unit/parser/ast/resource_reference.rb
+++ b/spec/unit/parser/ast/resource_reference.rb
@@ -60,4 +60,10 @@ describe Puppet::Parser::AST::ResourceReference do
         ref.evaluate(@scope)
     end
 
+    it "should return a correct representation when converting to string" do
+        type = stub 'type', :is_a? => true, :to_s => "file"
+        title = stub 'title', :is_a? => true, :to_s => "[/tmp/a, /tmp/b]"
+
+        ast::ResourceReference.new( :type => type, :title => title ).to_s.should == "File[/tmp/a, /tmp/b]"
+    end
 end