ruby-changes:51030
From: kou <ko1@a...>
Date: Sun, 22 Apr 2018 18:38:13 +0900 (JST)
Subject: [ruby-changes:51030] kou:r63237 (trunk): rexml: Fix XPath bug of //#{ELEMENT_NAME}[#{POSITION}]
kou 2018-04-22 18:38:06 +0900 (Sun, 22 Apr 2018) New Revision: 63237 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63237 Log: rexml: Fix XPath bug of //#{ELEMENT_NAME}[#{POSITION}] The position should be counted for each nodeset but the previous implementation counts position for union-ed nodeset. For example, "/a/*/*[1]" should be matched to "<c1/>" and "<c2/>" with the following XML. <a> <b> <c1/> </b> <b> <c2/> </b> </a> But the previous implementation just returns only "<c1/>". * lib/rexml/element.rb (REXML::Attributes#each_attribute): Support Enumerator for no block use. * lib/rexml/element.rb (REXML::Attributes#each): Support Enumerator for no block use. * lib/rexml/functions.rb (REXML::Functions.string): Support NaN again. * lib/rexml/xpath_parser.rb: Re-implement "Step" evaluator. It should evaluate "AxisSpecifier", "NodeTest" and "Predicate" in one step to respect position for each nodeset. * test/rexml/test_jaxen.rb: Enable more tests. Remained tests should be also enabled but it'll not be near future. * test/rexml/xpath/test_base.rb: Fix expected value. Modified files: trunk/lib/rexml/element.rb trunk/lib/rexml/functions.rb trunk/lib/rexml/xpath_parser.rb trunk/test/rexml/test_jaxen.rb trunk/test/rexml/xpath/test_base.rb Index: lib/rexml/functions.rb =================================================================== --- lib/rexml/functions.rb (revision 63236) +++ lib/rexml/functions.rb (revision 63237) @@ -154,12 +154,16 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/functions.rb#L154 case object when Array string(object[0]) - when Numeric - integer = object.to_i - if object == integer - "%d" % integer + when Float + if object.nan? + "NaN" else - object.to_s + integer = object.to_i + if object == integer + "%d" % integer + else + object.to_s + end end when nil "" Index: lib/rexml/element.rb =================================================================== --- lib/rexml/element.rb (revision 63236) +++ lib/rexml/element.rb (revision 63237) @@ -1033,6 +1033,7 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/element.rb#L1033 # p attr.expanded_name+" => "+attr.value # } def each_attribute # :yields: attribute + return to_enum(__method__) unless block_given? each_value do |val| if val.kind_of? Attribute yield val @@ -1048,6 +1049,7 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/element.rb#L1049 # doc = Document.new '<a x="1" y="2"/>' # doc.root.attributes.each {|name, value| p name+" => "+value } def each + return to_enum(__method__) unless block_given? each_attribute do |attr| yield [attr.expanded_name, attr.value] end Index: lib/rexml/xpath_parser.rb =================================================================== --- lib/rexml/xpath_parser.rb (revision 63236) +++ lib/rexml/xpath_parser.rb (revision 63237) @@ -76,7 +76,7 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L76 def predicate path, nodeset path_stack = @parser.parse( path ) - expr( path_stack, nodeset ) + match( path_stack, nodeset ) end def []=( variable_name, value ) @@ -124,9 +124,18 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L124 end - def match( path_stack, nodeset ) - r = expr( path_stack, nodeset ) - r + def match(path_stack, nodeset) + nodeset = nodeset.collect.with_index do |node, i| + position = i + 1 + XPathNode.new(node, position: position) + end + result = expr(path_stack, nodeset) + case result + when Array # nodeset + unnode(result) + else + result + end end private @@ -149,11 +158,8 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L158 # Expr takes a stack of path elements and a set of nodes (either a Parent # or an Array and returns an Array of matching nodes - ALL = [ :attribute, :element, :text, :processing_instruction, :comment ] - ELEMENTS = [ :element ] def expr( path_stack, nodeset, context=nil ) # enter(:expr, path_stack, nodeset) - node_types = ELEMENTS return nodeset if path_stack.length == 0 || nodeset.length == 0 while path_stack.length > 0 # trace(:while, path_stack, nodeset) @@ -161,251 +167,167 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L167 path_stack.clear return [] end - case (op = path_stack.shift) + op = path_stack.shift + case op when :document - nodeset = [ nodeset[0].root_node ] - - when :qname - prefix = path_stack.shift - name = path_stack.shift - # enter(:qname, path_stack, prefix, name, nodeset) - nodeset.select! do |node| - if node.node_type == :element - if prefix.nil? - node.name == name - elsif prefix.empty? - node.name == name and node.namespace == "" - else - node.name == name and - # FIXME: This DOUBLES the time XPath searches take - node.namespace == get_namespace(node, prefix) - end - else - false - end - end - # leave(:qname, path_stack, nodeset) - node_types = ELEMENTS - - when :any - nodeset.delete_if { |node| !node_types.include?(node.node_type) } - + first_raw_node = nodeset.first.raw_node + nodeset = [XPathNode.new(first_raw_node.root_node, position: 1)] when :self - # This space left intentionally blank - - when :processing_instruction - target = path_stack.shift - nodeset.delete_if do |node| - (node.node_type != :processing_instruction) or - ( target!='' and ( node.target != target ) ) + nodeset = step(path_stack) do + [nodeset] end - - when :text - nodeset.delete_if { |node| node.node_type != :text } - - when :comment - nodeset.delete_if { |node| node.node_type != :comment } - - when :node - # This space left intentionally blank - node_types = ALL - when :child - new_nodeset = [] - nt = nil - nodeset.each do |node| - nt = node.node_type - # trace(:child, nt, node) - case nt - when :element - new_nodeset.concat(node.children) - when :document - node.children.each do |child| - case child - when XMLDecl, Text - # ignore - else - new_nodeset << child - end - end - end + nodeset = step(path_stack) do + child(nodeset) end - nodeset = new_nodeset - node_types = ELEMENTS - when :literal # trace(:literal, path_stack, nodeset) return path_stack.shift - when :attribute - new_nodeset = [] - case path_stack.shift - when :qname - prefix = path_stack.shift - name = path_stack.shift - for element in nodeset - if element.node_type == :element - attrib = element.attribute( name, get_namespace(element, prefix) ) - new_nodeset << attrib if attrib + nodeset = step(path_stack, any_type: :attribute) do + nodesets = [] + nodeset.each do |node| + raw_node = node.raw_node + next unless raw_node.node_type == :element + attributes = raw_node.attributes + next if attributes.empty? + nodesets << attributes.each_attribute.collect.with_index do |attribute, i| + XPathNode.new(attribute, position: i + 1) end end - when :any - for element in nodeset - if element.node_type == :element - new_nodeset += element.attributes.to_a + nodesets + end + when :namespace + pre_defined_namespaces = { + "xml" => "http://www.w3.org/XML/1998/namespace", + } + nodeset = step(path_stack, any_type: :namespace) do + nodesets = [] + nodeset.each do |node| + raw_node = node.raw_node + case raw_node.node_type + when :element + if @namespaces + nodesets << pre_defined_namespaces.merge(@namespaces) + else + nodesets << pre_defined_namespaces.merge(raw_node.namespaces) + end + when :attribute + if @namespaces + nodesets << pre_defined_namespaces.merge(@namespaces) + else + nodesets << pre_defined_namespaces.merge(raw_node.element.namespaces) + end end end + nodesets end - nodeset = new_nodeset - when :parent - new_nodeset = [] - nodeset.each do |node| - if node.is_a?(Attribute) - parent = node.element - else - parent = node.parent + nodeset = step(path_stack) do + nodesets = [] + nodeset.each do |node| + raw_node = node.raw_node + if raw_node.node_type == :attribute + parent = raw_node.element + else + parent = raw_node.parent + end + nodesets << [XPathNode.new(parent, position: 1)] if parent end - new_nodeset << parent if parent + nodesets end - nodeset = new_nodeset - node_types = ELEMENTS - when :ancestor - new_nodeset = [] - nodeset.each do |node| - while node.parent - node = node.parent - new_nodeset << node unless new_nodeset.include? node - end - end - nodeset = new_nodeset - node_types = ELEMENTS - - when :ancestor_or_self - new_nodeset = [] - nodeset.each do |node| - if node.node_type == :element - new_nodeset << node - while ( node.parent ) - node = node.parent - new_nodeset << node unless new_nodeset.include? node + nodeset = step(path_stack) do + nodesets = [] + # new_nodes = {} + nodeset.each do |node| + raw_node = node.raw_node + new_nodeset = [] + while raw_node.parent + raw_node = raw_node.parent + # next if new_nodes.key?(node) + new_nodeset << XPathNode.new(raw_node, + position: new_nodeset.size + 1) + # new_nodes[node] = true end + nodesets << new_nodeset unless new_nodeset.empty? end + nodesets end - nodeset = new_nodeset - node_types = ELEMENTS - - when :predicate - new_nodeset = [] - subcontext = { :size => nodeset.size } - pred = path_stack.shift - # enter(:predicate, pred, nodeset) - nodeset.each_with_index { |node, index| - subcontext[ :node ] = node - subcontext[ :index ] = index+1 - pc = pred.dclone - result = expr( pc, [node], subcontext ) - result = result[0] if result.kind_of? Array and result.length == 1 - if result.kind_of? Numeric - new_nodeset << node if result == (index+1) - elsif result.instance_of? Array - if result.size > 0 and result.inject(false) {|k,s| s or k} - new_nodeset << node if result.size > 0 + when :ancestor_or_self + nodeset = step(path_stack) do + nodesets = [] + # new_nodes = {} + nodeset.each do |node| + raw_node = node.raw_node + next unless raw_node.node_type == :element + new_nodeset = [XPathNode.new(raw_node, position: 1)] + # new_nodes[node] = true + while raw_node.parent + raw_node = raw_node.parent + # next if new_nodes.key?(node) + new_nodeset << XPathNode.new(raw_node, + position: new_nodeset.size + 1) + # new_nodes[node] = true end - else - new_nodeset << node if result + nodesets << new_nodeset unless new_nodeset.empty? end - } - nodeset = new_nodeset - # leave(:predicate_return, nodeset) -=begin - predicate = path_stack.shift - ns = nodeset.clone - result = expr( predicate, ns ) - if result.kind_of? Array - nodeset = result.zip(ns).collect{|m,n| n if m}.compact - else - nodeset = result ? nodeset : [] + nodesets end -=end - when :descendant_or_self - rv = descendant_or_self( path_stack, nodeset ) - path_stack.clear - nodeset = rv - node_types = ELEMENTS - + nodeset = step(path_stack) do + descendant(nodeset, true) + end when :descendant - results = [] - nt = nil - nodeset.each do |node| - nt = node.node_type - results += expr( path_stack.dclone.unshift( :descendant_or_self ), - node.children ) if nt == :element or nt == :document + nodeset = step(path_stack) do + descendant(nodeset, false) end - nodeset = results - node_types = ELEMENTS - when :following_sibling - results = [] - nodeset.each do |node| - next if node.parent.nil? - all_siblings = node.parent.children - current_index = all_siblings.index( node ) - following_siblings = all_siblings[ current_index+1 .. -1 ] - results += following_siblings + nodeset = step(path_stack) do + nodesets = [] + nodeset.each do |node| + raw_node = node.raw_node + next unless raw_node.respond_to?(:parent) + next if raw_node.parent.nil? + all_siblings = raw_node.parent.children + current_index = all_siblings.index(raw_node) + following_siblings = all_siblings[(current_index + 1)..-1] + next if following_siblings.empty? + nodesets << following_siblings.collect.with_index do |sibling, i| + XPathNode.new(sibling, position: i + 1) + end + end + nodesets end - nodeset = results - node_types = ELEMENTS - when :preceding_sibling - results = [] - nodeset.each do |node| - next if node.parent.nil? - all_siblings = node.parent.children - current_index = all_siblings.index( node ) - preceding_siblings = all_siblings[ 0, current_index ].reverse - results += preceding_siblings + nodeset = step(path_stack, order: :reverse) do + nodesets = [] + nodeset.each do |node| + raw_node = node.raw_node + next unless raw_node.respond_to?(:parent) + next if raw_node.parent.nil? + all_siblings = raw_node.parent.children + current_index = all_siblings.index(raw_node) + preceding_siblings = all_siblings[0, current_index].reverse + next if preceding_siblings.empty? + nodesets << preceding_siblings.collect.with_index do |sibling, i| + XPathNode.new(sibling, position: i + 1) + end + end + nodesets end - nodeset = results - node_types = ELEMENTS - when :preceding - new_nodeset = [] - nodeset.each do |node| - new_nodeset += preceding( node ) + nodeset = step(path_stack, order: :reverse) do + unnode(nodeset) do |node| + preceding(node) + end end - nodeset = new_nodeset - node_types = ELEMENTS - when :following - new_nodeset = [] - nodeset.each do |node| - new_nodeset += following( node ) - end - nodeset = new_nodeset - node_types = ELEMENTS - - when :namespace - new_nodeset = [] - prefix = path_stack.shift - nodeset.each do |node| - if (node.node_type == :element or node.node_type == :attribute) - if @namespaces - namespaces = @namespaces - elsif (node.node_type == :element) - namespaces = node.namespaces - else - namespaces = node.element.namesapces - end - if (node.namespace == namespaces[prefix]) - new_nodeset << node - end + nodeset = step(path_stack) do + unnode(nodeset) do |node| + following(node) end end - nodeset = new_nodeset - when :variable var_name = path_stack.shift return [@variables[var_name]] @@ -431,40 +353,37 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L353 res = equality_relational_compare( left, op, right ) return res - when :div - left = Functions::number(expr(path_stack.shift, nodeset, context)).to_f - right = Functions::number(expr(path_stack.shift, nodeset, context)).to_f - return (left / right) - - when :mod - left = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - right = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - return (left % right) - - when :mult - left = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - right = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - return (left * right) - - when :plus - left = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - right = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - return (left + right) - - when :minus - left = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - right = Functions::number(expr(path_stack.shift, nodeset, context )).to_f - return (left - right) - + when :div, :mod, :mult, :plus, :minus + left = expr(path_stack.shift, nodeset, context) + right = expr(path_stack.shift, nodeset, context) + left = unnode(left) if left.is_a?(Array) + right = unnode(right) if right.is_a?(Array) + left = Functions::number(left) + right = Functions::number(right) + case op + when :div + return left / right + when :mod + return left % right + when :mult + return left * right + when :plus + return left + right + when :minus + return left - right + else + raise "[BUG] Unexpected operator: <#{op.inspect}>" + end when :union left = expr( path_stack.shift, nodeset, context ) right = expr( path_stack.shift, nodeset, context ) + left = unnode(left) if left.is_a?(Array) + right = unnode(right) if right.is_a?(Array) return (left | right) - when :neg res = expr( path_stack, nodeset, context ) - return -(res.to_f) - + res = unnode(res) if res.is_a?(Array) + return -Functions.number(res) when :not when :function func_name = path_stack.shift.tr('-','_') @@ -473,21 +392,30 @@ module REXML https://github.com/ruby/ruby/blob/trunk/lib/rexml/xpath_parser.rb#L392 res = [] cont = context - nodeset.each_with_index { |n, i| + nodeset.each_with_index do |node, i| if subcontext - subcontext[:node] = n - subcontext[:index] = i + if node.is_a?(XPathNode) + subcontext[:node] = node.raw_node + subcontext[:index] = node.position + else + subcontext[:node] = node + subcontext[:index] = i + (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/