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

Commit

Permalink
FC019: Ignore custom patches to node, refs #22.
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrew Crump committed May 2, 2012
1 parent 8c996be commit 935a712
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 5 deletions.
5 changes: 5 additions & 0 deletions features/019_check_for_consistent_node_access.feature
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ Feature: Check for consistency in node access
| updates | strings | foo = node[:foo].strip | shown |
| updates | symbols | node['foo'].strip | shown |

Scenario: Ignore method calls on patched node values
Given a cookbook with a single recipe that calls a patched node method
When I check the cookbook
Then the attribute consistency warning 019 should be not shown

Scenario: Two cookbooks with differing approaches
Given a cookbook with a single recipe that reads node attributes via strings only
And another cookbook with a single recipe that reads node attributes via symbols only
Expand Down
17 changes: 17 additions & 0 deletions features/step_definitions/cookbook_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,23 @@
write_recipe %q{node[:foo][:foo2] = 'bar'}
end

Given 'a cookbook with a single recipe that calls a patched node method' do
write_library 'search', %q{
class Chef
class Node
def in_tier?(*tier)
tier.flatten.include?(node['tier'])
end
end
end
}
write_recipe %q{
if node['something']['bar'] || node.in_tier?('foof')
Chef::Log.info("Node has been patched")
end
}
end

Given 'a cookbook with a single recipe that passes node attributes accessed via symbols to a template' do
write_recipe %q{
template "/etc/foo" do
Expand Down
21 changes: 18 additions & 3 deletions lib/foodcritic/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def attribute_access(ast, options = {})
end

if options[:type] == :vivified
vivified_attribute_access(ast)
vivified_attribute_access(ast, options[:cookbook_dir])
else
standard_attribute_access(ast, options)
end
Expand Down Expand Up @@ -357,6 +357,21 @@ def build_xml(node, doc = nil, xml_node=nil)
xml_node
end

def node_method?(meth, cookbook_dir)
chef_dsl_methods.include?(meth) || patched_node_method?(meth, cookbook_dir)
end

def patched_node_method?(meth, cookbook_dir)
return false if cookbook_dir.nil? || ! Dir.exists?(cookbook_dir)
cbk_tree_path = Pathname.new(File.join(cookbook_dir, '..'))
libs = Dir[File.join(cbk_tree_path.realpath, '*/libraries/*.rb')]
libs.any? do |lib|
! read_ast(lib).xpath(%Q{//class[count(descendant::const[@value='Chef'])
> 0][count(descendant::const[@value='Node']) > 0]/descendant::def/
ident[@value='#{meth.to_s}']}).empty?
end
end

# If the provided node is the line / column information.
#
# @param [Nokogiri::XML::Node] node A node within the AST
Expand All @@ -382,14 +397,14 @@ def standard_attribute_access(ast, options)
ast.xpath(expr, AttFilter.new).sort
end

def vivified_attribute_access(ast)
def vivified_attribute_access(ast, cookbook_dir)
calls = ast.xpath(%q{//*[self::call or self::field]
[is_att_type(vcall/ident/@value) or
is_att_type(var_ref/ident/@value)][@value='.']}, AttFilter.new)
calls.select do |call|
call.xpath("aref/args_add_block").size == 0 and
(call.xpath("descendant::ident").size > 1 and
! chef_dsl_methods.include?(call.xpath("ident/@value").to_s.to_sym))
! node_method?(call.xpath("ident/@value").to_s.to_sym, cookbook_dir))
end.sort
end

Expand Down
4 changes: 2 additions & 2 deletions lib/foodcritic/rules.rb
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,8 @@
end
types = [:string, :symbol, :vivified].map do |type|
{:access_type => type, :count => files.map do |file|
attribute_access(file[:ast], :type => type,
:ignore_calls => true).tap do |ast|
attribute_access(file[:ast], :type => type, :ignore_calls => true,
:cookbook_dir => cookbook_dir).tap do |ast|
if (! ast.empty?) and (! asts.has_key?(type))
asts[type] = {:ast => ast, :path => file[:path]}
end
Expand Down

0 comments on commit 935a712

Please sign in to comment.