From 05ed61b1703f49a85eb7b7a21048c51e7b066b32 Mon Sep 17 00:00:00 2001 From: tompng Date: Thu, 1 May 2025 02:26:10 +0900 Subject: [PATCH 1/4] Drop accepting array as an element in XPath.match, first and each XPath.match, first, each accepted array as an element. This behavior is not documented, and making hard to optimize and refactor. The second argument of XPathParser#parse and XPathParser#match is also changed from nodeset to node --- lib/rexml/xpath.rb | 3 --- lib/rexml/xpath_parser.rb | 19 ++++++++----------- test/test_jaxen.rb | 13 +++++++------ test/xpath/test_base.rb | 7 ++++--- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/lib/rexml/xpath.rb b/lib/rexml/xpath.rb index a0921bd8..666d764f 100644 --- a/lib/rexml/xpath.rb +++ b/lib/rexml/xpath.rb @@ -35,7 +35,6 @@ def XPath::first(element, path=nil, namespaces=nil, variables={}, options={}) parser.namespaces = namespaces parser.variables = variables path = "*" unless path - element = [element] unless element.kind_of? Array parser.parse(path, element).flatten[0] end @@ -64,7 +63,6 @@ def XPath::each(element, path=nil, namespaces=nil, variables={}, options={}, &bl parser.namespaces = namespaces parser.variables = variables path = "*" unless path - element = [element] unless element.kind_of? Array parser.parse(path, element).each( &block ) end @@ -74,7 +72,6 @@ def XPath::match(element, path=nil, namespaces=nil, variables={}, options={}) parser.namespaces = namespaces parser.variables = variables path = "*" unless path - element = [element] unless element.kind_of? Array parser.parse(path,element) end end diff --git a/lib/rexml/xpath_parser.rb b/lib/rexml/xpath_parser.rb index 5eb1e5a9..533877e5 100644 --- a/lib/rexml/xpath_parser.rb +++ b/lib/rexml/xpath_parser.rb @@ -76,19 +76,19 @@ def variables=( vars={} ) @variables = vars end - def parse path, nodeset + def parse path, node path_stack = @parser.parse( path ) - match( path_stack, nodeset ) + match( path_stack, node ) end - def get_first path, nodeset + def get_first path, node path_stack = @parser.parse( path ) - first( path_stack, nodeset ) + first( path_stack, node ) end - def predicate path, nodeset + def predicate path, node path_stack = @parser.parse( path ) - match( path_stack, nodeset ) + match( path_stack, node ) end def []=( variable_name, value ) @@ -136,11 +136,8 @@ def first( path_stack, node ) end - def match(path_stack, nodeset) - nodeset = nodeset.collect.with_index do |node, i| - position = i + 1 - XPathNode.new(node, position: position) - end + def match(path_stack, node) + nodeset = [XPathNode.new(node, position: 1)] result = expr(path_stack, nodeset) case result when Array # nodeset diff --git a/test/test_jaxen.rb b/test/test_jaxen.rb index 6038e88e..7fb3e27a 100644 --- a/test/test_jaxen.rb +++ b/test/test_jaxen.rb @@ -56,7 +56,9 @@ def process_test_case(name) # processes a tests/document/context node def process_context(doc, context) - test_context = XPath.match(doc, context.attributes["select"]) + matched = XPath.match(doc, context.attributes["select"]) + assert_equal(matched.size, 1) + test_context = matched.first namespaces = context.namespaces namespaces.delete("var") namespaces = nil if namespaces.empty? @@ -104,7 +106,8 @@ def process_nominal_test(context, variables, namespaces, test) end XPath.each(test, "valueOf") do |value_of| - process_value_of(matched, variables, namespaces, value_of) + assert_equal(1, matched.size) + process_value_of(matched.first, variables, namespaces, value_of) end end @@ -118,10 +121,8 @@ def process_exceptional_test(context, variables, namespaces, test) def user_message(context, xpath, matched) message = "" - context.each_with_index do |node, i| - message << "Node#{i}:\n" - message << "#{node}\n" - end + message << "Node:\n" + message << "#{context}\n" message << "XPath: <#{xpath}>\n" message << "Matched <#{matched}>" message diff --git a/test/xpath/test_base.rb b/test/xpath/test_base.rb index 1dacd69d..c58a571c 100644 --- a/test/xpath/test_base.rb +++ b/test/xpath/test_base.rb @@ -411,9 +411,10 @@ def test_preceding s = "" d = REXML::Document.new(s) - c = REXML::XPath.match( d, "//c[@id = '5']") - cs = REXML::XPath.match( c, "preceding::c" ) - assert_equal( 4, cs.length ) + c = REXML::XPath.match(d, "//c[@id = '5']") + assert_equal(1, c.length) + cs = REXML::XPath.match(c.first, "preceding::c") + assert_equal(4, cs.length) end def test_following From 4de2ac12c91bb9d0fe2a8f1b68469301e6a990ff Mon Sep 17 00:00:00 2001 From: tomoya ishida Date: Mon, 5 May 2025 02:33:15 +0900 Subject: [PATCH 2/4] Apply suggestions of fixing test Co-authored-by: Sutou Kouhei --- test/test_jaxen.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/test_jaxen.rb b/test/test_jaxen.rb index 7fb3e27a..2fd0abf1 100644 --- a/test/test_jaxen.rb +++ b/test/test_jaxen.rb @@ -57,7 +57,7 @@ def process_test_case(name) # processes a tests/document/context node def process_context(doc, context) matched = XPath.match(doc, context.attributes["select"]) - assert_equal(matched.size, 1) + assert_equal(1, matched.size) test_context = matched.first namespaces = context.namespaces namespaces.delete("var") @@ -106,8 +106,9 @@ def process_nominal_test(context, variables, namespaces, test) end XPath.each(test, "valueOf") do |value_of| - assert_equal(1, matched.size) - process_value_of(matched.first, variables, namespaces, value_of) + matched.each do |subcontext| + process_value_of(subcontext, variables, namespaces, value_of) + end end end From eccf0f50863271c06fa5438967101490159e909a Mon Sep 17 00:00:00 2001 From: tompng Date: Mon, 5 May 2025 02:35:59 +0900 Subject: [PATCH 3/4] Ensure matching nodes for valueOf assertion to be always present --- test/test_jaxen.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_jaxen.rb b/test/test_jaxen.rb index 2fd0abf1..548120d6 100644 --- a/test/test_jaxen.rb +++ b/test/test_jaxen.rb @@ -103,6 +103,8 @@ def process_nominal_test(context, variables, namespaces, test) assert_equal(Integer(expected, 10), matched.size, user_message(context, xpath, matched)) + else + assert_operator(matched.size, :>, 0, user_message(context, xpath, matched)) end XPath.each(test, "valueOf") do |value_of| From e2968dc5cf3ec28f544c5da14540f31986fc7927 Mon Sep 17 00:00:00 2001 From: tompng Date: Mon, 5 May 2025 02:46:22 +0900 Subject: [PATCH 4/4] Add a deprecation warning for REXML::XPath.each/find/match with nodeset --- lib/rexml/xpath_parser.rb | 5 +++++ test/xpath/test_base.rb | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/rexml/xpath_parser.rb b/lib/rexml/xpath_parser.rb index 533877e5..3bf796ae 100644 --- a/lib/rexml/xpath_parser.rb +++ b/lib/rexml/xpath_parser.rb @@ -137,6 +137,11 @@ def first( path_stack, node ) def match(path_stack, node) + if node.is_a?(Array) + Kernel.warn("REXML::XPath.each, REXML::XPath.first, REXML::XPath.match dropped support for nodeset...", uplevel: 1) + return [] if node.empty? + node = node.first + end nodeset = [XPathNode.new(node, position: 1)] result = expr(path_stack, nodeset) case result diff --git a/test/xpath/test_base.rb b/test/xpath/test_base.rb index c58a571c..354a31b1 100644 --- a/test/xpath/test_base.rb +++ b/test/xpath/test_base.rb @@ -1159,5 +1159,15 @@ def test_or_and end assert_equal(["/"], hrefs, "Bug #3842 [ruby-core:32447]") end + + def test_match_with_deprecated_usage + verbose, $VERBOSE = $VERBOSE, nil + doc = Document.new("") + assert_equal(['b'], XPath.match([doc, doc], '//b').map(&:name)) + assert_equal(['b'], XPath.match([doc], '//b').map(&:name)) + assert_equal([], XPath.match([], '//b').map(&:name)) + ensure + $VERBOSE = verbose + end end end