diff --git a/lib/foodcritic/api.rb b/lib/foodcritic/api.rb index e480c66b..a99f3782 100644 --- a/lib/foodcritic/api.rb +++ b/lib/foodcritic/api.rb @@ -182,7 +182,8 @@ def match(node) # Decode resource notifications. # # @param [Nokogiri::XML::Node] ast The AST to check for notifications. - # @return [Array] A flat array of notifications. + # @return [Array] A flat array of notifications. The resource_name may be + # a string or a Node if the resource name is an expression. def notifications(ast) return [] unless ast.respond_to?(:xpath) ast.xpath('//command[ident/@value="notifies" or @@ -194,14 +195,22 @@ def notifications(ast) symbol/ident[1]/@value') if params.empty? target = notifies.xpath('args_add_block/args_add/ - descendant::tstring_content[1]/@value').to_s - match = target.match(/^([^\[]+)\[(.+)\]$/) + descendant::tstring_content/@value').to_s + match = target.match(/^([^\[]+)\[(.*)\]$/) next unless match resource_type, resource_name = match.captures.tap{|m| m[0] = m[0].to_sym} + if notifies.xpath('descendant::string_embexpr').empty? + next if resource_name.empty? + else + resource_name = + notifies.xpath('args_add_block/args_add/string_literal') + end else resource_type = params.xpath('symbol[1]/ident/@value').to_s.to_sym - resource_name = params.xpath('string_add[1]/tstring_content/@value').to_s + resource_name = params.xpath('string_add[1][count(../ + descendant::string_add) = 1]/tstring_content/@value').to_s + resource_name = params if resource_name.empty? end { :type => diff --git a/spec/foodcritic/api_spec.rb b/spec/foodcritic/api_spec.rb index 75ae78bb..6568e897 100644 --- a/spec/foodcritic/api_spec.rb +++ b/spec/foodcritic/api_spec.rb @@ -415,6 +415,48 @@ def parse_ast(str) })).must_be_empty end end + describe "returns empty if the resource name is missing" do + it "old-style notifications" do + api.notifications(parse_ast(%q{ + template "/etc/nscd.conf" do + source "nscd.conf" + owner "root" + group "root" + notifies :restart, resources(:service) + end + })).must_be_empty + end + it "old-style subscriptions" do + api.notifications(parse_ast(%q{ + template "/etc/nscd.conf" do + source "nscd.conf" + owner "root" + group "root" + subscribes :restart, resources(:service) + end + })).must_be_empty + end + it "new-style notifications" do + api.notifications(parse_ast(%q{ + template "/etc/nscd.conf" do + source "nscd.conf" + owner "root" + group "root" + notifies :restart, "service[]" + end + })).must_be_empty + end + it "new-style subscriptions" do + api.notifications(parse_ast(%q{ + template "/etc/nscd.conf" do + source "nscd.conf" + owner "root" + group "root" + subscribes :restart, "service[]" + end + })).must_be_empty + end + end end it "understands the old-style notifications" do api.notifications(parse_ast(%q{ @@ -860,6 +902,70 @@ def parse_ast(str) })).first[:notification_timing].must_equal(:immediately) end end + describe "resource names as expressions" do + describe "returns the AST for an embedded string" do + it "old-style notifications" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :create, resources(:template => "/etc/bar/#{resource}.bar") + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with string expression to respond to #xpath" + end + it "new-style notifications" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :create, "template[/etc/bar/#{resource}.bar]" + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with string expression to respond to #xpath" + end + it "new-style notifications - complete resource_name" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :create, "template[#{template_path}]" + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with string expression to respond to #xpath" + end + end + describe "returns the AST for node attribute" do + it "old-style notifications" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :restart, resources(:service => node['foo']['service']) + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with node attribute to respond to #xpath" + end + it "new-style notifications" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :restart, "service[#{node['foo']['service']}]" + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with node attribute to respond to #xpath" + end + end + describe "returns the AST for variable reference" do + it "old-style notifications" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :restart, resources(:service => my_service) + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with var ref to respond to #xpath" + end + it "new-style notifications" do + assert api.notifications(parse_ast(%q{ + template "/etc/foo.conf" do + notifies :restart, "service[#{my_service}]" + end + })).first[:resource_name].respond_to?(:xpath), + "Expected resource_name with var ref to respond to #xpath" + end + end + end end describe "#os_command?" do