Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

whitespace check bug #339

Closed
zimbatm opened this issue Oct 7, 2014 · 10 comments
Closed

whitespace check bug #339

zimbatm opened this issue Oct 7, 2014 · 10 comments
Milestone

Comments

@zimbatm
Copy link

zimbatm commented Oct 7, 2014

I think the parser has some trouble parsing this horrible file:

class foo {
  file{'/some/file': content    =>inline_template('
FOO=bar
    '),
    require =>Package[somepackage],
    }
  }

https://gist.github.com/zimbatm/c48ba94108aeb52a79a9

When running puppet-lint on it if fails with the following error:

`gems/puppet-lint-1.1.0/lib/puppet-lint/plugins/check_whitespace.rb:115:in `[]': no implicit conversion from nil to integer (TypeError)`

EDIT: removing the require part doesn't trigger the issue.

@rodjek
Copy link
Owner

rodjek commented Oct 8, 2014

Yikes! Thanks for providing the example, i'll take a look into it.

@rodjek rodjek added this to the 1.1.1 milestone Oct 8, 2014
@zimbatm
Copy link
Author

zimbatm commented Oct 8, 2014

Thanks @rodjek . I found another case that would trigger the same error if it's helpful: https://gist.github.com/zimbatm/18b050f8ea79fdb47e34

@duritong
Copy link
Contributor

@zimbatm
Copy link
Author

zimbatm commented Oct 10, 2014

@duritong just finished cleaning up 15k lines of puppet code so by experience: whenever you see the error, try splitting all the resource properties on it's own lines. puppet-lint seem to get confused when you have a mix of single-line and multi-line properties like you have on line 31.

I also found it helpful to patch the "check_whitespace.rb" file to abort early whenever the error occurred.

          prev_indent_token = resource_tokens[0..idx].rindex { |t| t.type == :INDENT }
          # added this block at line 115
          if prev_indent_token.nil?
            p :PARSER_FAIL
            return
          end
          indent_length = resource_tokens[prev_indent_token].to_manifest.length + token.prev_code_token.to_manifest.length + 2

@duritong
Copy link
Contributor

Thanks! Yeah, this is also what I figured out and wanted to provide another example.

@nvtkaszpir
Copy link

I have experienced the same issue when linting below code:

  package { [
    'php5-cli',
    'php5-gd',
    'php5-curl',
    'php5-geoip',
    'php5-memcached',
    'php5-memcache',
    'php5-mysql',
    'php-apc',
    'php5-mcrypt',
    'php5-pgsql',
    ]: ensure => installed,
    require   => Exec['apt update'],
  }

adding newline after colon fixes the issue:

  package { [
    'php5-cli',
    'php5-gd',
    'php5-curl',
    'php5-geoip',
    'php5-memcached',
    'php5-memcache',
    'php5-mysql',
    'php-apc',
    'php5-mcrypt',
    'php5-pgsql',
    ]:
    ensure => installed,
    require   => Exec['apt update'],
  }

@dalen
Copy link
Contributor

dalen commented Nov 17, 2014

Had the same issue on some code that looked similar to this (which also triggers it):

file { [
  'foo',
  'bar',
]: require => [
  File['foobaz'],
  File['barbaz'],
], before => [
  File['foobax'],
  File['barbax'],
]}

@gregsio
Copy link

gregsio commented Nov 19, 2014

Had the same error with the following code (not indented)

Ini_setting {
ensure => present,
path => "${::settings::confdir}/puppet.conf",
}
ini_setting { 'Configure environmentpath':
section => 'main',
setting => 'environmentpath',
value => '$confdir/environments',
}
ini_setting { 'Configure basemodulepath':
section => 'main',
setting => 'basemodulepath',
value => '$confdir/modules:/opt/puppet/share/puppet/modules',
}

I fixed the issue by indenting the code properly

Ini_setting {
  ensure => present,
  path => "${::settings::confdir}/puppet.conf",
}
ini_setting { 'Configure environmentpath':
  section => 'main',
  setting => 'environmentpath',
  value => '$confdir/environments',
}
ini_setting { 'Configure basemodulepath':
  section => 'main',
  setting => 'basemodulepath',
  value => '$confdir/modules:/opt/puppet/share/puppet/modules',
}

@fxlv
Copy link

fxlv commented Feb 20, 2015

thanks for the patch suggestion @zimbatm !

@rodjek
Copy link
Owner

rodjek commented Apr 12, 2015

This should be fixed as part of #409 (arrow_alignment check expecting an INDENT token at the start of each line)

@rodjek rodjek closed this as completed Apr 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants