diff --git a/tdom/exc.py b/tdom/exc.py new file mode 100644 index 0000000..67fc85e --- /dev/null +++ b/tdom/exc.py @@ -0,0 +1,2 @@ +class TemplatingError(Exception): + pass diff --git a/tdom/parser.py b/tdom/parser.py index 40277b8..db059e2 100644 --- a/tdom/parser.py +++ b/tdom/parser.py @@ -3,6 +3,7 @@ from html.parser import HTMLParser from string.templatelib import Template +from .exc import TemplatingError from .htmlspec import VOID_ELEMENTS from .parser_utils import ( HTMLAttribute, @@ -36,6 +37,18 @@ ) +class ParsingError(TemplatingError): + pass + + +class ParsingAssertionError(ParsingError): + pass + + +class AttributeParsingError(ParsingError): + pass + + @dataclass(frozen=True, slots=True) class OpenTagSourceInfo: """ @@ -208,6 +221,9 @@ class TemplateParser(HTMLParser): stack: list[OpenTag] source: SourceTracker | None + tcomponent_children: dict[TComponent, list[TNode]] + "List of children for each finished tcomponent, stored at closing. " + sinfo_table: dict[PartPosition, TagSourceInfo] """Tags with more source info than just a position are tracked in this mapping.""" @@ -269,11 +285,11 @@ def make_tattr(self, attr: HTMLAttribute) -> TAttribute: else: return TTemplatedAttribute(name=name, value_ref=value_ref) if value_ref is not None: - raise ValueError( + raise AttributeParsingError( "Attribute names cannot contain interpolations if the value is also interpolated." ) if not name_ref.is_singleton: - raise ValueError( + raise AttributeParsingError( "Spread attributes must have exactly one interpolation in the name." ) return TSpreadAttribute(i_index=name_ref.i_start) @@ -310,7 +326,7 @@ def make_open_tag( ) if not tag_ref.is_singleton: - raise ValueError( + raise ParsingError( "Component element tags must have exactly one interpolation." ) @@ -368,6 +384,7 @@ def finalize_tag( attrs=attrs, source_pos=source_pos, sinfo=sinfo, + children=children, ): children_span = ( TemplateSpan(start=children_start, stop=endtag_pos) @@ -375,59 +392,129 @@ def finalize_tag( else None ) self.sinfo_table[source_pos] = sinfo.close(endtag_pos=endtag_pos) - return TComponent( + tnode = TComponent( start_i_index=start_i_index, end_i_index=endtag_i_index, children_span=children_span, attrs=attrs, source_pos=source_pos, ) + # Save children for introspection after some parsing errors otherwise + # they are discarded since we extract the children_span in the processor + # for the components. + self.tcomponent_children[tnode] = children + return tnode + + def make_mismatch_error( + self, + starttag_sinfo: OpenTagSourceInfo, + starttag_attrs: tuple[TAttribute, ...], + endtag_ref: TemplateRef, + endtag_pos: PartPosition, + ) -> ParsingError: + reader = self.get_source().get_reader() + starttag_repr = reader.span_to_repr(starttag_sinfo.starttag_span) + starttag_pos_msg = reader.make_template_pos_msg(starttag_sinfo.starttag_pos) + endtag_repr = reader.ref_to_repr(endtag_ref) + endtag_pos_msg = reader.make_template_pos_msg(endtag_pos) + e = ParsingError( + f"Mismatched closing tag at {endtag_pos_msg} for {starttag_repr} at {starttag_pos_msg}." + ) + if self.has_ambiguous_forward_slash(starttag_sinfo, starttag_attrs): + e.add_note( + f'Did you mean to quote the last attribute or put a space before "/>" for "{starttag_repr}" at {starttag_pos_msg}?' + ) + return e + + def make_invalid_endtag_error( + self, endtag_ref: TemplateRef, endtag_pos: PartPosition + ) -> ParsingError: + reader = self.get_source().get_reader() + endtag_repr = reader.ref_to_repr(endtag_ref) + endtag_pos_msg = reader.make_template_pos_msg(endtag_pos) + raise ParsingError( + f"Component end tags must have exactly one interpolation, {endtag_repr} at {endtag_pos_msg}." + ) def validate_end_tag(self, tag: str, open_tag: OpenTag) -> int | None: """Validate that closing tag matches open tag. Return component end index if applicable.""" source = self.get_source() - tag_ref = source.remove_placeholders(tag) + tag_ref = source.placeholders.remove_placeholders(tag) match open_tag: case OpenTElement(): - if not tag_ref.is_literal: - raise ValueError( - f"Component closing tag found for element <{open_tag.tag}>." - ) - if tag != open_tag.tag: - raise ValueError( - f"Mismatched closing tag for element <{open_tag.tag}>." + if tag_ref.is_singleton or (tag_ref.is_literal and tag != open_tag.tag): + raise self.make_mismatch_error( + open_tag.sinfo, open_tag.attrs, tag_ref, self.get_source_pos() ) + elif not tag_ref.is_singleton and not tag_ref.is_literal: + raise self.make_invalid_endtag_error(tag_ref, self.get_source_pos()) return None - case OpenTFragment(): - raise NotImplementedError("We do not support anonymous fragments.") - - case OpenTComponent(start_i_index=start_i_index): + raise ParsingAssertionError("We do not support anonymous fragments.") + case OpenTComponent(): if tag_ref.is_literal: - raise ValueError( - f"Mismatched closing tag for component starting at {source.format_starttag(start_i_index)}." + raise self.make_mismatch_error( + open_tag.sinfo, open_tag.attrs, tag_ref, self.get_source_pos() ) if not tag_ref.is_singleton: - raise ValueError( - "Component end tags must have exactly one interpolation." - ) - # HERE BE DRAGONS: the interpolation at end_i_index shuld be a - # component callable that matches the start tag. We do not check - # any of this in the parser, instead relying on higher layers. + raise self.make_invalid_endtag_error(tag_ref, self.get_source_pos()) return tag_ref.i_start def get_starttag_span(self) -> TemplateSpan: """Return the source span occupied by the current start tag.""" starttag_text = self.get_starttag_text() - assert starttag_text is not None, ( - "Expected the parser to have starttag_text set." - ) + if starttag_text is None: + raise ParsingAssertionError( + "Expected the parser to have starttag_text set." + ) source = self.get_source() line_pos = self.get_parser_pos() return source.translate_parser_span(line_pos, len(starttag_text)) + def has_ambiguous_forward_slash( + self, + sinfo: OpenTagSourceInfo | TagSourceInfo | None, + attrs: tuple[TAttribute, ...], + ) -> bool: + """ + Detect when an unquoted attribute value consumes a trailing "/" that + *might* have been meant to attempt to self-close a tag, ie. "/>". + + This can come up with literal values or values with interpolations. + + Such as "
" or "<{Component} title=test/>". + + Or more often "<{Component} title={title}/>" which should be corrected + with "<{Component} title={title} />". + """ + source = self.get_source() + reader = source.get_reader() + return ( + # has source info + sinfo is not None + # has attributes + and len(attrs) > 0 + # last attribute ends with "/" + # @NOTE: spread and interpolated attrs never do + and ( + ( + isinstance(attrs[-1], TLiteralAttribute) + and attrs[-1].value is not None + and attrs[-1].value.endswith("/") + ) + or ( + isinstance(attrs[-1], TTemplatedAttribute) + and attrs[-1].value_ref.strings[-1].endswith("/") + ) + ) + # original starttag ends with "/>", + and reader.span_to_template(sinfo.starttag_span).strings[-1].endswith("/>") + # if parsed AS startend already then its not ambiguous + and not sinfo.startend + ) + # ------------------------------------------ # HTMLParser tag callbacks # ------------------------------------------ @@ -449,15 +536,54 @@ def handle_startendtag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> None: def handle_endtag(self, tag: str) -> None: endtag_pos = self.get_source_pos() if not self.stack: - raise ValueError(f"Unexpected closing tag with no open tag.") - + source = self.get_source() + reader = source.get_reader() + endtag_ref = source.find_placeholders(tag) + endtag_repr = reader.ref_to_repr(endtag_ref) + endtag_pos_msg = reader.make_template_pos_msg(endtag_pos) + if endtag_ref.is_literal or endtag_ref.is_singleton: + raise ParsingError( + f"Unexpected closing tag with no open tag, {endtag_pos_msg}." + ) + else: + raise self.make_invalid_endtag_error(endtag_ref, endtag_pos) open_tag = self.stack.pop() endtag_i_index = self.validate_end_tag(tag, open_tag) final_tag = self.finalize_tag( - open_tag, endtag_i_index=endtag_i_index, endtag_pos=endtag_pos + open_tag, + endtag_i_index=endtag_i_index, + endtag_pos=endtag_pos, ) self.append_child(final_tag) + def get_closed_tcomps( + self, root: OpenTag | None, recurse_component_children: bool = False + ) -> list[TComponent]: + """ + Get TComponents that were closed during parsing starting from `root`. + + If `root` is None then use the parser's default `root`. + + TComponents should be returned in the order they were closed in: + from first closed to last closed. + + @NOTE: That the root is an `OpenTag` but its `children` are actually `TNode`s. + """ + if root is None: + root = self.root + tcomps = [] + nodes = list(root.children) + while nodes: + node = nodes.pop() + if isinstance(node, TComponent): + tcomps.append(node) + if recurse_component_children: + children = self.tcomponent_children.get(node, []) + nodes.extend(children) + elif isinstance(node, (TElement, TFragment)): + nodes.extend(node.children) + return tcomps + # ------------------------------------------ # HTMLParser other callbacks # ------------------------------------------ @@ -486,13 +612,13 @@ def handle_decl(self, decl: str) -> None: source = self.get_source() ref = source.remove_placeholders(decl) if not ref.is_literal: - raise ValueError("Interpolations are not allowed in declarations.") + raise ParsingError("Interpolations are not allowed in declarations.") elif decl.upper().startswith("DOCTYPE "): doctype_content = decl[7:].strip() doctype = TDocumentType(doctype_content, source_pos=self.get_source_pos()) self.append_child(doctype) else: - raise NotImplementedError( + raise ParsingError( "Only well formed DOCTYPE declarations are currently supported." ) @@ -502,22 +628,107 @@ def reset(self): self.stack = [] self.source = None self.sinfo_table = {} + self.tcomponent_children = {} + + def run_unclosed_ambiguous_slash_checks( + self, parent: OpenTag, e: ParsingError + ) -> None: + """ + Check for cases where ambiguous slash might create a confusing error. + + @NOTE: This adds exception notes to the exception but does not throw it. + """ + source = self.get_source() + reader = source.get_reader() + if isinstance( + parent, (OpenTElement, OpenTComponent) + ) and self.has_ambiguous_forward_slash(parent.sinfo, parent.attrs): + # CASE: "<{C1} attr={value}/>" -- maybe user meant to self-close? + # CASE: "
" -- mayber user meant to self-close? + starttag_span = parent.sinfo.starttag_span + starttag_repr = reader.span_to_repr(starttag_span) + pos_msg = reader.make_template_pos_msg(parent.source_pos) + e.add_note( + f'Did you mean to quote the last attribute or put a space before "/>" for "{starttag_repr}" at {pos_msg}?' + ) + elif isinstance(parent, OpenTElement): + # ie. t"
", looks + # like we missed a closing
but really we meant to + # self-close the middle div. + children = parent.children[:] + while children: + child = children.pop(0) + if isinstance(child, TElement) and child.tag == parent.tag: + sinfo = ( + self.sinfo_table.get(child.source_pos) + if child.source_pos is not None + else None + ) + if sinfo and self.has_ambiguous_forward_slash(sinfo, child.attrs): + full_starttag_repr = reader.span_to_repr(sinfo.starttag_span) + e.add_note( + f'Did you mean to quote the last attribute or put a space before "/>" for "{full_starttag_repr}"?' + ) + children.extend(child.children) + elif isinstance(parent, OpenTComponent): + # This is a special case where a component accidentally closes + # another component but we don't check the actual values in + # the parser so we can't tell until we are generating an error + # (when we can check the values). + # + # CASE: t"<{C2}><{C1} attr=/>" + # Maybe user meant to self-close <{C1} ...>, but closed by leaving <{C2}...> open? + # CASE: t"<{C3}><{C2}><{C1} attr=/>" + for comp in reversed( + self.get_closed_tcomps(parent, recurse_component_children=True) + ): + if ( + comp.end_i_index is not None + and comp.start_i_index != comp.end_i_index + and not reader.values_match(comp.start_i_index, comp.end_i_index) + ): + starttag_repr = reader.make_interpolation_repr(comp.start_i_index) + endtag_repr = reader.make_interpolation_repr(comp.end_i_index) + e.add_note( + f"Component start tag, <{starttag_repr} ...>, and end tag, , have values that do not match." + ) + sinfo = ( + self.sinfo_table.get(comp.source_pos) + if comp.source_pos is not None + else None + ) + if sinfo and self.has_ambiguous_forward_slash(sinfo, comp.attrs): + full_starttag_repr = reader.span_to_repr(sinfo.starttag_span) + e.add_note( + f'Did you mean to quote the last attribute or put a space before "/>" for "{full_starttag_repr}"?' + ) def close(self) -> None: + source = self.get_source() if self.waiting_for_data(): # We apply heuristics here to try to guess why the parser didn't finish. if self.rawdata.count('"') % 2 == 1 or self.rawdata.count("'") % 2 == 1: - raise ValueError( + raise ParsingError( "Parser expects more data, maybe you left an attribute quote unclosed?" ) else: - raise ValueError( + raise ParsingError( "Parser expects more data, is the template valid html?" ) if self.stack: - raise ValueError("Invalid HTML structure: unclosed tags remain.") + parent = self.stack[-1] + if isinstance(parent, (OpenTElement, OpenTComponent)): + reader = source.get_reader() + starttag_repr = reader.span_to_repr(parent.sinfo.starttag_span) + pos_msg = reader.make_template_pos_msg(parent.source_pos) + unclosed_msg = f"unclosed tag {starttag_repr} at {pos_msg}" + else: + unclosed_msg = "unclosed tags remain" + e = ParsingError(f"Invalid HTML structure: {unclosed_msg}.") + self.run_unclosed_ambiguous_slash_checks(parent, e) + raise e if self.source and self.source.has_placeholders(): - raise ValueError("Some placeholders were never resolved.") + raise ParsingError("Some placeholders were never resolved.") super().close() def waiting_for_data(self): @@ -556,12 +767,12 @@ def get_ttree(self) -> TTree: def get_source(self) -> SourceTracker: if self.source is None: - raise AssertionError("Source has not been initialized.") + raise ParsingAssertionError("Source has not been initialized.") return self.source def track_source(self, template: Template) -> SourceTracker: if self.source: - raise AssertionError("Did you forget to call reset?") + raise ParsingAssertionError("Did you forget to call reset?") source = self.source = configure_source_tracker(template) return source diff --git a/tdom/parser_test.py b/tdom/parser_test.py index 8d32785..547b1ac 100644 --- a/tdom/parser_test.py +++ b/tdom/parser_test.py @@ -2,7 +2,12 @@ import pytest -from .parser import TemplateParser, configure_source_tracker +from .parser import ( + AttributeParsingError, + ParsingError, + TemplateParser, + configure_source_tracker, +) from .placeholders import make_placeholder_config from .template_utils import PartPosition, TemplateRef, TemplateSpan from .tnodes import ( @@ -223,17 +228,17 @@ def test_parse_title_unusual(): def test_parse_mismatched_tags(): - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="Mismatch"): _ = parse_root(t"
Mismatched
") -def test_parse_unclosed_tag(): - with pytest.raises(ValueError): +def test_parse_unclosed_element(): + with pytest.raises(ParsingError, match="unclosed tag
"): _ = parse_root(t"
Unclosed") def test_parse_unexpected_closing_tag(): - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="Unexpected closing tag"): _ = parse_root(t"Unopened
") @@ -257,12 +262,12 @@ def test_nested_self_closing_tags(): def test_self_closing_tags_unexpected_closing_tag(): - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="Unexpected closing tag"): _ = parse_root(t"
") def test_self_closing_void_tags_unexpected_closing_tag(): - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="Unexpected closing tag"): _ = parse_root(t"") @@ -351,20 +356,28 @@ def test_spread_attr(): def test_templated_attribute_name_error(): - with pytest.raises(ValueError): + with pytest.raises( + AttributeParsingError, + match="cannot contain interpolations if the value is also interpolated", + ): attr_name = "some-attr" _ = parse_root(t'
') def test_templated_attribute_name_and_value_error(): - with pytest.raises(ValueError): + with pytest.raises( + AttributeParsingError, + match="cannot contain interpolations if the value is also interpolated", + ): attr_name = "some-attr" value = "value" _ = parse_root(t'
') def test_adjacent_spread_attrs_error(): - with pytest.raises(ValueError): + with pytest.raises( + AttributeParsingError, match="must have exactly one interpolation in the name" + ): attrs1 = {} attrs2 = {} _ = parse_root(t"
") @@ -394,14 +407,16 @@ def test_parse_doctype(): def test_parse_doctype_interpolation_error(): extra = "SYSTEM" - with pytest.raises(ValueError): + with pytest.raises( + ParsingError, match="Interpolations are not allowed in declarations" + ): _ = parse_root(t"") def test_unsupported_decl_error(): - with pytest.raises(NotImplementedError): + with pytest.raises(ParsingError, match="Only well formed DOCTYPE declarations"): _ = parse_root(t"") # Unknown declaration - with pytest.raises(NotImplementedError): + with pytest.raises(ParsingError, match="Only well formed DOCTYPE declarations"): _ = parse_root(t"") # missing DTD @@ -460,7 +475,7 @@ def test_component_element_invalid_closing_tag(): def Component(): pass - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="Mismatched closing tag
"): _ = parse_root(t"<{Component}>
") @@ -468,7 +483,8 @@ def test_component_element_invalid_opening_tag(): def Component(): pass - with pytest.raises(ValueError): + # @NOTE: intentional expression + with pytest.raises(ParsingError, match="Mismatched closing tag "): _ = parse_root(t"
") @@ -476,7 +492,7 @@ def test_adjacent_start_component_tag_error(): def Component(): pass - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="must have exactly one interpolation"): _ = parse_root(t"<{Component}{Component}>") @@ -484,10 +500,26 @@ def test_adjacent_end_component_tag_error(): def Component(): pass - with pytest.raises(ValueError): + with pytest.raises(ParsingError, match="must have exactly one interpolation"): _ = parse_root(t"<{Component}>") +def test_unmatched_end_component_tag_error(): + def Component(): + pass + + with pytest.raises(ParsingError, match="Unexpected closing tag "): + _ = TemplateParser.parse(t"") + + +def test_unclosed_component_tag_error(): + def Component(): + pass + + with pytest.raises(ParsingError, match="unclosed tag <{Component}>"): + _ = TemplateParser.parse(t"<{Component}>") + + def test_placeholder_collision_avoidance(): config = make_placeholder_config() # This test is to ensure that our placeholder detection avoids collisions @@ -513,7 +545,7 @@ def test_unresolved_placeholder(): # This would be a bug in the parser so we have to fabricate # this error manually. tp.get_source().placeholders.add_placeholder(3) - with pytest.raises(ValueError, match="Some placeholders were never resolved"): + with pytest.raises(ParsingError, match="Some placeholders were never resolved"): tp.close() @@ -558,17 +590,17 @@ def test_iter(self): class TestIncompleteParsing: def test_dangling_quotes(self): - with pytest.raises(ValueError, match="Parser expects more data"): + with pytest.raises(ParsingError, match="Parser expects more data"): _ = parse_root(t"
") + + def test_nested_unclosed_error(self): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*attr[=]nested/", + ): + _ = TemplateParser.parse(t"
") + + def test_double_nested_unclosed_error(self): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*attr[=]nested/", + ): + _ = TemplateParser.parse(t"
") + + def test_mismatch_with_element_error(self): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*attr[=]mismatch/", + ): + _ = TemplateParser.parse(t"
") + + def test_mismatch_with_component_error(self): + def Comp(children: Template) -> Template: + return t"" + + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*attr[=]mismatch/", + ): + _ = TemplateParser.parse(t"<{Comp}>
") + + +class TestComponentWithAmbiguousSlash: + @pytest.fixture + def Comp1(self): + def _Comp1(children: Template, title: str) -> Template: + return children + + return _Comp1 + + @pytest.fixture + def Comp2(self): + def _Comp2(children: Template, title: str) -> Template: + return children + + return _Comp2 + + @pytest.fixture + def Comp3(self): + def _Comp3(children: Template, title: str) -> Template: + return children + + return _Comp3 + + def test_mismatch_with_element_error(self, Comp1): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*title[=]today/", + ): + _ = TemplateParser.parse(t"
<{Comp1} title=today/>
") + + def test_root_unclosed_error(self, Comp1): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*title[=]today/", + ): + _ = TemplateParser.parse(t"<{Comp1} title=today/>") + + def test_single_nested_unclosed_error(self, Comp1, Comp2): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*title[=]today/", + ): + _ = TemplateParser.parse(t"<{Comp2}><{Comp1} title=today/>") + + def test_double_nested_unclosed_error(self, Comp1, Comp2, Comp3): + with pytest.raises( + ParsingError, + match="Did you mean to quote the last attribute.*title[=]today/", + ): + _ = TemplateParser.parse( + t"<{Comp2}><{Comp1}><{Comp3} title=today/>" + ) + + class TestSourcePos: """ Test that common nodes have a source position translated and set during parsing. diff --git a/tdom/source.py b/tdom/source.py index 9a8ca04..706d42d 100644 --- a/tdom/source.py +++ b/tdom/source.py @@ -73,6 +73,16 @@ def ref_to_repr(self, ref: TemplateRef, limit: int | None = None) -> str: filled_template = ref.bind(self.template.interpolations) return template_repr(filled_template)[:limit] + def span_to_repr(self, span: TemplateSpan, limit: int | None = None) -> str: + """ + Extract template span and convert to string representation. + """ + filled_template = span.extract(self.template) + return template_repr(filled_template)[:limit] + + def span_to_template(self, span: TemplateSpan) -> Template: + return span.extract(self.template) + def make_template_pos_msg(self, source_pos: PartPosition) -> str: """ Make a message to display the line number and offset number.