diff --git a/lib/foodcritic/api.rb b/lib/foodcritic/api.rb index 6dfdbdb9..23701b3f 100644 --- a/lib/foodcritic/api.rb +++ b/lib/foodcritic/api.rb @@ -137,12 +137,21 @@ def gem_version(version) # Retrieve the recipes that are included within the given recipe AST. # # @param [Nokogiri::XML::Node] ast The recipe AST + # @param [Hash] options Options to filter recipes to include + # @option [Symbol] :with_partial_names Include string literals for recipes + # that have an embedded sub-expression. # @return [Hash] include_recipe nodes keyed by included recipe name - def included_recipes(ast) + def included_recipes(ast, options = {:with_partial_names => true}) raise_unless_xpath!(ast) + + filter = ['[count(descendant::args_add) = 1]'] + unless options[:with_partial_names] + filter << '[count(descendant::string_embexpr) = 0]' + end + # we only support literal strings, ignoring sub-expressions - included = ast.xpath(%q{//command[ident/@value = 'include_recipe'][count( - descendant::args_add) = 1][count(descendant::string_embexpr) = 0] + included = ast.xpath(%Q{//command[ident/@value = 'include_recipe'] + #{filter.join} [descendant::args_add/string_literal]/descendant::tstring_content}) included.inject(Hash.new([])){|h, i| h[i['value']] += [i]; h} end diff --git a/lib/foodcritic/rules.rb b/lib/foodcritic/rules.rb index 7dc24037..03de009f 100644 --- a/lib/foodcritic/rules.rb +++ b/lib/foodcritic/rules.rb @@ -85,11 +85,12 @@ metadata_path =Pathname.new( File.join(File.dirname(filename), '..', 'metadata.rb')).cleanpath next unless File.exists? metadata_path - undeclared = included_recipes(ast).keys.map do |recipe| + actual_included = included_recipes(ast, :with_partial_names => false) + undeclared = actual_included.keys.map do |recipe| recipe.split('::').first end - [cookbook_name(filename)] - declared_dependencies(read_ast(metadata_path)) - included_recipes(ast).map do |recipe, include_stmts| + actual_included.map do |recipe, include_stmts| if undeclared.include?(recipe) || undeclared.any?{|u| recipe.start_with?("#{u}::")} include_stmts diff --git a/spec/foodcritic/api_spec.rb b/spec/foodcritic/api_spec.rb index 1b15792c..f75bcd16 100644 --- a/spec/foodcritic/api_spec.rb +++ b/spec/foodcritic/api_spec.rb @@ -2,6 +2,10 @@ describe FoodCritic::Api do + def parse_ast(str) + api.send(:build_xml, Ripper::SexpBuilder.new(str).parse) + end + let(:api) { Object.new.extend(FoodCritic::Api) } describe "#attribute_access" do @@ -196,6 +200,62 @@ [String] api.included_recipes(ast).values.must_equal [[{'value' => 'foo::bar'}]] end + it "correctly keys an included recipe specified as a string literal" do + api.included_recipes(parse_ast(%q{ + include_recipe "foo::default" + })).keys.must_equal ["foo::default"] + end + describe "embedded expression - recipe name" do + let(:ast) do + parse_ast(%q{ + include_recipe "foo::#{bar}" + }) + end + it "returns the literal string component by default" do + api.included_recipes(ast).keys.must_equal ["foo::"] + end + it "returns the literal string part of the AST" do + api.included_recipes(ast)['foo::'].first.must_respond_to(:xpath) + end + it "returns empty if asked to exclude statements with embedded expressions" do + api.included_recipes(ast, :with_partial_names => false).must_be_empty + end + it "returns the literals if asked to include statements with embedded expressions" do + api.included_recipes(ast, :with_partial_names => true).keys.must_equal ["foo::"] + end + end + describe "embedded expression - cookbook name" do + let(:ast) do + parse_ast(%q{ + include_recipe "#{foo}::bar" + }) + end + it "returns the literal string component by default" do + api.included_recipes(ast).keys.must_equal ["::bar"] + end + it "returns the literal string part of the AST" do + api.included_recipes(ast)['::bar'].first.must_respond_to(:xpath) + end + it "returns empty if asked to exclude statements with embedded expressions" do + api.included_recipes(ast, :with_partial_names => false).must_be_empty + end + end + describe "embedded expression - partial cookbook name" do + let(:ast) do + parse_ast(%q{ + include_recipe "#{foo}_foo::bar" + }) + end + it "returns the literal string component by default" do + api.included_recipes(ast).keys.must_equal ["_foo::bar"] + end + it "returns the literal string part of the AST" do + api.included_recipes(ast)['_foo::bar'].first.must_respond_to(:xpath) + end + it "returns empty if asked to exclude statements with embedded expressions" do + api.included_recipes(ast, :with_partial_names => false).must_be_empty + end + end end describe :AttFilter do @@ -275,9 +335,6 @@ end describe "#notifications" do - def parse_ast(str) - ast = api.send(:build_xml, Ripper::SexpBuilder.new(str).parse) - end it "returns empty if the provided AST does not support XPath" do api.notifications(nil).must_be_empty end @@ -1115,8 +1172,7 @@ def xpath(str) describe "#resource_attributes" do def str_to_atts(str) - ast = api.send(:build_xml, Ripper::SexpBuilder.new(str).parse) - api.resource_attributes(api.find_resources(ast).first) + api.resource_attributes(api.find_resources(parse_ast(str)).first) end it "raises if the resource does not support XPath" do lambda{api.resource_attributes(nil)}.must_raise ArgumentError