+1
On Jun 28, 2009, at 7:34 AM, Brice Figureau wrote:
>
> 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 <[email protected]>
> ---
> lib/puppet/parser/lexer.rb | 22 ++++++++++++++--------
> lib/puppet/parser/parser_support.rb | 2 +-
> spec/unit/parser/lexer.rb | 14 ++++++++++++++
> 3 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb
> index e296872..5bab6d6 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 2fb0755..480fd4a 100644
> --- a/lib/puppet/parser/parser_support.rb
> +++ b/lib/puppet/parser/parser_support.rb
> @@ -59,7 +59,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 c6b6e82..3c765d4 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.
> --
> 1.6.0.2
>
>
> >
--
There are three social classes in America: upper middle class, middle
class, and lower middle class. --Judith Martin
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
