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

Commit

Permalink
FC010: Check for invalid search syntax.
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrew Crump committed Dec 11, 2011
1 parent 2936559 commit c735c3b
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 0 deletions.
20 changes: 20 additions & 0 deletions features/010_check_search_syntax.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
Feature: Check for invalid search syntax

In order to identify invalid search syntax that will cause my converge to fail
As a developer
I want to verify that search expressions use valid Lucene syntax

Scenario: Invalid search syntax
Given a cookbook recipe that attempts to perform a search with invalid syntax
When I check the cookbook
Then the invalid search syntax warning 010 should be displayed

Scenario: Valid search syntax
Given a cookbook recipe that attempts to perform a search with valid syntax
When I check the cookbook
Then the invalid search syntax warning 010 should not be displayed

Scenario: Search with subexpression
Given a cookbook recipe that attempts to perform a search with a subexpression
When I check the cookbook
Then the invalid search syntax warning 010 should not be displayed
24 changes: 24 additions & 0 deletions features/step_definitions/cookbook_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -438,4 +438,28 @@
action :create
end
}.strip
end

Given /^a cookbook recipe that attempts to perform a search with invalid syntax$/ do
write_recipe %q{
search(:node, 'run_list:recipe[foo::bar]') do |matching_node|
puts matching_node.to_s
end
}.strip
end

Given /^a cookbook recipe that attempts to perform a search with valid syntax$/ do
write_recipe %q{
search(:node, 'run_list:recipe\[foo\:\:bar\]') do |matching_node|
puts matching_node.to_s
end
}.strip
end

Given /^a cookbook recipe that attempts to perform a search with a subexpression$/ do
write_recipe %q{
search(:node, "roles:#{node['foo']['role']}") do |matching_node|
puts matching_node.to_s
end
}.strip
end
2 changes: 2 additions & 0 deletions features/support/lint_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ def expect_warning(code, options={})
'Generated cookbook metadata needs updating'
when 'FC009' then
'Resource attribute not recognised'
when 'FC010' then
'Invalid search syntax'
end

if opt[:expect_warning]
Expand Down
24 changes: 24 additions & 0 deletions lib/foodcritic/helpers.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'nokogiri'
require 'chef/solr_query/query_transform'

module FoodCritic

Expand Down Expand Up @@ -31,6 +32,29 @@ def searches(ast)
ast.xpath("//fcall/ident[@value = 'search']")
end

# Searches performed by the specified recipe that are literal strings. Searches with a query formed from a
# subexpression will be ignored.
#
# @param [Nokogiri::XML::Node] ast The AST of the cookbook recipe to check.
# @return [Nokogiri::XML::Node] The matching nodes
def literal_searches(ast)
ast.xpath("//method_add_arg[fcall/ident/@value = 'search' and count(descendant::string_embexpr) = 0]/descendant::tstring_content")
end

# Is this a valid Lucene query?
#
# @param [String] query The query to check for syntax errors
# @return [Boolean] True if the query is well-formed
def valid_query?(query)
# Exceptions for flow control. Alternatively we could re-implement the parse method.
begin
Chef::SolrQuery::QueryTransform.parse(query)
true
rescue Chef::Exceptions::QueryParseError
false
end
end

# Find Chef resources of the specified type.
# TODO: Include blockless resources
#
Expand Down
8 changes: 8 additions & 0 deletions lib/foodcritic/rules.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,12 @@
end
matches
end
end

rule "FC010", "Invalid search syntax" do
description "The search expression in the recipe could not be parsed. Please check your syntax."
recipe do |ast|
# This only works for literal search strings
literal_searches(ast).reject{|search| valid_query?(search['value'])}.map{|search| match(search)}
end
end

0 comments on commit c735c3b

Please sign in to comment.