From 5fcadb7cd55cb32e4fdba43c9ea16753ef324da0 Mon Sep 17 00:00:00 2001 From: Andrew Crump Date: Sat, 2 Jun 2012 11:12:57 +0100 Subject: [PATCH] FC027: Resource sets internal attribute. --- ...7_check_for_internal_attribute_use.feature | 22 +++++++++++++++++++ features/step_definitions/cookbook_steps.rb | 12 ++++++++++ features/support/command_helpers.rb | 1 + lib/foodcritic/rules.rb | 10 +++++++++ 4 files changed, 45 insertions(+) create mode 100644 features/027_check_for_internal_attribute_use.feature diff --git a/features/027_check_for_internal_attribute_use.feature b/features/027_check_for_internal_attribute_use.feature new file mode 100644 index 00000000..582dfbd6 --- /dev/null +++ b/features/027_check_for_internal_attribute_use.feature @@ -0,0 +1,22 @@ +Feature: Check for use of internal attributes + + In order to avoid confusion about the expected state of a converged node + As a developer + I want to identify calls to 'internal' resource attributes + + Scenario Outline: Access of internal attributes + Given a cookbook recipe that declares a resource with the attribute set to + When I check the cookbook + Then the resource sets internal attribute warning 027 should be + + Examples: + | resource | attribute | value | show_warning | + | service | enabled | true | shown | + | service | enabled | false | shown | + | service | enabled | [].include?('foo') | shown | + | service | running | true | shown | + | service | running | false | shown | + | service | running | [].include?('foo') | shown | + | service | service_name | "foo" | not shown | + | my_lwrp | enabled | true | not shown | + | my_lwrp | running | true | not shown | diff --git a/features/step_definitions/cookbook_steps.rb b/features/step_definitions/cookbook_steps.rb index 4675c0c6..d141a05c 100644 --- a/features/step_definitions/cookbook_steps.rb +++ b/features/step_definitions/cookbook_steps.rb @@ -25,6 +25,14 @@ }.strip end +Given /^a cookbook recipe that declares a ([^ ]+) resource with the ([^ ]+) attribute set to (.*)$/ do |resource, attribute, value| + write_recipe %Q{ + #{resource} "foo" do + #{attribute} #{value} + end + } +end + Given 'a cookbook provider that declares execute resources varying only in the command in separate actions' do write_provider 'site', %q{ action :start do @@ -1015,6 +1023,10 @@ def search(bag_name, query=nil, sort=nil, start=0, rows=1000, &block) expect_output "cookbooks/example/recipes/default.rb" end +Then /^the resource sets internal attribute warning 027 should be (not )?shown$/ do |should_not| + expect_warning('FC027', :line => nil, :expect_warning => should_not.nil?) +end + Then 'the review should include the matching rules' do repl_review_includes_match?(@rule_code, @rule_name).should be_true end diff --git a/features/support/command_helpers.rb b/features/support/command_helpers.rb index 1ec765bd..66f9a907 100644 --- a/features/support/command_helpers.rb +++ b/features/support/command_helpers.rb @@ -33,6 +33,7 @@ module CommandHelpers 'FC024' => 'Consider adding platform equivalents', 'FC025' => 'Prefer chef_gem to compile-time gem install', 'FC026' => 'Conditional execution block attribute contains only string', + 'FC027' => 'Resource sets internal attribute', 'FCTEST001' => 'Test Rule' } diff --git a/lib/foodcritic/rules.rb b/lib/foodcritic/rules.rb index 3f4d4ca1..7f2fce69 100644 --- a/lib/foodcritic/rules.rb +++ b/lib/foodcritic/rules.rb @@ -378,3 +378,13 @@ end end end + +rule "FC027", "Resource sets internal attribute" do + tags %w{correctness} + recipe do |ast| + find_resources(ast, :type => :service).map do |service| + service unless (resource_attributes(service).keys & + ['enabled', 'running']).empty? + end.compact + end +end