fix(sync): a Liquid section or snippet reference links its file when the file appears later

CHANGELOG.md

@@ -66,6 +66,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - `codegraph sync` and the file watcher now link an import to the file it names when that file is added after the importing file was indexed, or comes back after being deleted: `import 'package:app/b.dart';` in Dart, `import './side'` in JavaScript or TypeScript, `#include "net/socket.h"` in C and C++, `require 'inc/db.php'` in PHP, `import pkg.mod` in Python, a Lua `require`, or a C# `using` of a namespace declared later. Before, the import stayed unlinked until the importing file itself changed or the project was re-indexed, so `codegraph affected` and the files listed as using the new file missed it. Re-index after upgrading.
 - 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.
 - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading.
 - In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList<const char*, KeyComparator> Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading.
 - In C++, a base class like `Message` in `class DynamicMessage final : public Message` now links to the class C++ itself would pick: the one declared in the namespaces around the class, or brought in by the file's `using` declarations and namespace aliases, and the class a `typedef` or `using` alias names. Before, a base class was matched by its name alone, so it could land on a class of the same name in another namespace or another language: in protocolbuffers/protobuf, `DynamicMessage` derived from the JSON parser's internal `Message` class, and in google/leveldb and RocksDB, iterators derived from an unrelated nested `Iterator` class such as the skip list's. The wrong base showed up in `codegraph_explore`'s type hierarchy and in impact, and tied the base's methods to the wrong overrides. A template parameter used as a base no longer links to a class that shares its name, and a base written from the global scope, like `public ::testing::Test`, now links to its class. Re-index C++ projects after upgrading.

__tests__/extraction.test.ts

@@ -7543,10 +7543,10 @@ describe('Liquid Shopify JSON template section resolution', () => {
     expect(detectLanguage('shop/templates/product.json', undefined, { '.json': 'yaml' }, tempDir)).toBe('liquid');
   });
 
-  const writeFiles = (files: Record<string, string>): void => {
+  const writeFiles = (files: Record<string, string>, root = tempDir): void => {
     for (const [file, text] of Object.entries(files)) {
-      fs.mkdirSync(path.dirname(path.join(tempDir, file)), { recursive: true });
-      fs.writeFileSync(path.join(tempDir, file), text);
+      fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
+      fs.writeFileSync(path.join(root, file), text);
     }
   };
   // A theme at the project root, a theme in a folder marked only by its settings
@@ -7608,6 +7608,67 @@ describe('Liquid Shopify JSON template section resolution', () => {
     expect(cg.getFileDependencies('examples/tailwind/templates/404.json')).toEqual([]);
   });
 
+  it('links a section or snippet that appears after the files naming it', async () => {
+    // Sync retries a reference it could not resolve once a file it may name
+    // appears. A section or snippet reference was parked under its extension,
+    // `liquid`, which no file is named, so it stayed unlinked until the file
+    // naming it changed or the project was indexed again.
+    const later: Record<string, string> = {
+      'sections/404.liquid': `<h1>404</h1>\n`,
+      'snippets/price.liquid': severalThemes['snippets/price.liquid'],
+      // Only straps/dusk had a 404 section, which is no stand-in for this theme's own.
+      'examples/tailwind/sections/404.liquid': `<h1>404</h1>\n`,
+      'extensions/reviews/snippets/star.liquid': severalThemes['extensions/reviews/snippets/star.liquid'],
+    };
+    const initial = Object.fromEntries(Object.entries(severalThemes).filter(([file]) => !(file in later)));
+    writeFiles({ ...initial, 'templates/404.json': jsonTemplate('404') });
+    cg = CodeGraph.initSync(tempDir);
+    await cg.indexAll();
+    const naming = ['templates/404.json', 'sections/header.liquid', 'examples/tailwind/templates/404.json',
+      'examples/tailwind/sections/header.liquid', 'extensions/reviews/blocks/stars.liquid'];
+    const links = (graph = cg) => Object.fromEntries(naming.map((file) => [file, graph.getFileDependencies(file).sort()]));
+    expect(links()).toEqual({
+      'templates/404.json': [],
+      'sections/header.liquid': [],
+      'examples/tailwind/templates/404.json': [],
+      'examples/tailwind/sections/header.liquid': ['examples/tailwind/snippets/icon.liquid'],
+      'extensions/reviews/blocks/stars.liquid': [],
+    });
+
+    writeFiles(later);
+    expect((await cg.sync()).filesAdded).toBe(4);
+    const synced = links();
+    expect(synced).toEqual({
+      'templates/404.json': ['sections/404.liquid'],
+      'sections/header.liquid': ['snippets/price.liquid'],
+      'examples/tailwind/templates/404.json': ['examples/tailwind/sections/404.liquid'],
+      // The root theme's new price snippet is not this theme's.
+      'examples/tailwind/sections/header.liquid': ['examples/tailwind/snippets/icon.liquid'],
+      'extensions/reviews/blocks/stars.liquid': ['extensions/reviews/snippets/star.liquid'],
+    });
+    expect(cg.getPendingReferenceCount()).toBe(0);
+    // A fresh index of the same files links the same. (Indexing again over
+    // this index would skip the unchanged files that name the new ones.)
+    const freshDir = createTempDir();
+    writeFiles({ ...severalThemes, 'templates/404.json': jsonTemplate('404'), ...later }, freshDir);
+    const fresh = CodeGraph.initSync(freshDir);
+    try {
+      await fresh.indexAll();
+      expect(links(fresh)).toEqual(synced);
+    } finally {
+      fresh.close();
+      fs.rmSync(freshDir, { recursive: true, force: true });
+    }
+
+    // A deleted snippet's references wait for it the same way.
+    fs.rmSync(path.join(tempDir, 'snippets/price.liquid'));
+    await cg.sync();
+    expect(cg.getFileDependencies('sections/header.liquid')).toEqual([]);
+    writeFiles({ 'snippets/price.liquid': later['snippets/price.liquid']! });
+    await cg.sync();
+    expect(cg.getFileDependencies('sections/header.liquid')).toEqual(['snippets/price.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);

__tests__/sync-path-reference-retry.test.ts

@@ -0,0 +1,138 @@
+/**
+ * A sync links a reference written as a path once the file it names appears.
+ *
+ * Sync retries parked failed refs by name tail (#1240): the tail is matched
+ * against the names of the nodes a sync adds, and a file's node is named for
+ * the file. A reference written as a path — Liquid's `{% render 'price' %}` is
+ * `snippets/price.liquid`, a JSON template's section `"type": "404"` is
+ * `sections/404.liquid` — was parked under its extension, `liquid`, which no
+ * node is named, so it linked only once the file naming it changed or on a
+ * full index. #2392 fixed the same gap for imports.
+ */
+import { describe, it, expect, afterEach } from 'vitest';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import CodeGraph from '../src/index';
+import { DatabaseConnection, getDatabasePath } from '../src/db';
+import { CURRENT_SCHEMA_VERSION, getCurrentVersion, runMigrations } from '../src/db/migrations';
+import { QueryBuilder } from '../src/db/queries';
+import { referenceNameTail } from '../src/db/reference-tail';
+
+let root: string | undefined;
+let cg: CodeGraph | undefined;
+let db: DatabaseConnection | undefined;
+
+afterEach(() => {
+  cg?.close();
+  cg = undefined;
+  db?.close();
+  db = undefined;
+  if (root) fs.rmSync(root, { recursive: true, force: true });
+  root = undefined;
+});
+
+function write(files: Record<string, string>): void {
+  for (const [file, text] of Object.entries(files)) {
+    fs.mkdirSync(path.dirname(path.join(root!, file)), { recursive: true });
+    fs.writeFileSync(path.join(root!, file), text);
+  }
+}
+
+describe('referenceNameTail', () => {
+  it('parks a reference written as a path under the name of the file it names', () => {
+    expect(referenceNameTail('snippets/price.liquid', 'references')).toBe('price.liquid');
+    expect(referenceNameTail('sections/404.liquid', 'references')).toBe('404.liquid');
+    expect(referenceNameTail('snippets/icon.logo.liquid', 'references')).toBe('icon.logo.liquid');
+    expect(referenceNameTail('lazy-import:./routes/about.tsx', 'references')).toBe('about.tsx');
+  });
+
+  it('keeps the symbol tail of a reference that names no file', () => {
+    // No path, a path with no file name at its end, or an Erlang arity.
+    expect(referenceNameTail('Foo.Bar', 'references')).toBe('Bar');
+    expect(referenceNameTail('import:./home.component#HomeComponent', 'references')).toBe('component#HomeComponent');
+    expect(referenceNameTail('lists::map/2', 'references')).toBe('map');
+    // A call's slashes sit in its arguments, a comment or a division.
+    expect(referenceNameTail('assert.logfile("logs/error.log").has.line', 'calls')).toBe('line');
+    expect(referenceNameTail('(rss_kb / 1024.0).to_i', 'calls')).toBe('to_i');
+    // An import keeps the stem of its path (#2392).
+    expect(referenceNameTail('package:app/b.dart', 'imports')).toBe('b');
+  });
+});
+
+describe('sync links a path reference whose file appears later', () => {
+  it('in an index whose references an older version parked', async () => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-path-ref-'));
+    write({
+      'layout/theme.liquid': "{% render 'price' %}{{ content_for_layout }}\n",
+      'templates/404.json': JSON.stringify({ sections: { main: { type: '404' } }, order: ['main'] }),
+    });
+    cg = await CodeGraph.init(root, { index: true });
+    cg.close();
+    cg = undefined;
+
+    // What a version before 13 left: the extension as the tail.
+    db = DatabaseConnection.open(getDatabasePath(root));
+    db.getDb().exec(`UPDATE unresolved_refs SET name_tail = 'liquid' WHERE reference_kind = 'references';
+      DELETE FROM schema_versions WHERE version >= 13;
+      INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (12, 0, 'legacy fixture');`);
+    db.close();
+    db = undefined;
+
+    cg = await CodeGraph.open(root);
+    write({ 'sections/404.liquid': '<h1>404</h1>\n', 'snippets/price.liquid': '{{ product.price }}\n' });
+    expect((await cg.sync()).filesAdded).toBe(2);
+    expect(cg.getFileDependencies('templates/404.json')).toEqual(['sections/404.liquid']);
+    expect(cg.getFileDependencies('layout/theme.liquid')).toEqual(['snippets/price.liquid']);
+    expect(cg.getPendingReferenceCount()).toBe(0);
+  }, 60_000);
+});
+
+describe('schema v13', () => {
+  function fixture(): QueryBuilder {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-path-ref-'));
+    db = DatabaseConnection.initialize(path.join(root, 'test.db'));
+    const queries = new QueryBuilder(db.getDb());
+    queries.insertNode({ id: 'a', kind: 'file', name: 'theme.liquid', qualifiedName: 'layout/theme.liquid', filePath: 'layout/theme.liquid',
+      language: 'liquid', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 });
+    for (const [referenceName, referenceKind, status, tail] of [
+      ['snippets/price.liquid', 'references', 'failed', 'liquid'],
+      ['sections/icon.logo.liquid', 'references', 'failed', 'liquid'],
+      ['snippets/card.liquid', 'references', 'pending', ''],
+      ['package:app/b.dart', 'imports', 'failed', 'b'],
+      ['assert.logfile("logs/error.log").has.line', 'calls', 'failed', 'line'],
+      ['lists::map/2', 'calls', 'failed', 'map'],
+    ] as const) {
+      queries.insertUnresolvedRef({ fromNodeId: 'a', referenceName, referenceKind, line: 1, column: 0, filePath: 'layout/theme.liquid', language: 'liquid' });
+      db.getDb().prepare('UPDATE unresolved_refs SET status = ?, name_tail = ? WHERE reference_name = ?').run(status, tail, referenceName);
+    }
+    return queries;
+  }
+
+  const tails = () => db!.getDb().prepare('SELECT reference_name AS name, name_tail AS tail FROM unresolved_refs ORDER BY id').all();
+
+  it('rewrites the tail of a path reference parked by an older version, and replays cleanly', () => {
+    fixture();
+    db!.getDb().exec(`DELETE FROM schema_versions WHERE version >= 13;
+      INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (12, 0, 'legacy fixture');`);
+    db!.close();
+    db = DatabaseConnection.open(path.join(root!, 'test.db'));
+    expect(getCurrentVersion(db.getDb())).toBe(CURRENT_SCHEMA_VERSION);
+    const migrated = tails();
+    expect(migrated).toEqual([
+      { name: 'snippets/price.liquid', tail: 'price.liquid' },
+      { name: 'sections/icon.logo.liquid', tail: 'icon.logo.liquid' },
+      // Only a failed row is looked up by its tail.
+      { name: 'snippets/card.liquid', tail: '' },
+      { name: 'package:app/b.dart', tail: 'b' },
+      { name: 'assert.logfile("logs/error.log").has.line', tail: 'line' },
+      { name: 'lists::map/2', tail: 'map' },
+    ]);
+    const queries = new QueryBuilder(db.getDb());
+    expect(queries.getRetryableFailedReferences(['price.liquid', 'liquid']).map((ref) => ref.referenceName)).toEqual(['snippets/price.liquid']);
+
+    db.getDb().exec('DELETE FROM schema_versions WHERE version >= 13');
+    runMigrations(db.getDb(), 12);
+    expect(tails()).toEqual(migrated);
+  });
+});

src/db/migrations.ts

@@ -10,7 +10,7 @@ import { referenceNameTail } from './reference-tail';
 /**
  * Current schema version
  */
-export const CURRENT_SCHEMA_VERSION = 12;
+export const CURRENT_SCHEMA_VERSION = 13;
 
 /**
  * Migration definition
@@ -238,6 +238,24 @@ const migrations: Migration[] = [
       for (const row of rows) update.run(referenceNameTail(row.reference_name, 'imports'), row.id);
     },
   },
+  {
+    version: 13,
+    description: 'Retry a failed path reference when sync adds the file it names: file-name tails',
+    up: (db) => {
+      // A path reference parked before this version — a Liquid
+      // `snippets/price.liquid` — carries its extension as its tail, which no
+      // node is named. Rewrite it to the file name a failed path reference is
+      // parked under now. Idempotent: a rewritten tail rewrites to itself.
+      const update = db.prepare('UPDATE unresolved_refs SET name_tail = ? WHERE id = ?');
+      const rows = db
+        .prepare("SELECT id, reference_name, name_tail FROM unresolved_refs WHERE status = 'failed' AND reference_kind = 'references' AND reference_name LIKE '%/%.%'")
+        .all() as Array<{ id: number; reference_name: string; name_tail: string }>;
+      for (const row of rows) {
+        const tail = referenceNameTail(row.reference_name, 'references');
+        if (tail !== row.name_tail) update.run(tail, row.id);
+      }
+    },
+  },
 ];
 
 /**