Optimize the parse_attributes method to use Source#match to parse XML. (#119)

## Why? Improve maintainability by consolidating processing into `Source#match`. ## Benchmark ``` RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.0/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [arm64-darwin22] Calculating ------------------------------------- before after before(YJIT) after(YJIT) dom 10.891 10.622 16.356 17.403 i/s - 100.000 times in 9.182130s 9.414177s 6.113806s 5.746133s sax 30.335 29.845 49.749 54.877 i/s - 100.000 times in 3.296483s 3.350595s 2.010071s 1.822259s pull 35.514 34.801 61.123 66.908 i/s - 100.000 times in 2.815793s 2.873484s 1.636041s 1.494591s stream 35.141 34.475 52.110 56.836 i/s - 100.000 times in 2.845646s 2.900638s 1.919017s 1.759456s Comparison: dom after(YJIT): 17.4 i/s before(YJIT): 16.4 i/s - 1.06x slower before: 10.9 i/s - 1.60x slower after: 10.6 i/s - 1.64x slower sax after(YJIT): 54.9 i/s before(YJIT): 49.7 i/s - 1.10x slower before: 30.3 i/s - 1.81x slower after: 29.8 i/s - 1.84x slower pull after(YJIT): 66.9 i/s before(YJIT): 61.1 i/s - 1.09x slower before: 35.5 i/s - 1.88x slower after: 34.8 i/s - 1.92x slower stream after(YJIT): 56.8 i/s before(YJIT): 52.1 i/s - 1.09x slower before: 35.1 i/s - 1.62x slower after: 34.5 i/s - 1.65x slower ``` - YJIT=ON : 1.06x - 1.10x faster - YJIT=OFF : 0.97x - 0.98x faster

Author: naitoh

lib/rexml/parsers/baseparser.rb

@@ -114,7 +114,7 @@ class BaseParser
 
  module Private
  INSTRUCTION_END = /#{NAME}(\s+.*?)?\?>/um
- TAG_PATTERN = /((?>#{QNAME_STR}))/um
+ TAG_PATTERN = /((?>#{QNAME_STR}))\s*/um
  CLOSE_PATTERN = /(#{QNAME_STR})\s*>/um
  ATTLISTDECL_END = /\s+#{NAME}(?:#{ATTDEF})*\s*>/um
  NAME_PATTERN = /\s*#{NAME}/um
@@ -128,7 +128,6 @@ module Private
  def initialize( source )
  self.stream = source
  @listeners = []
- @attributes_scanner = StringScanner.new('')
  end
 
  def add_listener( listener )
@@ -614,87 +613,60 @@ def process_instruction(start_position)
  def parse_attributes(prefixes, curr_ns)
  attributes = {}
  closed = false
- match_data = @source.match(/^(.*?)(\/)?>/um, true)
- if match_data.nil?
- message = "Start tag isn't ended"
- raise REXML::ParseException.new(message, @source)
- end
-
- raw_attributes = match_data[1]
- closed = !match_data[2].nil?
- return attributes, closed if raw_attributes.nil?
- return attributes, closed if raw_attributes.empty?
-
- @attributes_scanner.string = raw_attributes
- scanner = @attributes_scanner
- until scanner.eos?
- if scanner.scan(/\s+/)
- break if scanner.eos?
- end
-
- start_position = scanner.pos
- while true
- break if scanner.scan(ATTRIBUTE_PATTERN)
- unless scanner.scan(QNAME)
- message = "Invalid attribute name: <#{scanner.rest}>"
- raise REXML::ParseException.new(message, @source)
- end
- name = scanner[0]
- unless scanner.scan(/\s*=\s*/um)
+ while true
+ if @source.match(">", true)
+ return attributes, closed
+ elsif @source.match("/>", true)
+ closed = true
+ return attributes, closed
+ elsif match = @source.match(QNAME, true)
+ name = match[1]
+ prefix = match[2]
+ local_part = match[3]
+
+ unless @source.match(/\s*=\s*/um, true)
  message = "Missing attribute equal: <#{name}>"
  raise REXML::ParseException.new(message, @source)
  end
- quote = scanner.scan(/['"]/)
- unless quote
- message = "Missing attribute value start quote: <#{name}>"
- raise REXML::ParseException.new(message, @source)
- end
- unless scanner.scan(/.*#{Regexp.escape(quote)}/um)
- @source.ensure_buffer
- match_data = @source.match(/^(.*?)(\/)?>/um, true)
- if match_data
- scanner << "/" if closed
- scanner << ">"
- scanner << match_data[1]
- scanner.pos = start_position
- closed = !match_data[2].nil?
- next
+ unless match = @source.match(/(['"])(.*?)\1\s*/um, true)
+ if match = @source.match(/(['"])/, true)
+ message =
+ "Missing attribute value end quote: <#{name}>: <#{match[1]}>"
+ raise REXML::ParseException.new(message, @source)
+ else
+ message = "Missing attribute value start quote: <#{name}>"
+ raise REXML::ParseException.new(message, @source)
  end
- message =
- "Missing attribute value end quote: <#{name}>: <#{quote}>"
- raise REXML::ParseException.new(message, @source)
  end
- end
- name = scanner[1]
- prefix = scanner[2]
- local_part = scanner[3]
- # quote = scanner[4]
- value = scanner[5]
- if prefix == "xmlns"
- if local_part == "xml"
- if value != "http://www.w3.org/XML/1998/namespace"
- msg = "The 'xml' prefix must not be bound to any other namespace "+
+  value = match[2]
+  if prefix == "xmlns"
+  if local_part == "xml"
+  if value != "http://www.w3.org/XML/1998/namespace"
+  msg = "The 'xml' prefix must not be bound to any other namespace "+
+  "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
+  raise REXML::ParseException.new( msg, @source, self )
+  end
+ elsif local_part == "xmlns"
+ msg = "The 'xmlns' prefix must not be declared "+
  "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
- raise REXML::ParseException.new( msg, @source, self )
+ raise REXML::ParseException.new( msg, @source, self)
  end
- elsif local_part == "xmlns"
- msg = "The 'xmlns' prefix must not be declared "+
- "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
- raise REXML::ParseException.new( msg, @source, self)
+ curr_ns << local_part
+ elsif prefix
+ prefixes << prefix unless prefix == "xml"
  end
- curr_ns << local_part
- elsif prefix
- prefixes << prefix unless prefix == "xml"
- end
 
- if attributes.has_key?(name)
- msg = "Duplicate attribute #{name.inspect}"
- raise REXML::ParseException.new(msg, @source, self)
- end
+  if attributes.has_key?(name)
+  msg = "Duplicate attribute #{name.inspect}"
+  raise REXML::ParseException.new(msg, @source, self)
+  end
 
- attributes[name] = value
+ attributes[name] = value
+ else
+ message = "Invalid attribute name: <#{@source.buffer.split(%r{[/>\s]}).first}>"
+ raise REXML::ParseException.new(message, @source)
+ end
  end
- return attributes, closed
  end
  end
  end

test/parse/test_element.rb

@@ -41,9 +41,9 @@ def test_empty_namespace_attribute_name
  assert_equal(<<-DETAIL.chomp, exception.to_s)
 Invalid attribute name: <:a="">
 Line: 1
-Position: 9
+Position: 13
 Last 80 unconsumed characters:
-
+:a=""></x>
  DETAIL
  end
 

test/test_core.rb

@@ -116,11 +116,12 @@ def test_attribute
 
  def test_attribute_namespace_conflict
  # https://www.w3.org/TR/xml-names/#uniqAttrs
- message = <<-MESSAGE
+ message = <<-MESSAGE.chomp
 Duplicate attribute "a"
 Line: 4
 Position: 140
 Last 80 unconsumed characters:
+/>
  MESSAGE
  assert_raise(REXML::ParseException.new(message)) do
  Document.new(<<-XML)
@@ -1323,11 +1324,26 @@ def test_ticket_21
  exception = assert_raise(ParseException) do
  Document.new(src)
  end
- assert_equal(<<-DETAIL, exception.to_s)
+ assert_equal(<<-DETAIL.chomp, exception.to_s)
 Missing attribute value start quote: <bar>
 Line: 1
 Position: 16
 Last 80 unconsumed characters:
+value/>
+ DETAIL
+ end
+
+ def test_parse_exception_on_missing_attribute_end_quote
+ src = '<foo bar="value/>'
+ exception = assert_raise(ParseException) do
+ Document.new(src)
+ end
+ assert_equal(<<-DETAIL.chomp, exception.to_s)
+Missing attribute value end quote: <bar>: <">
+Line: 1
+Position: 17
+Last 80 unconsumed characters:
+value/>
  DETAIL
  end