tidy: CSS.xpath_for can take keyword options

Related to #3200
sparklemotion · Jun 11, 2024 · a84ad7f · a84ad7f
1 parent fb19c55
commit a84ad7f
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 24 deletions.
diff --git a/lib/nokogiri/css.rb b/lib/nokogiri/css.rb
@@ -70,22 +70,24 @@ def parse(selector) # :nodoc:
       #
       #   Nokogiri::CSS.xpath_for("h1, h2, h3") # => ["//h1", "//h2", "//h3"]
       #
-      def xpath_for(selector, options = {})
-        raise TypeError, "no implicit conversion of #{selector.inspect} to String" unless selector.respond_to?(:to_str)
+      def xpath_for(
+        selector, options = nil,
+        prefix: options&.delete(:prefix), visitor: options&.delete(:visitor), ns: options&.delete(:ns)
+      )
+        unless options.nil?
+          warn("Passing options as an explicit hash is deprecated. Use keyword arguments instead. This will become an error in a future release.", uplevel: 1, category: :deprecated)
+        end
 
-        selector = selector.to_str
-        raise Nokogiri::CSS::SyntaxError, "empty CSS selector" if selector.empty?
+        raise(TypeError, "no implicit conversion of #{selector.inspect} to String") unless selector.respond_to?(:to_str)
 
-        if options.key?(:prefix) && options.key?(:visitor)
-          raise ArgumentError, "cannot provide both :prefix and :visitor"
-        end
+        selector = selector.to_str
+        raise(Nokogiri::CSS::SyntaxError, "empty CSS selector") if selector.empty?
 
-        ns = options.fetch(:ns, {})
+        raise ArgumentError, "cannot provide both :prefix and :visitor" if prefix && visitor
 
-        visitor = if options.key?(:prefix)
-          Nokogiri::CSS::XPathVisitor.new(prefix: options.fetch(:prefix))
-        elsif options.key?(:visitor)
-          options.fetch(:visitor)
+        ns ||= {}
+        visitor ||= if prefix
+          Nokogiri::CSS::XPathVisitor.new(prefix: prefix)
         else
           Nokogiri::CSS::XPathVisitor.new
         end

diff --git a/lib/nokogiri/xml/searchable.rb b/lib/nokogiri/xml/searchable.rb
@@ -249,10 +249,7 @@ def xpath_query_from_css_rule(rule, ns)
             doctype: document.xpath_doctype,
             prefix: implied_xpath_context,
           )
-          CSS.xpath_for(rule.to_s, {
-            ns: ns,
-            visitor: visitor,
-          })
+          CSS.xpath_for(rule.to_s, ns: ns, visitor: visitor)
         end.join(" | ")
       end
 

diff --git a/test/css/test_css.rb b/test/css/test_css.rb
@@ -22,14 +22,19 @@
     end
 
     it "accepts an options hash" do
-      assert_equal(
-        ["./foo"],
-        Nokogiri::CSS.xpath_for("foo", { prefix: "./" }),
-      )
-      assert_equal(
-        ["./foo"],
-        Nokogiri::CSS.xpath_for("foo", { visitor: Nokogiri::CSS::XPathVisitor.new(prefix: "./") }),
-      )
+      assert_output(nil, /Passing options as an explicit hash is deprecated/) do
+        assert_equal(
+          ["./foo"],
+          Nokogiri::CSS.xpath_for("foo", { prefix: "./" }),
+        )
+      end
+
+      assert_output(nil, /Passing options as an explicit hash is deprecated/) do
+        assert_equal(
+          ["./foo"],
+          Nokogiri::CSS.xpath_for("foo", { visitor: Nokogiri::CSS::XPathVisitor.new(prefix: "./") }),
+        )
+      end
     end
 
     it "accepts keyword arguments" do