diff --git a/features/026_check_for_conditional_block_string.feature b/features/026_check_for_conditional_block_string.feature new file mode 100644 index 00000000..fb4fd92b --- /dev/null +++ b/features/026_check_for_conditional_block_string.feature @@ -0,0 +1,20 @@ +Feature: Check for conditional attribute blocks with only strings + + In order to avoid wrongly actioning a resource + As a developer + I want to identify conditional attribute blocks that consist only of strings + + Scenario Outline: + Given a cookbook recipe that declares a resource with a + When I check the cookbook + Then the conditional block contains only string warning 026 should be + + Examples: + | conditional_attribute | show_warning | + | not_if { "ls foo" } | shown | + | not_if do "ls foo" end | shown | + | only_if { "ls #{node['foo']['path']}" } | shown | + | not_if { "ls #{foo.method()}" } | shown | + | only_if { foo.bar } | not shown | + | not_if { foo.to_s } | not shown | + | not_if { File.exists?("/etc/foo") } | not shown | diff --git a/features/step_definitions/cookbook_steps.rb b/features/step_definitions/cookbook_steps.rb index 45e34497..4c53b509 100644 --- a/features/step_definitions/cookbook_steps.rb +++ b/features/step_definitions/cookbook_steps.rb @@ -943,6 +943,10 @@ def search(bag_name, query=nil, sort=nil, start=0, rows=1000, &block) expect_warning('FC024', :line => should_not.nil? ? @expected_line : nil, :expect_warning => should_not.nil?) end +Then /^the conditional block contains only string warning 026 should be (shown|not shown)$/ do |show_warning| + expect_warning('FC026', :line => nil, :expect_warning => show_warning == 'shown') +end + Then /^the conditional string looks like ruby warning 020 should be (shown|not shown)$/ do |show_warning| expect_warning('FC020', :line => nil, :expect_warning => show_warning == 'shown') end diff --git a/features/support/command_helpers.rb b/features/support/command_helpers.rb index efbb834b..1ec765bd 100644 --- a/features/support/command_helpers.rb +++ b/features/support/command_helpers.rb @@ -32,6 +32,7 @@ module CommandHelpers 'FC023' => 'Prefer conditional attributes', 'FC024' => 'Consider adding platform equivalents', 'FC025' => 'Prefer chef_gem to compile-time gem install', + 'FC026' => 'Conditional execution block attribute contains only string', 'FCTEST001' => 'Test Rule' } diff --git a/lib/foodcritic/rules.rb b/lib/foodcritic/rules.rb index e732d5ed..3f4d4ca1 100644 --- a/lib/foodcritic/rules.rb +++ b/lib/foodcritic/rules.rb @@ -362,3 +362,19 @@ end end end + +rule "FC026", "Conditional execution block attribute contains only string" do + tags %w{correctness} + applies_to {|version| version >= gem_version("0.7.4")} + recipe do |ast| + find_resources(ast).map{|r| resource_attributes(r)}.map do |resource| + [resource['not_if'], resource['only_if']] + end.flatten.compact.select do |condition| + condition.respond_to?(:xpath) and + ! condition.xpath('descendant::string_literal').empty? and + ! condition.xpath('stmts_add/string_literal').empty? and + condition.xpath('descendant::stmts_add[count(ancestor:: + string_literal) = 0]').size == 1 + end + end +end