Merge pull request #2701 from sparklemotion/2700-empty-css-query

feat: better exception message when a CSS selector is empty

CHANGELOG.md

@@ -66,6 +66,7 @@ This version of Nokogiri uses [`jar-dependencies`](https://github.com/mkristian/
 * Remove calls to `vasprintf` in favor of platform-independent `rb_vsprintf`
 * Prefer `ruby_xmalloc` to `malloc` within the C extension. [[#2480](https://github.com/sparklemotion/nokogiri/issues/2480)] (Thanks, [@Garfield96](https://github.com/Garfield96)!)
 * Installation from source on systems missing libiconv will once again generate a helpful error message (broken since v1.11.0). [[#2505](https://github.com/sparklemotion/nokogiri/issues/2505)]
+* Empty CSS selectors now raise a clearer `Nokogiri::CSS::SyntaxError` message, "empty CSS selector". Previously the exception raised from the bowels of `racc` was "unexpected '$' after ''". [[#2700](https://github.com/sparklemotion/nokogiri/issues/2700)]
 
 
 ### Deprecated

lib/nokogiri/css.rb

@@ -40,9 +40,15 @@ def parse(selector) # :nodoc:
       # 💡 Note that translated queries are cached for performance concerns.
       #
       def xpath_for(selector, options = {})
+        raise TypeError, "no implicit conversion of #{selector.inspect} to String" unless selector.respond_to?(:to_str)
+
+        selector = selector.to_str
+        raise Nokogiri::CSS::SyntaxError, "empty CSS selector" if selector.empty?
+
         prefix = options.fetch(:prefix, Nokogiri::XML::XPath::GLOBAL_SEARCH_PREFIX)
         visitor = options.fetch(:visitor) { Nokogiri::CSS::XPathVisitor.new }
         ns = options.fetch(:ns, {})
+
         Parser.new(ns).xpath_for(selector, prefix, visitor)
       end
     end

test/css/test_css.rb

@@ -2,7 +2,7 @@
 
 require "helper"
 
-class TestNokogiriCSS < Nokogiri::TestCase
+describe Nokogiri::CSS do
   describe ".xpath_for" do
     it "accepts just a selector" do
       assert_equal(["//foo"], Nokogiri::CSS.xpath_for("foo"))
@@ -33,5 +33,23 @@ class TestNokogiriCSS < Nokogiri::TestCase
         Nokogiri::CSS.xpath_for("foo", prefix: "./", visitor: Nokogiri::CSS::XPathVisitor.new),
       )
     end
+
+    describe "error handling" do
+      it "raises a SyntaxError exception if the query is not valid CSS" do
+        assert_raises(Nokogiri::CSS::SyntaxError) { Nokogiri::CSS.xpath_for("'") }
+        assert_raises(Nokogiri::CSS::SyntaxError) { Nokogiri::CSS.xpath_for("a[x=]") }
+      end
+
+      it "raises a SyntaxError exception if the query is empty" do
+        e = assert_raises(Nokogiri::CSS::SyntaxError) { Nokogiri::CSS.xpath_for("") }
+        assert_includes("empty CSS selector", e.message)
+      end
+
+      it "raises an TypeError exception if the query is not a string" do
+        assert_raises(TypeError) { Nokogiri::CSS.xpath_for(nil) }
+        assert_raises(TypeError) { Nokogiri::CSS.xpath_for(3) }
+        assert_raises(TypeError) { Nokogiri::CSS.xpath_for(Object.new) }
+      end
+    end
   end
 end