Get filter names via reflection on block parameters

lib/capybara/selector.rb

@@ -75,7 +75,7 @@ Capybara.add_selector(:field) do
 end
 
 Capybara.add_selector(:fieldset) do
-  xpath(:legend) do |locator, legend: nil, **|
+  xpath do |locator, legend: nil, **|
     locator_matchers = (XPath.attr(:id) == locator.to_s) | XPath.child(:legend)[XPath.string.n.is(locator.to_s)]
     locator_matchers |= XPath.attr(test_id) == locator if test_id
     xpath = XPath.descendant(:fieldset)
@@ -88,7 +88,7 @@ Capybara.add_selector(:fieldset) do
 end
 
 Capybara.add_selector(:link) do
-  xpath(:title, :alt) do |locator, href: true, alt: nil, title: nil, **|
+  xpath do |locator, href: true, alt: nil, title: nil, **|
     xpath = XPath.descendant(:a)
     xpath = xpath[@href_conditions = builder.attribute_conditions(href: href)]
 
@@ -192,8 +192,8 @@ end
 Capybara.add_selector(:fillable_field) do
   label 'field'
 
-  xpath(:allow_self) do |locator, **options|
-    xpath = XPath.axis(options[:allow_self] ? :"descendant-or-self" : :descendant, :input, :textarea)[
+  xpath do |locator, allow_self: nil, **options|
+    xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input, :textarea)[
       !XPath.attr(:type).one_of('submit', 'image', 'radio', 'checkbox', 'hidden', 'file')
     ]
     locate_field(xpath, locator, options)
@@ -223,8 +223,8 @@ end
 Capybara.add_selector(:radio_button) do
   label 'radio button'
 
-  xpath(:allow_self) do |locator, **options|
-    xpath = XPath.axis(options[:allow_self] ? :"descendant-or-self" : :descendant, :input)[
+  xpath do |locator, allow_self: nil, **options|
+    xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input)[
       XPath.attr(:type) == 'radio'
     ]
     locate_field(xpath, locator, options)
@@ -241,8 +241,8 @@ Capybara.add_selector(:radio_button) do
 end
 
 Capybara.add_selector(:checkbox) do
-  xpath(:allow_self) do |locator, **options|
-    xpath = XPath.axis(options[:allow_self] ? :"descendant-or-self" : :descendant, :input)[
+  xpath do |locator, allow_self: nil, **options|
+    xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input)[
       XPath.attr(:type) == 'checkbox'
     ]
     locate_field(xpath, locator, options)
@@ -379,8 +379,8 @@ end
 
 Capybara.add_selector(:file_field) do
   label 'file field'
-  xpath(:allow_self) do |locator, **options|
-    xpath = XPath.axis(options[:allow_self] ? :"descendant-or-self" : :descendant, :input)[
+  xpath do |locator, allow_self: nil, **options|
+    xpath = XPath.axis(allow_self ? :"descendant-or-self" : :descendant, :input)[
       XPath.attr(:type) == 'file'
     ]
     locate_field(xpath, locator, options)
@@ -411,14 +411,13 @@ Capybara.add_selector(:label) do
   end
 
   node_filter(:for) do |node, field_or_value|
-    if field_or_value.is_a? Capybara::Node::Element
-      if node[:for]
-        field_or_value[:id] == node[:for]
-      else
-        field_or_value.find_xpath('./ancestor::label[1]').include? node.base
-      end
+    # Non element values were handled through the expression filter
+    next true unless field_or_value.is_a? Capybara::Node::Element
+
+    if (for_val = node[:for])
+      field_or_value[:id] == for_val
     else
-      true # Non element values were handled through the expression filter
+      field_or_value.find_xpath('./ancestor::label[1]').include? node.base
     end
   end
 
@@ -431,7 +430,7 @@ Capybara.add_selector(:label) do
 end
 
 Capybara.add_selector(:table) do
-  xpath(:caption) do |locator, caption: nil, **|
+  xpath do |locator, caption: nil, **|
     xpath = XPath.descendant(:table)
     unless locator.nil?
       locator_matchers = (XPath.attr(:id) == locator.to_s) | XPath.descendant(:caption).is(locator.to_s)
@@ -448,15 +447,14 @@ Capybara.add_selector(:table) do
 end
 
 Capybara.add_selector(:frame) do
-  xpath(:name) do |locator, **options|
+  xpath do |locator, name: nil, **|
     xpath = XPath.descendant(:iframe).union(XPath.descendant(:frame))
     unless locator.nil?
       locator_matchers = (XPath.attr(:id) == locator.to_s) | (XPath.attr(:name) == locator.to_s)
       locator_matchers |= XPath.attr(test_id) == locator if test_id
       xpath = xpath[locator_matchers]
     end
-    xpath = expression_filters.keys.inject(xpath) { |memo, ef| memo[find_by_attr(ef, options[ef])] }
-    xpath
+    xpath[find_by_attr(:name, name)]
   end
 
   describe_expression_filters do |name: nil, **|

lib/capybara/selector/selector.rb

@@ -217,7 +217,7 @@ module Capybara
     # Define a selector by an xpath expression
     #
     # @overload xpath(*expression_filters, &block)
-    #   @param [Array<Symbol>] expression_filters ([])  Names of filters that can be implemented via this expression
+    #   @param [Array<Symbol>] expression_filters ([])  Names of filters that are implemented via this expression, if not specified the names of any keyword parameters in the block will be used
     #   @yield [locator, options]                       The block to use to generate the XPath expression
     #   @yieldparam [String] locator                    The locator string passed to the query
     #   @yieldparam [Hash] options                      The options hash passed to the query
@@ -229,6 +229,7 @@ module Capybara
     def xpath(*allowed_filters, &block)
       if block
         @format, @expression = :xpath, block
+        allowed_filters = parameter_names(block) if allowed_filters.empty?
         allowed_filters.flatten.each { |ef| expression_filters[ef] = Filters::IdentityExpressionFilter.new(ef) }
       end
       format == :xpath ? @expression : nil
@@ -251,6 +252,7 @@ module Capybara
     def css(*allowed_filters, &block)
       if block
         @format, @expression = :css, block
+        allowed_filters = parameter_names(block) if allowed_filters.empty?
         allowed_filters.flatten.each { |ef| expression_filters[ef] = Filters::IdentityExpressionFilter.new(ef) }
       end
       format == :css ? @expression : nil
@@ -459,6 +461,10 @@ module Capybara
     def find_by_class_attr(classes)
       Array(classes).map { |klass| XPath.attr(:class).contains_word(klass) }.reduce(:&)
     end
+
+    def parameter_names(block)
+      block.parameters.select { |(type, _name)| %i[key keyreq].include? type }.map { |(_type, name)| name }
+    end
   end
 end
 

spec/selector_spec.rb

@@ -54,7 +54,7 @@ RSpec.describe Capybara do
       end
 
       Capybara.add_selector :custom_css_selector do
-        css(:name) do |selector, name: nil, **|
+        css(:name, :other_name) do |selector, name: nil, **|
           selector ||= ''
           selector += "[name='#{name}']" if name
           selector
@@ -74,7 +74,7 @@ RSpec.describe Capybara do
       end
 
       Capybara.add_selector :custom_xpath_selector do
-        xpath { |selector| selector }
+        xpath(:valid1, :valid2) { |selector| selector }
       end
     end
 
@@ -120,7 +120,34 @@ RSpec.describe Capybara do
       end
     end
 
-    describe 'css based selectors' do
+    describe 'xpath' do
+      it 'uses filter names passed in' do
+        selector = Capybara::Selector.new :test do
+          xpath(:something, :other) { |_locator| XPath.descendant }
+        end
+
+        expect(selector.expression_filters.keys).to include(:something, :other)
+      end
+
+      it 'gets filter names from block if none passed to xpath method' do
+        selector = Capybara::Selector.new :test do
+          xpath { |_locator, valid3:, valid4: nil| "#{valid3} #{valid4}" }
+        end
+
+        expect(selector.expression_filters.keys).to include(:valid3, :valid4)
+      end
+
+      it 'ignores block parameters if names passed in' do
+        selector = Capybara::Selector.new :test do
+          xpath(:valid1) { |_locator, valid3:, valid4: nil| "#{valid3} #{valid4}" }
+        end
+
+        expect(selector.expression_filters.keys).to include(:valid1)
+        expect(selector.expression_filters.keys).not_to include(:valid3, :valid4)
+      end
+    end
+
+    describe 'css' do
       it "supports filters specified in 'css' definition" do
         expect(string).to have_selector(:custom_css_selector, 'input', name: 'form[my_text_input]')
         expect(string).to have_no_selector(:custom_css_selector, 'input', name: 'form[not_my_text_input]')
@@ -131,6 +158,31 @@ RSpec.describe Capybara do
         expect(string).to have_no_selector(:custom_css_selector, placeholder: 'not my text')
         expect(string).to have_selector(:custom_css_selector, value: 'click me', title: 'submit button')
       end
+
+      it 'uses filter names passed in' do
+        selector = Capybara::Selector.new :text do
+          css(:name, :other_name) { |_locator| '' }
+        end
+
+        expect(selector.expression_filters.keys).to include(:name, :other_name)
+      end
+
+      it 'gets filter names from block if none passed to css method' do
+        selector = Capybara::Selector.new :test do
+          css { |_locator, valid3:, valid4: nil| "#{valid3} #{valid4}" }
+        end
+
+        expect(selector.expression_filters.keys).to include(:valid3, :valid4)
+      end
+
+      it 'ignores block parameters if names passed in' do
+        selector = Capybara::Selector.new :test do
+          css(:valid1) { |_locator, valid3:, valid4: nil| "#{valid3} #{valid4}" }
+        end
+
+        expect(selector.expression_filters.keys).to include(:valid1)
+        expect(selector.expression_filters.keys).not_to include(:valid3, :valid4)
+      end
     end
 
     describe 'builtin selectors' do