From c505d0de0840c69809fd90b05f531334b2eae9be Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 27 Sep 2026 13:04:30 +0000 Subject: [PATCH] fix(liquid): extract statements inside liquid blocks (#1906) (#2008) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(liquid): read tags inside liquid blocks Extract render/include, section, and assign from liquid block bodies while preserving source positions and avoiding double counting. Add seven regression tests. Co-Authored-By: Claude * fix(liquid): extract statements inside liquid blocks (#1906) Liquid extraction required individual braced tags, missing statements inside liquid blocks. Build on PR #1907’s shared scanner while suppressing comment and raw regions. Preserve source positions and verify snippet and section resolution with real SQLite. Co-authored-by: Claude Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude --- __tests__/extraction.test.ts | 138 +++++++++++++++++++++++++++++ src/extraction/liquid-extractor.ts | 113 +++++++++++++++++++---- 2 files changed, 232 insertions(+), 19 deletions(-) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index e1454a2eb..d095e895b 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -5060,6 +5060,121 @@ import 'package:flutter/material.dart'; expect(importNode?.name).toBe('price'); }); + /* Inside a {% liquid %} tag, each body line is a tag with no braces of + its own. Patterns anchored on `{%` miss these references. */ + it('should extract render inside a {% liquid %} block', () => { + const code = [ + '{% liquid', + ' assign heading = section.settings.title', + " render 'card', title: heading", + '%}', + ].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + + const names = result.nodes.filter((n) => n.kind === 'import').map((n) => n.name); + expect(names).toContain('card'); + }); + + it('should extract assign inside a {% liquid %} block', () => { + const code = ['{% liquid', ' assign heading = section.settings.title', '%}'].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + + const vars = result.nodes.filter((n) => n.kind === 'variable').map((n) => n.name); + expect(vars).toContain('heading'); + }); + + it('should extract section inside a {% liquid %} block', () => { + const code = ['{% liquid', " section 'header'", '%}'].join('\n'); + const result = extractFromSource('layout/theme.liquid', code); + + const names = result.nodes.filter((n) => n.kind === 'import').map((n) => n.name); + expect(names).toContain('header'); + }); + + it('should report the real line number for a tag inside a {% liquid %} block', () => { + const code = ['
', '{% liquid', ' assign x = 1', " render 'card'", '%}'].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + + const card = result.nodes.find((n) => n.kind === 'import' && n.name === 'card'); + expect(card?.startLine).toBe(4); + }); + + it('should not count a tag twice when both spellings appear', () => { + const code = ["{% render 'card' %}", '{% liquid', " render 'card'", '%}'].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + + const cards = result.nodes.filter((n) => n.kind === 'import' && n.name === 'card'); + expect(cards.length).toBe(2); + expect(new Set(cards.map((n) => n.startLine)).size).toBe(2); + }); + + it('should not read a bare `render` outside a {% liquid %} block as a tag', () => { + // Prose and filters mentioning the word must not become references. + const code = ['

We render the card below.

', "{{ product | render_as: 'card' }}"].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + + const names = result.nodes.filter((n) => n.kind === 'import').map((n) => n.name); + expect(names).not.toContain('card'); + }); + + it('should handle whitespace control on the {% liquid %} tag itself', () => { + const code = ['{%- liquid', " render 'card'", '-%}'].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + + const names = result.nodes.filter((n) => n.kind === 'import').map((n) => n.name); + expect(names).toContain('card'); + }); + + it('does not scan tag-like strings or inline comments twice', () => { + const code = [ + `{% assign example = "{% render 'ghost'" %}`, + `{% # {% render 'ghost' %}`, + '{% liquid', ` assign example = "{% include 'ghost'"`, + " # section 'ghost'", " echo 'render ghost'", '%}', + "{% liquid render 'live' %}", "{% liquid include 'after' %}", + ].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + expect(result.nodes.filter((n) => n.kind === 'import').map((n) => [n.name, n.startLine, n.startColumn])) + .toEqual([['live', 8, 10], ['after', 9, 10]]); + expect(result.unresolvedReferences.map((r) => r.referenceName)) + .toEqual(['snippets/live.liquid', 'snippets/after.liquid']); + }); + + it.each(['\n', '\r\n'])('preserves Liquid block positions with %j line endings', (newline) => { + const code = [ + '
', '{%- liquid', " assign heading = 'x'", "\tinclude 'legacy'", + " render 'card'", " section 'footer'", '-%}', " {% render 'after' %}", + ].join(newline); + const result = extractFromSource('sections/featured.liquid', code); + expect(result.nodes.filter((n) => n.kind === 'variable' || n.kind === 'import') + .map((n) => [n.name, n.startLine, n.startColumn, n.endColumn]).sort()) + .toEqual([ + ['after', 8, 2, 19], ['card', 5, 2, 15], ['footer', 6, 2, 18], + ['heading', 3, 2, 18], ['legacy', 4, 1, 17], + ]); + expect(result.unresolvedReferences.map((r) => [r.referenceName, r.line, r.column]).sort()) + .toEqual([ + ['sections/footer.liquid', 6, 2], ['snippets/after.liquid', 8, 2], + ['snippets/card.liquid', 5, 2], ['snippets/legacy.liquid', 4, 1], + ]); + }); + + it.each(['comment', 'raw'])('ignores %s regions in both tag spellings', (tag) => { + const code = [ + `{%- ${tag} -%}`, "{% render 'ghost' %}", "{% include 'ghost' %}", + "{% section 'ghost' %}", '{% assign ghost = 1 %}', + '{% liquid', " render 'ghost'", '%}', `{%- end${tag} -%}`, + '{% liquid', ` ${tag}`, " render 'ghost'", " include 'ghost'", + " section 'ghost'", ' assign ghost = 1', ` end${tag}`, + " # render 'ghost'", " render 'live'", '%}', "{% include 'after' %}", + ].join('\n'); + const result = extractFromSource('sections/featured.liquid', code); + expect(result.nodes.filter((n) => n.kind !== 'file').map((n) => n.name)) + .toEqual(['live', 'live', 'after', 'after']); + expect(result.unresolvedReferences.map((r) => [r.referenceName, r.line])) + .toEqual([['snippets/live.liquid', 18], ['snippets/after.liquid', 20]]); + }); + it('should extract multiple imports', () => { const code = ` {% section 'header' %} @@ -7038,6 +7153,29 @@ describe('Liquid Shopify JSON template section resolution', () => { if (fs.existsSync(tempDir)) fs.rmSync(tempDir, { recursive: true, force: true }); }); + it('resolves Liquid block snippets and sections without linking commented references', async () => { + for (const dir of ['sections', 'snippets', 'layout']) fs.mkdirSync(path.join(tempDir, dir)); + for (const file of ['snippets/card.liquid', 'snippets/legacy.liquid', 'snippets/ghost.liquid', 'sections/footer.liquid']) { + fs.writeFileSync(path.join(tempDir, file), '
content
'); + } + fs.writeFileSync(path.join(tempDir, 'layout/theme.liquid'), [ + '{% liquid', " assign heading = 'x'", " render 'card'", " include 'legacy'", + " section 'footer'", ' comment', " render 'ghost'", ' endcomment', '%}', + '{% raw %}', "{% render 'ghost' %}", '{% endraw %}', + ].join('\n')); + cg = CodeGraph.initSync(tempDir); + await cg.indexAll(); + cg.resolveReferences(); + + for (const name of ['card', 'legacy', 'footer', 'ghost']) { + const file = cg.getNodesByKind('file').find((n) => n.name === `${name}.liquid`)!; + expect(file).toBeDefined(); + expect(cg.getFileDependents(file.filePath).some((p) => p.endsWith('layout/theme.liquid'))) + .toBe(name !== 'ghost'); + } + expect(cg.getNodesByKind('variable').some((n) => n.name === 'heading')).toBe(true); + }); + it('links a Shopify JSON template section `type` to its sections/.liquid', async () => { // Shopify OS 2.0 templates are JSON, referencing sections by `type` — not // a `{% section %}` Liquid tag — so a section used only from a JSON template diff --git a/src/extraction/liquid-extractor.ts b/src/extraction/liquid-extractor.ts index 5bd9bacc0..895101df7 100644 --- a/src/extraction/liquid-extractor.ts +++ b/src/extraction/liquid-extractor.ts @@ -124,16 +124,96 @@ export class LiquidExtractor { } } + /** + * Every occurrence of a Liquid tag, in BOTH spellings it can be written in. + * + * Inside a `{% liquid %}` tag, each line of the body is a tag + * WITHOUT braces of its own: + * + * {% liquid + * assign heading = section.settings.title + * render 'card', title: heading + * %} + * + * A pattern anchored on `{%` misses these references and assignments. + * + * Returns real offsets into `this.source`, so callers keep using + * getLineNumber/getLineStart unchanged. + */ + private findTagOccurrences( + tagPattern: string, + argPattern: string, + ): Array<{ fullMatch: string; groups: string[]; index: number }> { + const found: Array<{ fullMatch: string; groups: string[]; index: number }> = []; + + // Consume complete braced tags so strings and comments cannot start a + // second match inside the same tag. Skip non-executing regions entirely. + const blocks: Array<{ bodyStart: number; body: string }> = []; + const tags = /\{%[-]?\s*(\w+|#)([\s\S]*?)[-]?%\}/g; + const braced = new RegExp(`^\\{%[-]?\\s*(${tagPattern})\\s+${argPattern}`); + let suppressed: string | undefined; + let tag; + while ((tag = tags.exec(this.source)) !== null) { + const name = tag[1]!; + if (suppressed) { + if (name === `end${suppressed}`) suppressed = undefined; + continue; + } + if (name === 'comment' || name === 'raw') { + suppressed = name; + continue; + } + if (name === 'liquid') { + blocks.push({ + bodyStart: tag.index + tag[0].indexOf(name) + name.length, + body: tag[2]!, + }); + continue; + } + const match = braced.exec(tag[0]); + if (match) { + found.push({ fullMatch: match[0], groups: match.slice(1) as string[], index: tag.index }); + } + } + + /* Inside a `{% liquid %}` body each tag starts its own line. Anchoring on + the line start keeps prose, filters and inline `#` comments out. */ + const bare = new RegExp(`^[ \\t]*(${tagPattern})[ \\t]+${argPattern}`); + for (const b of blocks) { + let offset = b.bodyStart; + let suppressed: string | undefined; + for (const line of b.body.split('\n')) { + const name = /^[ \t]*(\w+)/.exec(line)?.[1]; + if (suppressed) { + if (name === `end${suppressed}`) suppressed = undefined; + } else if (name === 'comment' || name === 'raw') { + suppressed = name; + } else { + const inner = bare.exec(line); + if (inner) { + found.push({ + fullMatch: inner[0].trimStart(), + groups: inner.slice(1) as string[], + index: offset + (inner[0].length - inner[0].trimStart().length), + }); + } + } + offset += line.length + 1; + } + } + + return found.sort((a, b) => a.index - b.index); + } + /** * Extract {% render 'snippet' %} and {% include 'snippet' %} references */ private extractSnippetReferences(fileNodeId: string): void { - // Match {% render 'name' %} or {% include 'name' %} with optional parameters - const renderRegex = /\{%[-]?\s*(render|include)\s+['"]([^'"]+)['"]/g; - let match; - - while ((match = renderRegex.exec(this.source)) !== null) { - const [fullMatch, tagType, snippetName] = match; + // Both spellings: {% render 'name' %} and a bare `render 'name'` line + // inside a {% liquid %} block. + for (const match of this.findTagOccurrences('render|include', `['"]([^'"]+)['"]`)) { + const fullMatch = match.fullMatch; + const [tagType, snippetName] = match.groups; const line = this.getLineNumber(match.index); // Create an import node for searchability @@ -202,12 +282,10 @@ export class LiquidExtractor { * Extract {% section 'name' %} references */ private extractSectionReferences(fileNodeId: string): void { - // Match {% section 'name' %} - const sectionRegex = /\{%[-]?\s*section\s+['"]([^'"]+)['"]/g; - let match; - - while ((match = sectionRegex.exec(this.source)) !== null) { - const [fullMatch, sectionName] = match; + // Both spellings, as for render/include above. + for (const match of this.findTagOccurrences('section', `['"]([^'"]+)['"]`)) { + const fullMatch = match.fullMatch; + const sectionName = match.groups[1]; const line = this.getLineNumber(match.index); // Create an import node for searchability @@ -335,12 +413,9 @@ export class LiquidExtractor { * Extract {% assign var = value %} statements */ private extractAssignments(fileNodeId: string): void { - // Match {% assign variable_name = ... %} - const assignRegex = /\{%[-]?\s*assign\s+(\w+)\s*=/g; - let match; - - while ((match = assignRegex.exec(this.source)) !== null) { - const [, variableName] = match; + // Both spellings. Most of a modern theme's assigns are the bare kind. + for (const match of this.findTagOccurrences('assign', `(\\w+)\\s*=`)) { + const variableName = match.groups[1]; const line = this.getLineNumber(match.index); // Create a variable node @@ -356,7 +431,7 @@ export class LiquidExtractor { startLine: line, endLine: line, startColumn: match.index - this.getLineStart(line), - endColumn: match.index - this.getLineStart(line) + match[0].length, + endColumn: match.index - this.getLineStart(line) + match.fullMatch.length, updatedAt: Date.now(), };