diff --git a/CHANGELOG.md b/CHANGELOG.md index 94b262e8e..942a9ae63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - JSON files in a `templates` or `sections` folder are now read as Shopify Liquid only inside a Shopify theme, where the folder that contains `templates` or `sections` also has `layout/theme.liquid` or `config/settings_schema.json`. Before, all such JSON was, so a .NET project template's `template.json`, a schematic or a CMS's content showed up in the index as an empty Liquid file, and one that lists `sections` could be linked to an unrelated Liquid file of the same name. Shopify themes, including one kept in a subfolder of a repository, are indexed as before, and the next sync removes JSON that an earlier version indexed this way. - 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) - `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. diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index ed460dd4c..eefa535c9 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -5417,6 +5417,40 @@ import 'package:flutter/material.dart'; .toEqual([['snippets/live.liquid', 18], ['snippets/after.liquid', 20]]); }); + /* A layout renders a section group, the JSON file `sections/.json`, + with the plural tag; only the singular one was read (#2494). */ + it.each([ + "{% sections 'header-group' %}", + '{%- sections "header-group" -%}', + "{% liquid\n sections 'header-group'\n%}", + ])('reads a section group from %j', (code) => { + const result = extractFromSource('layout/theme.liquid', code); + expect(result.nodes.filter((n) => n.kind === 'import').map((n) => n.name)).toEqual(['header-group']); + expect(result.unresolvedReferences.map((r) => r.referenceName)).toEqual(['sections/header-group.json']); + }); + + it('keeps a section and a section group of the same name apart', () => { + const result = extractFromSource('layout/theme.liquid', "{% section 'header' %}{% sections 'header' %}"); + expect(result.unresolvedReferences.map((r) => [r.referenceName, r.line, r.column])) + .toEqual([['sections/header.liquid', 1, 0], ['sections/header.json', 1, 22]]); + expect(result.nodes.filter((n) => n.kind === 'component').map((n) => n.qualifiedName)) + .toEqual(['layout/theme.liquid::section:header', 'layout/theme.liquid::sections:header']); + expect(new Set(result.nodes.map((n) => n.id)).size).toBe(result.nodes.length); + }); + + it.each(['comment', 'raw'])('indexes no schema written inside %s', (tag) => { + // An example schema in a comment is not the section's, and a lone + // opening tag in one must not swallow the real schema after it (#2494). + const code = [ + `{% ${tag} %}`, '{% schema %}', '{"name": "Ghost"}', '{% endschema %}', `{% end${tag} %}`, + `{%- ${tag} -%}{% schema %}{%- end${tag} -%}`, + '{% schema %}', '{"name": "Live"}', '{% endschema %}', + ].join('\n'); + const result = extractFromSource('sections/header.liquid', code); + expect(result.nodes.filter((n) => n.kind === 'constant').map((n) => [n.name, n.startLine])) + .toEqual([['Live', 7]]); + }); + it('should extract multiple imports', () => { const code = ` {% section 'header' %} @@ -7751,6 +7785,97 @@ describe('Liquid Shopify JSON template section resolution', () => { expect(cg.getFileDependencies('sections/header.liquid')).toEqual(['snippets/price.liquid']); }, 60_000); + // The notice Shopify's theme editor writes at the top of every JSON template + // and section group it saves. + const editorNotice = [ + '/*', + ' * ------------------------------------------------------------', + ' * IMPORTANT: The contents of this file are auto-generated.', + ' * ------------------------------------------------------------', + ' */', + ]; + const referencesFrom = (file: string): Array<[string | undefined, number | undefined, number | undefined]> => { + const fileNode = cg.getNodesInFile(file).find((n) => n.kind === 'file')!; + return cg.getOutgoingEdges(fileNode.id) + .filter((e) => e.kind === 'references') + .map((e): [string | undefined, number | undefined, number | undefined] => [cg.getNode(e.target)?.filePath, e.line, e.column]) + .sort((a, b) => a[1]! - b[1]!); + }; + + it.each(['\n', '\r\n'])('links the sections an editor-saved template and section group list, where each `type` is (%j)', async (newline) => { + // JSON.parse rejects the editor's comment, so such a file linked none of + // its sections, and a file without one linked them at line 1 (#2494). + writeFiles({ + 'layout/theme.liquid': `{% sections 'header-group' %}{{ content_for_layout }}\n`, + 'sections/header-group.json': [ + ...editorNotice, '{', ' "type": "header",', ' "name": "Header",', ' "sections": {', + ' "bar": { "type": "announcement-bar" },', ' "header": {', ' "type": "header"', ' }', + ' },', ' "order": ["bar", "header"]', '}', + ].join(newline), + 'sections/announcement-bar.liquid': `
\n`, + 'sections/header.liquid': `
\n`, + 'sections/main-product.liquid': `
{{ product.title }}
\n`, + 'templates/product.json': [ + ...editorNotice, '{', ' "sections": {', ' "main": {', ' "type": "main-product",', + ' "settings": { "note": "https://example.com/*not-a-comment*/ // nor this" }', ' },', + ' "again": { "type": "main-product" }', ' },', ' "order": ["main", "again"]', '}', + ].join(newline), + 'templates/index.json': `{\n "sections": {\n "main": { "type": "header" }\n },\n "order": ["main"]\n}\n`, + }); + cg = CodeGraph.initSync(tempDir); + await cg.indexAll(); + cg.resolveReferences(); + + expect(referencesFrom('templates/product.json')).toEqual([['sections/main-product.liquid', 9, 14]]); + expect(referencesFrom('sections/header-group.json')).toEqual([ + ['sections/announcement-bar.liquid', 10, 21], + ['sections/header.liquid', 12, 14], + ]); + expect(referencesFrom('templates/index.json')).toEqual([['sections/header.liquid', 3, 22]]); + expect(referencesFrom('layout/theme.liquid')).toEqual([['sections/header-group.json', 1, 0]]); + }); + + it('reads a JSON template with trailing commas, which Shopify accepts in theme JSON', () => { + const text = '// a note\n{"sections": {"main": {"type": "main-product",}, "hero": {"type": "hero"},}, "order": ["main", "hero",],}'; + expect(extractFromSource('templates/product.json', text, 'liquid').unresolvedReferences.map((r) => r.referenceName)) + .toEqual(['sections/main-product.liquid', 'sections/hero.liquid']); + }); + + it.each([ + ['a missing brace', '{"sections": {"main": {"type": "main-product"}}'], + ['an unclosed comment', '/* saved by the editor {"sections": {"main": {"type": "main-product"}}}'], + ['a missing comma', '{"sections": {"main": {"type": "main-product"} "hero": {"type": "hero"}}}'], + ])('links nothing from a JSON template with %s', (_case, text) => { + expect(extractFromSource('templates/product.json', text, 'liquid').unresolvedReferences).toEqual([]); + }); + + it("links a layout's section groups inside its own theme, and on to the sections they list", async () => { + // `{% sections 'header-group' %}` renders the theme's + // sections/header-group.json, whose sections render snippets (#2494). + writeFiles({ + ...severalThemes, + 'layout/theme.liquid': `{% sections 'header-group' %}{{ content_for_layout }}\n`, + 'sections/header-group.json': [...editorNotice, jsonTemplate('header')].join('\n'), + // This theme has no header group of its own, and the root theme's is not its. + 'examples/tailwind/layout/theme.liquid': `{% liquid\n sections 'header-group'\n%}{{ content_for_layout }}\n`, + }); + cg = CodeGraph.initSync(tempDir); + await cg.indexAll(); + cg.resolveReferences(); + + const linksOf = (file: string): string[] => cg.getFileDependencies(file).sort(); + expect(linksOf('layout/theme.liquid')).toEqual(['sections/header-group.json']); + expect(linksOf('sections/header-group.json')).toEqual(['sections/header.liquid']); + expect(linksOf('sections/header.liquid')).toEqual(['snippets/price.liquid']); + expect(linksOf('examples/tailwind/layout/theme.liquid')).toEqual([]); + + // Once the theme has a header group of its own, sync links the layout to it. + writeFiles({ 'examples/tailwind/sections/header-group.json': [...editorNotice, jsonTemplate('header')].join('\n') }); + await cg.sync(); + expect(linksOf('examples/tailwind/layout/theme.liquid')).toEqual(['examples/tailwind/sections/header-group.json']); + expect(linksOf('examples/tailwind/sections/header-group.json')).toEqual(['examples/tailwind/sections/header.liquid']); + }, 60_000); + it('finds the theme a file is in from the theme folder it sits in', () => { const markers = new Set(['layout/theme.liquid', 'examples/tailwind/config/settings_schema.json']); const exists = (relativePath: string): boolean => markers.has(relativePath); diff --git a/src/extraction/liquid-extractor.ts b/src/extraction/liquid-extractor.ts index f6d9aee60..15850ff82 100644 --- a/src/extraction/liquid-extractor.ts +++ b/src/extraction/liquid-extractor.ts @@ -1,3 +1,4 @@ +import { parseTree, findNodeAtLocation, type ParseError } from 'jsonc-parser'; import { Node, Edge, ExtractionResult, ExtractionError, UnresolvedReference } from '../types'; import { generateNodeId, NodeIdAllocator } from './tree-sitter-helpers'; @@ -6,9 +7,11 @@ import { generateNodeId, NodeIdAllocator } from './tree-sitter-helpers'; * * 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 references ({% section 'name' %}) and section group references + * ({% sections 'header-group' %} → `sections/header-group.json`) * - Snippet references ({% render 'name' %} and {% include 'name' %}) * - Schema blocks ({% schema %}...{% endschema %}) + * - The sections a Shopify JSON template or section group lists */ export class LiquidExtractor { private filePath: string; @@ -18,6 +21,8 @@ export class LiquidExtractor { private edges: Edge[] = []; private unresolvedReferences: UnresolvedReference[] = []; private errors: ExtractionError[] = []; + /** Where Liquid runs nothing — see `silentRegions`. Computed once per file. */ + private silent?: Array<[start: number, end: number]>; constructor(filePath: string, source: string) { this.filePath = filePath; @@ -100,31 +105,68 @@ export class LiquidExtractor { * mapping an id → `{ "type": "", ... }`; the `type` names a * `sections/.liquid` file. Emit a `references` edge to each, so a section * used only from a JSON template (the OS 2.0 norm) is no longer orphaned. + * + * The theme editor writes a block comment ("IMPORTANT: The contents of this + * file are auto-generated…") at the top of every template and section group + * it saves, and Shopify's theme JSON allows comments and trailing commas. + * Anything else that is not JSON, like a file cut off partway, links nothing. + * Each reference sits where its `type` value is written. */ 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; + const errors: ParseError[] = []; + const root = parseTree(this.source, errors, { allowTrailingComma: true }); + if (errors.length > 0 || root?.type !== 'object') return; + const sections = findNodeAtLocation(root, ['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; - seen.add(type); + for (const section of sections.children ?? []) { + const value = section.children?.[1]; + const type = value?.type === 'object' ? findNodeAtLocation(value, ['type']) : undefined; + if (type?.type !== 'string' || typeof type.value !== 'string' || seen.has(type.value)) continue; + seen.add(type.value); + const line = this.getLineNumber(type.offset); this.unresolvedReferences.push({ fromNodeId, - referenceName: `sections/${type}.liquid`, + referenceName: `sections/${type.value}.liquid`, referenceKind: 'references', - line: 1, - column: 0, + line, + column: type.offset - this.getLineStart(line), }); } } + /** + * The regions Liquid runs nothing in: `{% comment %}…{% endcomment %}` and + * `{% raw %}…{% endraw %}`, each from its opening tag to the end of its + * closing one (or to the end of the file, when it is never closed). The tags, + * schemas and declarations written inside one are examples, not code. + */ + private silentRegions(): Array<[start: number, end: number]> { + if (this.silent) return this.silent; + const regions: Array<[number, number]> = []; + const tags = /\{%[-]?\s*(\w+|#)([\s\S]*?)[-]?%\}/g; + let open: { name: string; start: number } | undefined; + let tag; + while ((tag = tags.exec(this.source)) !== null) { + const name = tag[1]!; + if (open) { + if (name === `end${open.name}`) { + regions.push([open.start, tags.lastIndex]); + open = undefined; + } + } else if (name === 'comment' || name === 'raw') { + open = { name, start: tag.index }; + } + } + if (open) regions.push([open.start, this.source.length]); + return (this.silent = regions); + } + + /** The silent region (see `silentRegions`) holding `index`, if any. */ + private silentRegionAt(index: number): [start: number, end: number] | undefined { + return this.silentRegions().find(([start, end]) => index >= start && index < end); + } + /** * Every occurrence of a Liquid tag, in BOTH spellings it can be written in. * @@ -152,18 +194,14 @@ 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; let tag; while ((tag = tags.exec(this.source)) !== null) { + const silent = this.silentRegionAt(tag.index); + if (silent) { + tags.lastIndex = silent[1]; + continue; + } 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, @@ -280,13 +318,16 @@ export class LiquidExtractor { } /** - * Extract {% section 'name' %} references + * Extract {% section 'name' %} and {% sections 'group' %} references. A + * section is `sections/.liquid`; a section group, which a layout renders + * with the plural tag, is the JSON file `sections/.json` that lists its + * sections. */ 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('sections?', `['"]([^'"]+)['"]`)) { const fullMatch = match.fullMatch; - const sectionName = match.groups[1]; + const [tagType, sectionName] = match.groups; const line = this.getLineNumber(match.index); // Create an import node for searchability @@ -315,13 +356,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, @@ -340,10 +381,10 @@ export class LiquidExtractor { kind: 'contains', }); - // Add unresolved reference to the section file + // Add unresolved reference to the section file, or the group's JSON 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), @@ -360,6 +401,13 @@ export class LiquidExtractor { let match; while ((match = schemaRegex.exec(this.source)) !== null) { + // A schema inside {% comment %} or {% raw %} is an example, not the + // section's: look again after that region ends. + const silent = this.silentRegionAt(match.index); + if (silent) { + schemaRegex.lastIndex = silent[1]; + 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 3c2f3c8b5..99076599f 100644 --- a/src/resolution/shopify-themes.ts +++ b/src/resolution/shopify-themes.ts @@ -2,13 +2,14 @@ * A Shopify theme's section and snippet references → that theme's own files. * * `{% 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 - * examples, a store's seasonal themes) a theme missing a section linked to - * another theme's copy of it. + * file belongs to; `{% section 'header' %}`, a JSON template's section + * `"type"` and `{% sections 'header-group' %}` name a file in that theme's + * `sections/`. Shopify never looks in another theme. The Liquid extractor + * records these as `snippets/.liquid`, `sections/.liquid` and, for + * a section 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. * * From a file inside a theme (see `shopifyThemeRoot`), these references * resolve to the theme's own file or to nothing, and never fall through to the @@ -20,7 +21,8 @@ import type { ResolvedRef, ResolutionContext, UnresolvedRef } from './types'; /** * The file a section or snippet reference made inside a Shopify theme names: - * the theme's own `sections/.liquid` or `snippets/.liquid`. + * the theme's own `sections/.liquid`, `sections/.json` or + * `snippets/.liquid`. * undefined means the reference is not one, or is made outside any theme; * null means the theme has no such file, so no other strategy may guess one. */