Improve error message for unclosed components whose attribute value might have consumed the slash.

tdom/parser.py

@@ -25,20 +25,34 @@
 type HTMLAttributesDict = dict[str, str | None]
 
 
-@dataclass
+@dataclass(frozen=True)
+class ParseInfo:
+    "Track parse info for error reporting."
+
+    starttag_text: str
+    " Entire starttag as parsed, includes placeholders, . "
+    raw_attrs: Sequence[HTMLAttribute]
+    " Attrs as parsed, includes placeholders. "
+    startend: bool
+    " Was parsed as startend tag, ie. <tag />. "
+
+
+@dataclass(frozen=True)
 class OpenTElement:
+    parse_info: ParseInfo
     tag: str
     attrs: tuple[TAttribute, ...]
     children: list[TNode] = field(default_factory=list)
 
 
-@dataclass
+@dataclass(frozen=True)
 class OpenTFragment:
     children: list[TNode] = field(default_factory=list)
 
 
-@dataclass
+@dataclass(frozen=True)
 class OpenTComponent:
+    parse_info: ParseInfo
     start_i_index: int
     children_start_s_index: int
     """The strings index where the component's children template starts."""
@@ -72,6 +86,19 @@ class SourceTracker:
     def interpolations(self) -> tuple[Interpolation, ...]:
         return self.template.interpolations
 
+    def _check_indices(self, index1: int, index2: int):
+        last_index = len(self.interpolations) - 1
+        if max(index1, index2) > last_index or min(index1, index2) < 0:
+            raise ValueError(
+                f"Interpolation indices exceed bounds: {index1} {index2}: [0...{last_index}]"
+            )
+
+    def values_match(self, i_index1: int, i_index2: int) -> bool:
+        self._check_indices(i_index1, i_index2)
+        return (
+            self.interpolations[i_index1].value == self.interpolations[i_index2].value
+        )
+
     def advance_interpolation(self) -> int:
         """Call before processing an interpolation to move to the next one."""
         self.i_index += 1
@@ -95,12 +122,17 @@ def format_starttag(self, i_index: int) -> str:
         """Format a component start tag for error messages."""
         return self.get_expression(i_index, fallback_prefix="component-starttag")
 
+    def format_endtag(self, i_index: int) -> str:
+        return self.get_expression(i_index, fallback_prefix="component-endtag")
+
 
 class TemplateParser(HTMLParser):
     root: OpenTFragment
     stack: list[OpenTag]
     placeholders: PlaceholderState
     source: SourceTracker | None
+    tmap: dict[TComponent | TElement | TFragment, OpenTag]
+    " Map from completed tnodes back to their opentag for error reporting. "
 
     def __init__(self, *, convert_charrefs: bool = True):
         # This calls HTMLParser.reset() which we override to set up our state.
@@ -159,12 +191,22 @@ def make_tattrs(self, attrs: Sequence[HTMLAttribute]) -> tuple[TAttribute, ...]:
     # Tag Helpers
     # ------------------------------------------
 
-    def make_open_tag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> OpenTag:
+    def make_open_tag(
+        self, tag: str, attrs: Sequence[HTMLAttribute], startend: bool = False
+    ) -> OpenTag:
         """Build an OpenTag from a raw tag and attribute tuples."""
         tag_ref = self.placeholders.remove_placeholders(tag)
 
         if tag_ref.is_literal:
-            return OpenTElement(tag=tag, attrs=self.make_tattrs(attrs))
+            return OpenTElement(
+                parse_info=ParseInfo(
+                    starttag_text=self.get_starttag_text(),
+                    raw_attrs=attrs,
+                    startend=startend,
+                ),
+                tag=tag,
+                attrs=self.make_tattrs(attrs),
+            )
 
         if not tag_ref.is_singleton:
             raise ValueError(
@@ -189,11 +231,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.get_starttag_text(
+            f"Expected startag_text to be set when parsing component at {i_index}."
+        )
 
         tattrs = self.make_tattrs(attrs)
 
@@ -205,6 +245,9 @@ def make_open_tag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> OpenTag:
         )
 
         return OpenTComponent(
+            parse_info=ParseInfo(
+                starttag_text=starttag_text, raw_attrs=attrs, startend=startend
+            ),
             start_i_index=i_index,
             children_start_s_index=children_start_s_index,
             offset_into_children_start_s=offset_into_children_start_s,
@@ -263,9 +306,13 @@ def finalize_tag(
         """Finalize an OpenTag into a TNode."""
         match open_tag:
             case OpenTElement(tag=tag, attrs=attrs, children=children):
-                return TElement(tag=tag, attrs=attrs, children=tuple(children))
+                tnode = TElement(tag=tag, attrs=attrs, children=tuple(children))
+                self.tmap[tnode] = open_tag
+                return tnode
             case OpenTFragment(children=children):
-                return TFragment(children=tuple(children))
+                tnode = TFragment(children=tuple(children))
+                self.tmap[tnode] = open_tag
+                return tnode
             case OpenTComponent(
                 start_i_index=start_i_index,
                 children_start_s_index=children_start_s_index,
@@ -279,12 +326,14 @@ def finalize_tag(
                     offset_into_children_start_s=offset_into_children_start_s,
                     template=self.get_source().template,
                 )
-                return TComponent(
+                tnode = TComponent(
                     start_i_index=start_i_index,
                     end_i_index=endtag_i_index,
                     children_ref=children_ref,
                     attrs=attrs,
                 )
+                self.tmap[tnode] = open_tag
+                return tnode
 
     def extract_component_children_ref(
         self,
@@ -339,7 +388,7 @@ def extract_component_children_ref(
 
     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."""
-        assert self.source, "Parser source tracker not initialized."
+        source = self.get_source()
         tag_ref = self.placeholders.remove_placeholders(tag)
 
         match open_tag:
@@ -359,18 +408,60 @@ def validate_end_tag(self, tag: str, open_tag: OpenTag) -> int | None:
 
             case OpenTComponent(start_i_index=start_i_index):
                 if tag_ref.is_literal:
-                    raise ValueError(
-                        f"Mismatched closing tag </{tag}> for component starting at {self.source.format_starttag(start_i_index)}."
+                    starttag = source.format_starttag(start_i_index)
+                    e = ValueError(
+                        f"Mismatched closing tag </{tag}> for component with tag {{{starttag}}}."
                     )
+                    if self.has_ambiguous_forward_slash(open_tag):
+                        e.add_note(
+                            f'Did you mean to quote the last attribute or put a space before "/>" for "<{{{starttag}}} .../>"?'
+                        )
+                    raise e
                 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.
                 return tag_ref.i_indexes[0]
 
+    def 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 = super().get_starttag_text()
+        if starttag_text is None:
+            raise AssertionError(msg)
+        return starttag_text
+
+    def has_ambiguous_forward_slash(self, open_tag: OpenTag) -> 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 "<div title=test/>" or "<{Component} title=test/>".
+
+        Or more often "<{Component} title={title}/>" which should be corrected
+        with "<{Component} title={title} />".
+        """
+        if isinstance(open_tag, (OpenTElement, OpenTComponent)):
+            parse_info = open_tag.parse_info
+            return (
+                # has attributes
+                len(parse_info.raw_attrs) > 0
+                # last attr not bare attribute
+                and parse_info.raw_attrs[-1][1] is not None
+                # last char of last attr is "/"
+                and parse_info.raw_attrs[-1][1][-1] == "/"
+                # parsed starttag ends with "/>"
+                and parse_info.starttag_text.endswith("/>")
+                # if parsed as startend then its not ambiguous
+                and not parse_info.startend
+            )
+        return False
+
     # ------------------------------------------
     # HTMLParser tag callbacks
     # ------------------------------------------
@@ -385,19 +476,54 @@ def handle_starttag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> None:
 
     def handle_startendtag(self, tag: str, attrs: Sequence[HTMLAttribute]) -> None:
         """Dispatch a self-closing tag, `<tag />` to specialized handlers."""
-        open_tag = self.make_open_tag(tag, attrs)
+        open_tag = self.make_open_tag(tag, attrs, startend=True)
         final_tag = self.finalize_tag(open_tag)
         self.append_child(final_tag)
 
     def handle_endtag(self, tag: str) -> None:
         if not self.stack:
-            raise ValueError(f"Unexpected closing tag </{tag}> with no open tag.")
-
+            tag_ref = self.placeholders.copy().remove_placeholders(tag)
+            if tag_ref.is_literal:
+                raise ValueError(f"Unexpected closing tag </{tag}> with no open tag.")
+            if not tag_ref.is_singleton:
+                # @TODO: Also it doesn't match anything
+                raise ValueError(
+                    "Component end tags must have exactly one interpolation."
+                )
+            # Component tag endtag but no component tag is open...
+            source = self.get_source()
+            unmatched_endtag = source.format_endtag(tag_ref.i_indexes[0])
+            raise ValueError(
+                f"Unexpected closing component tag </{{{unmatched_endtag}}}> with no open tag."
+            )
         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)
         self.append_child(final_tag)
 
+    def get_closed_tcomps(self, root: OpenTag | None) -> 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)
+            elif isinstance(node, (TElement, TFragment)):
+                nodes.extend(node.children)
+        return tcomps
+
     # ------------------------------------------
     # HTMLParser other callbacks
     # ------------------------------------------
@@ -436,6 +562,7 @@ def reset(self):
         self.stack = []
         self.placeholders = PlaceholderState()
         self.source = None
+        self.tmap = {}
 
     def close(self) -> None:
         if self.waiting_for_data():
@@ -449,7 +576,42 @@ def close(self) -> None:
                     "Parser expects more data, is the template valid html?"
                 )
         if self.stack:
-            raise ValueError("Invalid HTML structure: unclosed tags remain.")
+            source = self.get_source()
+            e = ValueError("Invalid HTML structure: unclosed tags remain.")
+            # @TODO: We need to determine which tags this might apply to,
+            # this only applies to components.
+            parent = self.stack[-1]
+            if isinstance(parent, OpenTComponent) and self.has_ambiguous_forward_slash(
+                parent
+            ):
+                # CASE: "<{C1} attr={value}/>" -- meant to self-close
+                # Maybe user meant to self-close?
+                starttag = source.format_starttag(parent.start_i_index)
+                e.add_note(
+                    f'Did you mean to quote the last attribute or put a space before "/>" for "<{{{starttag}}} .../>"?'
+                )
+            else:
+                # CASE: "<{C2}><{C1} attr=/></{C2}>"
+                # Maybe user meant to self-close <{C1} ...>, but closed by </{C2}> leaving <{C2}...> open?
+                for comp in reversed(self.get_closed_tcomps(parent)):
+                    if (
+                        comp.end_i_index is not None
+                        and comp.start_i_index != comp.end_i_index
+                        and not source.values_match(
+                            comp.start_i_index, comp.end_i_index
+                        )
+                    ):
+                        closed_tag = self.tmap[comp]
+                        starttag = source.format_starttag(comp.start_i_index)
+                        endtag = source.format_endtag(comp.end_i_index)
+                        e.add_note(
+                            f"Component start tag, <{{{starttag}}}>, and end tag, </{{{endtag}}}>, have values that do not match."
+                        )
+                        if self.has_ambiguous_forward_slash(closed_tag):
+                            e.add_note(
+                                f'Did you mean to quote the last attribute or put a space before "/>" for "<{{{starttag}}} .../>"?'
+                            )
+            raise e
         if not self.placeholders.is_empty:
             raise ValueError("Some placeholders were never resolved.")
         super().close()