diff --git a/README.md b/README.md index bc0cbc6..b6891b8 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ with open("out.ged", "wb") as f: Each record is a `GedcomStructure` with a `tag`, an optional `xref` and `pointer`, the raw `text` payload, and `children`. Its `value` property casts the payload to the data type the specification gives that structure type. -`loads` and `dumps` are the string equivalents. Non-conforming input raises `GedcomParseError`, a `ValueError` carrying `line_number`. +`loads` and `dumps` are the string equivalents. Non-conforming input raises `GedcomParseError`, a `ValueError` carrying `line_number`. A pointer to a record that does not exist is kept rather than rejected; `gedcom7.validate(records)` lists each one as a `dangling-pointer` error. ## Development diff --git a/gedcom7/parser.py b/gedcom7/parser.py index 96b99ac..b2c6185 100644 --- a/gedcom7/parser.py +++ b/gedcom7/parser.py @@ -3,6 +3,7 @@ from __future__ import annotations import re +import sys from typing import TYPE_CHECKING from . import const, grammar @@ -10,6 +11,7 @@ from .types import GedcomStructure if TYPE_CHECKING: + from collections.abc import Iterator from typing import BinaryIO # EOL = %x0D [%x0A] / %x0A -- CR-LF, CR, or LF @@ -32,6 +34,21 @@ def _unescape(linestr: str) -> str: return linestr[1:] if linestr.startswith("@@") else linestr +def _lines(string: str) -> Iterator[str]: + """Yield the lines of a data stream, without their EOLs, one at a time. + + A terminating EOL does not start a further, empty line. A data stream whose + last line lacks its EOL is tolerated: the line is complete and unambiguous, + and dropping it would silently lose data. + """ + start = 0 + for eol in _EOL.finditer(string): + yield string[start : eol.start()] + start = eol.end() + if start < len(string): + yield string[start:] + + def load(fp: BinaryIO) -> list[GedcomStructure]: """Load a GEDCOM 7 dataset from a binary file object. @@ -56,6 +73,10 @@ def loads(string: str) -> list[GedcomStructure]: Raises :class:`~gedcom7.exceptions.GedcomParseError` if the data stream does not conform to the specification. Non-conforming lines are never skipped. + + A pointer that matches no cross-reference identifier is kept as written, so + one broken pointer does not cost the rest of the dataset; + :func:`~gedcom7.validate` reports each one as a ``dangling-pointer`` error. """ string = string.removeprefix(_BOM) @@ -66,24 +87,16 @@ def loads(string: str) -> list[GedcomStructure]: line_number=string.count("\n", 0, banned.start()) + 1, ) - lines = _EOL.split(string) - # A terminating EOL leaves a final empty element that is not itself a line. - # A data stream whose last line lacks its EOL is tolerated: the line is - # complete and unambiguous, and dropping it would silently lose data. - if lines and lines[-1] == "": - lines.pop() - records: list[GedcomStructure] = [] # stack[i] is the structure encoded by the nearest preceding line of level i stack: list[GedcomStructure] = [] # extension tag -> URIs declared for it by the header schema schema: dict[str, list[str]] = {} xrefs: set[str] = set() - pointers: list[tuple[str, int]] = [] # the structure a CONT on the very next line would continue continuable: GedcomStructure | None = None - for number, text in enumerate(lines, start=1): + for number, text in enumerate(_lines(string), start=1): match = _LINE.fullmatch(text + "\n") if match is None: raise GedcomParseError( @@ -91,7 +104,9 @@ def loads(string: str) -> list[GedcomStructure]: ) level = int(match.group("level")) - tag = match.group("tag") + # A dataset repeats a few dozen tags across every line, so each line's + # tag shares one string object rather than holding its own copy. + tag = sys.intern(match.group("tag")) xref = match.group("xref") pointer = match.group("pointer") linestr = match.group("linestr") @@ -188,19 +203,6 @@ def loads(string: str) -> list[GedcomStructure]: stack.append(structure) continuable = structure - if pointer is not None and pointer != const.VOIDPTR: - pointers.append((pointer, number)) - - # Pointers may be forward references, so they are resolved once the whole - # data stream has been read. - for pointer, number in pointers: - if pointer not in xrefs: - raise GedcomParseError( - f"pointer {pointer} matches no cross-reference identifier in the " - "data stream", - line_number=number, - ) - if not records: raise GedcomParseError("a dataset must contain a header and a trailer") if records[0].tag != const.HEAD: diff --git a/gedcom7/types.py b/gedcom7/types.py index ff55283..d245edb 100644 --- a/gedcom7/types.py +++ b/gedcom7/types.py @@ -8,7 +8,7 @@ from . import cast, const -@dataclass +@dataclass(slots=True) class GedcomStructure: """Gedcom structure class.""" diff --git a/gedcom7/validator.py b/gedcom7/validator.py index 7781e57..6cf5b1b 100644 --- a/gedcom7/validator.py +++ b/gedcom7/validator.py @@ -201,17 +201,18 @@ def _check_structure( structure, ) _check_substructure(structure, errors) - if type_id is not None: - payload = const.payloads.get(type_id) - if payload is not None: - _check_payload_kind(structure, payload, errors) - # Only one of these can say anything useful. Where the payload is of - # the wrong kind entirely, chasing its pointer or casting its text - # would report the same mistake a second time. - if payload.startswith("@<"): - _check_pointer(structure, payload, xrefs, errors) - else: - _check_payload_value(structure, type_id, errors) + payload = const.payloads.get(type_id) if type_id is not None else None + points = payload is not None and payload.startswith("@<") + if type_id is not None and payload is not None: + _check_payload_kind(structure, payload, errors) + # A pointer payload's text is not cast: where it holds the pointer by + # mistake, the kind check above has already said so. + if not points: + _check_payload_value(structure, type_id, errors) + # Any structure may carry a pointer, whether the specification gives it a + # pointer payload, a value, or nothing it knows of, as for an extension. A + # pointer to a missing record is reported wherever it is found. + _check_pointer(structure, payload if points else None, xrefs, errors) for child in structure.children: _check_structure(child, xrefs, declared, errors) @@ -267,11 +268,14 @@ def _check_payload_kind( def _check_pointer( structure: types.GedcomStructure, - payload: str, + payload: str | None, xrefs: dict[str, types.GedcomStructure], errors: list[Error], ) -> None: - """Check that a pointer reaches a record, and one of the right type.""" + """Check that a pointer reaches a record, and one of the type ``payload`` names. + + With no ``payload``, as for an extension, any record will do. + """ if not structure.pointer or structure.pointer == const.VOIDPTR: return target = xrefs.get(structure.pointer) @@ -282,7 +286,7 @@ def _check_pointer( f"{structure.pointer} is not a record in this dataset", structure, ) - else: + elif payload is not None: wanted = payload[2:-2] if target.type_id != wanted: _report( diff --git a/test/test_conformance.py b/test/test_conformance.py index 071ea17..177a5e7 100644 --- a/test/test_conformance.py +++ b/test/test_conformance.py @@ -1,7 +1,8 @@ """Conformance tests against the FamilySearch GEDCOM 7 specification. Each test cites the requirement it covers. Cases that the specification -prohibits must raise, not be silently skipped or reinterpreted. +prohibits must raise, not be silently skipped or reinterpreted. The exception +is a pointer to a missing record, which parses and is reported by ``validate``. """ import pytest @@ -185,10 +186,24 @@ def test_pointer_and_voidptr() -> None: assert indi.children[1].pointer == "@VOID@" -def test_unresolved_pointer_rejected() -> None: - """A pointer must match an xref of a structure within the document.""" - with pytest.raises(GedcomParseError, match="matches no cross-reference"): - parse("0 @I1@ INDI\n1 ALIA @I9@\n") +def test_unresolved_pointers_parse_and_validate_reports_each() -> None: + """A pointer must match an xref of a structure within the document. + + The parser keeps an unresolved pointer as written, and ``validate`` reports + every one, not only the first. + """ + records = parse( + "0 @I1@ INDI\n1 ALIA @I9@\n1 FAMS @F9@\n1 FAMC @VOID@\n" + "0 @F1@ FAM\n1 HUSB @I1@\n1 WIFE @I8@\n" + ) + assert records[1].children[0].pointer == "@I9@" + assert records[1].children[1].pointer == "@F9@" + errors = [e for e in gedcom7.validate(records) if e.category == "dangling-pointer"] + assert [e.path for e in errors] == [ + "@I1@ INDI > ALIA", + "@I1@ INDI > FAMS", + "@F1@ FAM > WIFE", + ] def test_forward_pointer_allowed() -> None: diff --git a/test/test_parser.py b/test/test_parser.py index 38381b6..99e5499 100644 --- a/test/test_parser.py +++ b/test/test_parser.py @@ -1,6 +1,9 @@ +import itertools import pathlib +import re import gedcom7 +from gedcom7.parser import _lines GEDCOM_MIN = """0 HEAD 1 GEDC @@ -103,3 +106,15 @@ def test_empty_line_value() -> None: assert file_struct.tag == "FILE" assert file_struct.text == "" assert file_struct.children[0].tag == "FORM" + + +def test_lines_matches_splitting_on_every_eol() -> None: + """Every string over a character, CR and LF, up to seven long, splits into + the lines that splitting on EOL and dropping a final empty piece gives.""" + for n in range(8): + for chars in itertools.product("a\r\n", repeat=n): + string = "".join(chars) + expected = re.split(r"\r\n|\r|\n", string) + if expected[-1] == "": + expected.pop() + assert list(_lines(string)) == expected, repr(string) diff --git a/test/test_validator.py b/test/test_validator.py index 30ecb16..6d8c723 100644 --- a/test/test_validator.py +++ b/test/test_validator.py @@ -1,6 +1,7 @@ """Tests for reporting what is wrong with a dataset.""" import pathlib +from collections.abc import Callable import pytest @@ -62,6 +63,70 @@ def test_void_pointer_is_not_dangling() -> None: assert gedcom7.validate(records) == [] +def _pointer_standard(pointer: str) -> types.GedcomStructure: + return individual(types.GedcomStructure(tag="FAMS", pointer=pointer)) + + +def _pointer_on_a_value(pointer: str) -> types.GedcomStructure: + return individual(types.GedcomStructure(tag="SEX", pointer=pointer)) + + +def _pointer_on_no_payload(pointer: str) -> types.GedcomStructure: + return individual(types.GedcomStructure(tag="BAPL", pointer=pointer)) + + +def _pointer_documented_extension(pointer: str) -> types.GedcomStructure: + return individual(types.GedcomStructure(tag=FOAF, pointer=pointer)) + + +def _pointer_undocumented_extension(pointer: str) -> types.GedcomStructure: + return individual(types.GedcomStructure(tag="_MINE", pointer=pointer)) + + +def _pointer_below_an_extension(pointer: str) -> types.GedcomStructure: + extension = types.GedcomStructure(tag="_MINE") + extension.append_child(types.GedcomStructure(tag="FAMS", pointer=pointer)) + return individual(extension) + + +def _pointer_misplaced_standard_tag(pointer: str) -> types.GedcomStructure: + birth = types.GedcomStructure(tag="BIRT") + birth.append_child(types.GedcomStructure(tag="FAMS", pointer=pointer)) + return individual(birth) + + +def _pointer_on_a_record(pointer: str) -> types.GedcomStructure: + return types.GedcomStructure(tag="_REC", xref="@X1@", pointer=pointer) + + +# Every place a pointer can sit, by what the tables know of its structure: a +# pointer payload, a value payload, no payload, a documented or undocumented +# extension, a standard tag with no standard type here, or a record. +POINTER_HOLDERS = [ + _pointer_standard, + _pointer_on_a_value, + _pointer_on_no_payload, + _pointer_documented_extension, + _pointer_undocumented_extension, + _pointer_below_an_extension, + _pointer_misplaced_standard_tag, + _pointer_on_a_record, +] + + +@pytest.mark.parametrize("holder", POINTER_HOLDERS) +@pytest.mark.parametrize( + ("pointer", "dangles"), [("@F9@", True), ("@VOID@", False), ("@I1@", False)] +) +def test_dangling_pointer_is_reported_wherever_it_sits( + holder: Callable[[str], types.GedcomStructure], pointer: str, dangles: bool +) -> None: + """loads keeps a pointer to a missing record, so validate must find every one.""" + built = holder(pointer) + records = dataset(built) if built.tag == "INDI" else dataset(individual(), built) + assert ("dangling-pointer" in categories(records)) is dangles + + def test_pointer_at_the_wrong_record_type() -> None: """FAMS names a family, so pointing it at an individual is wrong.""" other = types.GedcomStructure(tag="INDI", xref="@I2@") @@ -125,7 +190,7 @@ def test_text_where_a_pointer_belongs() -> None: def test_pointer_where_text_belongs() -> None: - records = dataset(individual(types.GedcomStructure(tag="SEX", pointer="@I2@"))) + records = dataset(individual(types.GedcomStructure(tag="SEX", pointer="@I1@"))) assert categories(records) == ["misplaced-payload"]