From c735c3bd0b41a87459fbdd43235f23ca5f76dd73 Mon Sep 17 00:00:00 2001
From: Andrew Crump <andrew.crump@ieee.org>
Date: Sun, 11 Dec 2011 21:50:16 +0000
Subject: [PATCH] FC010: Check for invalid search syntax.

---
 features/010_check_search_syntax.feature    | 20 +++++++++++++++++
 features/step_definitions/cookbook_steps.rb | 24 +++++++++++++++++++++
 features/support/lint_helpers.rb            |  2 ++
 lib/foodcritic/helpers.rb                   | 24 +++++++++++++++++++++
 lib/foodcritic/rules.rb                     |  8 +++++++
 5 files changed, 78 insertions(+)
 create mode 100644 features/010_check_search_syntax.feature

diff --git a/features/010_check_search_syntax.feature b/features/010_check_search_syntax.feature
new file mode 100644
index 00000000..fba9feb3
--- /dev/null
+++ b/features/010_check_search_syntax.feature
@@ -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
diff --git a/features/step_definitions/cookbook_steps.rb b/features/step_definitions/cookbook_steps.rb
index cd53f512..183ab249 100644
--- a/features/step_definitions/cookbook_steps.rb
+++ b/features/step_definitions/cookbook_steps.rb
@@ -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
\ No newline at end of file
diff --git a/features/support/lint_helpers.rb b/features/support/lint_helpers.rb
index 631cbe0b..05b08e5e 100644
--- a/features/support/lint_helpers.rb
+++ b/features/support/lint_helpers.rb
@@ -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]
diff --git a/lib/foodcritic/helpers.rb b/lib/foodcritic/helpers.rb
index 2eca3f99..0a595305 100644
--- a/lib/foodcritic/helpers.rb
+++ b/lib/foodcritic/helpers.rb
@@ -1,4 +1,5 @@
 require 'nokogiri'
+require 'chef/solr_query/query_transform'
 
 module FoodCritic
 
@@ -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
     #
diff --git a/lib/foodcritic/rules.rb b/lib/foodcritic/rules.rb
index f67b0366..26847b1e 100644
--- a/lib/foodcritic/rules.rb
+++ b/lib/foodcritic/rules.rb
@@ -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
\ No newline at end of file