puppetlabs / puppet-lint

Check that your Puppet manifests conform to the style guide
https://puppetlabs.github.io/puppet-lint/
MIT License
18 stars 12 forks source link

pdk validate fails if code has specific style guide incompatibility #171

Open tuxmea opened 8 months ago

tuxmea commented 8 months ago

Describe the Bug

In some cases lint is unable to autofix the string containing only a variable lint check.

Expected Behavior

Lint will autofix

Steps to Reproduce

See code below

Environment

Additional Context


puppet-lint version: 4.0.0 ruby version: 3.2.2-p53 platform: x86_64-darwin21 file path: manifests/init.pp file contents:

# @summary A short summary of the purpose of this class
#
# A description of what this class does
#
# @example
#   include test
class test {
  $var = '/tmp/test'
  if $::operatingsystemmajrelease == 'RedHat' {
    file {"${test}":
      ensure => file,
    }
  }
}

error:


NoMethodError: undefined method `-' for nil:NilClass
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/data.rb:67:in `insert'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checkplugin.rb:62:in `add_token'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-manifest_whitespace-check-0.3.0/lib/puppet-lint/plugins/check_manifest_whitespace_opening_brace.rb:87:in `fix'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checkplugin.rb:42:in `block in fix_problems'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checkplugin.rb:38:in `each'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checkplugin.rb:38:in `fix_problems'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checks.rb:67:in `block in run'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checks.rb:65:in `each'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/checks.rb:65:in `run'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint.rb:226:in `run'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/bin.rb:85:in `block in run'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/bin.rb:80:in `each'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/lib/puppet-lint/bin.rb:80:in `run'
/opt/puppetlabs/pdk/share/cache/ruby/3.2.0/gems/puppet-lint-4.0.0/bin/puppet-lint:7:in `<top (required)>'
ekohl commented 8 months ago

I get the impression that it's a bug in the plugin. If we check the stack trace:

https://github.com/puppetlabs/puppet-lint/blob/cc275ca0da4a7589b3194790d7e6353595e54499/lib/puppet-lint/data.rb#L65-L67

That ends up being nil - 1. In https://github.com/puppetlabs/puppet-lint/pull/172 I tried to formalize the API, but implicitly it says index must be an integer.

Looking at the next frame:

https://github.com/puppetlabs/puppet-lint/blob/cc275ca0da4a7589b3194790d7e6353595e54499/lib/puppet-lint/checkplugin.rb#L61-L62

That just passes it along, so the same API.

The next frame is the plugin:

https://github.com/voxpupuli/puppet-lint-manifest_whitespace-check/blob/ff2a72941ef32686f36ff41d98b9f0bbac965b76/lib/puppet-lint/plugins/check_manifest_whitespace_opening_brace.rb#L87

So tokens.index(token) ends up being nil. Note it is in the fix(problem) method and you have multiple problems there, so I think 2 plugins both fix the token stream. What probably happens is that one rewrites it to

    file {$test:
      ensure => file,
    }

And then the whitespace check wants to rewrite it to:

    file { "${test}":
      ensure => file,
    }

But the " token no longer exists, so it is nil.