Skip to content
This repository has been archived by the owner on Sep 19, 2020. It is now read-only.

FC022 - Foodcritic complains about resources defined in a loop, but resource not defined in a loop #632

Open
jayhendren opened this issue May 18, 2017 · 3 comments
Labels

Comments

@jayhendren
Copy link

jayhendren commented May 18, 2017

[birdsnest ~/local/data/sis/git/cookbooks/cub_jenkins](polishing|✚1)[I]% foodcritic .
Checking 12 files
.......x....
FC022: Resource condition within loop may not behave as expected: ./recipes/master.rb:17

And here is recipes/master.rb up to the resource defined on line 17 (ruby_block[trigger jenkins ...]):

include_recipe 'cub_java'
include_recipe 'jenkins::master'
include_recipe 'cub_jenkins::proxy'

node['cub_jenkins']['plugins'].each do |name, ver|
  jenkins_plugin name do
    version ver
    # don't do dependency resolution on plugins. see attributes/master.rb and
    # https://github.com/chef-cookbooks/jenkins/issues/534
    install_deps false
  end
end

# restart jenkins now if any of above plugins have been updated.
# keypair credentials below depends on plugins being loaded
# which won't happen until jenkins is restarted.
ruby_block 'trigger jenkins restart to enable plugins' do
  block {}
  notifies :restart, 'service[jenkins]', :immediately
  only_if do
    node['cub_jenkins']['plugins'].map do |name, _version|
      resources("jenkins_plugin[#{name}]").updated_by_last_action?
    end.any?
  end
end

Foodcritic is complaining that the ruby_block resource is defined with a fixed name in a loop, but it's not defined in a loop. The previously defined resources are in a loop, which seems to trigger the issue. if I comment out either the node['cub_jenkins']['plugins'].each loop or the ruby_block resource, then foodcritic stops complaining.

[birdsnest ~/local/data/sis/git/cookbooks/cub_jenkins](polishing|✚1)[I]% foodcritic --version
foodcritic 10.2.2
@coderanger
Copy link
Contributor

We can probably just deprecate this rule since in 13+ there is no error (other than maybe some confusion around notifications).

@tas50
Copy link
Contributor

tas50 commented May 18, 2017

I'm ok with nuking this one if someone (@coderanger) wants to cut a PR that explains why this isn't an issue anymore and why we don't need it.

@tas50 tas50 added the Bug label May 26, 2017
@WalterS
Copy link

WalterS commented Jan 26, 2018

I get the same for

package 'python-xml' do
  action :install
  only_if { node['os_family'] == 'suse' && check_package(new_package).empty? } 
end

with foodcritic 12.3.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants