From 730bc3a89276d58378ba2126b2140e589ab6c1de Mon Sep 17 00:00:00 2001 From: Ian Wilson Date: Sun, 31 May 2026 23:56:39 -0700 Subject: [PATCH 1/2] Raise an error if tag's self-closing / seems ambiguous for #132. --- tdom/parser.py | 36 +++++++++++++++++++---- tdom/parser_test.py | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/tdom/parser.py b/tdom/parser.py index 2eec36d..e73e3d4 100644 --- a/tdom/parser.py +++ b/tdom/parser.py @@ -189,11 +189,9 @@ def make_open_tag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> OpenTag: # @NOTE: This must be called when the tag is handled since it is # populated based on the most recently finished start tag. Otherwise # the value will be out of sync. - starttag_text = self.get_starttag_text() - if starttag_text is None: - raise AssertionError( - f"Expected startag_text to be set when parsing component at {i_index}." - ) + starttag_text = self.always_get_starttag_text( + f"Expected startag_text to be set when parsing component at {i_index}." + ) tattrs = self.make_tattrs(attrs) @@ -371,12 +369,40 @@ def validate_end_tag(self, tag: str, open_tag: OpenTag) -> int | None: # any of this in the parser, instead relying on higher layers. return tag_ref.i_indexes[0] + def always_get_starttag_text( + self, msg: str = "Expecting starttag text to be set." + ) -> str: + """ + Wrap get_starttag_text and just raise if None is returned. + + Do this so we don't guard for `None` everywhere. + """ + starttag_text = self.get_starttag_text() + if starttag_text is None: + raise AssertionError(msg) + return starttag_text + # ------------------------------------------ # HTMLParser tag callbacks # ------------------------------------------ def handle_starttag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> None: open_tag = self.make_open_tag(tag, attrs) + # An unquoted attribute value within a self-closing tag can consume + # the "/" as part of the attribute's value if not separated by whitespace. + # This is the CORRECT behavior but can can be especially confusing if + # preceded by an interpolation, such as `<{Comp} name={value}/>`. + # We explicitly dissallow this usage without preceding ascii whitespace. + # This applies to all tags (components or not). + if self.always_get_starttag_text().endswith("/>"): + if isinstance(open_tag, OpenTComponent): + error_tag = self.get_source().format_starttag(open_tag.start_i_index) + else: + error_tag = tag + raise ValueError( + f'Ambiguous self-closing tag, "<{error_tag}.../>", please precede "/>" with whitespace or apply quotes around the last attribute value.' + ) + if isinstance(open_tag, OpenTElement) and open_tag.tag in VOID_ELEMENTS: final_tag = self.finalize_tag(open_tag) self.append_child(final_tag) diff --git a/tdom/parser_test.py b/tdom/parser_test.py index d1650ae..702ef59 100644 --- a/tdom/parser_test.py +++ b/tdom/parser_test.py @@ -602,3 +602,75 @@ def test_extract_with_templated_attr_gt_char(self, Component): strings=("
Hello, World!
",), i_indexes=() ), ) + + +class TestAmbiguousSelfCloseCheck: + @pytest.fixture + def comp(self): + def component( + active: bool = False, title: str = "Title", children: Template = t"" + ) -> Template: + dataset = {"active": active} + return t"
{children}
" + + return component + + def test_component_ok(self, comp): + dynamic = "dynamic" + attrs = {"active": True} + for template in [ + t"<{comp}/>", + t"<{comp} active/>", # Still ok because attr name cannot contain / + t"<{comp} {attrs}/>", # Still ok because attr name cannot contain / + t"<{comp} />", + t"<{comp} title=literal />", + t'<{comp} title="literal"/>', + t"<{comp} title={dynamic} />", + t'<{comp} title="{dynamic}"/>', + t"<{comp} title={dynamic}literal />", + t'<{comp} title="{dynamic}literal"/>', + ]: + tnode = TemplateParser.parse(template) + assert isinstance(tnode, TComponent) and tnode.start_i_index == 0 + + def test_component_ambiguous_error(self, comp): + dynamic = "dynamic" + for template in ( + t"<{comp} title=literal/>", + t"<{comp} title={dynamic}/>", + t"<{comp} title={dynamic}literal/>", + t"<{comp} title=/>", + t"<{comp} title= />", # WS between = and value is ignored, so title=/ + ): + with pytest.raises(ValueError, match="Ambiguous self-closing tag"): + _ = TemplateParser.parse(template) + + def test_element_ok(self): + dynamic = "dynamic" + attrs = {"active": True} + for template in ( + t"
", + t"
", # Still ok because attr name cannot contain / + t"
", # Still ok because attr name cannot contain / + t"
", + t"
", + t'
', + t"
", + t'
', + t"
", + t'
', + ): + tnode = TemplateParser.parse(template) + assert isinstance(tnode, TElement) and tnode.tag == "div" + + def test_element_ambiguous_error(self): + dynamic = "dynamic" + for template in ( + t"
", + t"
", + t"
", + t"
", + t"
", # WS between = and value is ignored, so title=/ + ): + with pytest.raises(ValueError, match="Ambiguous self-closing tag"): + _ = TemplateParser.parse(template) From c04c5cf0e1b7d7c2308b9b72a8deef036cc6c4c3 Mon Sep 17 00:00:00 2001 From: Ian Wilson Date: Mon, 1 Jun 2026 00:42:22 -0700 Subject: [PATCH 2/2] Add another couple cases that should work but don't look OK. --- tdom/parser_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tdom/parser_test.py b/tdom/parser_test.py index 702ef59..8983e40 100644 --- a/tdom/parser_test.py +++ b/tdom/parser_test.py @@ -624,6 +624,7 @@ def test_component_ok(self, comp): t"<{comp} {attrs}/>", # Still ok because attr name cannot contain / t"<{comp} />", t"<{comp} title=literal />", + t"<{comp} title=literal/ >", # This is really gross but shouldn't be common. t'<{comp} title="literal"/>', t"<{comp} title={dynamic} />", t'<{comp} title="{dynamic}"/>', @@ -654,6 +655,7 @@ def test_element_ok(self): t"
", # Still ok because attr name cannot contain / t"
", t"
", + t"
", # This is really gross but shouldn't be common. t'
', t"
", t'
',