From e3e53981ee7ca5c7c027db3517c54f1b667c5609 Mon Sep 17 00:00:00 2001 From: Conor Bronsdon Date: Tue, 7 Jul 2026 21:38:53 -0700 Subject: [PATCH 1/2] Parse errors carry line/column + a context snippet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New src/xml/errors.mojo (exported from the package): - line_col(source: Span[UInt8, _], offset: Int) -> Tuple[Int, Int] — 1-based line and column from a byte offset; the column is the 1-based BYTE offset within the line (no UTF-8 decode, deterministic). Clamps out-of-range offsets; only LF terminates a line, so CRLF adds no phantom column. - parse_error(msg, source, offset) -> Error — message is exactly " at line , column : " with the snippet a ~30-byte whitespace-trimmed window of the offending line centered on the column, ... where truncated, nudged off UTF-8 continuation bytes, never multi-line. Wired into every pull-parser raise site with a genuine byte offset: strict errors (previously a bare parenthesized location, no snippet) and the liberal-mode structural errors (previously no position): unterminated construct/start tag/attribute/attribute value, unquoted attribute value, malformed start/end tag, empty element name. Error mechanism unchanged — still raise Error(...). 23 new tests (line_col edge cases, exact parse_error formats, 3 hand-verified malformed-XML integration positions); suite now 61 pull + 74 etree + 23 errors, anchor + short fuzz green. Co-Authored-By: Claude Fable 5 --- .github/workflows/test.yml | 3 + CHANGELOG.md | 18 ++++ pixi.toml | 2 +- src/xml/__init__.mojo | 4 + src/xml/errors.mojo | 135 ++++++++++++++++++++++++ src/xml/pull.mojo | 73 ++++++------- test/test_errors.mojo | 211 +++++++++++++++++++++++++++++++++++++ test/test_pull.mojo | 2 +- 8 files changed, 408 insertions(+), 40 deletions(-) create mode 100644 src/xml/errors.mojo create mode 100644 test/test_errors.mojo diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4c5d844..1618fe8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,6 +34,9 @@ jobs: - name: Unit tests — XML pull parser run: .venv/bin/mojo run -I src test/test_pull.mojo + - name: Unit tests — parse-error positions + run: .venv/bin/mojo run -I src test/test_errors.mojo + - name: Conformance — byte-match vs CPython xml.etree run: MOJO=.venv/bin/mojo python3 test/anchor_run.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 478357e..e494d0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,23 @@ # Changelog +## Unreleased + +- New `xml.errors` module (exported from the package): `line_col(source, + offset)` maps a byte offset to a 1-based (line, column) pair — the column + is the 1-based BYTE offset within the line, no UTF-8 decoding — and + `parse_error(msg, source, offset)` builds an `Error` reading + ` at line , column : ''`, where the snippet is up to + ~30 bytes of the offending line centered on the column, + whitespace-trimmed, with `...` where truncated, and never multi-line. +- Pull-parser errors now carry that position + snippet wherever a byte + offset exists at the raise site: every strict-mode error (previously a + bare `(line L, column C)` suffix with no snippet) and the structural + errors both modes raise — unterminated constructs / start tags / + attributes / attribute values, unquoted attribute values, malformed + start/end tags, and empty element names (previously no position at all). +- No mechanism change: parsers still `raise Error(...)`, no new error + types, and existing `contains=`-style message checks keep matching. + ## v0.1.0 — 2026-07-06 First release. General-purpose XML parsing in pure Mojo, mirroring Python's diff --git a/pixi.toml b/pixi.toml index 50f701f..59da865 100644 --- a/pixi.toml +++ b/pixi.toml @@ -7,7 +7,7 @@ platforms = ["linux-64", "osx-arm64"] version = "0.1.0" [tasks] -test = "mojo run -I src test/test_etree.mojo && mojo run -I src test/test_pull.mojo" +test = "mojo run -I src test/test_etree.mojo && mojo run -I src test/test_pull.mojo && mojo run -I src test/test_errors.mojo" demo = "mojo run -I src examples/catalog.mojo" bench = "mojo build -I src bench/bench_parse.mojo -o .bench_parse && ./.bench_parse" fmt = "mojo format src/ test/ examples/ bench/" diff --git a/src/xml/__init__.mojo b/src/xml/__init__.mojo index e07f7bf..e5dd418 100644 --- a/src/xml/__init__.mojo +++ b/src/xml/__init__.mojo @@ -16,3 +16,7 @@ from xml.etree import ( tostring, SubElement, ) +from xml.errors import ( + line_col, + parse_error, +) diff --git a/src/xml/errors.mojo b/src/xml/errors.mojo new file mode 100644 index 0000000..a9c0039 --- /dev/null +++ b/src/xml/errors.mojo @@ -0,0 +1,135 @@ +"""Position-aware parse errors. + +`line_col` maps a byte offset in a source buffer to a 1-based +(line, column) pair, and `parse_error` builds an `Error` whose message +carries that position plus a short snippet of the offending line: + + at line , column : '' + +Positions are byte-based: the column is the 1-based BYTE offset within +the line, not a codepoint or display column. That keeps the computation +deterministic and free of UTF-8 decode cost; for ASCII-heavy markup the +byte column and the visual column coincide. + +This module is the error-reporting pattern shared across the mojo-* +parser suite. +""" + +comptime _LF = UInt8(0x0A) +comptime _CR = UInt8(0x0D) + +# Snippet size budget, in bytes, before the `...` truncation markers are +# added. Wide enough to show meaningful context, narrow enough that error +# messages stay one readable line. +comptime _SNIPPET_BUDGET = 30 + + +def _is_ws(b: UInt8) -> Bool: + return b == 0x20 or b == 0x09 or b == _CR or b == _LF + + +def line_col(source: Span[UInt8, _], offset: Int) -> Tuple[Int, Int]: + """1-based (line, column) of byte `offset` in `source`. + + The column is the 1-based BYTE offset within the line — codepoints + are never decoded, so the result is cheap and deterministic even on + invalid UTF-8. Only LF (0x0A) terminates a line: after a CRLF + sequence the next byte is column 1 of the next line, with no phantom + column contributed by the CR. An offset pointing AT an LF reports + the line that newline terminates (column = line length + 1). Offsets + past the end of `source` clamp to the end; an empty source yields + (1, 1). + """ + var limit = offset + if limit > len(source): + limit = len(source) + if limit < 0: + limit = 0 + var line = 1 + var last_nl = -1 + for i in range(limit): + if source[i] == _LF: + line += 1 + last_nl = i + return (line, limit - last_nl) + + +def _snippet(source: Span[UInt8, _], offset: Int) -> String: + """Up to ~`_SNIPPET_BUDGET` bytes of the line containing `offset`. + + The line is trimmed of leading/trailing whitespace (which also drops + the CR of a CRLF line ending), then windowed around the offset with + `...` marking each side that was cut. Window edges are nudged off + UTF-8 continuation bytes so the result is always valid UTF-8. The + result never contains a newline. + """ + var n = len(source) + var anchor = offset + if anchor > n: + anchor = n + if anchor < 0: + anchor = 0 + # Line bounds around the anchor; an anchor sitting AT an LF belongs + # to the line that newline terminates. + var line_start = anchor + while line_start > 0 and source[line_start - 1] != _LF: + line_start -= 1 + var line_end = anchor + while line_end < n and source[line_end] != _LF: + line_end += 1 + # Trim surrounding whitespace. + while line_start < line_end and _is_ws(source[line_start]): + line_start += 1 + while line_end > line_start and _is_ws(source[line_end - 1]): + line_end -= 1 + var win_start = line_start + var win_end = line_end + var cut_left = False + var cut_right = False + if line_end - line_start > _SNIPPET_BUDGET: + win_start = anchor - _SNIPPET_BUDGET // 2 + if win_start > line_end - _SNIPPET_BUDGET: + win_start = line_end - _SNIPPET_BUDGET + if win_start < line_start: + win_start = line_start + win_end = win_start + _SNIPPET_BUDGET + # Never split a multi-byte UTF-8 sequence at a window edge. + while win_start < win_end and (source[win_start] & 0xC0) == 0x80: + win_start += 1 + while win_end < line_end and (source[win_end] & 0xC0) == 0x80: + win_end += 1 + cut_left = win_start > line_start + cut_right = win_end < line_end + var out = String() + if cut_left: + out += "..." + out += String(StringSlice(unsafe_from_utf8=source[win_start:win_end])) + if cut_right: + out += "..." + return out^ + + +def parse_error(msg: String, source: Span[UInt8, _], offset: Int) -> Error: + """An `Error` locating `msg` at byte `offset` of `source`. + + The message is exactly: + + at line , column : '' + + where line/column come from `line_col` (1-based; column is a byte + offset within the line) and the snippet is the offending line, + whitespace-trimmed and truncated to ~30 bytes centered on the + column, with `...` where truncated. The message never contains a + newline, so it renders on one line in test output and logs. + """ + var lc = line_col(source, offset) + return Error( + msg + + " at line " + + String(lc[0]) + + ", column " + + String(lc[1]) + + ": '" + + _snippet(source, offset) + + "'" + ) diff --git a/src/xml/pull.mojo b/src/xml/pull.mojo index d3c353e..c5765dc 100644 --- a/src/xml/pull.mojo +++ b/src/xml/pull.mojo @@ -20,6 +20,8 @@ CPython's `xml.etree` for self-contained documents; parameter entities access — no XXE surface). """ +from xml.errors import parse_error + comptime EVENT_START = 0 comptime EVENT_END = 1 comptime EVENT_TEXT = 2 @@ -458,10 +460,13 @@ struct XmlPullParser(Copyable, Movable): unknown entities, invalid element/attribute names, valueless or duplicate attributes, a raw `<` in an attribute value, a literal `]]>` in character data, `--` inside a comment, and out-of-Char-production - character references all raise with a line/column location instead of - being liberally recovered. Useful for debugging a feed you produce; - leave it off for feeds you merely consume (liberal mode stays - deliberately tolerant of these). + character references all raise instead of being liberally recovered. + Useful for debugging a feed you produce; leave it off for feeds you + merely consume (liberal mode stays deliberately tolerant of these). + + Errors — strict-mode and the structural ones both modes raise — carry + a `line L, column C` location plus a snippet of the offending line + (see `xml.errors.parse_error`). """ var src: String @@ -481,30 +486,14 @@ struct XmlPullParser(Copyable, Movable): self._open = List[String]() self._entities = Dict[String, String]() - def _location(self, p: Int) -> String: - """Human-readable "line L, column C" for byte offset `p`. - - Computed lazily (only on error paths), so the happy path pays - nothing for location tracking. - """ - var bytes = self.src.as_bytes() - var line = 1 - var col = 1 - var limit = p - if limit > len(bytes): - limit = len(bytes) - for i in range(limit): - if bytes[i] == 0x0A: - line += 1 - col = 1 - else: - col += 1 - return String("line ") + String(line) + ", column " + String(col) - def _strict_error(self, msg: String, p: Int) -> Error: - return Error( - "mojo-xml [strict]: " + msg + " (" + self._location(p) + ")" - ) + # Position + snippet are computed lazily (only on error paths), + # so the happy path pays nothing for location tracking. + return parse_error("mojo-xml [strict]: " + msg, self.src.as_bytes(), p) + + def _error(self, msg: String, p: Int) -> Error: + """A positioned parse error (both strict and liberal mode).""" + return parse_error("mojo-xml: " + msg, self.src.as_bytes(), p) def _len(self) -> Int: return self.src.byte_length() @@ -527,15 +516,20 @@ struct XmlPullParser(Copyable, Movable): return True def _find(self, start: Int, literal: StaticString) raises -> Int: - """Byte offset of `literal` at or after `start`, or raises.""" + """Byte offset of `literal` at or after `start`, or raises. + + The error position is `start` — the beginning of the search, + i.e. just inside the construct that was never terminated — + rather than the uninformative end of input. + """ var i = start while i < self._len(): if self._starts_with(i, literal): return i i += 1 - raise Error( - String("mojo-xml: unterminated construct, expected: ") - + String(literal) + raise self._error( + String("unterminated construct, expected: ") + String(literal), + start, ) def _skip_space(mut self): @@ -722,7 +716,7 @@ struct XmlPullParser(Copyable, Movable): while True: self._skip_space() if self.pos >= self._len(): - raise Error("mojo-xml: unterminated start tag") + raise self._error("unterminated start tag", self.pos) var b = self._at(self.pos) if b == _GT or b == _SLASH: return attrs^ @@ -739,16 +733,19 @@ struct XmlPullParser(Copyable, Movable): self.pos += 1 self._skip_space() if self.pos >= self._len(): - raise Error("mojo-xml: unterminated attribute") + raise self._error("unterminated attribute", name_pos) var quote = self._at(self.pos) if quote != _SQUOTE and quote != _DQUOTE: - raise Error("mojo-xml: unquoted attribute value") + raise self._error("unquoted attribute value", self.pos) self.pos += 1 var vstart = self.pos while self.pos < self._len() and self._at(self.pos) != quote: self.pos += 1 if self.pos >= self._len(): - raise Error("mojo-xml: unterminated attribute value") + # Point at the opening quote that was never closed. + raise self._error( + "unterminated attribute value", vstart - 1 + ) if self.strict: # A raw '<' is never allowed in an attribute value. for k in range(vstart, self.pos): @@ -922,7 +919,7 @@ struct XmlPullParser(Copyable, Movable): var name = self._read_name() self._skip_space() if self.pos >= self._len() or self._at(self.pos) != _GT: - raise Error("mojo-xml: malformed end tag: " + name) + raise self._error("malformed end tag: " + name, tag_start) self.pos += 1 if self.strict: if len(self._open) == 0: @@ -946,7 +943,7 @@ struct XmlPullParser(Copyable, Movable): var name_pos = self.pos var name = self._read_name() if name.byte_length() == 0: - raise Error("mojo-xml: empty element name") + raise self._error("empty element name", name_pos) if self.strict: self._validate_name(name, name_pos) var attrs = self._read_attrs() @@ -956,7 +953,7 @@ struct XmlPullParser(Copyable, Movable): self.pos += 1 self._skip_space() if self.pos >= self._len() or self._at(self.pos) != _GT: - raise Error("mojo-xml: malformed start tag: " + name) + raise self._error("malformed start tag: " + name, name_pos) self.pos += 1 if self_closing: self._pending_end = name.copy() diff --git a/test/test_errors.mojo b/test/test_errors.mojo new file mode 100644 index 0000000..92ebbd2 --- /dev/null +++ b/test/test_errors.mojo @@ -0,0 +1,211 @@ +from std.testing import assert_equal, assert_true, assert_raises, TestSuite + +from xml.errors import line_col, parse_error +from xml.pull import XmlPullParser, EVENT_EOF +from xml.etree import fromstring + + +def _assert_lc(source: String, offset: Int, line: Int, col: Int) raises: + var lc = line_col(source.as_bytes(), offset) + assert_equal(lc[0], line) + assert_equal(lc[1], col) + + +def _msg(e: Error) -> String: + return String.write(e) + + +def _strict_drain(var source: String) raises: + var parser = XmlPullParser(source^, strict=True) + while True: + var event = parser.next_event() + if event.kind == EVENT_EOF: + break + + +# -------------------------------------------------------------------------- +# line_col unit tests — every documented edge case. +# -------------------------------------------------------------------------- + + +def test_line_col_offset_zero() raises: + _assert_lc("abc", 0, 1, 1) + + +def test_line_col_empty_source() raises: + _assert_lc("", 0, 1, 1) + _assert_lc("", 7, 1, 1) + + +def test_line_col_negative_offset_clamps() raises: + _assert_lc("abc", -5, 1, 1) + + +def test_line_col_middle_of_lines() raises: + # "ab\ncd": a=0 b=1 \n=2 c=3 d=4 + _assert_lc("ab\ncd", 1, 1, 2) + _assert_lc("ab\ncd", 3, 2, 1) + _assert_lc("ab\ncd", 4, 2, 2) + + +def test_line_col_offset_at_newline() raises: + # An offset AT a '\n' reports the line that newline terminates. + _assert_lc("ab\ncd", 2, 1, 3) + + +def test_line_col_offset_past_end_clamps() raises: + _assert_lc("ab\ncd", 5, 2, 3) # == len + _assert_lc("ab\ncd", 99, 2, 3) # > len + + +def test_line_col_crlf_no_phantom_column() raises: + # "ab\r\ncd": a=0 b=1 \r=2 \n=3 c=4 d=5. The byte after a CRLF is + # column 1 of the next line — the '\r' contributes no phantom column. + _assert_lc("ab\r\ncd", 4, 2, 1) + _assert_lc("ab\r\ncd", 5, 2, 2) + _assert_lc("ab\r\ncd", 2, 1, 3) # at the '\r' + _assert_lc("ab\r\ncd", 3, 1, 4) # at the '\n' of the CRLF + + +def test_line_col_consecutive_newlines() raises: + # "a\n\nb": a=0 \n=1 \n=2 b=3 + _assert_lc("a\n\nb", 2, 2, 1) + _assert_lc("a\n\nb", 3, 3, 1) + + +def test_line_col_trailing_newline() raises: + _assert_lc("ab\n", 3, 2, 1) + + +# -------------------------------------------------------------------------- +# parse_error unit tests — exact message format + snippet behavior. +# -------------------------------------------------------------------------- + + +def test_parse_error_exact_format() raises: + var e = parse_error("boom", "hello".as_bytes(), 2) + assert_equal(_msg(e), "boom at line 1, column 3: 'hello'") + + +def test_parse_error_multiline_source_single_line_message() raises: + # Offset 8 is the 'n' of "line2"; snippet is that line only — the + # message never embeds a newline. + var e = parse_error("bad", "line1\nline2\nline3".as_bytes(), 8) + assert_equal(_msg(e), "bad at line 2, column 3: 'line2'") + + +def test_parse_error_snippet_trims_whitespace() raises: + # " pad ": offset 4 is the 'a'. Column counts the raw bytes, the + # snippet is the trimmed line content. + var e = parse_error("boom", " pad ".as_bytes(), 4) + assert_equal(_msg(e), "boom at line 1, column 5: 'pad'") + + +def test_parse_error_offset_at_newline_snippet_is_ended_line() raises: + var e = parse_error("boom", "ab\ncd".as_bytes(), 2) + assert_equal(_msg(e), "boom at line 1, column 3: 'ab'") + + +def test_parse_error_crlf_line_has_no_stray_cr() raises: + # The snippet for a CRLF-terminated line drops the '\r' (whitespace + # trim), so the quoted snippet is clean. + var e = parse_error("boom", "ab\r\ncd".as_bytes(), 1) + assert_equal(_msg(e), "boom at line 1, column 2: 'ab'") + + +def test_parse_error_offset_past_end_clamps() raises: + var e = parse_error("eof", "ab\ncd".as_bytes(), 99) + assert_equal(_msg(e), "eof at line 2, column 3: 'cd'") + + +def test_parse_error_empty_source() raises: + var e = parse_error("boom", "".as_bytes(), 0) + assert_equal(_msg(e), "boom at line 1, column 1: ''") + + +def test_parse_error_truncates_both_sides() raises: + # An 80-byte line with the offending '!' at offset 40: the snippet is + # a 30-byte window centered on it, with '...' on both cut sides. + var source = String() + for _ in range(40): + source += "x" + source += "!" + for _ in range(39): + source += "x" + var e = parse_error("bang", source.as_bytes(), 40) + assert_equal( + _msg(e), + "bang at line 1, column 41: '...xxxxxxxxxxxxxxx!xxxxxxxxxxxxxx...'", + ) + + +def test_parse_error_truncates_right_only() raises: + var source = String("abcde") + for _ in range(75): + source += "x" + var e = parse_error("bang", source.as_bytes(), 0) + assert_equal( + _msg(e), + "bang at line 1, column 1: 'abcdexxxxxxxxxxxxxxxxxxxxxxxxx...'", + ) + + +def test_parse_error_truncates_left_only() raises: + var source = String() + for _ in range(79): + source += "x" + source += "!" + var e = parse_error("bang", source.as_bytes(), 79) + assert_equal( + _msg(e), + "bang at line 1, column 80: '...xxxxxxxxxxxxxxxxxxxxxxxxxxxxx!'", + ) + + +def test_parse_error_snippet_never_splits_utf8() raises: + # 40 'é' (2 bytes each, 80 bytes total); an offset landing mid-sequence + # still yields a valid-UTF-8 snippet: window edges are nudged off + # continuation bytes, so the snippet is whole codepoints only. + var source = String() + for _ in range(40): + source += "é" + var e = parse_error("bang", source.as_bytes(), 40) + var rendered = _msg(e) + assert_true(rendered.startswith("bang at line 1, column 41: '...")) + # 30-byte budget at a 2-byte-char boundary nudge = 15 whole 'é'. + var expected_snippet = String("'...") + for _ in range(15): + expected_snippet += "é" + expected_snippet += "...'" + assert_true(expected_snippet in rendered) + + +# -------------------------------------------------------------------------- +# Integration — real malformed-XML parses report hand-verified positions. +# -------------------------------------------------------------------------- + + +def test_integration_mismatched_end_tag_position() raises: + # "\n\nx\n": the offending "" starts at byte 9 — + # line 3 (newlines at 3 and 7), column 9-7 = 2. Line 3 is "x". + with assert_raises(contains="at line 3, column 2: 'x'"): + _strict_drain("\n\nx\n") + + +def test_integration_unquoted_attribute_position() raises: + # "": the unquoted value 'c' is at byte 5 — line 1, column 6. + # Liberal (non-strict) mode: structural errors carry positions too. + var parser = XmlPullParser("") + with assert_raises(contains="at line 1, column 6: ''"): + _ = parser.next_event() + + +def test_integration_fromstring_stray_end_tag_position() raises: + # "x\n": the stray "" starts at byte 9 — line 2, + # column 1. Positions surface through the DOM API unchanged. + with assert_raises(contains="stray end tag at line 2, column 1"): + _ = fromstring("x\n") + + +def main() raises: + TestSuite.discover_tests[__functions_in_module()]().run() diff --git a/test/test_pull.mojo b/test/test_pull.mojo index f25fed3..672934a 100644 --- a/test/test_pull.mojo +++ b/test/test_pull.mojo @@ -262,7 +262,7 @@ def test_strict_bare_ampersand() raises: def test_strict_error_reports_location() raises: - with assert_raises(contains="line 3"): + with assert_raises(contains="at line 3, column 2"): _strict_events("\n\nx\n") From 9bd0d0400d7fbcd70f59d9c9ab15b6a8b4f697e7 Mon Sep 17 00:00:00 2001 From: Conor Bronsdon Date: Tue, 7 Jul 2026 21:52:27 -0700 Subject: [PATCH 2/2] Point unterminated-start-tag at the tag construct start, not EOF (review finding); dedicated position test --- src/xml/pull.mojo | 6 +++--- test/test_pull.mojo | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/xml/pull.mojo b/src/xml/pull.mojo index c5765dc..e6b6e1e 100644 --- a/src/xml/pull.mojo +++ b/src/xml/pull.mojo @@ -711,12 +711,12 @@ struct XmlPullParser(Copyable, Movable): "invalid character in name '" + name + "'", p ) - def _read_attrs(mut self) raises -> Dict[String, String]: + def _read_attrs(mut self, tag_start: Int) raises -> Dict[String, String]: var attrs = Dict[String, String]() while True: self._skip_space() if self.pos >= self._len(): - raise self._error("unterminated start tag", self.pos) + raise self._error("unterminated start tag", tag_start) var b = self._at(self.pos) if b == _GT or b == _SLASH: return attrs^ @@ -946,7 +946,7 @@ struct XmlPullParser(Copyable, Movable): raise self._error("empty element name", name_pos) if self.strict: self._validate_name(name, name_pos) - var attrs = self._read_attrs() + var attrs = self._read_attrs(name_pos - 1) var self_closing = False if self._at(self.pos) == _SLASH: self_closing = True diff --git a/test/test_pull.mojo b/test/test_pull.mojo index 672934a..761e1b0 100644 --- a/test/test_pull.mojo +++ b/test/test_pull.mojo @@ -272,7 +272,13 @@ def test_strict_self_closing_ok() raises: def test_unterminated_tag_raises() raises: var parser = XmlPullParser("