Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: rodjek/puppet-lint
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 1.0.1
Choose a base ref
...
head repository: rodjek/puppet-lint
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 1.1.0
Choose a head ref

Commits on Sep 18, 2014

  1. Cache parsing state in Lexer rather than recalculating

    Reported in #315, puppet-lint is painfully slow when processing large files. I
    reproduced this by creating a very large manifest (basically the same block of
    code concatenated over and over).
    
    ```
    $ ls -la ~/test.pp
    -rw-r--r--  1 tsharpe  staff  165928 18 Sep 14:23 /Users/tsharpe/test.pp
    $ wc -l ~/test.pp
        6296 /Users/tsharpe/test.pp
    ```
    
    On 1.0.1, running the full suite of checks:
    ```
    $ time bin/puppet-lint ~/test.pp
    bin/puppet-lint ~/test.pp > 1.0.1.output  169.96s user 0.26s system 99% cpu 2:50.26 total
    ```
    
    Basically, the problem was that every time the lexer was creating a new token
    it would search through every byte of the file tokenised so far to count how
    many newlines had been found in order to add the line number to the Tokens. By
    changing the lexer so that it now keeps a counter which it increments everytime
    a :NEWLINE Token is created, we can avoid all this stupidity (keeping track of
    the state in regards to column number also provided a small win, but not as
    large as line number).
    
    On this branch, running the full suite of checks:
    ```
    $ time bin/puppet-lint ~/test.pp
    bin/puppet-lint ~/test.pp > 315.output  9.43s user 0.08s system 99% cpu 9.513 total
    ```
    rodjek committed Sep 18, 2014
    Copy the full SHA
    af3bf4e View commit details
  2. Insert :WHITESPACE token between :NAME and :FARROW if needed

    When fixing arrow alignment, ensure that we insert a new :WHITESPACE token
    if there isn't one there or we'll end up wiping out the parameter name when
    fixing the arrow alignment.
    rodjek committed Sep 18, 2014
    Copy the full SHA
    d7edd29 View commit details
  3. Merge pull request #317 from rodjek/issue-315

    Cache parsing state in Lexer rather than recalculating
    timtim123456 committed Sep 18, 2014
    Copy the full SHA
    1c98182 View commit details
  4. Merge pull request #318 from rodjek/issue-311

    Insert :WHITESPACE token between :NAME and :FARROW if needed
    timtim123456 committed Sep 18, 2014
    Copy the full SHA
    61d4b8e View commit details

Commits on Sep 19, 2014

  1. Don't parse class body when searching for parameter tokens

    Fixes #319
    Dominic Cleal committed Sep 19, 2014
    Copy the full SHA
    817abfd View commit details

Commits on Sep 21, 2014

  1. Copy the full SHA
    d6bcfed View commit details
  2. Copy the full SHA
    b5bdb73 View commit details
  3. Merge pull request #321 from rodjek/issue-310

    Support of metaparameter variables in variable_scope check
    timtim123456 committed Sep 21, 2014
    Copy the full SHA
    d84c3b7 View commit details
  4. Merge pull request #320 from domcleal/319-param-body

    Don't parse class body when searching for parameter tokens
    timtim123456 committed Sep 21, 2014
    Copy the full SHA
    d6bf899 View commit details
  5. Add spec for issue #312

    rodjek committed Sep 21, 2014
    Copy the full SHA
    d79ba24 View commit details
  6. Copy the full SHA
    7026a07 View commit details
  7. Copy the full SHA
    2d79fb8 View commit details
  8. Handle multiple params on a line when fixing arrow_alignment problems

    Move the additional params onto their own lines and indent them properly before
    adusting the whitespace between param and arrow to fix alignment.
    rodjek committed Sep 21, 2014
    Copy the full SHA
    43ef2cb View commit details
  9. Merge pull request #322 from rodjek/issue-312

    Handle multiple parameters on a line when fixing arrow_alignment problems
    timtim123456 committed Sep 21, 2014
    Copy the full SHA
    93153dc View commit details

Commits on Sep 22, 2014

  1. Copy the full SHA
    d2b116e View commit details
  2. Copy the full SHA
    3f5c3a9 View commit details
  3. Merge pull request #324 from rodjek/issue-323

    Support for multiple node names in unquoted_node_name
    timtim123456 committed Sep 22, 2014
    Copy the full SHA
    fe3a7ae View commit details
  4. Copy the full SHA
    b37c98e View commit details
  5. Copy the full SHA
    bcddef9 View commit details
  6. Merge pull request #325 from rodjek/issue-314

    Support multiple commands in a single control comment
    timtim123456 committed Sep 22, 2014
    Copy the full SHA
    3aa4d6d View commit details
  7. Copy the full SHA
    8cfa590 View commit details
  8. Document rake block in README

    rodjek committed Sep 22, 2014
    Copy the full SHA
    763575c View commit details
  9. Remove unnecessary line

    rodjek committed Sep 22, 2014
    Copy the full SHA
    c7c36bb View commit details
  10. Merge pull request #326 from rodjek/issue-305

    Extend the rake task to support setting configuration options in the block
    timtim123456 committed Sep 22, 2014
    Copy the full SHA
    c08bf75 View commit details

Commits on Sep 23, 2014

  1. Clean this up a bit

    rodjek committed Sep 23, 2014
    Copy the full SHA
    7c52636 View commit details
  2. Bump to 1.1.0

    rodjek committed Sep 23, 2014
    Copy the full SHA
    879b7eb View commit details
34 changes: 34 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -27,6 +27,40 @@ If you want to test your entire Puppet manifest directory, you can add

rake lint

If you want to modify the default behaviour of the rake task, you can modify
the PuppetLint configuration by defining the task yourself.

PuppetLint::RakeTask.new :lint do |config|
# Pattern of files to check, defaults to `**/*.pp`
config.pattern = 'modules'

# Pattern of files to ignore
config.ignore_paths = ['modules/apt', 'modules/stdlib']

# List of checks to disable
config.disable_checks = ['documentation', '80chars']

# Should puppet-lint prefix it's output with the file being checked,
# defaults to true
config.with_filename = false

# Should the task fail if there were any warnings, defaults to false
config.fail_on_warnings = true

# Format string for puppet-lint's output (see the puppet-lint help output
# for details
config.log_format = '%{filename} - %{message}'

# Print out the context for the problem, defaults to false
config.with_context = true

# Enable automatic fixing of problems, defaults to false
config.fix = true

# Show ignored problems in the output, defaults to false
config.show_ignored = true
end

## Implemented tests

At the moment, the following tests have been implemented:
45 changes: 29 additions & 16 deletions lib/puppet-lint/data.rb
Original file line number Diff line number Diff line change
@@ -255,6 +255,9 @@ def param_tokens(these_tokens)
rparen_idx = i
break
end
elsif token.type == :LBRACE && depth == 0
# no parameters
break
end
end

@@ -289,6 +292,7 @@ def ignore_overrides
# Returns nothing.
def parse_control_comments
@ignore_overrides.each_key { |check| @ignore_overrides[check].clear }
control_re = /\A(lint:\S+)(\s+lint:\S+)*(.*)/

comment_token_types = Set[:COMMENT, :MLCOMMENT, :SLASH_COMMENT]

@@ -301,28 +305,37 @@ def parse_control_comments

stack = []
control_comment_tokens.each do |token|
control, reason = token.value.strip.split(' ', 2)
split_control = control.split(':')
command = split_control[1]
comment_data = control_re.match(token.value.strip).to_a[1..-1].compact.map(&:strip)
if comment_data.last =~ /\Alint:(ignore|endignore)/
comment_data << ''
end
reason = comment_data.pop
stack_add = []
comment_data.each do |control|
split_control = control.split(':')
command = split_control[1]

if command == 'ignore'
check = split_control[2].to_sym
if command == 'ignore'
check = split_control[2].to_sym

if token.prev_token && !Set[:NEWLINE, :INDENT].include?(token.prev_token.type)
# control comment at the end of the line, override applies to
# a single line only
(ignore_overrides[check] ||= {})[token.line] = reason
if token.prev_token && !Set[:NEWLINE, :INDENT].include?(token.prev_token.type)
# control comment at the end of the line, override applies to
# a single line only
(ignore_overrides[check] ||= {})[token.line] = reason
else
stack_add << [token.line, reason, check]
end
else
stack << [token.line, reason, check]
end
else
start = stack.pop
unless start.nil?
(start[0]..token.line).each do |i|
(ignore_overrides[start[2]] ||= {})[i] = start[1]
stack.pop.each do |start|
unless start.nil?
(start[0]..token.line).each do |i|
(ignore_overrides[start[2]] ||= {})[i] = start[1]
end
end
end
end
end
stack << stack_add unless stack_add.empty?
end
end
end
127 changes: 60 additions & 67 deletions lib/puppet-lint/lexer.rb
Original file line number Diff line number Diff line change
@@ -15,24 +15,22 @@ class LexerError < StandardError

# Internal: Initialise a new PuppetLint::LexerError object.
#
# code - The String manifest code being tokenised.
# offset - The Integer position in the code string that the tokeniser was
# at when it encountered the error.
def initialize(code, offset)
chunk = code[0..offset]
@line_no = chunk.scan(/(\r\n|\r|\n)/).size + 1
if @line_no == 1
@column = chunk.length
else
@column = chunk.length - chunk.rindex(/(\r\n|\r|\n)/) - 1
end
@column = 1 if @column == 0
# line_no - The Integer line number of the location of the error.
# column - The Integer column number of the location of the error.
def initialize(line_no, column)
@line_no = line_no
@column = column
end
end

# Internal: The puppet-lint lexer. Converts your manifest into its tokenised
# form.
class Lexer
def initialize
@line_no = 1
@column = 1
end

# Internal: A Hash whose keys are Strings representing reserved keywords in
# the Puppet DSL.
KEYWORDS = {
@@ -153,85 +151,85 @@ def tokenise(code)

KNOWN_TOKENS.each do |type, regex|
if value = chunk[regex, 1]
length = value.size
if type == :NAME
if KEYWORDS.include? value
tokens << new_token(value.upcase.to_sym, value, :chunk => code[0..i])
tokens << new_token(value.upcase.to_sym, value, length)
else
tokens << new_token(type, value, :chunk => code[0..i])
tokens << new_token(type, value, length)
end
else
tokens << new_token(type, value, :chunk => code[0..i])
tokens << new_token(type, value, length)
end
i += value.size
i += length
found = true
break
end
end

unless found
if var_name = chunk[/\A\$((::)?([\w-]+::)*[\w-]+(\[.+?\])*)/, 1]
tokens << new_token(:VARIABLE, var_name, :chunk => code[0..i])
i += var_name.size + 1
length = var_name.size + 1
tokens << new_token(:VARIABLE, var_name, length)

elsif chunk.match(/\A'(.*?)'/m)
str_content = StringScanner.new(code[i+1..-1]).scan_until(/(\A|[^\\])(\\\\)*'/m)
tokens << new_token(:SSTRING, str_content[0..-2], :chunk => code[0..i])
i += str_content.size + 1
length = str_content.size + 1
tokens << new_token(:SSTRING, str_content[0..-2], length)

elsif chunk.match(/\A"/)
str_contents = StringScanner.new(code[i+1..-1]).scan_until(/(\A|[^\\])(\\\\)*"/m)
_ = code[0..i].split("\n")
interpolate_string(str_contents, _.count, _.last.length)
i += str_contents.size + 1
length = str_contents.size + 1

elsif comment = chunk[/\A(#.*)/, 1]
comment_size = comment.size
length = comment.size
comment.sub!(/#/, '')
tokens << new_token(:COMMENT, comment, :chunk => code[0..i])
i += comment_size
tokens << new_token(:COMMENT, comment, length)

elsif slash_comment = chunk[/\A(\/\/.*)/, 1]
slash_comment_size = slash_comment.size
length = slash_comment.size
slash_comment.sub!(/\/\//, '')
tokens << new_token(:SLASH_COMMENT, slash_comment, :chunk => code[0..i])
i += slash_comment_size
tokens << new_token(:SLASH_COMMENT, slash_comment, length)

elsif mlcomment = chunk[/\A(\/\*.*?\*\/)/m, 1]
mlcomment_size = mlcomment.size
length = mlcomment.size
mlcomment.sub!(/\A\/\* ?/, '')
mlcomment.sub!(/ ?\*\/\Z/, '')
mlcomment.gsub!(/ *\* ?/, '')
mlcomment.strip!
tokens << new_token(:MLCOMMENT, mlcomment, :chunk => code[0..i])
i += mlcomment_size
tokens << new_token(:MLCOMMENT, mlcomment, length)

elsif chunk.match(/\A\/.*?\//) && possible_regex?
str_content = StringScanner.new(code[i+1..-1]).scan_until(/(\A|[^\\])(\\\\)*\//m)
tokens << new_token(:REGEX, str_content[0..-2], :chunk => code[0..i])
i += str_content.size + 1
length = str_content.size + 1
tokens << new_token(:REGEX, str_content[0..-2], length)

elsif eolindent = chunk[/\A((\r\n|\r|\n)[ \t]+)/m, 1]
eol = eolindent[/\A([\r\n]+)/m, 1]
indent = eolindent[/\A[\r\n]+([ \t]+)/m, 1]
tokens << new_token(:NEWLINE, eol, :chunk => code[0..i])
tokens << new_token(:INDENT, indent, :chunk => code[0..i+eol.size])
i += indent.size + eol.size
tokens << new_token(:NEWLINE, eol, eol.size)
tokens << new_token(:INDENT, indent, indent.size)
length = indent.size + eol.size

elsif whitespace = chunk[/\A([ \t]+)/, 1]
tokens << new_token(:WHITESPACE, whitespace, :chunk => code[0..i])
i += whitespace.size
length = whitespace.size
tokens << new_token(:WHITESPACE, whitespace, length)

elsif eol = chunk[/\A(\r\n|\r|\n)/, 1]
tokens << new_token(:NEWLINE, eol, :chunk => code[0..i])
i += eol.size
length = eol.size
tokens << new_token(:NEWLINE, eol, length)

elsif chunk.match(/\A\//)
tokens << new_token(:DIV, '/', :chunk => code[0..i])
i += 1
length = 1
tokens << new_token(:DIV, '/', length)

else
raise PuppetLint::LexerError.new(code, i)
raise PuppetLint::LexerError.new(@line_no, @column)
end

i += length
end
end

@@ -259,29 +257,18 @@ def possible_regex?
# Internal: Create a new PuppetLint::Lexer::Token object, calculate its
# line number and column and then add it to the Linked List of tokens.
#
# type - The Symbol token type.
# value - The token value.
# opts - A Hash of additional values required to determine line number and
# type - The Symbol token type.
# value - The token value.
# length - The Integer length of the token's value.
# opts - A Hash of additional values required to determine line number and
# column:
# :chunk - The String chunk of the manifest that has been tokenised so
# far.
# :line - The Integer line number if calculated externally.
# :column - The Integer column number if calculated externally.
#
# Returns the instantiated PuppetLint::Lexer::Token object.
def new_token(type, value, opts = {})
if opts[:chunk]
line_no = opts[:chunk].scan(/(\r\n|\r|\n)/).size + 1
if line_no == 1
column = opts[:chunk].length
else
column = opts[:chunk].length - opts[:chunk].rindex(/(\r\n|\r|\n)/) - 1
end
column += 1 if column == 0
else
column = opts[:column]
line_no = opts[:line]
end
def new_token(type, value, length, opts = {})
column = opts[:column] || @column
line_no = opts[:line] || @line_no

token = Token.new(type, value, line_no, column)
unless tokens.last.nil?
@@ -298,6 +285,12 @@ def new_token(type, value, opts = {})
end
end

@column += length
if type == :NEWLINE
@line_no += 1
@column = 1
end

token
end

@@ -333,30 +326,30 @@ def interpolate_string(string, line, column)
until value.nil?
if terminator == "\""
if first
tokens << new_token(:STRING, value, :line => line, :column => column)
tokens << new_token(:STRING, value, value.size + 2, :line => line, :column => column)
first = false
else
line += value.scan(/(\r\n|\r|\n)/).size
token_column = column + (ss.pos - value.size)
tokens << new_token(:DQPOST, value, :line => line, :column => token_column)
tokens << new_token(:DQPOST, value, value.size + 1, :line => line, :column => token_column)
end
else
if first
tokens << new_token(:DQPRE, value, :line => line, :column => column)
tokens << new_token(:DQPRE, value, value.size + 1, :line => line, :column => column)
first = false
else
line += value.scan(/(\r\n|\r|\n)/).size
token_column = column + (ss.pos - value.size)
tokens << new_token(:DQMID, value, :line => line, :column => token_column)
tokens << new_token(:DQMID, value, value.size, :line => line, :column => token_column)
end
if ss.scan(/\{/).nil?
var_name = ss.scan(/(::)?([\w-]+::)*[\w-]+/)
if var_name.nil?
token_column = column + ss.pos - 1
tokens << new_token(:DQMID, "$", :line => line, :column => token_column)
tokens << new_token(:DQMID, "$", 1, :line => line, :column => token_column)
else
token_column = column + (ss.pos - var_name.size)
tokens << new_token(:UNENC_VARIABLE, var_name, :line => line, :column => token_column)
tokens << new_token(:UNENC_VARIABLE, var_name, var_name.size, :line => line, :column => token_column)
end
else
contents = ss.scan_until(/\}/)[0..-2]
@@ -368,7 +361,7 @@ def interpolate_string(string, line, column)
lexer.tokens.each do |token|
tok_col = column + token.column + (ss.pos - contents.size - 1)
tok_line = token.line + line - 1
tokens << new_token(token.type, token.value, :line => tok_line, :column => tok_col)
tokens << new_token(token.type, token.value, token.value.size, :line => tok_line, :column => tok_col)
end
end
end
11 changes: 11 additions & 0 deletions lib/puppet-lint/plugins/check_classes.rb
Original file line number Diff line number Diff line change
@@ -186,6 +186,17 @@ def check
'serverip',
'serverversion',
'caller_module_name',
'alias',
'audit',
'before',
'loglevel',
'noop',
'notify',
'require',
'schedule',
'stage',
'subscribe',
'tag',
]
POST_VAR_TOKENS = Set[:COMMA, :EQUALS, :RPAREN]

Loading