direct support for negative index in XPath instead of requiring looping

watir · Oct 13, 2018 · 55b041c · 55b041c
1 parent dec8599
commit 55b041c
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/lib/watir/locators/element/selector_builder/xpath.rb b/lib/watir/locators/element/selector_builder/xpath.rb
@@ -27,14 +27,7 @@ def build(selector)
             xpath = "#{start_string}#{adjacent_string}#{tag_string}#{class_string}#{attribute_string}" \
 "#{converted_attribute_string}#{text_string}"
 
-            xpath = if index && !adjacent_string.empty?
-                      "#{xpath}[#{index + 1}]"
-                    elsif index&.positive? && @requires_matches.empty? && use_index?
-                      "(#{xpath})[#{index + 1}]"
-                    else
-                      @requires_matches[:index] = index if index
-                      xpath
-                    end
+            xpath = index ? add_index(xpath, index, !adjacent_string.empty?) : xpath
 
             @selector.merge! @requires_matches
 
@@ -43,6 +36,24 @@ def build(selector)
 
           protected
 
+          # TODO: Remove this on refactor of adjacent & index
+          # rubocop:disable Metrics/CyclomaticComplexity:
+          def add_index(xpath, index, adjacent)
+            if adjacent
+              "#{xpath}[#{index + 1}]"
+            elsif index&.positive? && @requires_matches.empty? && use_index?
+              "(#{xpath})[#{index + 1}]"
+            elsif index&.negative? && @requires_matches.empty? && use_index?
+              last_value = 'last()'
+              last_value << (index + 1).to_s if index < -1
+              "(#{xpath})[#{last_value}]"
+            else
+              @requires_matches[:index] = index
+              xpath
+            end
+          end
+          # rubocop:enable Metrics/CyclomaticComplexity:
+
           def use_index?
             true
           end

diff --git a/spec/watirspec/selector_builder/element_spec.rb b/spec/watirspec/selector_builder/element_spec.rb
@@ -320,6 +320,18 @@
         @data_locator = 'content'
       end
 
+      it 'negative' do
+        @selector = {tag_name: 'div', index: -7}
+        @wd_locator = {xpath: "(.//*[local-name()='div'])[last()-6]"}
+        @data_locator = 'second div'
+      end
+
+      it 'last' do
+        @selector = {tag_name: 'div', index: -1}
+        @wd_locator = {xpath: "(.//*[local-name()='div'])[last()]"}
+        @data_locator = 'content'
+      end
+
       it 'does not return index if it is zero' do
         @selector = {tag_name: 'div', index: 0}
         @wd_locator = {xpath: ".//*[local-name()='div']"}
@@ -545,12 +557,6 @@
         @remaining = {visible_text: 'foo'}
       end
 
-      it 'negative index' do
-        @selector = {tag_name: 'span', index: -1}
-        @wd_locator = {xpath: ".//*[local-name()='span']"}
-        @remaining = {index: -1}
-      end
-
       it 'raises exception when visible is not boolean', skip_after: true do
         selector = {visible: 'foo'}
         msg = 'expected boolean, got "foo":String'