diff --git a/CHANGELOG.md b/CHANGELOG.md index b74c5d37ef..5a6c8015e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In a Shopify theme, a JSON template or section group that starts with the comment the theme editor writes at the top now links the sections it lists, at the line of each section's `type`. Before, such a file linked nothing, so most sections of an editor-saved theme looked unused. Re-index Shopify themes after upgrading. (#2494) +- In a Shopify theme layout, `{% sections 'header-group' %}` now links to the section group `sections/header-group.json`. Before, only the singular `{% section %}` tag was linked. (#2494) +- A `{% schema %}` written inside `{% comment %}` or `{% raw %}` in a Liquid file is no longer indexed as a definition. (#2494) - In VB.NET, every member of a `Structure` is now indexed, including its fields, properties, methods, constructors and nested enums. Before, only the first member was, so the rest could not be found and their callers looked empty. - In VB.NET and C#, what a property's `Get` and `Set` code calls, creates and reads now belongs to that property, as do C#'s `get => …` accessors and `=> …` property bodies. Before, it was dropped, so a method used only from a property looked unused. - In VB.NET, a field or property initializer like `= Compute()` or `As New List(Of Order)` now links what it calls and creates, and so do a `Custom Event`'s `AddHandler`, `RemoveHandler` and `RaiseEvent` blocks. Re-index VB.NET and C# projects after upgrading. diff --git a/__tests__/liquid-shopify-editor-regressions.test.ts b/__tests__/liquid-shopify-editor-regressions.test.ts new file mode 100644 index 0000000000..8f57ad51a0 --- /dev/null +++ b/__tests__/liquid-shopify-editor-regressions.test.ts @@ -0,0 +1,163 @@ +import { describe, it, expect } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { LiquidExtractor } from '../src/extraction/liquid-extractor'; +import { CodeGraph } from '../src/index'; + +const extract = (file: string, source: string) => new LiquidExtractor(file, source).extract(); +const refs = (file: string, source: string) => extract(file, source).unresolvedReferences; + +describe('Shopify editor JSON and section groups', () => { + it.each(['\n', '\r\n'])('reads generated comments and reports type positions with %j', (newline) => { + const source = [ + '/* IMPORTANT: This file is generated by the Shopify editor. */', + '{', + ' "sections": {', + ' "main": {', + ' "type": "main-product",', + ' "settings": { "text": "https://host/path/*literal*/" }', + ' },', + ' "duplicate": { "type": "main-product" }', + ' },', + ' "order": ["main", "duplicate"]', + '}', + ].join(newline); + expect(refs('templates/product.json', source).map(r => [r.referenceName, r.line, r.column])) + .toEqual([['sections/main-product.liquid', 5, 14]]); + }); + + it.each([ + ['templates/customers/login.json', 'main-login'], + ['sections/header-group.json', 'header'], + ])('reads comments in %s', (file, type) => { + const source = `/* Generated */\n{"sections":{"main":{"type":"${type}"}},"order":["main"]}`; + expect(refs(file, source).map(r => r.referenceName)).toEqual([`sections/${type}.liquid`]); + }); + + it.each(['null', '[]', '{', '{"sections":[]}', '{"sections":{"main":null}}', + '{"sections":{"main":{"type":17}}}', '{"sections":{"main":{"type":"header"}}', + '{"sections":{"main":{"type":"header"}},}', + '/* unfinished {"sections":{"main":{"type":"header"}}}'])('does not invent dependencies from %s', source => { + expect(refs('templates/product.json', source)).toEqual([]); + }); + + it.each([ + "{% sections 'header-group' %}", + '{%- sections "header-group" -%}', + "{% liquid\n sections 'header-group'\n%}", + ])('links a section group to JSON: %s', source => { + expect(refs('layout/theme.liquid', source).map(r => r.referenceName)) + .toEqual(['sections/header-group.json']); + }); + + it('keeps singular sections distinct from groups of the same name', () => { + const result = extract('layout/theme.liquid', "{% section 'header' %}{% sections 'header' %}"); + expect(result.unresolvedReferences.map(r => r.referenceName)) + .toEqual(['sections/header.liquid', 'sections/header.json']); + expect(new Set(result.nodes.map(n => n.id)).size).toBe(result.nodes.length); + }); + + it('preserves strict JSON section dependencies and gives their actual positions', () => { + const source = '{\n "sections": { "main": { "type": "header" } },\n "order": ["main"]\n}'; + expect(refs('templates/index.json', source).map(r => [r.referenceName, r.line, r.column])) + .toEqual([['sections/header.liquid', 2, 34]]); + }); + + it('preserves standard Liquid assignments and suppresses commented/raw assignments', () => { + const source = "{% assign keep = 'hello' %}\n{% comment %}{% assign hidden = 1 %}{% endcomment %}\n" + + "{% raw %}{% assign hidden2 = 2 %}{% endraw %}\n{% assign after = 'world' %}"; + const result = extract('views/page.liquid', source); + expect(result.nodes.filter(n => n.kind === 'variable').map(n => [n.name, n.startLine])) + .toEqual([['keep', 1], ['after', 4]]); + expect(result.unresolvedReferences).toEqual([]); + }); + + it.each([ + "{% # sections 'ghost' %}", + "{% liquid\n # sections 'ghost'\n%}", + ])('does not infer group dependencies from inline comments: %s', source => { + expect(refs('layout/theme.liquid', source)).toEqual([]); + }); + + it.each(['comment', 'raw'])('ignores schemas and group references inside %s', tag => { + const source = `{% ${tag} %}{% sections 'ghost' %}{% schema %}{"name":"Ghost"}{% endschema %}{% end${tag} %}\n` + + '{% schema %}{"name":"Live"}{% endschema %}'; + const result = extract('sections/header.liquid', source); + expect(result.unresolvedReferences).toEqual([]); + expect(result.nodes.filter(n => n.kind === 'constant').map(n => [n.name, n.startLine])) + .toEqual([['Live', 2]]); + }); + + it('resolves the complete layout to group to section to snippet chain', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-shopify-editor-')); + let cg: CodeGraph | undefined; + try { + for (const dir of ['layout', 'sections', 'snippets', 'config']) fs.mkdirSync(path.join(root, dir)); + const files = { + 'config/settings_schema.json': '[]', + 'layout/theme.liquid': "{% sections 'header-group' %}", + 'sections/header-group.json': '/* Generated */\n{"type":"header","name":"Header","sections":{"main":{"type":"header"}},"order":["main"]}', + 'sections/header.liquid': "{% render 'logo' %}", + 'snippets/logo.liquid': 'Logo', + }; + for (const [file, source] of Object.entries(files)) fs.writeFileSync(path.join(root, file), source); + cg = CodeGraph.initSync(root); + await cg.indexAll(); + cg.resolveReferences(); + expect(cg.getFileDependencies('layout/theme.liquid')).toEqual(['sections/header-group.json']); + expect(cg.getFileDependencies('sections/header-group.json')).toEqual(['sections/header.liquid']); + expect(cg.getFileDependencies('sections/header.liquid')).toEqual(['snippets/logo.liquid']); + expect(cg.getFileDependents('sections/header.liquid')).toEqual(['sections/header-group.json']); + } finally { + cg?.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('leaves commented JSON in a non-Shopify project outside the Liquid index', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-non-shopify-json-')); + let cg: CodeGraph | undefined; + try { + for (const dir of ['templates', 'sections', 'views']) fs.mkdirSync(path.join(root, dir)); + fs.writeFileSync(path.join(root, 'templates/page.json'), '/* CMS data */\n{"sections":{"main":{"type":"hero"}}}'); + fs.writeFileSync(path.join(root, 'sections/hero.liquid'), '

{{ title }}

'); + fs.writeFileSync(path.join(root, 'views/page.liquid'), "{% assign title = 'Hello' %}"); + cg = CodeGraph.initSync(root); + await cg.indexAll(); + cg.resolveReferences(); + expect(cg.getNodesByKind('file').map(n => n.filePath)).not.toContain('templates/page.json'); + expect(cg.getFileDependents('sections/hero.liquid')).toEqual([]); + expect(cg.getNodesByKind('variable').some(n => n.name === 'title')).toBe(true); + } finally { + cg?.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('keeps group references within their own theme even when a group is missing', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-group-isolation-')); + let cg: CodeGraph | undefined; + try { + const files = { + 'first/layout/theme.liquid': "{% sections 'header-group' %}", + 'second/layout/theme.liquid': "{% sections 'header-group' %}", + 'second/sections/header-group.json': '/* Generated */\n{"type":"header","name":"Header","sections":{"main":{"type":"header"}},"order":["main"]}', + 'second/sections/header.liquid': '
Header
', + }; + for (const [file, source] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), source); + } + cg = CodeGraph.initSync(root); + await cg.indexAll(); + cg.resolveReferences(); + expect(cg.getFileDependencies('first/layout/theme.liquid')).toEqual([]); + expect(cg.getFileDependencies('second/layout/theme.liquid')).toEqual(['second/sections/header-group.json']); + expect(cg.getFileDependencies('second/sections/header-group.json')).toEqual(['second/sections/header.liquid']); + } finally { + cg?.close(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/src/extraction/liquid-extractor.ts b/src/extraction/liquid-extractor.ts index f6d9aee60a..e90cf86b9d 100644 --- a/src/extraction/liquid-extractor.ts +++ b/src/extraction/liquid-extractor.ts @@ -1,12 +1,13 @@ import { Node, Edge, ExtractionResult, ExtractionError, UnresolvedReference } from '../types'; import { generateNodeId, NodeIdAllocator } from './tree-sitter-helpers'; +import { parseTree, findNodeAtLocation, ParseError } from 'jsonc-parser'; /** * LiquidExtractor - Extracts relationships from Liquid template files * * Liquid is a templating language (used by Shopify, Jekyll, etc.) that doesn't * have traditional functions or classes. Instead, we extract: - * - Section references ({% section 'name' %}) + * - Section and section-group references ({% section/sections 'name' %}) * - Snippet references ({% render 'name' %} and {% include 'name' %}) * - Schema blocks ({% schema %}...{% endschema %}) */ @@ -18,6 +19,7 @@ export class LiquidExtractor { private edges: Edge[] = []; private unresolvedReferences: UnresolvedReference[] = []; private errors: ExtractionError[] = []; + private suppressedRanges?: Array<[number, number]>; constructor(filePath: string, source: string) { this.filePath = filePath; @@ -102,25 +104,29 @@ export class LiquidExtractor { * used only from a JSON template (the OS 2.0 norm) is no longer orphaned. */ private extractShopifyJsonSections(fromNodeId: string): void { - let parsed: unknown; - try { - parsed = JSON.parse(this.source); - } catch { - return; // not valid JSON (or a partial) — nothing to link - } - const sections = (parsed as { sections?: Record })?.sections; - if (!sections || typeof sections !== 'object') return; + // Shopify's editor prepends a comment to generated templates and groups. + // Parse with offsets, but reject incomplete input rather than invent links. + const errors: ParseError[] = []; + const parsed = parseTree(this.source, errors, { allowTrailingComma: false }); + if (errors.length || parsed?.type !== 'object') return; + const sections = findNodeAtLocation(parsed, ['sections']); + if (sections?.type !== 'object') return; const seen = new Set(); - for (const key of Object.keys(sections)) { - const type = sections[key]?.type; - if (typeof type !== 'string' || seen.has(type)) continue; + for (const property of sections.children ?? []) { + const value = property.children?.[1]; + if (value?.type !== 'object') continue; + const typeNode = findNodeAtLocation(value, ['type']); + if (typeNode?.type !== 'string') continue; + const type = typeNode.value as string; + if (seen.has(type)) continue; seen.add(type); + const line = this.getLineNumber(typeNode.offset); this.unresolvedReferences.push({ fromNodeId, referenceName: `sections/${type}.liquid`, referenceKind: 'references', - line: 1, - column: 0, + line, + column: typeNode.offset - this.getLineStart(line), }); } } @@ -152,18 +158,13 @@ export class LiquidExtractor { 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; + const ranges = this.getSuppressedRanges(); + let rangeIndex = 0; 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; - } + while (rangeIndex < ranges.length && tag.index >= ranges[rangeIndex]![1]) rangeIndex++; + if (rangeIndex < ranges.length && tag.index >= ranges[rangeIndex]![0]) continue; if (name === 'liquid') { blocks.push({ bodyStart: tag.index + tag[0].indexOf(name) + name.length, @@ -280,12 +281,13 @@ export class LiquidExtractor { } /** - * Extract {% section 'name' %} references + * Extract {% section 'name' %} and {% sections 'group' %} references */ private extractSectionReferences(fileNodeId: string): void { // Both spellings, as for render/include above. - for (const match of this.findTagOccurrences('section', `['"]([^'"]+)['"]`)) { + for (const match of this.findTagOccurrences('section|sections', `['"]([^'"]+)['"]`)) { const fullMatch = match.fullMatch; + const tagType = match.groups[0]; const sectionName = match.groups[1]; const line = this.getLineNumber(match.index); @@ -315,13 +317,13 @@ export class LiquidExtractor { }); // Create a component node for the section reference - const nodeId = this.nodeIds.generate(this.filePath, 'component', `section:${sectionName}`, line, match.index - this.getLineStart(line)); + const nodeId = this.nodeIds.generate(this.filePath, 'component', `${tagType}:${sectionName}`, line, match.index - this.getLineStart(line)); const node: Node = { id: nodeId, kind: 'component', name: sectionName!, - qualifiedName: `${this.filePath}::section:${sectionName}`, + qualifiedName: `${this.filePath}::${tagType}:${sectionName}`, filePath: this.filePath, language: 'liquid', startLine: line, @@ -343,7 +345,7 @@ export class LiquidExtractor { // Add unresolved reference to the section file this.unresolvedReferences.push({ fromNodeId: fileNodeId, - referenceName: `sections/${sectionName}.liquid`, + referenceName: `sections/${sectionName}.${tagType === 'sections' ? 'json' : 'liquid'}`, referenceKind: 'references', line, column: match.index - this.getLineStart(line), @@ -351,15 +353,40 @@ export class LiquidExtractor { } } + /** One cached braced comment/raw scanner shared by tags and schemas. */ + private getSuppressedRanges(): Array<[number, number]> { + if (this.suppressedRanges) return this.suppressedRanges; + const ranges: [number, number][] = []; + const tags = /\{%[-]?\s*(\w+|#)([\s\S]*?)[-]?%\}/g; + let suppressed: { name: string; start: number } | undefined; + let tag: RegExpExecArray | null; + while ((tag = tags.exec(this.source)) !== null) { + if (suppressed) { + if (tag[1] === `end${suppressed.name}`) { + ranges.push([suppressed.start, tags.lastIndex]); + suppressed = undefined; + } + } else if (tag[1] === 'comment' || tag[1] === 'raw') { + suppressed = { name: tag[1], start: tag.index }; + } + } + if (suppressed) ranges.push([suppressed.start, this.source.length]); + this.suppressedRanges = ranges; + return ranges; + } + /** * Extract {% schema %}...{% endschema %} blocks */ private extractSchema(fileNodeId: string): void { // Match {% schema %}...{% endschema %} const schemaRegex = /\{%[-]?\s*schema\s*[-]?%\}([\s\S]*?)\{%[-]?\s*endschema\s*[-]?%\}/g; + const suppressedRanges = this.getSuppressedRanges(); let match; while ((match = schemaRegex.exec(this.source)) !== null) { + const index = match.index; + if (suppressedRanges.some(([start, end]) => index >= start && index < end)) continue; const [fullMatch, schemaContent] = match; const startLine = this.getLineNumber(match.index); const endLine = this.getLineNumber(match.index + fullMatch.length); diff --git a/src/resolution/shopify-themes.ts b/src/resolution/shopify-themes.ts index 3c2f3c8b5c..16b72b6320 100644 --- a/src/resolution/shopify-themes.ts +++ b/src/resolution/shopify-themes.ts @@ -4,9 +4,10 @@ * `{% render 'price' %}` renders the `snippets/price.liquid` of the theme the * file belongs to; `{% section 'header' %}` and a JSON template's section * `"type"` name a file in that theme's `sections/`. Shopify never looks in - * another theme. The Liquid extractor records these as `snippets/.liquid` - * and `sections/.liquid`, and the path matcher took any file whose path - * ends with that, so in a repository holding several themes (a theme tool's + * another theme. The Liquid extractor records these as `snippets/.liquid`, + * `sections/.liquid` and, for a `{% sections %}` group, + * `sections/.json`, and the path matcher took any file whose path ends + * with that, so in a repository holding several themes (a theme tool's * examples, a store's seasonal themes) a theme missing a section linked to * another theme's copy of it. *