trending/codegraph
1
0
Fork 0
mirror of https://github.com/colbymchenry/codegraph.git synced 2026-10-23 06:50:54 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(liquid): variables declared with {% capture %} are indexed, as assigned ones are (#2814)

Browse Source 
The Liquid extractor indexed a variable declared with {% assign %} as a
variable node, in both spellings (braced, and as a line of a
{% liquid %} tag) and skipping comment and raw regions, but it never
read {% capture %}, Liquid's other way to declare a variable. A
captured variable could not be found: search missed it, and
codegraph_explore reported its name as not in the index.

Fix:
- extractAssignments reads capture tags through the same
  findTagOccurrences scanner as assign, so both spellings and the
  comment/raw rule apply. The name is word characters, as for assign,
  optionally in single or double quotes (Liquid reads 'title' as
  title), and must end at whitespace or the tag's end (-%} included). A
  name that goes on with "-" or ".", which the assign rule doesn't read
  either, gets no node rather than a cut-short one.
- Captures are read after assigns. Node ids are keyed by file, kind,
  name and line, so an assign keeps its id when a capture of the same
  name shares its line.
- One helper now builds the variable node and its contains edge for
  both. It computes the declaration's column once, where the assign
  code computed it three times, each a split of the whole file.

Validation:
- 13 new tests in __tests__/extraction.test.ts. 11 fail with main's
  extractor and pass with the fix: 8 declaration forms (braced,
  whitespace-trimmed, tight -%}, single- and double-quoted, {% liquid %}
  lines with LF and CRLF, a quoted {% liquid %} line), an assign and two
  captures of one name on one line (distinct ids, the assign keeps
  its), tags inside a capture body, and an index-level test that search
  finds captured variables in a non-theme folder. 2 guards pass on
  both: no capture from comment, raw, inline comments, prose, a
  nameless tag or a tag inside a string, and no cut-short hyphenated
  or dotted name. Red/green on the committed tree: 42 passed and 11
  failed with main's extractor, 53 passed with the fix.
- A/B on efb091ec, each arm built from its own checkout: 8 Shopify
  theme repos (Shopify/dawn, Shopify/horizon, Shopify/skeleton-theme,
  ChumsInc/chums-impulse-dev, agencecinq/cinquante-et-un,
  noclocks/shopify-devstore-theme, panoply/syncify, Shopify/theme-tools)
  and 2 controls (express, requests). The controls and skeleton-theme,
  which has no capture, are byte-identical. Every other theme gains
  only variable nodes, each with one contains edge: dawn 36, horizon
  192, chums-impulse-dev 90, cinquante-et-un 25, shopify-devstore-theme
  27, syncify 25, theme-tools 49, 444 in all (383 braced, 61 in
  {% liquid %} tags). That is exactly the count of capture tags a
  separate scan finds outside comment and raw regions, and a script
  checked each new node against the source text: it sits on a capture
  tag naming it, with no edge but its contains. 444 right, 0 wrong.
  Nothing is removed or moved, no unresolved ref changes, and no node
  present on both arms changed its id. Every capture and assign name in
  these themes is a plain word.
- Cost: the extractor is faster with the fix. A microbenchmark over
  1,386 Liquid files (8 MB), 15 runs in each of 3 interleaved rounds,
  gave per-round medians of 443-465 ms on main and 316-387 ms with the
  fix, because whole-file line-start scans fall from 29,807 to 20,671.
  Index wall time, median of 3 interleaved runs, main vs fix: horizon
  2975 vs 2935 ms, chums-impulse-dev 2404 vs 2311, syncify 4329 vs
  3867, theme-tools 8405 vs 8108, and the byte-identical controls
  express 2427 vs 2417 and requests 2006 vs 1942, which is within the
  noise of a shared machine.
- Rebased onto 68fcf2f1, which adds only PR #2813 (JS/TS static and
  instance overrides, with a kernel change, so the kernel was rebuilt).
  There the Liquid tests pass, and re-indexing the 8 theme repos
  changes no Liquid or JSON line: 4 are byte-identical to the
  pre-rebase fix arm, and the others differ only in PR #2813's
  is_static flag on JavaScript methods (dawn 30, horizon 26,
  shopify-devstore-theme 14) and one synthesized TypeScript call in
  syncify.
- Kernel parity: not applicable, the kernel does not extract Liquid.

The issue also asks for README and docs-site wording about what Liquid
support covers. That is PR #2815, a docs-only change held for the
maintainer's review, so this change leaves the issue open.

Refs #2498

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-22 21:49:29 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 68fcf2f1c7
commit 53f72fec90
3 changed files with 121 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -119,6 +119,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In a repository that holds more than one Shopify theme, a theme's `{% render %}`, `{% include %}` and `{% section %}` tags and the sections its JSON templates list now link only to that theme's own snippets and sections, which is where Shopify looks for them. Before, a section or snippet the theme doesn't have was linked to another theme's file of the same name, and a theme kept in a subfolder could link to the root theme's copy instead of its own. A reference the theme can't satisfy now links to nothing. Projects with a single theme are indexed as before. Re-index projects with several Shopify themes after upgrading.
- `codegraph sync` and the file watcher now link a Shopify section or snippet to the files that name it when it is added after them, or comes back after being deleted, whether they name it in a `{% render %}`, `{% include %}` or `{% section %}` tag or as a section `type` in a JSON template. Before, the link appeared only once a file naming it changed or the project was re-indexed, even in a repository with a single theme, so the new section or snippet looked unused and `codegraph affected` missed the files that use it. Re-index Shopify themes after upgrading to pick up links an earlier version missed.
- A Shopify JSON template or section group saved by the theme editor now links the sections it lists. The editor writes a comment at the top of every file it saves, and a file with that comment, or with a trailing comma, both of which Shopify accepts, linked none of its sections, so in an editor-saved theme most sections looked unused. A layout's `{% sections 'header-group' %}` tag now links to the section group it renders, each section a JSON file lists is linked from the line that names it rather than from the top of the file, and a `{% schema %}` written inside `{% comment %}` or `{% raw %}` is no longer indexed as the section's own. Re-index Shopify themes after upgrading. Thanks @Puma7 for the report and the pull request. (#2494)
- In Liquid, a variable declared with `{% capture %}`, like `{% capture title %}…{% endcapture %}`, a quoted `{% capture 'title' %}` or a `capture title` line inside `{% liquid %}`, is now indexed like one declared with `{% assign %}`, so search and `codegraph_explore` can find it. Before, only assigned variables were indexed, so a captured one could not be found, and `codegraph_explore` said its name was not in the index. Re-index Liquid projects after upgrading. Thanks @Puma7 for the report and the pull request. (#2498)
- `codegraph sync` and the file watcher now link a route to the page or layout it loads lazily when that file is added after the router was indexed, comes back after being deleted, or gets the component the route renders in a later edit, whether or not the import names the file's extension: React Router's `lazy: () => import('./pages/Team')`, Vue Router's `component: () => import('@/views/Login')` and Angular's `loadComponent: () => import('./home/home.component')`. Before, the route stayed unlinked until the router file changed or the project was re-indexed, so the page looked unused and `codegraph affected` missed the tests that reach it through the router. Re-index after upgrading to pick up links an earlier version missed.
- A file named the way Google names its tests, like `wire_format_unittest.cc` in protocolbuffers/protobuf, `logging_unittest.cc` in google/glog or a Python `foo_unittest.py`, now counts as a test, as `wire_format_test.cc` already did. Before, it was taken for production code: search and `codegraph_explore` ranked it alongside the code it tests, `codegraph affected` never listed it, and calls in production code could link to a class or type a unittest declares for itself, as google/breakpad's `stack_frame_entries_.size()` did to a unittest's `StackHelper`. Re-index projects that have such files after upgrading.
- A file in a `unittests/` folder, where llvm/llvm-project and google/breakpad keep their unit tests, or in a folder named like a unittest, such as `dcheck_unittest/` in google/glog, now counts as a test whatever its own name says, like LLVM's `CountCopyAndMove.h` helper. Before, such a file was taken for production code unless its name said test, and none of them were kept apart from the program the way a `tests/` folder is: search and `codegraph_explore` ranked them alongside production code, `codegraph affected` missed them, and production code linked to a unit test's own declarations, as LLVM's `Ctx` and `OpInfo` variables did to ones its unit tests declare. A folder named just `unittest/`, like CPython's `Lib/unittest/`, is still production code. Re-index projects that have such folders after upgrading.
__tests__/extraction.test.ts
+76
View File
@@ -5325,6 +5325,64 @@ import 'package:flutter/material.dart';
expect(vars).toContain('heading');
});
/* `{% capture name %}…{% endcapture %}` declares a variable as `assign`
does; only assigns were read (#2498). */
it.each([
['{% capture captured %}Hello{% endcapture %}', 'captured', 1, 0],
['{%- capture trimmed -%}Hi{%- endcapture -%}', 'trimmed', 1, 0],
['{% capture tight-%}Hi{% endcapture %}', 'tight', 1, 0],
["<p>{% capture 'single' %}Hi{% endcapture %}</p>", 'single', 1, 3],
['{% capture "double" %}Hi{% endcapture %}', 'double', 1, 0],
['{% liquid\n capture bare\n echo "Hi"\n endcapture\n%}', 'bare', 2, 2],
['{% liquid\r\n capture crlf # a note\r\n endcapture\r\n%}', 'crlf', 2, 2],
["{%- liquid\n capture 'quoted_line'\n endcapture\n-%}", 'quoted_line', 2, 2],
] as const)('reads a capture declaration from %j', (code, name, line, column) => {
const result = extractFromSource('sections/card.liquid', code);
const variables = result.nodes.filter((n) => n.kind === 'variable');
expect(variables.map((n) => [n.name, n.qualifiedName, n.startLine, n.startColumn]))
.toEqual([[name, `sections/card.liquid::${name}`, line, column]]);
const file = result.nodes.find((n) => n.kind === 'file')!;
expect(result.edges).toContainEqual({ source: file.id, target: variables[0]!.id, kind: 'contains' });
});
it('reads no capture from comments, raw regions, prose or a tag without a name', () => {
const code = [
'{% comment %}{% capture ghost %}x{% endcapture %}{% endcomment %}',
'{%- raw -%}{% capture ghost %}x{% endcapture %}{%- endraw -%}',
'{% # capture ghost %}',
'{% liquid', ' # capture ghost', ' comment', ' capture ghost', ' endcomment', '%}',
'{% capture %}x{% endcapture %}',
'<p>We capture ghost moments.</p>',
'{% assign text = "{% capture ghost %}" %}',
].join('\n');
const result = extractFromSource('sections/card.liquid', code);
expect(result.nodes.filter((n) => n.kind === 'variable').map((n) => n.name)).toEqual(['text']);
});
it('never cuts a capture name short', () => {
// Liquid allows `-` and `.` in a name, which the assign rule doesn't read
// either; such a name gets no node rather than a wrong one.
const code = ['{% capture ghost-name %}x{% endcapture %}', '{% capture ghost.name %}x{% endcapture %}'].join('\n');
expect(extractFromSource('sections/card.liquid', code).nodes.filter((n) => n.kind === 'variable')).toEqual([]);
});
it('keeps an assign and captures of the same name on one line apart', () => {
const code = '{% assign title = 1 %}{% capture title %}one{% endcapture %}{% capture title %}two{% endcapture %}';
const variables = extractFromSource('sections/card.liquid', code).nodes.filter((n) => n.kind === 'variable');
expect(variables.map((n) => [n.name, n.startColumn]))
.toEqual([['title', 0], ['title', code.indexOf('{% capture')], ['title', code.lastIndexOf('{% capture')]]);
expect(new Set(variables.map((n) => n.id)).size).toBe(3);
// The assign keeps the id it had before captures were read.
expect(variables[0]!.id).toBe(generateNodeId('sections/card.liquid', 'variable', 'title', 1));
});
it('still reads the tags inside a capture', () => {
const code = "{% capture card_html %}{% render 'card' %}{% assign inner = 1 %}{% endcapture %}";
const result = extractFromSource('sections/product.liquid', code);
expect(result.nodes.filter((n) => n.kind === 'variable').map((n) => n.name).sort()).toEqual(['card_html', 'inner']);
expect(result.unresolvedReferences.map((r) => r.referenceName)).toEqual(['snippets/card.liquid']);
});
it('should extract section inside a {% liquid %} block', () => {
const code = ['{% liquid', " section 'header'", '%}'].join('\n');
const result = extractFromSource('layout/theme.liquid', code);
@@ -7537,6 +7595,24 @@ describe('Liquid Shopify JSON template section resolution', () => {
expect(cg.getNodesByKind('variable').some((n) => n.name === 'heading')).toBe(true);
});
it('indexes capture declarations as variables that search finds, in or out of a Shopify theme', async () => {
// A captured variable could not be found, while an assigned one could (#2498).
fs.mkdirSync(path.join(tempDir, '_includes'), { recursive: true });
fs.writeFileSync(path.join(tempDir, '_includes/card.liquid'), [
"{% assign existing = 'ok' %}",
'{% capture captured %}Hello{% endcapture %}',
'{% liquid', ' capture bare_capture', " echo 'x'", ' endcapture', '%}',
].join('\n'));
cg = CodeGraph.initSync(tempDir);
await cg.indexAll();
for (const [name, line] of [['existing', 1], ['captured', 2], ['bare_capture', 4]] as const) {
const hit = cg.searchNodes(name, { kinds: ['variable'] }).find((r) => r.node.name === name);
expect(hit?.node, name).toMatchObject({ filePath: '_includes/card.liquid', language: 'liquid', startLine: line });
expect(cg.getIncomingEdges(hit!.node.id)).toContainEqual(expect.objectContaining({ kind: 'contains' }));
}
});
it('links a Shopify JSON template section `type` to its sections/<type>.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
src/extraction/liquid-extractor.ts
+44 -30
View File
@@ -11,6 +11,7 @@ import { generateNodeId, NodeIdAllocator } from './tree-sitter-helpers';
* ({% sections 'header-group' %} → `sections/header-group.json`)
* - Snippet references ({% render 'name' %} and {% include 'name' %})
* - Schema blocks ({% schema %}...{% endschema %})
* - Variables declared with {% assign %} and {% capture %}
* - The sections a Shopify JSON template or section group lists
*/
export class LiquidExtractor {
@@ -55,7 +56,7 @@ export class LiquidExtractor {
// Extract schema block
this.extractSchema(fileNode.id);
// Extract assign statements as variables
// Extract assign and capture declarations as variables
this.extractAssignments(fileNode.id);
}
} catch (error) {
@@ -459,40 +460,53 @@ export class LiquidExtractor {
}
/**
* Extract {% assign var = value %} statements
* Extract {% assign var = value %} and {% capture var %}…{% endcapture %}
* declarations as variables
*/
private extractAssignments(fileNodeId: string): void {
// 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
const nodeId = this.nodeIds.generate(this.filePath, 'variable', variableName!, line, match.index - this.getLineStart(line));
const node: Node = {
id: nodeId,
kind: 'variable',
name: variableName!,
qualifiedName: `${this.filePath}::${variableName}`,
filePath: this.filePath,
language: 'liquid',
startLine: line,
endLine: line,
startColumn: match.index - this.getLineStart(line),
endColumn: match.index - this.getLineStart(line) + match.fullMatch.length,
updatedAt: Date.now(),
};
this.nodes.push(node);
// Add containment edge from file
this.edges.push({
source: fileNodeId,
target: nodeId,
kind: 'contains',
});
this.addVariable(fileNodeId, match.groups[1]!, match);
}
// A capture declares its name as an assign does, and Liquid reads a quoted
// name (`{% capture 'title' %}`) as the name inside the quotes. A name that
// goes on with `-` or `.`, which the assign rule doesn't read either, gets
// no node rather than a cut-short one. Captures come after assigns, so an
// assign keeps its id when a capture of the same name shares its line.
for (const match of this.findTagOccurrences('capture', `(?:'(\\w+)'|"(\\w+)"|(\\w+))(?=\\s|-?%\\}|$)`)) {
const [, singleQuoted, doubleQuoted, bare] = match.groups;
this.addVariable(fileNodeId, (singleQuoted ?? doubleQuoted ?? bare)!, match);
}
}
/** A `variable` node, contained by the file, for a declaration that `findTagOccurrences` found. */
private addVariable(fileNodeId: string, variableName: string, match: { fullMatch: string; index: number }): void {
const line = this.getLineNumber(match.index);
const column = match.index - this.getLineStart(line);
const nodeId = this.nodeIds.generate(this.filePath, 'variable', variableName, line, column);
const node: Node = {
id: nodeId,
kind: 'variable',
name: variableName,
qualifiedName: `${this.filePath}::${variableName}`,
filePath: this.filePath,
language: 'liquid',
startLine: line,
endLine: line,
startColumn: column,
endColumn: column + match.fullMatch.length,
updatedAt: Date.now(),
};
this.nodes.push(node);
// Add containment edge from file
this.edges.push({
source: fileNodeId,
target: nodeId,
kind: 'contains',
});
}
/**