Merge pull request #80 from rubocop/fix78-2

Fix an invalid attributes parse when name with multiple `[]`

Author: ydah

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Change to default `EnforcedStyle: link_or_button` for `Capybara/ClickLinkOrButtonStyle` cop. ([@ydah])
 - Fix a false negative for `RSpec/HaveSelector` when first argument is dstr node. ([@ydah])
+- Fix an invalid attributes parse when name with multiple `[]` for `Capybara/SpecificFinders` and `Capybara/SpecificActions` and `Capybara/SpecificMatcher`. ([@ydah])
 
 ## 2.19.0 (2023-09-20)
 

lib/rubocop-capybara.rb

@@ -6,6 +6,7 @@
 require 'rubocop'
 
 require_relative 'rubocop/cop/capybara/mixin/capybara_help'
+require_relative 'rubocop/cop/capybara/mixin/css_attributes_parser'
 require_relative 'rubocop/cop/capybara/mixin/css_selector'
 
 require_relative 'rubocop/cop/capybara_cops'

lib/rubocop/cop/capybara/mixin/css_attributes_parser.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Capybara
+      # Css selector parser.
+      # @api private
+      class CssAttributesParser
+        def initialize(selector)
+          @selector = selector
+          @state = :initial
+          @temp = ''
+          @results = {}
+          @bracket_count = 0
+        end
+
+        # @return [Array<String>]
+        def parse # rubocop:disable Metrics/MethodLength
+          @selector.chars do |char|
+            if char == '['
+              on_bracket_start
+            elsif char == ']'
+              on_bracket_end
+            elsif @state == :inside_attr
+              @temp += char
+            end
+          end
+          @results
+        end
+
+        private
+
+        def on_bracket_start
+          @bracket_count += 1
+          if @state == :initial
+            @state = :inside_attr
+          else
+            @temp += '['
+          end
+        end
+
+        def on_bracket_end
+          @bracket_count -= 1
+          if @bracket_count.zero?
+            @state = :initial
+            key, value = @temp.split('=')
+            @results[key] = normalize_value(value)
+            @temp.clear
+          else
+            @temp += ']'
+          end
+        end
+
+        # @param value [String]
+        # @return [Boolean, String]
+        # @example
+        #   normalize_value('true') # => true
+        #   normalize_value('false') # => false
+        #   normalize_value(nil) # => nil
+        #   normalize_value("foo") # => "'foo'"
+        def normalize_value(value)
+          case value
+          when 'true' then true
+          when 'false' then false
+          when nil then nil
+          else "'#{value.gsub(/"|'/, '')}'"
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/capybara/mixin/css_selector.rb

@@ -55,15 +55,9 @@ def attribute?(selector)
         #   attributes('a[foo-bar_baz]') # => {"foo-bar_baz=>nil}
         #   attributes('button[foo][bar=baz]') # => {"foo"=>nil, "bar"=>"'baz'"}
         #   attributes('table[foo=bar]') # => {"foo"=>"'bar'"}
+        #   attributes('[foo="bar[baz][qux]"]') # => {"foo"=>"'bar[baz][qux]'"}
         def attributes(selector)
-          # Extract the inner strings of attributes.
-          # For example, extract the following:
-          # 'button[foo][bar=baz]' => 'foo][bar=baz'
-          inside_attributes = selector.scan(/\[(.*)\]/).flatten.join
-          inside_attributes.split('][').to_h do |attr|
-            key, value = attr.split('=')
-            [key, normalize_value(value)]
-          end
+          CssAttributesParser.new(selector).parse
         end
 
         # @param selector [String]
@@ -89,22 +83,6 @@ def multiple_selectors?(selector)
           normalize = selector.gsub(/(\\[>,+~]|\(.*\))/, '')
           normalize.match?(/[ >,+~]/)
         end
-
-        # @param value [String]
-        # @return [Boolean, String]
-        # @example
-        #   normalize_value('true') # => true
-        #   normalize_value('false') # => false
-        #   normalize_value(nil) # => nil
-        #   normalize_value("foo") # => "'foo'"
-        def normalize_value(value)
-          case value
-          when 'true' then true
-          when 'false' then false
-          when nil then nil
-          else "'#{value.gsub(/"|'/, '')}'"
-          end
-        end
       end
     end
   end

spec/rubocop/cop/capybara/mixin/css_attributes_parser_spec.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Capybara::CssAttributesParser do
+  describe 'CssSelector.new.parse' do
+    it 'returns attributes hash when specify attributes' do
+      expect(described_class.new('a[foo-bar_baz]').parse).to eq(
+        'foo-bar_baz' => nil
+      )
+      expect(described_class.new('table[foo=bar]').parse).to eq(
+        'foo' => "'bar'"
+      )
+    end
+
+    it 'returns attributes hash when specify multiple attributes' do
+      expect(described_class.new('button[foo][bar=baz]').parse).to eq(
+        'foo' => nil, 'bar' => "'baz'"
+      )
+    end
+
+    it 'returns attributes hash when specify nested attributes' do
+      expect(described_class.new('[foo="bar[baz]"]').parse).to eq(
+        'foo' => "'bar[baz]'"
+      )
+    end
+
+    it 'returns attributes hash when specify nested and include ' \
+       'multiple bracket' do
+      expect(described_class.new('[foo="bar[baz][qux]"]').parse).to eq(
+        'foo' => "'bar[baz][qux]'"
+      )
+    end
+
+    it 'returns empty hash when specify not include attributes' do
+      expect(described_class.new('h1.cls#id').parse).to eq({})
+    end
+  end
+end

spec/rubocop/cop/capybara/mixin/css_selector_spec.rb

@@ -90,6 +90,13 @@
       )
     end
 
+    it 'returns attributes hash when specify nested and include ' \
+       'multiple bracket' do
+      expect(described_class.attributes('[foo="bar[baz][qux]"]')).to eq(
+        'foo' => "'bar[baz][qux]'"
+      )
+    end
+
     it 'returns empty hash when specify not include attributes' do
       expect(described_class.attributes('h1.cls#id')).to eq({})
     end
@@ -136,22 +143,4 @@
       expect(described_class.multiple_selectors?('a.cls\>b')).to be false
     end
   end
-
-  describe 'CssSelector.normalize_value' do
-    it 'returns true when "true"' do
-      expect(described_class.normalize_value('true')).to be true
-    end
-
-    it 'returns false when "false"' do
-      expect(described_class.normalize_value('false')).to be false
-    end
-
-    it 'returns nil when nil' do
-      expect(described_class.normalize_value(nil)).to be_nil
-    end
-
-    it "returns \"'string'\" when 'string'" do
-      expect(described_class.normalize_value('foo')).to eq "'foo'"
-    end
-  end
 end

spec/rubocop/cop/capybara/specific_matcher_spec.rb

@@ -171,6 +171,14 @@
     RUBY
   end
 
+  it 'registers an offense when using abstract matcher with ' \
+     'first argument is element with multiple brackets' do
+    expect_offense(<<-RUBY)
+      expect(page).to have_css('button[name="bar[baz][qux]"]', exact_text: 'foo')
+                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `have_button` over `have_css`.
+    RUBY
+  end
+
   it 'registers an offense when using abstract matcher with state' do
     expect_offense(<<-RUBY)
       expect(page).to have_css('button[disabled=true]', exact_text: 'foo')