Skip to content

Commit da93ee3

Browse files
graygilmoreclaude
andcommitted
Handle paired conditionals across sibling branches in UnclosedHTMLElement
Add a cross-identifier balancing pass that matches unmatched opens from one conditional against unmatched closes from a sibling conditional within the same grandparent. This handles patterns like the bento-grid where forloop.first opens a div and forloop.last closes it — different conditions but semantically paired. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 064a2be commit da93ee3

2 files changed

Lines changed: 100 additions & 2 deletions

File tree

packages/theme-check-common/src/checks/unclosed-html-element/index.spec.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,61 @@ describe('Module: UnclosedHTMLElement', () => {
188188
expect(highlightedOffenses(file, offenses)).to.include('</summary>');
189189
});
190190

191+
it('should not report for paired conditional open/close in for-loop boundary conditions', async () => {
192+
const testCases = [
193+
// Bento-grid pattern: open on forloop.first, close on forloop.last
194+
`
195+
{% for item in items %}
196+
{% if forloop.first or item_modulo == 1 %}
197+
<div class="bento-box">
198+
{% endif %}
199+
200+
<div class="bento-box__item">{{ item }}</div>
201+
202+
{% if forloop.last or item_modulo == 0 %}
203+
</div>
204+
{% endif %}
205+
{% endfor %}
206+
`,
207+
// Simpler variant: different conditions but balanced open/close in same grandparent
208+
`
209+
<div>
210+
{% if condition_a %}
211+
<section>
212+
{% endif %}
213+
214+
{% if condition_b %}
215+
</section>
216+
{% endif %}
217+
</div>
218+
`,
219+
];
220+
221+
for (const file of testCases) {
222+
const offenses = await runLiquidCheck(UnclosedHTMLElement, file);
223+
expect(offenses, file).to.have.length(0);
224+
}
225+
});
226+
227+
it('should still report when cross-identifier tags do not balance by name', async () => {
228+
const file = `
229+
<div>
230+
{% if condition_a %}
231+
<section>
232+
{% endif %}
233+
234+
{% if condition_b %}
235+
</article>
236+
{% endif %}
237+
</div>
238+
`;
239+
240+
const offenses = await runLiquidCheck(UnclosedHTMLElement, file);
241+
expect(offenses).to.have.length(2);
242+
expect(highlightedOffenses(file, offenses)).to.include('<section>');
243+
expect(highlightedOffenses(file, offenses)).to.include('</article>');
244+
});
245+
191246
it('should report offenses when conditions do not match', async () => {
192247
const file = `
193248
<div>

packages/theme-check-common/src/checks/unclosed-html-element/index.ts

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,13 @@ export const UnclosedHTMLElement: LiquidCheckDefinition = {
139139

140140
async onCodePathEnd() {
141141
for (const [grandparent, stacks] of stacksByGrandparent) {
142+
// First pass: match opens/closes within each condition identifier.
143+
// Collect unmatched nodes with their identifier for cross-identifier balancing.
144+
const unmatchedByIdentifier = new Map<
145+
ConditionIdentifer,
146+
(HtmlElement | HtmlDanglingMarkerClose)[]
147+
>();
148+
142149
for (const identifier of stacks.identifiers) {
143150
const openNodes = stacks.open.get(identifier) ?? [];
144151
const closeNodes = stacks.close.get(identifier) ?? [];
@@ -170,8 +177,44 @@ export const UnclosedHTMLElement: LiquidCheckDefinition = {
170177
}
171178
}
172179

173-
// At the end, whatever is left in the stack is a reported offense.
174-
for (const node of stack) {
180+
unmatchedByIdentifier.set(identifier, stack);
181+
}
182+
183+
// Second pass: try to balance unmatched opens from one condition
184+
// identifier against unmatched closes from sibling condition identifiers
185+
// within the same grandparent.
186+
//
187+
// This handles patterns like the bento-grid where an open tag is inside
188+
// `{% if forloop.first %}` and the matching close is inside
189+
// `{% if forloop.last %}` -- different conditions but semantically paired.
190+
const allUnmatched = ([] as (HtmlElement | HtmlDanglingMarkerClose)[])
191+
.concat(...unmatchedByIdentifier.values())
192+
.sort((a, b) => a.position.start - b.position.start);
193+
194+
const crossBalancedStack = [] as (HtmlElement | HtmlDanglingMarkerClose)[];
195+
for (const node of allUnmatched) {
196+
if (node.type === NodeTypes.HtmlElement) {
197+
crossBalancedStack.push(node);
198+
} else if (
199+
crossBalancedStack.length > 0 &&
200+
getName(node) === getName(crossBalancedStack.at(-1)!) &&
201+
crossBalancedStack.at(-1)!.type === NodeTypes.HtmlElement &&
202+
node.type === NodeTypes.HtmlDanglingMarkerClose
203+
) {
204+
crossBalancedStack.pop();
205+
} else {
206+
crossBalancedStack.push(node);
207+
}
208+
}
209+
210+
// Build a set of nodes that were resolved by cross-identifier balancing
211+
// so we can skip reporting them.
212+
const crossBalancedRemaining = new Set(crossBalancedStack);
213+
214+
for (const [identifier, unmatched] of unmatchedByIdentifier) {
215+
for (const node of unmatched) {
216+
if (!crossBalancedRemaining.has(node)) continue;
217+
175218
if (node.type === NodeTypes.HtmlDanglingMarkerClose) {
176219
context.report({
177220
message: `Closing tag does not have a matching opening tag for condition \`${identifier}\` in ${

0 commit comments

Comments
 (0)