From cb63f3bd9440fe74c4f58719afe93a60fcebd3ca Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Thu, 6 Jun 2024 12:56:22 -0400
Subject: [PATCH] tidy: CSS.xpath_for can take keyword options

Related to #3200
---
 lib/nokogiri/css.rb            | 26 ++++++++++++++------------
 lib/nokogiri/xml/searchable.rb |  5 +----
 test/css/test_css.rb           | 21 +++++++++++++--------
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/lib/nokogiri/css.rb b/lib/nokogiri/css.rb
index e9edb51ad3c..37a20c2e479 100644
--- a/lib/nokogiri/css.rb
+++ b/lib/nokogiri/css.rb
@@ -39,22 +39,24 @@ 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)
+      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
index 47457f5461a..120c3360ac4 100644
--- 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
index d047f695df1..f92030a4e63 100644
--- 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