Fix #2364 - Associates the correct comment to the right statement

Due to the problem that we associate documentation in the lexer and
not in the parser (which would be to complex and unmaintenable to
do), and since the parser reads new tokens before reducing
the current statement (thus creating the AST node), we could
sometimes associate comments seen after a statement associated
to this one.
Ex:
1. $foo = 1
2. # doc of next class
3. class test {

When we parse the first line, the parser can reduce this to the
correct VarDef only after it lexed the CLASS token.
But lexing this token means we already pushed on the comment stack
the "doc of next class" comment.
That means at the time we create the AST VarDef node, the parser thinks
it should associate this documentation to it, which is incorrect.

As soon as the parser uses token line number, we can enhance the lexer
to allow comments to be associated to current AST node only if
the statement line number is greater or equal than the last comment
line number.

This way it is impossible to associate a comment appearing later in the
source than a previous statement.

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

3 files changed, 29 insertions, 9 deletions

diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb
index e296872f9..5bab6d65d 100644
--- a/lib/puppet/parser/lexer.rb
+++ b/lib/puppet/parser/lexer.rb
@@ -332,7 +332,7 @@ class Puppet::Parser::Lexer
         @namestack = []
         @indefine = false
         @expected = []
-        @commentstack = ['']
+        @commentstack = [ ['', @line] ]
     end
 
     # Make any necessary changes to the token and/or value.
@@ -348,7 +348,9 @@ class Puppet::Parser::Lexer
         return unless token
 
         if token.accumulate?
-            @commentstack.last << value + "\n"
+            comment = @commentstack.pop
+            comment[0] << value + "\n"
+            @commentstack.push(comment)
         end
 
         return if token.skip
@@ -490,16 +492,20 @@ class Puppet::Parser::Lexer
 
     # returns the content of the currently accumulated content cache
     def commentpop
-        return @commentstack.pop
+        return @commentstack.pop[0]
     end
 
-    def getcomment
-        comment = @commentstack.pop
-        @commentstack.push('')
-        return comment
+    def getcomment(line = nil)
+        comment = @commentstack.last
+        if line.nil? or comment[1] <= line
+            @commentstack.pop
+            @commentstack.push(['', @line])
+            return comment[0]
+        end
+        return ''
     end
 
     def commentpush
-        @commentstack.push('')
+        @commentstack.push(['', @line])
     end
 end
diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index 92db9af5f..e1af2fe82 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -50,7 +50,7 @@ class Puppet::Parser::Parser
         end
 
         k = klass.new(hash)
-        k.doc = lexer.getcomment if !k.nil? and k.use_docs and k.doc.empty?
+        k.doc = lexer.getcomment(hash[:line]) if !k.nil? and k.use_docs and k.doc.empty?
         return k
     end
 
diff --git a/spec/unit/parser/lexer.rb b/spec/unit/parser/lexer.rb
index c6b6e82fa..3c765d431 100755
--- a/spec/unit/parser/lexer.rb
+++ b/spec/unit/parser/lexer.rb
@@ -490,6 +490,20 @@ describe Puppet::Parser::Lexer, "when lexing comments" do
         @lexer.string = "/* 1\n\n */ \ntest"
         @lexer.fullscan.should be_like([[:NAME, "test"],[false,false]])
     end
+
+    it "should not return comments seen after the current line" do
+        @lexer.string = "# 1\n\n# 2"
+        @lexer.fullscan
+
+        @lexer.getcomment(1).should == ""
+    end
+
+    it "should return a comment seen before the current line" do
+        @lexer.string = "# 1\n# 2"
+        @lexer.fullscan
+
+        @lexer.getcomment(2).should == "1\n2\n"
+    end
 end
 
 # FIXME: We need to rewrite all of these tests, but I just don't want to take the time right now.