diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b8ca92c1..fe1144fe4 100644 --- a/CHANGELOG.md +++ b/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 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. diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 6dfa8cb42..e84b7bf1b 100644 --- a/__tests__/extraction.test.ts +++ b/__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): void => { + const writeFiles = (files: Record, 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 = { + 'sections/404.liquid': `

404

\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': `

404

\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); diff --git a/__tests__/sync-path-reference-retry.test.ts b/__tests__/sync-path-reference-retry.test.ts new file mode 100644 index 000000000..a6c50a097 --- /dev/null +++ b/__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): 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': '

404

\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); + }); +}); diff --git a/src/db/migrations.ts b/src/db/migrations.ts index 5191519b5..7e5d1ccdc 100644 --- a/src/db/migrations.ts +++ b/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); + } + }, + }, ]; /** diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts index 4c7ed319b..af4ea5a7c 100644 --- a/src/db/reference-tail.ts +++ b/src/db/reference-tail.ts @@ -1,8 +1,9 @@ /** * The keys sync's failed-ref retry (#1240) matches on: the tail a failed ref * is parked under, and the names a newly added file can be imported as. - * Shared by the query layer, which writes and reads them, and the migration - * that rewrote the tails parked before path imports had one of their own. + * Shared by the query layer, which writes and reads them, and the migrations + * that rewrote the tails parked before path imports and path references had + * ones of their own. */ /** @@ -17,6 +18,13 @@ * `inc/db.php` → 'db' — the stem {@link importPathKeys} takes from a file * that could satisfy it. The dotted tail was the extension ('dart', 'php') or * a path fragment ('/foo'), which no file ever matched. + * + * Any other reference written as a path to a file names it by the file's own + * name: Liquid's `{% render 'price' %}` is `snippets/price.liquid`, parked as + * 'price.liquid' — the name path matching looks the file up by, and the name + * of the node a file that appears later is given. Its dotted tail was the + * extension, 'liquid'. A call's slashes are in its arguments, a comment or a + * division, so only a `references` ref is read as a path. */ export function referenceNameTail(referenceName: string, referenceKind?: string): string { if (referenceKind === 'imports') { @@ -26,6 +34,10 @@ export function referenceNameTail(referenceName: string, referenceKind?: string) // `..` and `.` name a folder only through the importing file's location. if (!/^\.*$/.test(stem)) return stem; } + if (referenceKind === 'references') { + const fileName = referenceName.slice(referenceName.lastIndexOf('/') + 1); + if (fileName !== referenceName && FILE_NAME.test(fileName)) return fileName; + } // Erlang refs carry a written arity (`f/1`, `mod::fn/2` — #1610); the tail a // new symbol's plain name could match is the arity-less function name. const base = referenceName.replace(/\/\d{1,3}$/, '') || referenceName; @@ -33,6 +45,9 @@ export function referenceNameTail(referenceName: string, referenceKind?: string) return idx >= 0 ? base.slice(idx + 1) : base; } +/** A file name with an extension: 'price.liquid', 'icon.logo.liquid', 'about.tsx' — not 'x.component#Name'. */ +const FILE_NAME = /^[\w$@+~.-]+\.[A-Za-z][A-Za-z0-9]*$/; + /** A file or folder name up to its first dot: 'b.dart' → 'b', 'types.d.ts' → 'types', '.eslintrc.js' → '.eslintrc'. */ function fileNameStem(name: string): string { const dot = name.indexOf('.', 1); diff --git a/src/index.ts b/src/index.ts index 70b7acfe6..862add024 100644 --- a/src/index.ts +++ b/src/index.ts @@ -982,8 +982,10 @@ export class CodeGraph { // now resolve, and nothing else ever revisits them (their rows // were parked as status='failed' by an earlier completed pass). // Look them up by the symbol names the changed files now carry - // and re-resolve just that set. On a sync where no failed ref - // matches, this is one indexed lookup. + // and re-resolve just that set. The names include each file's + // own, which a reference written as a path + // (`snippets/price.liquid`) waits under. On a sync where no failed + // ref matches, this is one indexed lookup. const tRetry = Date.now(); const retryable = this.queries.getRetryableFailedReferences( this.queries.getNodeNamesByFiles(result.changedFilePaths)