Foodcritic / foodcritic

Lint tool for Chef cookbooks.
http://foodcritic.io
MIT License
408 stars 153 forks source link

Detect if 'lazy' evaluation is passed do ... end instead of { ... } #757

Closed NaomiReeves closed 5 years ago

NaomiReeves commented 6 years ago

We should alert users if they have given lazy do ... end instead of using { ... }. Using a do .. end will result in a chef run failing with: ArgumentError: tried to create Proc object without a block which seems to contradict normal block syntax, unless you are familiar with Ruby precedence.

from Ruby:

{ ... } blocks have priority below all listed operations, but do ... end blocks have lower priority.

correct:

template 'template_name' do
  # some attributes
  path lazy { ' some Ruby code ' }
end

will fail:

template 'template_name' do
  # some attributes
  path lazy do
    # some Ruby code
  end
end

Reported originally in https://tickets.opscode.com/browse/CHEF-4595

coderanger commented 6 years ago

This might be a hard thing to do because path(lazy do ... end) is allowable syntax. It would depend on how much context info we get from Ripper.

coderanger commented 6 years ago

The deeper thing to check for probably is lazy called without a block.

lamont-granquist commented 6 years ago

yeah that's what dan suggested in CHEF-4595

jaymzh commented 6 years ago

@lamont-granquist someone should move that bug from Jira to GH so it actually gets noticed and fixed.

tas50 commented 5 years ago

Moved this over to Cookstyle. I already shipped the first guard check there that has detected a good number of bad guards that Foodcritic missed. This seems like it wouldn't be too hard to add there.

https://github.com/chef/cookstyle/issues/251