fix(parser): surface diagnostic when end tag skips an unclosed child#308
Merged
Merged
Conversation
…child The HTML template parser silently accepted templates like `<div><span></div>` — `</div>` implicitly closed the still-open `<span>` without surfacing any diagnostic, so the malformed template compiled cleanly with an empty `result.errors`. Angular's reference `_popContainer` in `ml_parser/parser.ts` tracks whether any implicitly-closed container required an explicit end tag (i.e. its tag definition lacks `closedByParent`) and emits an "Unexpected closing tag" diagnostic attached to the offending end tag. Mirror that behavior: - `pop_element_container` returns `(Option<HtmlNode>, bool)`; the bool is set whenever a non-`closed_by_parent` element (or any block) is implicitly closed above the matching one. - `consume_element_end` emits the diagnostic when that flag is true, while recovery still produces an AST node for the outer element. - `auto_close_element_if_needed` discards the flag — it pops one element at a time and never skips containers. Optional-end-tag elements (`<li>`, `<td>`, etc.) and the legitimate unclosed-at-EOF case remain silent, matching Angular's reference. Closes #290 (template portion; ambiguous DI unions `A | B` are a separate code path and left for follow-up). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<div><span></div>(and similar malformed templates) now produces anUnexpected closing tag "div"diagnostic inresult.errors/result.diagnosticsinstead of compiling silently._popContainer(ml_parser/parser.ts), which tracks whether any implicitly-closed container above the match required an explicit end tag and surfaces a diagnostic on the offending end tag.Why this was broken
pop_element_containerwas popping skipped containers above the match without distinguishing optional-end-tag elements (<li>,<td>, etc.) from ones that legitimately need an end tag. So</div>closing past an open<span>was treated the same as</ul>closing past an open<li>— completely silent.Test plan
cargo test— 2559 passed, 0 failedcrates/oxc_angular_compiler/src/parser/html/parser.rs:test_parse_unclosed_inner_tag_reports_error— the exact repro from the issuetest_parse_mismatched_close_does_not_error_for_optional_end_tag—<ul><li>...</ul>stays silenttest_parse_multiple_unclosed_inner_tags_reports_error—</section>skipping past<div>and<span>test_malformed_template_surfaces_parse_diagnosticcompiling theBadComponentfrom the issue and asserting the diagnostic propagates to the file-level transform resultcargo fmt --all -- --checkcleanOut of scope
The issue also mentions ambiguous DI unions (
A | B) — that's a separate code path (util/type_extract.rs::resolve_di_token_type) and worth its own PR.🤖 Generated with Claude Code
Note
Low Risk
Localized parser diagnostic logic with recovery unchanged; covered by new unit and integration tests.
Overview
Malformed templates where a closing tag skips still-open children (e.g.
<div><span></div>) now emit an Unexpected closing tag diagnostic on that end tag instead of compiling with empty errors, aligned with Angular’s_popContainerbehavior.pop_element_containernow returns whether any implicitly closed container above the match required an explicit end tag—usingclosed_by_parentfor elements and always treating @blocks as non-silent—while HTML5 auto-close from a new opening tag stays unchanged. Optional-end-tag cases like<ul><li>…</ul>remain error-free, and recovery still builds the outer AST. An integration test asserts the diagnostic reaches file-leveltransform_angular_fileresults.Reviewed by Cursor Bugbot for commit cae23ac. Bugbot is set up for automated code reviews on this repo. Configure here.