diff --git a/CHANGELOG.md b/CHANGELOG.md index 81206b95a5..f848a93cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -483,6 +483,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph_explore` now leaves out of its answers every file that search already treats as a test or an example: a C++ `FooTest.cpp` or `foo_test.cc`, a Google `foo_unittest.cc`, anything in an LLVM-style `unittests/` folder or an `e2e/` or `testing/` folder, and a project's `examples/`, `samples/` and `benchmarks/`. Before, explore went by a shorter list of its own, so a unit test or an example that called the code you asked about could take a place in the answer meant for that code: in protocolbuffers/protobuf, a question about `DescriptorPool` came back with three of its unit tests. The same files are no longer taken for a project's main routes file either, such as an end-to-end suite's mock server. No re-index is needed. - A `__tests__/` folder at the top of a repository now counts as a test, as one below `src/` or any other folder already did. Before, a file like `__tests__/foo.ts` was taken for production code: search and `codegraph_explore` ranked it alongside production code, and it could be taken for a project's main routes file. Re-index projects with a top-level `__tests__/` folder after upgrading. - `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed. +- `codegraph sync` and the file watcher now also link a call or read on a name a file imports, like `NODE_PATH_EXCLUDELIST.includes(path)` after `import { NODE_PATH_EXCLUDELIST } from './constants'`, `store.get(key)` through a default import or `api.get()` through a namespace import, when the module gets the export in a later edit, is added after the importing file was indexed, or comes back after being deleted, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, such a call stayed unlinked until the importing file changed or the project was re-indexed, because sync looked it up only by its method's name, which the module seldom declares, and skipped a name as common as `includes`, `get` or `map` altogether. Re-index after upgrading to pick up links an earlier version missed. +- In this fork, a call on an imported constant, like `NODE_PATH_EXCLUDELIST.includes(path)` or `store.get(key)` above, stays unlinked after a sync, as it does after a full index: the native resolver links a call through an import only to a function, method, class or component, or to a store through its `getState`. Sync's lookup by the module still links the calls that do resolve, like `api.get()` through a namespace import. - `codegraph sync` and the file watcher now do the same for a Python import under a name of its own, like `from .models import User as UserModel`, `from . import helpers as h` or `from app.pkg import sub as subpkg`: when the module or package is added after the importing file was indexed, comes back after being deleted, or gets the imported name in a later edit, the import and every use of the alias, such as a call, a base class or the class passed as a value, now link to it. Before, they stayed unlinked until the importing file changed or the project was re-indexed. Re-index Python projects after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now also link a Python import of a name a package re-exports, like `from fastapi import FastAPI` through `fastapi/__init__.py`, when that `__init__.py` is added after the importing files were indexed or gains the re-export in a later edit. Before, calls through the name stayed unlinked until each importing file changed or the project was re-indexed. Large projects gain the most: sync also used to skip, in any language, a module or package that more than 500 references were waiting for, and now restoring Django's `django/urls/__init__.py` relinks every file that imports `path` from it. Re-index Python projects after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now update the files that import through a module that re-exports what it imports, like a package's `__init__.py` or an `index.ts` barrel, when that module is deleted or added, and in Python also the files that reach it through another package re-exporting it. Before, deleting `shop/__init__.py` left every `from shop import Order` linked to the class it re-exported, and deleting `components/index.ts` left every `import { Button } from './components'` linked, until each importing file changed or the project was re-indexed. When such a module came back, the imports stayed linked the way they were while it was missing, and in a large Python project an import of a module whose name is as common as `models` or `test`, like `from django.test import TestCase`, stayed unlinked. Python, JavaScript, TypeScript, Vue, Svelte and Astro. @@ -495,7 +497,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a call a method makes with no receiver, or through `this->`, now links to the method C++ itself would call: one of the method's own class, else one the class inherits from a base class, else a static member of a class it is nested in, which a nested class can name without an object. The same goes for a call in a lambda inside the method, and of a class's overloads the one the call's arguments fit is taken. Before, the call was matched by its name alone, so another class's method of that name could win because it was declared nearer or found first: in protocolbuffers/protobuf, every generated message's `Swap` and move assignment calling `InternalSwap(…)` reached `Any::InternalSwap`, and the `GetArena()` messages inherit from `MessageLite` reached an arena helper's `GetArena`, while googletest's `~linked_ptr() { depart(); }`, vendored in facebook/rocksdb, reached `linked_ptr_internal::depart`. Calls C++ doesn't look up in the class are matched as before: a free function's, a call to a parameter or local of that name, and a template's bare call into a base class that depends on its template parameters. Re-index C++ projects after upgrading. Thanks @danusha2345 for reporting the C++ case in #1952. - In C++, a call on the variable of a range-based `for` loop over `auto` elements, like `cfd->Ref()` inside facebook/rocksdb's `for (auto cfd : cfds)`, now links to the method of the type the loop goes over. That type is read from what the loop iterates: a `std::vector`, `std::set`, `absl::Span` or similar container, protocolbuffers/protobuf's `RepeatedPtrField` or RocksDB's `autovector`, whether a variable, parameter or member holds it, an `auto` variable is set to it or a function or method returns it, and a project class you loop over through its `begin()`, like RocksDB's `ColumnFamilySet`. Before, the call was matched by the variable's name, or by an earlier variable of that name: in RocksDB, `blob_file.GetBlobFileNumber()` over a list of `BlobFileGarbage` reached `BlobFileAddition::GetBlobFileNumber`, and in protobuf's Objective-C generator, calls over a list of `std::unique_ptr` reached other generators' methods. A call on an element of a standard-library type, like `file.find(…)` over a `std::vector`, no longer links to a project method, and `->` on an element held in a `std::unique_ptr` or `std::shared_ptr` reaches the type it holds. Re-index C++ projects after upgrading. - In this fork, the two C++ changes above run in the native resolver. A call on a variable whose type is a template parameter, like `T* iter`, stays unlinked rather than reaching a class that happens to share the parameter's name. A call stays linked as before when the class has no method of that name but one of its base classes comes from an outside library other than the standard library, such as googletest's `testing::Test`. -- In C++, a call on a data member of the calling method's class, like `versions_->GetColumnFamilySet()` in facebook/rocksdb's `db_impl_write.cc`, now links to the method of the member's type wherever the class is declared. Before, only the call's own file and the header of the same name were read for the member, and RocksDB declares `DBImpl` in `db_impl.h` while its methods are spread over `db_impl_write.cc`, `db_impl_compaction_flush.cc`, `db_impl_open.cc` and more, so such calls were guessed by the member's name. A member declared below an inline method that uses it, one a base class declares, and, in a gtest `TEST_F` or `TEST_P` body, one of the test's fixture, like RocksDB's `db_`, are found the same way, and a member of a class nested in the caller's class is no longer taken for the caller's own. A call on a member of a standard-library type, like `std::atomic shutting_down_`, no longer links to a project method that only shares the method's name. A call on a member reached through other members now links the same way, like RocksDB's `thread->rand.Next()` on a `Random64` or `meta->largest.user_key()` on an `InternalKey`. Before, it went to whichever class had a method of that name, like `ArenaWrappedDBIter::Next` or `ClippingIterator::user_key`, or, inside a method, to that method's own class. When the member's class has no such method, the call no longer links to another class's. The member is read from the class the calling file sees: RocksDB's `db_bench_tool.cc` and its stress tool each declare their own `ThreadState`, whose `rand` is a `Random64` in one and a `Random` in the other. Re-index C++ projects after upgrading. +- In C++, a call on a data member of the calling method's class, like `versions_->GetColumnFamilySet()` in facebook/rocksdb's `db_impl_write.cc`, now links to the method of the member's type wherever the class is declared. Before, only the call's own file and the header of the same name were read for the member, and RocksDB declares `DBImpl` in `db_impl.h` while its methods are spread over `db_impl_write.cc`, `db_impl_compaction_flush.cc`, `db_impl_open.cc` and more, so such calls were guessed by the member's name. A member declared below an inline method that uses it, one a base class declares, and, in a gtest `TEST_F` or `TEST_P` body, one of the test's fixture, like RocksDB's `db_`, are found the same way, and a member of a class nested in the caller's class is no longer taken for the caller's own. A call on a member of a standard-library type, like `std::atomic shutting_down_`, no longer links to a project method that only shares the method's name. A call on a member reached through other members now links the same way, like RocksDB's `thread->rand.Next()` on a `Random64` or `meta->largest.user_key()` on an `InternalKey`. Before, it went to whichever class had a method of that name, like `ArenaWrappedDBIter::Next` or `ClippingIterator::user_key`, or, inside a method, to that method's own class. When the member's class has no such method, the call no longer links to another class's. The member is read from the class the calling file sees: RocksDB's `db_bench_tool.cc` and its stress tool each declare their own `ThreadState`, whose `rand` is a `Random64` in one and a `Random` in the other. It is also found in the classes that class derives from. RocksDB's `Options` inherits its members from `DBOptions` and `ColumnFamilyOptions`, so `options.statistics->getTickerCount(…)` now reaches `Statistics`, and `options.table_factory.reset(…)` on a `std::shared_ptr` no longer links to a project method of that name. Re-index C++ projects after upgrading. - In C++, a variable, parameter or member whose declaration puts a qualifier or an annotation around its name is now read for its type, so calls on it link to the declared class's methods: a `const` after the `*` or `&`, like facebook/rocksdb's `Iterator* const iter = iter_guard.get();` or google/leveldb's `Env* const env_;`, a type written before its `const`, like `Slice const& key`, a nullability annotation after the `*`, like protocolbuffers/protobuf's `io::EpsCopyOutputStream* PROTOBUF_NONNULL stream`, and a thread-safety annotation after the name, like LevelDB's `MemTable* imm_ GUARDED_BY(mutex_);`. Before, such a declaration was skipped, or the annotation was taken for its type, so the call took the type of an earlier variable of the same name or was guessed by the method's name: RocksDB's `iter->SeekToFirst()` reached the unrelated `ArenaWrappedDBIter`, and LevelDB's `env_->NewWritableFile(…)` reached `InMemoryEnv`. Re-index C++ projects after upgrading. - In C++, a call on a name a structured binding declares now links to the method of the type C++ gives it. That covers a range-based `for` over pairs or a map, like facebook/rocksdb's `for (auto& [cfh, iter] : cfh_iter_pairs_) { iter->Valid(); }`, and a declaration like protocolbuffers/protobuf's `auto [dst_ext, is_new] = Insert(…);`. A call on what a `std::function` or another function wrapper returns, like `create_iterator()->Valid()` on a `std::function`, links the same way, and so does a call on a class member whose type is written on the line above its name. Before, these calls were guessed by the method's name, and calls on a library container declared that way, like a `std::priority_queue` member's `pop()`, reached a project class's method of that name. Re-index C++ projects after upgrading. - In C++, a call written without a receiver in a gtest `TEST_F` or `TEST_P` body now links to the test fixture's method, its own or one it inherits, as C++ finds it: facebook/rocksdb's tests call `DBTestBase`'s `Put(…)`, `Flush()` and `NewIterator(…)` this way. A variable such a call initializes, like `auto iter = NewIterator(…)`, takes the type the method returns, so `iter->SeekToFirst()` reaches the iterator class. Before, those calls were guessed by name and often reached a namesake in another class, like RocksDB's `ColumnFamilyTestBase::Put` or protocolbuffers/protobuf's `EncodeDecodeTest::ExpectNoErrors`, and `iter->SeekToFirst()` reached the unrelated `MemTableRep::Iterator`. A `->` call on a value of a type from outside the project, like protobuf's `m->value()` on an `absl::StatusOr`, no longer links to a class of the project that shares the type's name, like protobuf's own `hpb::StatusOr`. Re-index C++ projects after upgrading. diff --git a/README.md b/README.md index 87bd723e23..2da57c8d1e 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Follow [@getcodegraph](https://x.com/getcodegraph) on X for updates. ## About this fork -This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`e8ec7fd8`](https://github.com/colbymchenry/codegraph/commit/e8ec7fd8) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. +This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`baf19067`](https://github.com/colbymchenry/codegraph/commit/baf19067) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. The fork publishes no releases. The install scripts, npm package, badges and `codegraph upgrade` further down this page install **upstream's** releases. To run the fork, build it from source (below). @@ -125,7 +125,7 @@ The native resolver includes upstream's Go import-name assumptions, type-positio Go interfaces link named embedded interfaces and aliases that resolve to interfaces. Named scalar or struct terms, unions, underlying-type constraints and basic types are not supertypes. Package-qualified embeddings stay in their imported package. Go imports follow the nearest indexed module and the longest matching module path, including module changes during incremental sync. Unexported receivers and embedded methods stay in their declaring package. Dart imports, exports and part directives follow their library URIs and visibility rules. Library directive reads stay inside the indexed project, including resolved symlink targets. Calls through import prefixes, annotations and member chains follow the visible declaration and written receiver types, including explicit generic lookup types. Parameters and locals shadow bare calls. Top-level and field initializers contribute calls; const constructors, redirecting factories and annotated members retain their declarations and dartdoc. Getter reads become calls only when the receiver type reaches that getter and no nearer field in the visible class hierarchy overrides it; enum extensions and type-position references participate in resolution. Rust enum-variant values retain their enum references. -Named JavaScript and TypeScript object literals own their function members, including local objects and classic-script global assignments. Member calls follow the visible object; loop-local objects stay within their scope. Destructured member calls resolve across lines while preserving the source binding at the destructure declaration; unrelated bare names stay unresolved. Vue template expressions contribute calls to script bindings while preserving component ownership. Encoded attribute expressions retain their original source positions, and template-local bindings stay within their scope. +Named JavaScript and TypeScript object literals own their function members, including local objects and classic-script global assignments. Member calls follow the visible object; loop-local objects stay within their scope. Destructured member calls resolve across lines while preserving the source binding at the destructure declaration; unrelated bare names stay unresolved. A call on an imported constant, like `LIST.includes(x)` after `import { LIST } from './constants'`, stays unresolved rather than linking to the constant, as upstream does. Vue template expressions contribute calls to script bindings while preserving component ownership. Encoded attribute expressions retain their original source positions, and template-local bindings stay within their scope. A COBOL copybook named in an explore query prioritizes its indexed source and lists its COPY and EXEC SQL INCLUDE sites. Missing indexed source is reported explicitly. diff --git a/__tests__/cpp-member-chain-base-members.test.ts b/__tests__/cpp-member-chain-base-members.test.ts new file mode 100644 index 0000000000..c2a2d5299b --- /dev/null +++ b/__tests__/cpp-member-chain-base-members.test.ts @@ -0,0 +1,131 @@ +/** + * A C++ call on a data member that the object's class inherits, reached + * through a member chain, calls a method of the member's class. rocksdb's + * `Options` declares no members of its own: `options.statistics` and + * `options.table_factory` are `DBOptions`' and `ColumnFamilyOptions`'. Only + * the object's own class was read for the member, so such calls were guessed + * by the method's name. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import type { Node } from '../src/types'; + +const lines = (...l: string[]): string => [...l, ''].join('\n'); + +describe('a C++ call on a member chain through an inherited member', () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-base-')); + const write = (file: string, text: string): void => { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + }; + // Each namesake comes first: a call matched by its bare name lands on it. + write('src/types.h', lines( + 'class ArenaStats {', + ' public:', + ' long getTickerCount(int t) const { return 0; }', + '};', + 'class Statistics {', + ' public:', + ' virtual long getTickerCount(int t) const { return 1; }', + '};', + 'class BlockNamesake {', + ' public:', + ' const char* Name() const { return "block"; }', + '};', + 'class TableFactory {', + ' public:', + ' virtual const char* Name() const { return "table"; }', + '};', + 'class MemTableNamesake {', + ' public:', + ' void CreateMemTableRep() {}', + '};', + 'class MemTableRepFactory {', + ' public:', + ' virtual void CreateMemTableRep() {}', + '};', + 'class FilePtr {', + ' public:', + ' void reset() {}', + '};', + 'class Other {', + ' public:', + ' void Flush() {}', + '};', + 'class Inner {', + ' public:', + ' void Flush() {}', + '};', + )); + write('src/options.h', lines( + '#include ', + '#include "src/types.h"', + 'struct DBOptions {', + ' Statistics* statistics = nullptr;', + '};', + 'struct AdvancedColumnFamilyOptions {', + ' MemTableRepFactory* memtable_factory = nullptr;', + '};', + 'struct ColumnFamilyOptions : public AdvancedColumnFamilyOptions {', + ' std::shared_ptr table_factory;', + '};', + 'struct Options : public DBOptions, public ColumnFamilyOptions {};', + 'struct Shadowing : public DBOptions {', + ' Inner statistics;', + '};', + )); + write('src/use.cc', lines( + '#include "src/options.h"', + 'void Stats(Options& options) { options.statistics->getTickerCount(0); }', + 'void Table(Options* options) { options->table_factory->Name(); }', + 'void Memtable(const Options& opts) { opts.memtable_factory->CreateMemTableRep(); }', + 'void HolderOwn(Options& options) { options.table_factory.reset(); }', + 'void Shadowed(Shadowing& s) { s.statistics.Flush(); }', + 'struct Holder {', + ' Options opts;', + '};', + 'void Deep(Holder* h) { h->opts.statistics->getTickerCount(1); }', + )); + cg = await CodeGraph.init(root, { index: true }); + }, 120_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + /** `calls` callees of the function `name`, as `qualifiedName`. */ + const calls = (name: string): string[] => { + const caller = cg.getNodesByKind('function').find((n: Node) => n.name === name); + expect(caller, name).toBeDefined(); + return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls').map((r) => r.node.qualifiedName).sort(); + }; + + it("a member the object's class inherits from either base", () => { + expect(calls('Stats')).toEqual(['Statistics::getTickerCount']); + expect(calls('Table')).toEqual(['TableFactory::Name']); + }); + + it('a member inherited through two levels', () => { + expect(calls('Memtable')).toEqual(['MemTableRepFactory::CreateMemTableRep']); + }); + + it("an inherited library member's own method calls none of the project's", () => { + expect(calls('HolderOwn')).toEqual([]); + }); + + it("the class's own member before a base's of the same name", () => { + expect(calls('Shadowed')).toEqual(['Inner::Flush']); + }); + + it('an inherited member further down a chain', () => { + expect(calls('Deep')).toEqual(['Statistics::getTickerCount']); + }); +}); diff --git a/__tests__/daemon-candidates.ts b/__tests__/daemon-candidates.ts index d7f31e777a..a47dac76f8 100644 --- a/__tests__/daemon-candidates.ts +++ b/__tests__/daemon-candidates.ts @@ -22,7 +22,8 @@ export function recordSpawns(fixture: string): { args: string[]; env: NodeJS.Pro return { args: ['--require', SPAWN_RECORDER], env: { CG_TEST_SPAWN_LOG: spawnLogFor(fixture) } }; } -function recordedPids(fixture: string): number[] { +/** The pids of every candidate the launchers started for `fixture` recorded. */ +export function recordedPids(fixture: string): number[] { let raw = ''; try { raw = fs.readFileSync(spawnLogFor(fixture), 'utf8'); } catch { return []; } return raw.split('\n').map(Number).filter((pid) => Number.isSafeInteger(pid) && pid > 0); diff --git a/__tests__/fixtures/record-detached-spawns.cjs b/__tests__/fixtures/record-detached-spawns.cjs index 50ef59f074..9ea89464a9 100644 --- a/__tests__/fixtures/record-detached-spawns.cjs +++ b/__tests__/fixtures/record-detached-spawns.cjs @@ -3,9 +3,11 @@ * the test learns the pid of every detached daemon candidate they spawn * (#1773). Racing launchers may each spawn one; the losers exit on their own * once they see the winner's lock, but on a loaded machine one can still be - * starting when the test tears down. Written synchronously at spawn time, so - * the pid is on disk before the launcher can be stopped. Candidates inherit - * the preload through `process.execArgv`; they spawn nothing detached. + * starting when the test tears down. Written synchronously as the spawn + * returns, so the pid is on disk before the launcher can exit. A launcher + * killed inside the spawn records nothing, and on Windows leaves that candidate + * suspended for good. Candidates inherit the preload through + * `process.execArgv`; they spawn nothing detached. */ const childProcess = require('child_process'); const fs = require('fs'); diff --git a/__tests__/index-daemon-rebuild.test.ts b/__tests__/index-daemon-rebuild.test.ts index 0e5e51c931..b7fb9d0143 100644 --- a/__tests__/index-daemon-rebuild.test.ts +++ b/__tests__/index-daemon-rebuild.test.ts @@ -6,6 +6,7 @@ import * as path from 'path'; import { CodeGraph } from '../src'; import { stopDaemonAt, isProcessAlive } from '../src/mcp/daemon-registry'; import { readWriterLock, releaseWriterLock, tryAcquireWriterLock } from '../src/mcp/writer-lock'; +import { recordedPids, recordSpawns, removeSpawnLog } from './daemon-candidates'; const bin = path.resolve(__dirname, '../dist/bin/codegraph.js'); let root: string; @@ -23,7 +24,10 @@ async function until(fn: () => T, timeout = 20000): Promise> { throw new Error('Timed out waiting for process/response'); } function start(args: string[]) { - const child = spawn(process.execPath, ['--liftoff-only', bin, ...args], { cwd: root, env }); + // Record the daemons a launcher spawns, for the teardown. + const recorder = recordSpawns(root); + const child = spawn(process.execPath, ['--liftoff-only', ...recorder.args, bin, ...args], + { cwd: root, env: { ...env, ...recorder.env } }); children.push(child); let out = '', err = ''; child.stdout.on('data', b => out += b); @@ -65,14 +69,26 @@ beforeEach(async () => { }); afterEach(async () => { releaseWriterLock(root, 'rebuild.pid'); - for (const child of children) { - child.stdin.end(); - if (child.exitCode === null && child.signalCode === null) child.kill(); + // Close each launcher's stdin and let it exit; never kill one. A launcher + // serving calls in-process retries its daemon on a timer, and a retry that + // comes due during a call spawns the daemon as soon as that call answers, + // just as this teardown begins. On Windows a launcher killed mid-spawn leaves + // that daemon suspended for good: it never runs, so no daemon.pid names it + // and nothing can stop it. + for (const child of children) child.stdin.end(); + await until(() => children.every(c => c.exitCode !== null || c.signalCode !== null), 30000); + // Then stop every daemon the launchers recorded. One spawned as its launcher + // exited may still be starting, and stopDaemonAt leaves a daemon running + // until it answers, so keep at it until all have exited. + const end = Date.now() + 30000; + while (recordedPids(root).some(isProcessAlive)) { + if (Date.now() > end) throw new Error(`Daemons ${recordedPids(root).filter(isProcessAlive).join(', ')} are still running`); + await stopDaemonAt(root, { preserveUnverified: true }); + await new Promise(r => setTimeout(r, 25)); } - await until(() => children.every(c => c.exitCode !== null || c.signalCode !== null)); - await stopDaemonAt(root); + removeSpawnLog(root); fs.rmSync(root, { recursive: true, force: true, maxRetries: 20, retryDelay: 100 }); -}); +}, 90000); it.runIf(process.platform === 'win32')('rebuilds after the real MCP client closes with SQLite held by its daemon (#1325)', async () => { const server = await client(); diff --git a/__tests__/sync-import-binding-retry.test.ts b/__tests__/sync-import-binding-retry.test.ts index da8471a1e8..e940653eda 100644 --- a/__tests__/sync-import-binding-retry.test.ts +++ b/__tests__/sync-import-binding-retry.test.ts @@ -224,8 +224,9 @@ describe('a failed reference through an import binding', () => { const root = tempRoot(); write(root, { ...IMPORTERS, - // A package's binding, an unaliased named import and a member of a - // namespace import wait for the name they use. + // A package's binding and an unaliased named import wait for the name + // they use; a member of a namespace import waits for its module too + // (sync-import-member-retry.test.ts). 'src/other.ts': "import express from 'express';\nimport { helper } from './util';\nimport * as NS from './ns';\nexport const app = express();\nexport const values = [helper, NS.make()];\n", }); (await CodeGraph.init(root, { index: true })).close(); @@ -251,7 +252,7 @@ describe('a failed reference through an import binding', () => { expect(tailOf('src/other.ts', 'express', 'imports')).toBe('express'); expect(tailOf('src/other.ts', 'helper', 'imports')).toBe('helper'); - expect(tailOf('src/other.ts', 'NS.make', 'calls')).toBe('make'); + expect(tailOf('src/other.ts', 'NS.make', 'calls')).toBe('module:ns'); // Each module's file is one of the keys its references wait under. for (const [file, module] of [ diff --git a/__tests__/sync-import-member-retry.test.ts b/__tests__/sync-import-member-retry.test.ts new file mode 100644 index 0000000000..446e0760d1 --- /dev/null +++ b/__tests__/sync-import-member-retry.test.ts @@ -0,0 +1,487 @@ +/** + * A sync links a member reference through an import binding once the module + * appears. + * + * `NODE_PATH_EXCLUDELIST.includes(p)` after `import { NODE_PATH_EXCLUDELIST } + * from './constants'` links the constant through the import, as do + * `store.get(k)` through a default import, `useStore.setState()` through an + * aliased one and `api.get()` through a namespace import. A reference that + * failed to resolve is parked under its tail (#1240), and a member's tail is + * the member's name: `includes`, `get`, `setState`. The module that appears + * later seldom declares that name, and one that common is shared by more + * failed references than the retry's ceiling, so the retry never looked. So a + * module added after its importers were indexed, one restored after a delete, + * or one that gained its export in a later edit stayed unlinked from those + * calls until each importer changed or the project was indexed again. + * + * Such a reference is now parked under its module's key, as #2450 parks a + * whole name through a default, namespace or aliased binding, and is still + * found by its member's name — a namespace import's barrel forwards the member + * from another file, a guess by name links a receiver whose module never + * resolves, a router's navigation retry looks calls up by method — and by its + * receiver's: a barrel forwards the binding's declaration from a file of + * another name. Only a direct member is: a chain (`CONFIG.load().then`) or a + * longer path (`CONFIG.api.fetch`) keeps its member's name, as what its last + * member hangs off is seldom a type the module declares. + */ +import { describe, it, expect, afterEach, vi } 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 { MEMBER_TAIL_SQL, MODULE_PARKED_SQL, moduleReferenceKeys, moduleTail, referenceNameTail } from '../src/db/reference-tail'; + +type Files = Record; + +/** 501 failed `get` calls on a receiver no binding names: `get` is past the retry's ceiling. */ +const NOISE = `export function noise() {\n${Array.from({ length: 501 }, (_, i) => ` extApi.get(${i});`).join('\n')}\n}\n`; + +/** The files that call members of what they import. */ +const IMPORTERS: Files = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { vue: '^3' } }), + 'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@/*': ['src/*'] } } }), + // A named import of a constant (infrahub). + 'src/diff-utils.ts': "import { NODE_PATH_EXCLUDELIST } from './constants';\nexport function keep(p: string) { return !NODE_PATH_EXCLUDELIST.includes(p); }\n", + // Through a tsconfig alias (luci-go's milo). + 'src/flags.ts': "import { REGISTERED_FLAGS } from '@/flags/registry';\nexport function flag(n: string) { return REGISTERED_FLAGS.get(n); }\n", + // A default import of a store. + 'src/save.ts': "import store from './store';\nexport function save(v: number) {\n store.set('k', v);\n return store.get('k');\n}\n", + // An aliased import (folo's zustand store). + 'src/entry.ts': "import { useEntryStore as useStore } from './entry-store';\nexport function reset() { useStore.setState({ entries: [] }); }\n", + // A namespace import whose member's name many other failed calls share. + 'src/client.ts': "import * as api from './api';\nexport function load() { return api.get('/me'); }\n", + // A Vue component's script. + 'src/components/Panel.vue': "\n\n", + 'src/noise.ts': NOISE, +}; + +const MODULES: Files = { + 'src/constants.ts': "export const NODE_PATH_EXCLUDELIST = ['a', 'b'];\n", + 'src/flags/registry.ts': 'export const REGISTERED_FLAGS = new Map();\n', + 'src/store.ts': "import { createStore } from 'jotai';\nconst store = createStore();\nexport default store;\n", + 'src/entry-store.ts': "import { create } from 'zustand';\nexport const useEntryStore = create(() => ({ entries: [] as number[] }));\n", + 'src/api.ts': 'export function get(path: string) { return path; }\n', + 'src/table.ts': "export const COLUMNS = [{ name: 'a' }];\n", +}; + +/** + * What a full index links through the bindings: `: -> ::`. + * + * Fork: the kernel links a member call through an import only to a callable + * target (a function, method, class or component, or a store's `getState`), + * so `NODE_PATH_EXCLUDELIST.includes`, `store.get` and the other calls on an + * imported constant stay unresolved in a full index and after a sync alike. + * Upstream links them to the constant. Each is still parked under its module + * (the tail assertions below), and every sync is compared with a full index. + */ +const LINKS = [ + 'api.get: calls function src/client.ts -> function src/api.ts::get', +]; +/** Fork: a member call on an imported constant is not linked (see LINKS). */ +const FLAGS_LINKS: string[] = []; + +let roots: string[] = []; +let graphs: CodeGraph[] = []; + +afterEach(() => { + for (const graph of graphs) graph.close(); + graphs = []; + for (const root of roots) fs.rmSync(root, { recursive: true, force: true }); + roots = []; + vi.restoreAllMocks(); +}); + +function tempRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-member-')); + roots.push(root); + return root; +} + +function write(root: string, files: Files): 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); + } +} + +async function index(root: string): Promise { + const graph = await CodeGraph.init(root, { index: true }); + graphs.push(graph); + return graph; +} + +/** Every edge, by its ends' natural keys, with where it is written and how it resolved. */ +function edgesOf(graph: CodeGraph): string[] { + const keys = new Map( + graph.getFiles() + .flatMap((file) => graph.getNodesInFile(file.path)) + .map((n) => [n.id, `${n.kind} ${n.filePath}:${n.qualifiedName}:${n.startLine}`] as const) + ); + return graph.getOutgoingEdgesFrom([...keys.keys()]) + .map((e) => `${e.kind} ${keys.get(e.source)} -> ${keys.get(e.target) ?? e.target} @${e.line}:${e.column} ${JSON.stringify(e.metadata ?? {})}`) + .sort(); +} + +/** The edges made by references whose name has a member (`x.y`): `: -> ::`. */ +function memberLinksOf(graph: CodeGraph): string[] { + const ids = graph.getFiles().flatMap((file) => graph.getNodesInFile(file.path)).map((n) => n.id); + const links: string[] = []; + for (const edge of graph.getOutgoingEdgesFrom(ids)) { + const ref = edge.metadata?.refName; + if (typeof ref !== 'string' || !/^[A-Za-z_$][\w$]*\./.test(ref)) continue; + const source = graph.getNode(edge.source)!; + const target = graph.getNode(edge.target)!; + links.push(`${ref}: ${edge.kind} ${source.kind} ${source.filePath} -> ${target.kind} ${target.filePath}::${target.name}`); + } + return [...new Set(links)].sort(); +} + +/** The final tree, indexed from scratch in a folder of its own: what the sync has to match. */ +async function fullIndexOf(files: Files): Promise { + const root = tempRoot(); + write(root, files); + return edgesOf(await index(root)); +} + +/** The tail each failed reference of `root`'s index is parked under, by ` `. */ +function failedTails(root: string): Map { + const db = DatabaseConnection.open(getDatabasePath(root)); + try { + const rows = db.getDb() + .prepare("SELECT reference_name AS name, reference_kind AS kind, file_path AS file, name_tail AS tail FROM unresolved_refs WHERE status = 'failed'") + .all() as Array<{ name: string; kind: string; file: string; tail: string }>; + return new Map(rows.map((r) => [`${r.file} ${r.kind} ${r.name}`, r.tail])); + } finally { + db.close(); + } +} + +/** A module before the edit that gives it what its importers import. */ +const stub = (file: string): string => (file.endsWith('.vue') ? '\n' : 'export const placeholder = 1;\n'); + +describe('sync links member references through an import binding to their module', () => { + it('when the module is added after its importers', async () => { + const root = tempRoot(); + write(root, IMPORTERS); + const graph = await index(root); + expect(memberLinksOf(graph)).toEqual([]); + + write(root, MODULES); + const result = await graph.sync(); + // The importers did not change: only the retry of their parked refs links them. + expect(result.filesAdded).toBe(Object.keys(MODULES).length); + expect(result.filesModified).toBe(0); + expect(memberLinksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + expect(graph.getPendingReferenceCount()).toBe(0); + }, 120_000); + + it('when the module comes back after it was deleted', async () => { + const root = tempRoot(); + write(root, { ...IMPORTERS, ...MODULES }); + const graph = await index(root); + expect(memberLinksOf(graph)).toEqual(LINKS); + + for (const file of Object.keys(MODULES)) fs.rmSync(path.join(root, file)); + expect((await graph.sync()).filesRemoved).toBe(Object.keys(MODULES).length); + expect(memberLinksOf(graph)).toEqual([]); + + write(root, MODULES); + expect((await graph.sync()).filesAdded).toBe(Object.keys(MODULES).length); + expect(memberLinksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('when an edit gives the module what its importers import', async () => { + const root = tempRoot(); + write(root, IMPORTERS); + write(root, Object.fromEntries(Object.keys(MODULES).map((file) => [file, stub(file)]))); + const graph = await index(root); + + write(root, MODULES); + const result = await graph.sync(); + expect(result.filesModified).toBe(Object.keys(MODULES).length); + expect(result.filesAdded).toBe(0); + expect(memberLinksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('and through its barrel when the file the barrel forwards from is added', async () => { + const initial: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/config.ts': "export * from './shared/flags';\n", + 'src/use-flags.ts': "import { FLAGS } from './config';\nexport function on(f: string) { return FLAGS.includes(f); }\n", + }; + const added: Files = { 'src/shared/flags.ts': "export const FLAGS = ['a'];\n" }; + // None of the added file's keys is the barrel's. + expect(moduleReferenceKeys('src/shared/flags.ts')).not.toContain(moduleTail('src/config')); + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + + write(root, added); + expect((await graph.sync()).filesAdded).toBe(1); + expect(memberLinksOf(graph)).toEqual(FLAGS_LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...added })); + }, 120_000); + + it('and through its barrel when the file the barrel forwards from gains the declaration', async () => { + // Parked under the barrel's key, the call is one the sync's walk up from + // an edited file to the barrels passing it on re-opens (#2560). + const initial: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/config.ts': "export * from './shared/flags';\n", + 'src/use-flags.ts': "import { FLAGS } from './config';\nexport function on(f: string) { return FLAGS.includes(f); }\n", + 'src/shared/flags.ts': 'export const placeholder = 1;\n', + }; + const edited: Files = { 'src/shared/flags.ts': "export const FLAGS = ['a'];\n" }; + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + + write(root, edited); + expect((await graph.sync()).filesModified).toBe(1); + expect(memberLinksOf(graph)).toEqual(FLAGS_LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...edited })); + }, 120_000); + + it('and by its receiver when the barrel names that file through an alias in a Vue project', async () => { + // With Vue's resolver, the barrel's `export * from '@/shared/flags'` + // links the constant the file declares, not the file, so no walk up from + // the edited file finds the barrel: only the receiver's name does. + const initial: Files = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { vue: '^3' } }), + 'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@/*': ['src/*'] } } }), + 'src/config.ts': "export * from '@/shared/flags';\n", + 'src/use-flags.ts': "import { FLAGS } from '@/config';\nexport function on(f: string) { return FLAGS.includes(f); }\n", + 'src/shared/flags.ts': 'export const placeholder = 1;\n', + }; + const edited: Files = { 'src/shared/flags.ts': "export const FLAGS = ['a'];\n" }; + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + + write(root, edited); + expect((await graph.sync()).filesModified).toBe(1); + expect(memberLinksOf(graph)).toEqual(FLAGS_LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...edited })); + }, 120_000); + + it('and by its member when a namespace import\'s barrel forwards it from a file of another name', async () => { + const initial: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/ns.ts': "export * from './impl/make';\n", + 'src/use-ns.ts': "import * as NS from './ns';\nexport function go() { return NS.make(); }\n", + }; + const added: Files = { 'src/impl/make.ts': 'export function make() { return 1; }\n' }; + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + expect(failedTails(root).get('src/use-ns.ts calls NS.make')).toBe('module:ns'); + + write(root, added); + expect((await graph.sync()).filesAdded).toBe(1); + expect(memberLinksOf(graph)).toEqual(['NS.make: calls function src/use-ns.ts -> function src/impl/make.ts::make']); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...added })); + }, 120_000); + + it('and by its member when a full index guesses the method by its name', async () => { + // The binding's module never resolves, so upstream's full index links the + // call by the method's name, to the one class declaring it. Fork: a + // receiver bound by an import is never guessed by the method's name, so + // the call stays unresolved; the sync must still match a full index. + const initial: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/a.ts': "import { store } from './missing-store';\nexport function a() { return store.fetchUserProfile(); }\n", + }; + const added: Files = { 'src/impl/profile.ts': 'export class ProfileStore {\n fetchUserProfile() { return 1; }\n}\n' }; + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + expect(failedTails(root).get('src/a.ts calls store.fetchUserProfile')).toBe('module:missing-store'); + + write(root, added); + expect((await graph.sync()).filesAdded).toBe(1); + expect(memberLinksOf(graph)).toEqual([]); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...added })); + }, 120_000); + + it('and by its method when a route it navigates to appears in another file', async () => { + // Vue Router: the router module default-exports a call, which no binding + // resolves to, so `router.push` fails and is parked under the module. A + // route added to the table in another file binds it again. + const initial: Files = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { vue: '3', 'vue-router': '4' } }), + 'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@/*': ['src/*'] } } }), + 'src/router/index.js': + "import { createRouter, createWebHistory } from 'vue-router'\n" + + "import { routes } from '../routes'\n" + + 'export default createRouter({ history: createWebHistory(), routes })\n', + 'src/routes.js': "export const routes = [\n { name: 'home', path: '/', component: () => import('@/views/Home.vue') },\n]\n", + 'src/views/Home.vue': '\n', + 'src/views/Login.vue': '\n', + 'src/auth.js': "import router from '@/router'\nexport function logout() {\n router.push('/login')\n}\n", + }; + const edited: Files = { + 'src/routes.js': + "export const routes = [\n { name: 'home', path: '/', component: () => import('@/views/Home.vue') },\n" + + " { name: 'login', path: '/login', component: () => import('@/views/Login.vue') },\n]\n", + }; + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + expect(failedTails(root).get('src/auth.js calls router.push')).toBe('module:router'); + // None of the edited file's keys or names is the call's module, receiver or method. + expect(moduleReferenceKeys('src/routes.js')).not.toContain('module:router'); + + write(root, edited); + expect((await graph.sync()).filesModified).toBe(1); + expect(memberLinksOf(graph)).toEqual(['router.push: navigates function src/auth.js -> route src/routes.js::/login']); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...edited })); + }, 120_000); +}); + +describe('a failed member reference through an import binding', () => { + it('waits for its module, and every other reference keeps its own tail', async () => { + const root = tempRoot(); + write(root, { + ...IMPORTERS, + // A member of a package's binding, the whole name of an unaliased named + // import, and a chain or a longer path through a binding wait for the + // name they use. + 'src/other.ts': + "import express from 'express';\nimport { helper } from './util';\nimport CONFIG from './config';\n" + + 'export const app = express.Router();\nexport const value = helper;\n' + + 'export const fetched = CONFIG.api.fetch();\nexport const loaded = CONFIG.load().then(() => 1);\n', + }); + (await CodeGraph.init(root, { index: true })).close(); + const tails = failedTails(root); + + expect(tails.get('src/diff-utils.ts calls NODE_PATH_EXCLUDELIST.includes')).toBe('module:constants'); + expect(tails.get('src/flags.ts calls REGISTERED_FLAGS.get')).toBe('module:registry'); + expect(tails.get('src/save.ts calls store.set')).toBe('module:store'); + expect(tails.get('src/save.ts calls store.get')).toBe('module:store'); + expect(tails.get('src/entry.ts calls useStore.setState')).toBe('module:entry-store'); + expect(tails.get('src/client.ts calls api.get')).toBe('module:api'); + expect(tails.get('src/components/Panel.vue calls COLUMNS.map')).toBe('module:table'); + + expect(tails.get('src/noise.ts calls extApi.get')).toBe('get'); + expect(tails.get('src/other.ts calls express.Router')).toBe('Router'); + expect(tails.get('src/other.ts imports helper')).toBe('helper'); + expect(tails.get('src/other.ts calls CONFIG.api.fetch')).toBe('fetch'); + expect(tails.get('src/other.ts calls CONFIG.load().then')).toBe('then'); + // The chain's own first call is a direct member. + expect(tails.get('src/other.ts calls CONFIG.load')).toBe('module:config'); + }, 60_000); +}); + +describe('schema v16', () => { + let db: DatabaseConnection | undefined; + + afterEach(() => { + db?.close(); + db = undefined; + }); + + function fixture(): QueryBuilder { + const root = tempRoot(); + db = DatabaseConnection.initialize(path.join(root, 'test.db')); + const queries = new QueryBuilder(db.getDb()); + queries.insertNode({ id: 'f', kind: 'file', name: 'save.ts', qualifiedName: 'src/save.ts', filePath: 'src/save.ts', + language: 'typescript', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 }); + for (const [referenceName, referenceKind, tail, line] of [ + ['store.get', 'calls', 'module:store', 1], + ['store.set', 'calls', 'module:store', 2], + ['router.push', 'calls', 'module:router', 3], + ['api.fetchUser', 'calls', 'module:api', 4], + ['tagsController', 'imports', 'module:tag', 5], + // A member of a receiver no binding names: parked under its member. + ['extApi.get', 'calls', 'get', 6], + ] as const) { + queries.insertUnresolvedRef({ fromNodeId: 'f', referenceName, referenceKind, line, column: 0, filePath: 'src/save.ts', language: 'typescript' }); + db.getDb().prepare("UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE line = ?").run(tail, line); + } + return queries; + } + + const retried = (queries: QueryBuilder, names: string[], ceiling?: number) => + queries.getRetryableFailedReferences(names, ceiling).map((ref) => `${ref.referenceKind} ${ref.referenceName}`).sort(); + + it('finds a member reference parked under its module by its module, its member and its receiver', () => { + const queries = fixture(); + expect(retried(queries, moduleReferenceKeys('src/store.ts'))).toEqual(['calls store.get', 'calls store.set']); + expect(retried(queries, ['get'])).toEqual(['calls extApi.get', 'calls store.get']); + expect(retried(queries, ['store'])).toEqual(['calls store.get', 'calls store.set']); + expect(retried(queries, ['fetchUser', 'router'])).toEqual(['calls api.fetchUser', 'calls router.push']); + // A whole name's lookup is #2450's, by its name. + expect(retried(queries, ['tagsController'])).toEqual(['imports tagsController']); + // A file's name, a key and a prefix of a receiver are no receiver. + expect(retried(queries, ['store.ts', 'module:store.ts', 'stor', 'rou'])).toEqual([]); + // The per-name ceiling holds for the member and receiver lookups too, and + // a member shares its name's with the refs still parked under that name, + // as one tail before. + expect(retried(queries, ['store'], 1)).toEqual([]); + expect(retried(queries, ['get'], 1)).toEqual([]); + expect(retried(queries, ['get'], 2)).toEqual(['calls extApi.get', 'calls store.get']); + expect(retried(queries, ['set', 'push'], 0)).toEqual([]); + }); + + it('finds a call parked under its module by its method for a router, and its member for the dead-code check', () => { + const queries = fixture(); + expect(queries.getFailedCallsByTail(['push', 'get']).map((ref) => ref.referenceName).sort()).toEqual(['extApi.get', 'router.push', 'store.get']); + expect([...queries.getUnresolvedNamesAmong(['fetchUser', 'push', 'set', 'store', 'nothing'])].sort()).toEqual(['fetchUser', 'push', 'set']); + }); + + it('reads the member as the tail its name gives', () => { + fixture(); + const memberOf = db!.getDb().prepare(`SELECT ${MEMBER_TAIL_SQL} AS member FROM (SELECT ? AS reference_name)`); + for (const name of ['store.get', 'NODE_PATH_EXCLUDELIST.includes', 'LogAPI.getVisitOverview().then', 'CONFIG.api.fetch', 'a.b.c.d', '$el.$on', 'helper', 'café.größe']) { + expect((memberOf.get(name) as { member: string }).member).toBe(referenceNameTail(name, 'calls')); + } + }); + + it('looks a member and a receiver up through the module indexes, not every failed row', () => { + const queries = fixture(); + const prepare = vi.spyOn(db!.getDb(), 'prepare'); + queries.getRetryableFailedReferences(['get', 'store']); + queries.getFailedCallsByTail(['push']); + queries.getUnresolvedNamesAmong(['fetchUser']); + const sqls = prepare.mock.calls.map(([sql]) => sql as string); + prepare.mockRestore(); + const plan = (sql: string) => db!.getDb().prepare(`EXPLAIN QUERY PLAN ${sql}`) + .all(...Array.from({ length: (sql.match(/\?/g) ?? []).length }, () => 'store')) + .map((row) => (row as { detail: string }).detail).join('; '); + + const memberSelects = sqls.filter((sql) => sql.includes(`${MEMBER_TAIL_SQL} IN`)); + // Counts and rows of the retry, the router's calls, the dead-code names. + expect(memberSelects).toHaveLength(4); + for (const sql of memberSelects) expect(plan(sql)).toMatch(/USING (COVERING )?INDEX idx_unresolved_failed_module_member/); + const receiverSelects = sqls.filter((sql) => sql.includes('reference_name >= ?')); + expect(receiverSelects).toHaveLength(1); + expect(plan(receiverSelects[0]!)).toMatch(/USING (COVERING )?INDEX idx_unresolved_failed_module_name \(status=\? AND reference_name>\? AND reference_name<\?\)/); + }); + + it('adds the member index to an older index, and replays cleanly', () => { + fixture(); + const indexes = () => db!.getDb().prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND name = 'idx_unresolved_failed_module_member'").all(); + db!.getDb().exec(`DROP INDEX idx_unresolved_failed_module_member; + DELETE FROM schema_versions WHERE version >= 22; + INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (21, 0, 'legacy fixture');`); + expect(indexes()).toHaveLength(0); + runMigrations(db!.getDb(), 21); + expect(getCurrentVersion(db!.getDb())).toBe(CURRENT_SCHEMA_VERSION); + expect(indexes()).toHaveLength(1); + db!.getDb().exec('DELETE FROM schema_versions WHERE version >= 22'); + runMigrations(db!.getDb(), 21); + expect(indexes()).toHaveLength(1); + }); + + it('defines the member index in schema.sql with the expression the queries spell', () => { + const schema = fs.readFileSync(path.join(__dirname, '..', 'src', 'db', 'schema.sql'), 'utf8'); + const definition = /CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_member[^;]*;/.exec(schema)?.[0]; + expect(definition).toBe(`CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_member ON unresolved_refs(status, ${MEMBER_TAIL_SQL}) WHERE ${MODULE_PARKED_SQL};`); + }); +}); diff --git a/src/codegraph.ts b/src/codegraph.ts index 327ce6dc12..b38be0c80d 100644 --- a/src/codegraph.ts +++ b/src/codegraph.ts @@ -1076,9 +1076,11 @@ 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 module keys for renamed/default imports, then re-resolve that set. - // On a sync where no failed ref - // matches, this is one indexed lookup. + // and module keys for renamed/default imports and for a direct + // member reference through any binding (`LIST.includes(x)` after + // `import { LIST } from './constants'`), then re-resolve that set. + // On a sync where no failed ref matches, this is a few indexed + // lookups. const tRetry = Date.now(); edgesBeforeRetry = this.queries.getMaxEdgeId(); const retryable = this.queries.getRetryableFailedReferences([...new Set([ diff --git a/src/db/migrations.ts b/src/db/migrations.ts index 40e51ad0ca..1a37aa8fb9 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -5,12 +5,12 @@ */ import { SqliteDatabase } from './sqlite-adapter'; -import { referenceNameTail } from './reference-tail'; +import { referenceNameTail, MEMBER_TAIL_SQL, MODULE_PARKED_SQL } from './reference-tail'; /** * Current schema version */ -export const CURRENT_SCHEMA_VERSION = 21; +export const CURRENT_SCHEMA_VERSION = 22; /** * Migration definition @@ -415,6 +415,23 @@ CREATE TABLE synth_skips ( `); }, }, + { + // Upstream's version 16; the fork's own migrations hold 16-20. + version: 22, + description: 'Retry a failed member reference through an import binding by its member: module-tail member index', + up: (db) => { + // A failed member reference through an import binding (`store.get`) is + // parked under the module's key from this version on, and still looked + // up by its member, the tail its name gives. Rows parked before keep + // that tail, which the tail lookup already finds; a re-index parks + // them under their modules. Keep the definition in lockstep with + // schema.sql: the expression is MEMBER_TAIL_SQL. + if (!db.prepare("SELECT 1 FROM pragma_table_info('unresolved_refs') WHERE name = 'name_tail'").get()) return; + db.exec(` + CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_member ON unresolved_refs(status, ${MEMBER_TAIL_SQL}) WHERE ${MODULE_PARKED_SQL}; + `); + }, + }, ]; /** diff --git a/src/db/queries.ts b/src/db/queries.ts index bcff61d25b..42d8032292 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -22,7 +22,7 @@ import { getStemVariants, isTestFile, kindBonus, nameMatchBonus, scorePathReleva import { parseQuery, boundedEditDistance } from '../search/query-parser'; import { isGeneratedFile } from '../extraction/generated-detection'; import { splitIdentifierSegments } from '../search/identifier-segments'; -import { isModuleKey, referenceNameTail } from './reference-tail'; +import { isModuleKey, referenceNameTail, MEMBER_TAIL_SQL, MODULE_PARKED_SQL } from './reference-tail'; /** * Files that should not be candidates for "dominant file" detection: tests, @@ -47,6 +47,9 @@ const SQLITE_PARAM_CHUNK_SIZE = 500; /** project_metadata key naming the build that wrote the synth_skips rows. */ const SYNTH_SKIPS_VERSION_KEY = 'synth_skips_version'; +/** A name a member reference's receiver can be: not a file's name, nor a `module:` key. */ +const RECEIVER_NAME = /^[A-Za-z_$][\w$]*$/; + /** * A SQL predicate: is the node aliased `alias` a member an INTERFACE declares? * @@ -2586,11 +2589,13 @@ export class QueryBuilder { * called unreferenced, whatever the edge table says. It is deliberately * matched loosely, on the reference name AND on its tail (`util.greet` → * `greet`), because the question being asked is "could this name be the one - * we failed to follow", and a maybe has to count as a yes. + * we failed to follow", and a maybe has to count as a yes. A reference + * parked under its module (`store.get` → 'module:store') is matched on the + * tail its name gives. * * Bounded-lookup like {@link getGeneratedPathsAmong}: the caller holds a - * candidate list, so this is a chunked probe over `idx_unresolved_name`, not - * a scan of the table. + * candidate list, so this is a chunked probe over `idx_unresolved_name` + * (and the module-member index), not a scan of the table. */ getUnresolvedNamesAmong(names: Iterable): Set { const unique = [...new Set(names)].filter((name) => name.length > 0); @@ -2606,9 +2611,12 @@ export class QueryBuilder { WHERE reference_name IN (${placeholders}) UNION SELECT DISTINCT name_tail AS name FROM unresolved_refs - WHERE name_tail IN (${placeholders})` + WHERE name_tail IN (${placeholders}) + UNION + SELECT DISTINCT ${MEMBER_TAIL_SQL} AS name FROM unresolved_refs + WHERE ${MODULE_PARKED_SQL} AND ${MEMBER_TAIL_SQL} IN (${placeholders})` ) - .all(...chunk, ...chunk) as Array<{ name: string }>; + .all(...chunk, ...chunk, ...chunk) as Array<{ name: string }>; for (const row of rows) found.add(row.name); } return found; @@ -4240,7 +4248,11 @@ export class QueryBuilder { * is a reference through an import binding of the module. Such a binding's * reference still waits for its own name too, looked up by its whole name: * a fresh index links a binding whose module never resolves it by that - * name alone, to whichever declaration carries it. + * name alone, to whichever declaration carries it. A member reference + * through a binding (`store.get`) waits for its member's name, the tail its + * name gives, under that name's ceiling, and for its receiver's, looked up + * as the prefix `store.`: the declaration of a name a barrel forwards is in + * a file of another name. * * A module key has a ceiling of its own, `perModuleCeiling`. What waits * under it imports a module of that name, so a large population is not @@ -4256,25 +4268,39 @@ export class QueryBuilder { // idx_unresolved_failed_module_name; its leading `status` is the // equality term that keeps the planner off idx_unresolved_status and // idx_unresolved_name, which read hundreds of thousands of rows here. - { column: 'reference_name', where: "status = 'failed' AND name_tail GLOB 'module:*'" }, + { column: 'reference_name', where: MODULE_PARKED_SQL }, + // A member reference's member: idx_unresolved_failed_module_member, for + // the same reason. A whole name is the lookup above's. + { column: MEMBER_TAIL_SQL, where: `${MODULE_PARKED_SQL} AND instr(reference_name, '.') > 0` }, ]; - const rows = new Map(); - for (const { column, where } of lookups) { - // Pass 1: per-key counts, chunked under the SQLite parameter limit. - const retryKeys: string[] = []; + // Pass 1: per-key counts, chunked under the SQLite parameter limit. + const counts = lookups.map(({ column, where }) => { + const byKey = new Map(); for (let i = 0; i < names.length; i += SQLITE_PARAM_CHUNK_SIZE) { const chunk = names.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); const placeholders = chunk.map(() => '?').join(','); - const counts = this.db + const found = this.db .prepare(`SELECT ${column} AS key, COUNT(*) AS count FROM unresolved_refs WHERE ${where} AND ${column} IN (${placeholders}) GROUP BY ${column}`) .all(...chunk) as Array<{ key: string; count: number }>; - for (const row of counts) { - const ceiling = column === 'name_tail' && isModuleKey(row.key) ? perModuleCeiling : perNameCeiling; - if (row.count <= ceiling) retryKeys.push(row.key); - } + for (const row of found) byKey.set(row.key, row.count); } + return byKey; + }); + // A member reference parked under its module had its member's name as its + // tail before, so the two lookups by that name share one ceiling: a name + // too common to look up stays so wherever its references are parked, and + // the references still under it are not retried more for it. + const [byTail, , byMember] = counts; + const population = (lookup: number, key: string, count: number): number => + lookup === 1 ? count : (byTail!.get(key) ?? 0) + (byMember!.get(key) ?? 0); + + const rows = new Map(); + for (const [lookup, { column, where }] of lookups.entries()) { + const retryKeys = [...counts[lookup]!] + .filter(([key, count]) => (lookup === 0 && isModuleKey(key) ? count <= perModuleCeiling : population(lookup, key, count) <= perNameCeiling)) + .map(([key]) => key); - // Pass 2: load the surviving rows; a row both lookups find is kept once. + // Pass 2: load the surviving rows; a row two lookups find is kept once. for (let i = 0; i < retryKeys.length; i += SQLITE_PARAM_CHUNK_SIZE) { const chunk = retryKeys.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); const placeholders = chunk.map(() => '?').join(','); @@ -4288,6 +4314,18 @@ export class QueryBuilder { } } + // By receiver: one range of idx_unresolved_failed_module_name per name, + // `store.` up to `store/` ('/' follows '.'), read one past the ceiling. + const byReceiver = this.db.prepare( + `SELECT * FROM unresolved_refs WHERE ${MODULE_PARKED_SQL} AND reference_name >= ? AND reference_name < ? LIMIT ?` + ); + for (const name of names) { + if (!RECEIVER_NAME.test(name)) continue; + const found = byReceiver.all(`${name}.`, `${name}/`, perNameCeiling + 1) as UnresolvedRefRow[]; + if (found.length > perNameCeiling) continue; + for (const row of found) rows.set(row.id, row); + } + return [...rows.values()].map((row) => ({ fromNodeId: row.from_node_id, referenceName: row.reference_name, @@ -4423,8 +4461,10 @@ export class QueryBuilder { /** * Failed `calls` refs whose name tail is one of `tails` — the navigation * calls (`history.push`, `navigate`) a sync looks through after a route - * appeared or went away. Read through the failed-tail index; the caller - * decides on each whole name. + * appeared or went away. Read through the failed-tail index, and a call + * through an import binding, parked under its module (`router.push` → + * 'module:router'), through the module-member index; the caller decides on + * each whole name. */ getFailedCallsByTail(tails: string[]): Array<{ rowId: number; referenceName: string; filePath: string }> { const out: Array<{ rowId: number; referenceName: string; filePath: string }> = []; @@ -4435,9 +4475,12 @@ export class QueryBuilder { const rows = this.db .prepare( `SELECT id, reference_name, file_path FROM unresolved_refs - WHERE status = 'failed' AND reference_kind = 'calls' AND name_tail IN (${placeholders})` + WHERE status = 'failed' AND reference_kind = 'calls' AND name_tail IN (${placeholders}) + UNION ALL + SELECT id, reference_name, file_path FROM unresolved_refs + WHERE ${MODULE_PARKED_SQL} AND reference_kind = 'calls' AND ${MEMBER_TAIL_SQL} IN (${placeholders})` ) - .all(...chunk) as Array<{ id: number; reference_name: string; file_path: string }>; + .all(...chunk, ...chunk) as Array<{ id: number; reference_name: string; file_path: string }>; for (const row of rows) out.push({ rowId: row.id, referenceName: row.reference_name, filePath: row.file_path }); } return out; diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts index 9ec033f345..dcc031a228 100644 --- a/src/db/reference-tail.ts +++ b/src/db/reference-tail.ts @@ -78,6 +78,21 @@ export function isModuleKey(key: string): boolean { return key.startsWith(MODULE_KEY); } +/** + * SQL for the tail a reference parked under its module (`module:*`) has by its + * own name: what follows the name's last dot — `store.get` → 'get' — or the + * whole name. A member reference through an import binding still waits for + * its member (see + * `importBindingTail`), so the retry, the navigation retry and the dead-code + * check look it up by that. idx_unresolved_failed_module_member indexes the + * expression, and SQLite uses an expression index only for a query that + * spells the expression exactly so: schema.sql and the v16 migration do. + */ +export const MEMBER_TAIL_SQL = "substr(reference_name, length(rtrim(reference_name, replace(reference_name, '.', ''))) + 1)"; + +/** The rows parked under a module: the partial WHERE of the indexes that find them. */ +export const MODULE_PARKED_SQL = "status = 'failed' AND name_tail GLOB 'module:*'"; + /** * The tail of a reference that waits for a module rather than for a name: the * module path's stem behind 'module:' — `./pages/Team` → 'module:Team', @@ -129,7 +144,8 @@ export function importPathKeys(filePath: string): string[] { * AND the ones it changes: a route renders the component its module exports, * so an edit that gives the module one is what the route waited for. So does * a reference through an import binding the module declares under another - * name (`importBindingTail`), which is parked under the same keys. + * name, and a direct member reference through any binding + * (`importBindingTail`), which are parked under the same keys. */ export function moduleReferenceKeys(filePath: string): string[] { const keys = importPathKeys(filePath); diff --git a/src/db/schema.sql b/src/db/schema.sql index 41bafcc3af..4c5303c89c 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -248,6 +248,11 @@ CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_name ON unresolved_refs( -- (`module:tag`) and looked up by its whole name as well. The leading status -- is the equality term that keeps the planner on this small index. CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_name ON unresolved_refs(status, reference_name) WHERE status = 'failed' AND name_tail GLOB 'module:*'; +-- A member reference through an import binding (`store.get`) is parked under +-- its module too, and still looked up by its member: the tail its name gives, +-- what follows its last dot. Spell the expression exactly as MEMBER_TAIL_SQL +-- (reference-tail.ts) does, or SQLite will not use this index. +CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_member ON unresolved_refs(status, substr(reference_name, length(rtrim(reference_name, replace(reference_name, '.', ''))) + 1)) WHERE status = 'failed' AND name_tail GLOB 'module:*'; CREATE INDEX IF NOT EXISTS idx_edges_provenance ON edges(provenance); -- Project metadata for version/provenance tracking diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index da452446a7..717b61d558 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -1053,13 +1053,52 @@ export function isBoundToOutOfRepoImport( return false; } -/** Park an unresolved renamed/default import under the module a later sync can introduce. */ +/** + * The tail a reference that failed to resolve is parked under when it goes + * through an import binding the module's key finds and its own tail does not: + * - the binding's whole name, when the module it imports does not declare it + * by that name: a default import (`import tagsController from + * './tag/tag.controller'`, a CommonJS `require`), a namespace import, or an + * aliased one (`import { Component as Wrapper }`, `{ default as X }`) — the + * import itself, a call, `new`, a JSX tag, a value. Its own name, the + * default tail, is one the module never declares. A sync still looks it up + * by that name as well: a binding its module never resolves is linked by + * the name alone. + * - a direct member of any binding, a named import's included + * (`NODE_PATH_EXCLUDELIST.includes(p)`, `store.get(k)`, `NS.make()`). It + * reaches the binding's declaration, or a member of it, through the module, + * and its default tail is the member's name (`includes`, `get`), which the + * module seldom declares and which too many other failed refs share for the + * retry to look it up. A sync still looks it up by that member's name, which + * is the member a namespace import's barrel forwards or the method a guess + * by name links, and by its receiver's, which a barrel's source declares. + * A chain (`URI.file(p).toString()`) or a longer path + * (`vscode.window.showErrorMessage`) keeps its member's name: what its last + * member hangs off is what another member returns or holds, seldom a type + * the module declares. Parked under the module as well, they tripled the + * refs an edit of a hot module retries on vscode, and none of them resolved. + * Resolution reaches either through the module, so a sync's retry has to find + * it by the module: under the module's {@link moduleTail} ('module:tag'), the + * key a sync looks up for every file it adds or changes, so the file that + * appears or gains the export finds it. + * + * Only for a module of the project: a package's binding keeps its tail. + * Undefined for every other reference, and for the whole name of an + * unaliased named import, which is the name its module declares. A Python + * reference is {@link pythonImportBindingTail}'s, by its whole name only. + */ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext): string | undefined { if (ref.language === 'python') return pythonImportBindingTail(ref, context); - if (!ESM_IMPORT_LANGUAGES.has(ref.language) || !IDENTIFIER.test(ref.referenceName)) return undefined; - const binding = context.getImportMappings(ref.filePath, ref.language).find(m => m.localName === ref.referenceName); - if (!binding || (!binding.isDefault && !binding.isNamespace && binding.exportedName === binding.localName)) return undefined; + if (!ESM_IMPORT_LANGUAGES.has(ref.language)) return undefined; + // An import's dotted name is a path, not a member. + const receiver = ref.referenceKind === 'imports' ? undefined : MEMBER_RECEIVER.exec(ref.referenceName)?.[1]; + const name = receiver ?? ref.referenceName; + if (!receiver && !IDENTIFIER.test(name)) return undefined; + const binding = context.getImportMappings(ref.filePath, ref.language).find((m) => m.localName === name); + if (!binding || (!receiver && !binding.isDefault && !binding.isNamespace && binding.exportedName === binding.localName)) return undefined; + // The importing file passes its nearest tsconfig aliases (`#/views/…` in a monorepo app). if (isExternalImport(binding.source, ref.language, context, ref.filePath)) return undefined; + // `..` names a folder through the importing file's own location. const modulePath = binding.source.startsWith('.') ? path.posix.join(path.posix.dirname(ref.filePath), binding.source) : binding.source; return moduleTail(modulePath) || undefined; @@ -1161,3 +1200,5 @@ export function pythonModulePath(source: string, fromFile: string): string | nul } const IDENTIFIER = /^[A-Za-z_$][\w$]*$/; +/** The receiver of a direct member reference: `store` of `store.get`, none of `store.get().then` or `a.b.c`. */ +const MEMBER_RECEIVER = /^([A-Za-z_$][\w$]*)\.[A-Za-z_$][\w$]*$/;