From 730bc3a89276d58378ba2126b2140e589ab6c1de Mon Sep 17 00:00:00 2001 From: Ian Wilson Date: Sun, 31 May 2026 23:56:39 -0700 Subject: [PATCH 1/3] 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/3] 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'
', From f3231844c0a2e208460a4b2ea07a67cc72a5bf9d Mon Sep 17 00:00:00 2001 From: Ian Wilson Date: Mon, 1 Jun 2026 09:36:25 -0700 Subject: [PATCH 3/3] Alternate: override attr value with self-closing in special case. --- tdom/parser.py | 21 ++++---- tdom/parser_test.py | 123 ++++++++++++++++++++++++++++++-------------- 2 files changed, 94 insertions(+), 50 deletions(-) diff --git a/tdom/parser.py b/tdom/parser.py index e73e3d4..b72b5f2 100644 --- a/tdom/parser.py +++ b/tdom/parser.py @@ -387,22 +387,23 @@ def always_get_starttag_text( # ------------------------------------------ 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. + # We correct this usage by removing the / from the last attr's value + # and treating the tag as self-closing. # 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 ( + self.always_get_starttag_text().endswith("/>") + and len(attrs) > 0 + and isinstance(attrs[-1][1], str) # attr is not boolean + and attrs[-1][1][-1] == "/" # attr value ends with / + ): + return self.handle_startendtag( + tag, (*attrs[:-1], (attrs[-1][0], attrs[-1][1][:-1])) ) - + open_tag = self.make_open_tag(tag, attrs) 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 8983e40..ed0f6c7 100644 --- a/tdom/parser_test.py +++ b/tdom/parser_test.py @@ -619,60 +619,103 @@ 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/ >", # This is really gross but shouldn't be common. - t'<{comp} title="literal"/>', - t"<{comp} title={dynamic} />", - t'<{comp} title="{dynamic}"/>', - t"<{comp} title={dynamic}literal />", - t'<{comp} title="{dynamic}literal"/>', + t"<{comp}/>abc", + t"<{comp} active/>abc", # Still ok because attr name cannot contain / + t"<{comp} {attrs}/>abc", # Still ok because attr name cannot contain / + t"<{comp} />abc", + t"<{comp} title=literal />abc", + t"<{comp} title=literal/ >abc", # This is really gross but shouldn't be common. + t'<{comp} title="literal"/>abc', + t"<{comp} title={dynamic} />abc", + t'<{comp} title="{dynamic}"/>abc', + t"<{comp} title={dynamic}literal />abc", + t'<{comp} title="{dynamic}literal"/>abc', ]: tnode = TemplateParser.parse(template) - assert isinstance(tnode, TComponent) and tnode.start_i_index == 0 + assert ( + isinstance(tnode, TFragment) + and len(tnode.children) == 2 + and isinstance(tnode.children[0], TComponent) + ) - def test_component_ambiguous_error(self, comp): + def test_component_ambiguous_corrected(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=/ + t"<{comp} title=literal/>abc", + t"<{comp} title={dynamic}/>abc", + t"<{comp} title={dynamic}literal/>abc", + t"<{comp} title=/>abc", + t"<{comp} title= />abc", # WS between = and value is ignored, so title=/ ): - with pytest.raises(ValueError, match="Ambiguous self-closing tag"): - _ = TemplateParser.parse(template) + tnode = TemplateParser.parse(template) + assert ( + isinstance(tnode, TFragment) + and len(tnode.children) == 2 + and isinstance(tnode.children[0], TComponent) + ) + + def test_component_ambiguous_corrected_attr_value_literal(self, comp): + tnode = TemplateParser.parse(t"<{comp} title=literal/>") + assert ( + isinstance(tnode, TComponent) + and isinstance(tnode.attrs[-1], TLiteralAttribute) + and tnode.attrs[-1].value == "literal" + ) + + def test_component_ambiguous_corrected_attr_value_interpolated(self, comp): + dynamic = "dynamic" + tnode = TemplateParser.parse(t"<{comp} title={dynamic}/>") + assert ( + isinstance(tnode, TComponent) + and isinstance(tnode.attrs[-1], TInterpolatedAttribute) + and tnode.attrs[-1].value_i_index == 1 + ) + + def test_component_ambiguous_corrected_attr_value_templated(self, comp): + dynamic = "dynamic" + tnode = TemplateParser.parse(t"<{comp} title=prefix{dynamic}/>") + assert ( + isinstance(tnode, TComponent) + and isinstance(tnode.attrs[-1], TTemplatedAttribute) + and tnode.attrs[-1].value_ref + == TemplateRef(strings=("prefix", ""), i_indexes=(1,)) + ) 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"
", # This is really gross but shouldn't be common. - t'
', - t"
", - t'
', - t"
", - t'
', + t"
abc", + t"
abc", # Still ok because attr name cannot contain / + t"
abc", # Still ok because attr name cannot contain / + t"
abc", + t"
abc", + t"
abc", # This is really gross but shouldn't be common. + t'
abc', + t"
abc", + t'
abc', + t"
abc", + t'
abc', ): tnode = TemplateParser.parse(template) - assert isinstance(tnode, TElement) and tnode.tag == "div" + assert ( + isinstance(tnode, TFragment) + and len(tnode.children) == 2 + and isinstance(tnode.children[0], TElement) + ) - def test_element_ambiguous_error(self): + def test_element_ambiguous_corrected(self): dynamic = "dynamic" for template in ( - t"
", - t"
", - t"
", - t"
", - t"
", # WS between = and value is ignored, so title=/ + t"
abc", + t"
abc", + t"
abc", + t"
abc", + t"
abc", # WS between = and value is ignored, so title=/ ): - with pytest.raises(ValueError, match="Ambiguous self-closing tag"): - _ = TemplateParser.parse(template) + tnode = TemplateParser.parse(template) + assert ( + isinstance(tnode, TFragment) + and len(tnode.children) == 2 + and isinstance(tnode.children[0], TElement) + )