From e2b0cf5eea53fa5fb2dc5990b798e66223ae6280 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 17 Oct 2026 02:05:39 +0000 Subject: [PATCH 1/3] =?UTF-8?q?fix(cpp):=20a=20call=20on=20an=20inherited?= =?UTF-8?q?=20member=20reached=20through=20a=20chain,=20like=20`options.st?= =?UTF-8?q?atistics->getTickerCount(=E2=80=A6)`=20on=20a=20RocksDB=20`Opti?= =?UTF-8?q?ons`,=20links=20to=20the=20member's=20class=20(#2628)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 3 of member chains, after #2626 and #2627. The type of `x.member` was read from x's class alone, so a member that class inherits was never found and the call fell back to its bare name. A temporary diagnostic on main measured why chain calls still fell through: 2,801 of 4,884 were a member declared in a base class. Nearly all were rocksdb's `Options`, which declares no members of its own and inherits `statistics`, `table_factory`, `listeners`, `prefix_extractor` and the rest from `DBOptions` and `ColumnFamilyOptions`. The other causes are left for later: - a chain whose base can't be typed: 875; - a class the index lacks, e.g. protobuf's generated `_impl_`: 626; - a member found nowhere: 337. cppExpressionType's member branch now uses cppInheritedDataMember, the lookup the calling class's own members already use (#2533): - the class's own declaration first; - then the classes it derives from, through their base edges, to depth 4; - a base that depends on the class's template parameters is skipped unless a using-declaration brings the name in. Kit A/B against main e8ec7fd8: | repo | removed | added | retargeted | same target, now typed | |---|---|---|---|---| | rocksdb | 1,410 | 634 | 392 | 184 | | protobuf | 0 | 2 | 149 | 55 | | leveldb, fmt, redis | identical | | | | - Removed: the bare-name guesses on inherited std::shared_ptr and std::vector members, such as `options.table_factory.reset(…)` → FSWritableFilePtr::reset (654) and `options_.db_paths.emplace_back(…)` → autovector::emplace_back (256). - Added: `options.statistics->getTickerCount(…)` → Statistics (432), `rate_limiter` → RateLimiter (51), `comparator` → Comparator (26). - Retargeted: DummyOldStats -> Statistics, DB -> Env, ClockCacheShard -> Cache. - protobuf's generated `this_._internal_metadata_.Clear<…>()`, `MergeFrom<…>` and `unknown_fields<…>` now reach InternalMetadata, a member every message inherits from MessageLite. Before, they reached the message's own `Clear` or `Any::MergeFrom`. All 50 random sites read by hand (36 rocksdb, 14 protobuf) were correct. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../cpp-member-chain-base-members.test.ts | 131 ++++++++++++++++++ src/resolution/name-matcher.ts | 9 +- 3 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 __tests__/cpp-member-chain-base-members.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 17eefc2b5..f418d584f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,7 +105,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C, a call now links to the function of that name its own file can see: its own, else one a header it includes defines, like a `static inline` helper, else one another source file defines without `static`. Before, the function nearest the call was taken, and when that was another source file's `static` function, which no other file can call, the call linked to nothing: in redis, the calls to `server.c`'s `mstime()` and `ustime()` from the rest of the server were lost to the `static` copies other files keep for themselves, and jemalloc's tests' calls to the `do_arena_create` their shared header defines to a neighbouring test's own. A call to a `static inline` function that several headers define reaches the copy in a header the file includes, so protocolbuffers/protobuf's upb code no longer links to the copies in its PHP extension's combined `php-upb.h`. A source file that `#include`s another, like redis's `ae.c` with its event-loop backend or hiredis's `async.c` with `dict.c`, now calls that file's `static` functions, and a header a file includes through an include directory, like jemalloc's `#include "jemalloc/internal/bit_util.h"`, counts as included. A function a header defines only for a build that turns on a flag, like lua-cjson's `fpconv_init` under `#ifdef USE_INTERNAL_FPCONV`, gives way to another source file's definition when nothing in the project's code or build files sets that flag, so redis's calls to `fpconv_init()` reach the `fpconv.c` its build compiles. Re-index C projects after upgrading. - In C++, text inside a string literal is no longer taken for a variable's declaration. That covers the C code protocolbuffers/protobuf's upb generator writes in a raw string (`R"cc( … upb_MiniTableField field = $field_init$; … )cc"`) and log messages like facebook/rocksdb's `"… in WriteBatch handler; …"`. Before, a later call like `field.ctype()` on the function's own `upb::FieldDefPtr field` parameter, or `handler->PutCF(…)` on a `WriteBatch::Handler* handler`, was matched against the type that text seemed to name, so it linked to nothing or to another class's method. Re-index C++ projects after upgrading. - In C++, a call on an `auto` variable now links to the method of the class C++ finds for the type its initializer gives it, as a call on a variable declared with that type does. The type is read from a cast or smart pointer written out, like facebook/rocksdb's `auto iter = std::unique_ptr(…)` or `auto* mutex = static_cast(arg)`, from `new` or `std::make_unique` of a class, and from what a function or method is declared to return, like the `Iterator*` of `txn->GetIterator(…)`, read to the end of an initializer that goes on to the next line. Before, such a variable took the type of an earlier variable of the same name, or none: in RocksDB, `iter->SeekToFirst()` reached `MemTableRep::Iterator` and `mutex->Lock()` reached `port::Mutex`. A chained call like `cfd->current()->storage_info()` now looks up the class the inner call is declared to return the same way, through a smart pointer too, so protocolbuffers/protobuf's Java generator no longer reaches the C# generator's `EnumGenerator`. A type from outside the project no longer links to a class of the project that shares its name: protobuf's `status.ok()` on an `absl::Status` reached `upb::Status::ok`, and fmtlib/fmt's `os.flush()` on a `std::ostream&` reached fmt's own `ostream::flush`. The same holds for a variable declared in a macro body, or typed by what a function in another file returns, and for a class the index doesn't hold whose name only another class shares: protobuf's `_status.ok()` in its `RETURN_IF_ERROR` macro reached `upb::Status::ok`, and `range.options()` on its generated `DescriptorProto::ExtensionRange` reached `Descriptor::ExtensionRange::options`. A namespace a macro stands for is the project's under either name: facebook/rocksdb declares its classes in `namespace ROCKSDB_NAMESPACE` with `#define ROCKSDB_NAMESPACE rocksdb`, so `db->Put(…)` on a `rocksdb::DB*` links to RocksDB's own `DB::Put`. 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. 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/__tests__/cpp-member-chain-base-members.test.ts b/__tests__/cpp-member-chain-base-members.test.ts new file mode 100644 index 000000000..c2a2d5299 --- /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/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index d87d84adf..d3cf05b88 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -10016,14 +10016,17 @@ function cppExpressionType(expr: string, at: UnresolvedRef, context: ResolutionC return followed && cppSubscriptElement(followed, at, context); } if (!text.endsWith(')')) { - // `compact_->sub_compact_states`: a data member of the object's class; - // `it->second` through a map's iterator: the member of the `std::pair`. + // `compact_->sub_compact_states`: a data member of the object's class, or + // of a class it derives from — rocksdb's `Options` declares none of its + // own, and inherits `table_factory`, `statistics` and the rest from + // `DBOptions` and `ColumnFamilyOptions`; `it->second` through a map's + // iterator: the member of the `std::pair`. const field = /^(.+?)\s*(\.|->)\s*([A-Za-z_]\w*)$/.exec(text); const object = field && cppMemberObject(field[1]!, field[2]!, at, context, depth); const element = object && cppPairMember(object, field![3]!, at, context); if (element) return element; const cls = object && cppWrittenClass(object, at, context); - return cls ? cppDataMemberIn(cls, field![3]!, context) : null; + return cls ? cppInheritedDataMember([cls], field![3]!, context, 0, new Set([cls.qualifiedName])) : null; } const open = cppOpeningParen(text, text.length - 1); const callee = open > 0 ? text.slice(0, open).trim() : ''; From 3be906fcf4a8c3c8e0c172ce9c281b03c09c0a37 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 17 Oct 2026 05:14:14 +0000 Subject: [PATCH 2/3] test: stop index-daemon-rebuild leaving a suspended daemon on Windows (#2629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A full suite run on Windows left a `serve --mcp --path %TEMP%\cg-rebuild-…` process behind. Its parent was gone, and its one thread was suspended with 0 CPU used. It would never exit, and it held the checkout's dist. libuv creates every detached child suspended on Windows and resumes it only at the end of `uv_spawn`. A launcher killed inside that call never resumes its daemon. The daemon never runs, so it writes no daemon.pid, and nothing can find or stop it. The rebuild test's teardown killed its launcher at once. The test stops the daemon on purpose, so by then the launcher is serving calls in-process with a retry timer running. When the retry comes due during the last call, the launcher spawns its daemon as soon as that call answers, just as the teardown begins. Under load the two coincide. The teardown now: - closes each launcher's stdin and waits for it to exit. A graceful exit can't land inside the synchronous spawn. - records the daemons each launcher spawns (#1773's recorder) and stops them until all have exited. One spawned as its launcher exited may still be starting, and stopDaemonAt leaves a daemon running until it answers. The recorder's comment now says that a launcher killed mid-spawn records nothing. CODEGRAPH_DAEMON_RETRY_MS=2500 puts the retry inside the last call on a quiet box. Under it, main's test left a daemon in 6 of 12 runs: 2 suspended ones, plus 4 live ones whose fixture then failed to delete. This test left nothing in 12 runs. Of the mutants, killing launchers again, a single stopDaemonAt and no spawn recorder each leaked. The other daemon test files left nothing in 2 runs each. Co-Authored-By: Claude Opus 5.5 --- __tests__/daemon-candidates.ts | 3 +- __tests__/fixtures/record-detached-spawns.cjs | 8 +++-- __tests__/index-daemon-rebuild.test.ts | 30 ++++++++++++++----- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/__tests__/daemon-candidates.ts b/__tests__/daemon-candidates.ts index d7f31e777..a47dac76f 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 50ef59f07..9ea89464a 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 0e5e51c93..b7fb9d014 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(); From baf190678f81845bc7ea77b9fdfc38b8810724b9 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 17 Oct 2026 07:56:42 +0000 Subject: [PATCH 3/3] fix(sync): a member reference through an import binding links its module when the module changes (#2630) `NODE_PATH_EXCLUDELIST.includes(p)` after `import { NODE_PATH_EXCLUDELIST } from './constants'`, `store.get(k)` through a default import or `api.get()` through a namespace import is linked by a fresh index through the import. When the module was missing or lacked the export, the failed reference was parked under its member's name (`includes`, `get`), which the module seldom declares and which the retry skips past its 500 ceiling. A sync that gave the module the export never relinked it, nor one that added the module where no import edge reached its file. A direct member reference through an import binding of a project module is now parked under the module's key, as whole names through a binding are since #2450, and keeps two more triggers. Its member's name, through a new expression index on the tail its name gives (schema v16), under the same ceiling as the references still parked under that name: a namespace import's barrel forwards the member from another file, and a guess by name links a receiver whose module never resolves. Its receiver's name, through a prefix range on the module-name index of #2450: a barrel's import of the declaring file that no walk finds. The router navigation retry and the dead-code check read module-parked calls by their member through the new index. A chain or a longer path keeps its member's tail: on vscode, parking them too tripled the refs an edit of a hot module retries, and none of them resolved. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/sync-import-binding-retry.test.ts | 7 +- __tests__/sync-import-member-retry.test.ts | 482 ++++++++++++++++++++ src/db/migrations.ts | 19 +- src/db/queries.ts | 87 +++- src/db/reference-tail.ts | 18 +- src/db/schema.sql | 5 + src/index.ts | 9 +- src/resolution/import-resolver.ts | 58 ++- 9 files changed, 637 insertions(+), 49 deletions(-) create mode 100644 __tests__/sync-import-member-retry.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f418d584f..d50f1de81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A file in a `unittests/` folder, where llvm/llvm-project and google/breakpad keep their unit tests, or in a folder named like a unittest, such as `dcheck_unittest/` in google/glog, now counts as a test whatever its own name says, like LLVM's `CountCopyAndMove.h` helper. Before, such a file was taken for production code unless its name said test, and none of them were kept apart from the program the way a `tests/` folder is: search and `codegraph_explore` ranked them alongside production code, `codegraph affected` missed them, and production code linked to a unit test's own declarations, as LLVM's `Ctx` and `OpInfo` variables did to ones its unit tests declare. A folder named just `unittest/`, like CPython's `Lib/unittest/`, is still production code. Re-index projects that have such folders after upgrading. - `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. - `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. - `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. diff --git a/__tests__/sync-import-binding-retry.test.ts b/__tests__/sync-import-binding-retry.test.ts index c34f4f1cb..37b1d64d7 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 000000000..1a8706577 --- /dev/null +++ b/__tests__/sync-import-member-retry.test.ts @@ -0,0 +1,482 @@ +/** + * 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: `: -> ::`. */ +const LINKS = [ + 'COLUMNS.map: calls component src/components/Panel.vue -> constant src/table.ts::COLUMNS', + 'NODE_PATH_EXCLUDELIST.includes: calls function src/diff-utils.ts -> constant src/constants.ts::NODE_PATH_EXCLUDELIST', + 'REGISTERED_FLAGS.get: calls function src/flags.ts -> constant src/flags/registry.ts::REGISTERED_FLAGS', + 'api.get: calls function src/client.ts -> function src/api.ts::get', + 'store.get: calls function src/save.ts -> constant src/store.ts::store', + 'store.set: calls function src/save.ts -> constant src/store.ts::store', + 'useStore.setState: calls function src/entry.ts -> constant src/entry-store.ts::useEntryStore', +]; + +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.includes: calls function src/use-flags.ts -> constant src/shared/flags.ts::FLAGS']); + 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.includes: calls function src/use-flags.ts -> constant src/shared/flags.ts::FLAGS']); + 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.includes: calls function src/use-flags.ts -> constant src/shared/flags.ts::FLAGS']); + 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 a full index links the call by + // the method's name, to the one class declaring it. + 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([ + 'store.fetchUserProfile: calls function src/a.ts -> method src/impl/profile.ts::fetchUserProfile', + ]); + 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 >= 16; + INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (15, 0, 'legacy fixture');`); + expect(indexes()).toHaveLength(0); + runMigrations(db!.getDb(), 15); + expect(getCurrentVersion(db!.getDb())).toBe(CURRENT_SCHEMA_VERSION); + expect(indexes()).toHaveLength(1); + db!.getDb().exec('DELETE FROM schema_versions WHERE version >= 16'); + runMigrations(db!.getDb(), 15); + 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/db/migrations.ts b/src/db/migrations.ts index 46ab416b9..8a83ed831 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 = 15; +export const CURRENT_SCHEMA_VERSION = 16; /** * Migration definition @@ -299,6 +299,21 @@ const migrations: Migration[] = [ `); }, }, + { + version: 16, + 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. + 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 6421eec36..fd8367f17 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -22,7 +22,7 @@ import { isTestFile, kindBonus, nameMatchBonus, scorePathRelevance } from '../se 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, @@ -45,6 +45,9 @@ function isLowValueFile(filePath: string, generated?: ReadonlySet): bool const SQLITE_PARAM_CHUNK_SIZE = 500; +/** 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? * @@ -2394,11 +2397,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); @@ -2414,9 +2419,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; @@ -4009,7 +4017,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 @@ -4025,25 +4037,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(','); @@ -4057,6 +4083,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, @@ -4190,8 +4228,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 }> = []; @@ -4202,9 +4242,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 9ec033f34..dcc031a22 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 f9acf1f83..3266a8287 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -207,6 +207,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); -- Sync's third-file wiring lookup must not scan every synthesized edge. -- CASE short-circuits malformed metadata; keep these expressions identical diff --git a/src/index.ts b/src/index.ts index 1f50d46ce..7a5a2d425 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1042,9 +1042,12 @@ export class CodeGraph { // `lazy-import:./pages/Team`), as does a reference through an // import binding the module declares under another name // (`module:tag` for `import tagsController from - // './tag/tag.controller'`): each resolves through the module, so - // an edit can satisfy it as well as an added file. On a sync - // where no failed ref matches, this is one indexed lookup. + // './tag/tag.controller'`) and a direct member reference through + // any binding (`module:constants` for `LIST.includes(x)` after + // `import { LIST } from './constants'`): each resolves through the + // module, so an edit can satisfy it as well as an added file. 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/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 9423eaebf..24a273290 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -3108,28 +3108,48 @@ export function isBoundToOutOfRepoImport( } /** - * The tail a reference that failed to resolve is parked under when its name - * is an import binding the module it imports does not declare 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 }`). Resolution reaches it 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. Its own name, the default tail, is one the - * module never declares, so the file that appears or gains the export never - * found it. A sync still looks it up by that name as well: a binding its - * module never resolves is linked by the name alone. + * 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 a whole-name reference to the binding — the import itself, a call, - * `new`, a JSX tag, a value — and only for a module of the project: a member - * read (`NS.member`) waits for the member's name, and a package's binding - * keeps its tail. Undefined for every other reference. + * 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; if (isExternalImport(binding.source, ref.language, context) && // A monorepo app's own tsconfig alias (`#/views/…`): isExternalImport reads the root's only. !context.getNearestAliases?.(ref.filePath)?.patterns.some((p) => p.prefix !== '' && binding.source.startsWith(p.prefix))) { @@ -3237,3 +3257,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$]*$/;