From 2344ce0722216c1aac913a5c5de2b2b77cc12115 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 16 Oct 2026 17:00:12 +0000 Subject: [PATCH 1/5] test: stop test runs leaving temp dirs behind (#2622) Every full test run left about 40 directories in the temp dir, even when it passed. On the Windows dev machine about 16,000 had piled up by mid-October. A full run with TEMP pointed at an empty folder showed where each one came from: - function-ref (20) and ts-field-classification (3): each case's `finally` cleared `tmpDir` right after `cg.destroy()`, so the `afterEach` that removes the dir found nothing to remove. - npm-sdk (3) and liveness-watchdog (3) never removed the dirs they made. - extraction's #1502 suite (3) made a dir in `beforeEach` and had no `afterEach`; pr19-improvements' schema-version case (1) made one inline. - setup-home-sandbox (6 on Windows): Vitest runs no hooks for a file whose every test is skipped, so each platform-gated file left its home sandbox. Each sandbox now records itself in a per-run registry, and a global teardown removes whatever is still listed and names any dir still in use. With the fix, the same full run leaves nothing behind but Node's own reused compile cache. test-home-sandbox gains two tests: a file's home is listed in the run's registry, and the teardown removes every listed home, then the registry. Co-Authored-By: Claude Opus 5.5 --- __tests__/extraction.test.ts | 4 +++ __tests__/function-ref.test.ts | 20 ----------- __tests__/global-setup-home-sandbox.ts | 42 +++++++++++++++++++++++ __tests__/liveness-watchdog.test.ts | 21 +++++++++--- __tests__/npm-sdk.test.ts | 13 +++++-- __tests__/pr19-improvements.test.ts | 5 +-- __tests__/setup-home-sandbox.ts | 12 +++++-- __tests__/test-home-sandbox.test.ts | 28 ++++++++++++++- __tests__/ts-field-classification.test.ts | 3 -- vitest.config.mts | 5 +-- 10 files changed, 117 insertions(+), 36 deletions(-) create mode 100644 __tests__/global-setup-home-sandbox.ts diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 8a1d7921d2..1ceb2271e1 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -13721,6 +13721,10 @@ describe('Unsupported-language projects report what they skipped (#1502)', () => tempDir = createTempDir(); }); + afterEach(() => { + cleanupTempDir(tempDir); + }); + it('counts files it could not index, by extension, on the git path', async () => { const runGit = (...args: string[]) => execFileSync('git', args, { cwd: tempDir, stdio: 'pipe' }); diff --git a/__tests__/function-ref.test.ts b/__tests__/function-ref.test.ts index 664b407c7c..1572c1eb9f 100644 --- a/__tests__/function-ref.test.ts +++ b/__tests__/function-ref.test.ts @@ -108,7 +108,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, callEdges)).toEqual(['direct_caller']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -152,7 +151,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, fnRefEdgesInto(cg, 'handleClick'))).toEqual(['wire']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -182,7 +180,6 @@ describe('Function-as-value capture (#756)', () => { expect(target?.filePath.endsWith('handlers.ts')).toBe(true); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -207,7 +204,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, edges)).not.toContain('wire'); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -235,7 +231,6 @@ describe('Function-as-value capture (#756)', () => { expect(target?.filePath.endsWith('real.c')).toBe(true); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -260,7 +255,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRef).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -284,7 +278,6 @@ describe('Function-as-value capture (#756)', () => { expect(selfLoops).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -349,7 +342,6 @@ describe('Function-as-value capture (#756)', () => { } } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -406,7 +398,6 @@ describe('Function-as-value capture (#756)', () => { ]); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -456,7 +447,6 @@ describe('Function-as-value capture (#756)', () => { } } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -496,7 +486,6 @@ describe('Function-as-value capture (#756)', () => { ).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -543,7 +532,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, fnRefEdgesInto(cg, 'run0'))).toEqual(['thisRef']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -588,7 +576,6 @@ describe('Function-as-value capture (#756)', () => { expect(cg.getIncomingEdges(decoy.id).filter((e) => e.metadata?.fnRef === true)).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -617,7 +604,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, edges)).toEqual(['wire']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -661,7 +647,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRefEdgesInto(cg, 'dupCmd')).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -702,7 +687,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, fnRefEdgesInto(cg, 'load'))).toEqual(['sorter']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -743,7 +727,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRefEdgesInto(cg, 'title')).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -823,7 +806,6 @@ describe('Function-as-value capture (#756)', () => { expect(callers.some((c) => c.node.name === 'get_serializer_class')).toBe(true); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -856,7 +838,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRefEdgesInto(cg, 'unknown_thing')).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -1598,7 +1579,6 @@ func Unknown(obj interface{}) { Submit(obj.Fetch) } expect(sourceNames(cg, fnRefEdgesInto(cg, 'cb_a'))).toEqual(['wire']); } finally { cg.destroy(); - tmpDir = undefined; } }); }); diff --git a/__tests__/global-setup-home-sandbox.ts b/__tests__/global-setup-home-sandbox.ts new file mode 100644 index 0000000000..cd259aa818 --- /dev/null +++ b/__tests__/global-setup-home-sandbox.ts @@ -0,0 +1,42 @@ +/** + * Remove the home sandboxes `setup-home-sandbox.ts` could not. + * + * Each test file removes its own sandbox in an `afterAll`, but Vitest runs no + * hooks at all for a file whose every test is skipped — a platform-gated suite + * skips that way, and each one left its home dir behind on every run — and a + * straggling child can still hold a file there when `afterAll` runs. So every + * sandbox records its path in this run's registry dir, and the end of the run + * removes whatever is still listed. + */ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { GlobalSetupContext } from 'vitest/node'; + +declare module 'vitest' { + export interface ProvidedContext { + /** The dir each test file's home sandbox records its path in. */ + homeSandboxRegistry: string; + } +} + +export default function setup({ provide }: GlobalSetupContext): () => void { + const registry = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-homes-')); + provide('homeSandboxRegistry', registry); + return () => { + const held: string[] = []; + for (const entry of fs.readdirSync(registry)) { + const home = fs.readFileSync(path.join(registry, entry), 'utf8'); + try { + fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 }); + } catch { + held.push(home); + } + } + fs.rmSync(registry, { recursive: true, force: true }); + if (held.length > 0) { + process.stderr.write(`[test teardown] ${held.length} home sandbox dir(s) still in use, left behind:\n` + + held.map((home) => ` ${home}\n`).join('')); + } + }; +} diff --git a/__tests__/liveness-watchdog.test.ts b/__tests__/liveness-watchdog.test.ts index 9bc6983ecd..026d8781ce 100644 --- a/__tests__/liveness-watchdog.test.ts +++ b/__tests__/liveness-watchdog.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterEach } from 'vitest'; import { spawn } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; @@ -140,6 +140,19 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { // when the watched DB files keep advancing (a slow synchronous SQLite // statement on degraded storage). --- + // The dirs holding each case's watched file, removed after it. + const progressDirs: string[] = []; + afterEach(() => { + for (const dir of progressDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + /** A fresh path for a case's watchdog to watch, in its own temp dir. */ + function progressFile(): string { + const dir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')); + progressDirs.push(dir); + return path.join(dir, 'db-wal'); + } + /** Grow `file` every 150ms for `forMs`; resolves when done. */ function growFile(file: string, forMs: number): Promise { return new Promise((resolve) => { @@ -149,7 +162,7 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { } it('does NOT kill a blocked loop while the watched files advance (slow store, not a wedge)', async () => { - const tmp = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')), 'db-wal'); + const tmp = progressFile(); fs.writeFileSync(tmp, 'seed'); // Base timeout 500ms; the loop blocks for 2.5s (5 timeouts, under the 10× // cap) while the test process grows the watched file. Old behavior: killed @@ -168,7 +181,7 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { }, 15000); it('still kills a blocked loop when the watched files do NOT advance (a true wedge)', async () => { - const tmp = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')), 'db-wal'); + const tmp = progressFile(); fs.writeFileSync(tmp, 'seed'); // Same blocked loop, nobody grows the file: the base timeout kills it long // before its own exit(5) at 2.5s. @@ -182,7 +195,7 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { }, 15000); it('kills at the hard cap even with ongoing file activity (bounded deferral)', async () => { - const tmp = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')), 'db-wal'); + const tmp = progressFile(); fs.writeFileSync(tmp, 'seed'); // Base timeout 300ms ⇒ cap 3s. The loop blocks for 8s with continuous file // growth: deferral carries it past 300ms but the cap kills it around ~3s, diff --git a/__tests__/npm-sdk.test.ts b/__tests__/npm-sdk.test.ts index c27bfc5fef..0a3385633b 100644 --- a/__tests__/npm-sdk.test.ts +++ b/__tests__/npm-sdk.test.ts @@ -13,7 +13,7 @@ * hermetically with no real bundle, network, or registry. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, afterEach } from 'vitest'; import { spawnSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; @@ -23,8 +23,13 @@ const SDK_SRC = path.join(__dirname, '..', 'scripts', 'npm-sdk.js'); const target = `${process.platform}-${process.arch}`; const VERSION = '9.9.9-test'; +// Every temp dir a case makes, removed after it. +const tmpDirs: string[] = []; + function mkTmp(label: string): string { - return fs.mkdtempSync(path.join(os.tmpdir(), `cg-sdk-${label}-`)); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-sdk-${label}-`)); + tmpDirs.push(dir); + return dir; } // A temp node_modules with the main package (npm-sdk.js + package.json). The @@ -75,6 +80,10 @@ function requireSdk(mainPkg: string, env: Record = {}) { } describe('npm-sdk programmatic entry', () => { + afterEach(() => { + for (const dir of tmpDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + it('re-exports the installed platform bundle library', () => { const { root, mainPkg } = makeConsumer(); installPlatformPackage(root, 'platform-lib'); diff --git a/__tests__/pr19-improvements.test.ts b/__tests__/pr19-improvements.test.ts index 06c365ec7f..0e4dd55423 100644 --- a/__tests__/pr19-improvements.test.ts +++ b/__tests__/pr19-improvements.test.ts @@ -305,12 +305,13 @@ describe('Schema v2 Migration', () => { // just makes every schema change edit this test (v9/#1500 was the latest). // A fresh database records the current version, so nothing is pending; // ask a version-0 database instead to see the full migration list. - const dbPath = path.join(createTempDir(), 'schema-version.db'); - const conn = DatabaseConnection.initialize(dbPath); + const dir = createTempDir(); + const conn = DatabaseConnection.initialize(path.join(dir, 'schema-version.db')); const raw = conn.getDb(); raw.prepare('DELETE FROM schema_versions').run(); const highest = Math.max(...getPendingMigrations(raw).map((m) => m.version)); conn.close(); + cleanupTempDir(dir); expect(CURRENT_SCHEMA_VERSION).toBe(highest); }); diff --git a/__tests__/setup-home-sandbox.ts b/__tests__/setup-home-sandbox.ts index 5acb473944..ba5c4fe3a8 100644 --- a/__tests__/setup-home-sandbox.ts +++ b/__tests__/setup-home-sandbox.ts @@ -21,7 +21,7 @@ * Tests that redirect the home dir themselves (spying on `os.homedir`, or * setting HOME and restoring it) keep working: they restore to this sandbox. */ -import { afterAll } from 'vitest'; +import { afterAll, inject } from 'vitest'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -30,6 +30,13 @@ import * as path from 'path'; // `/private/var`) sees the same home everything else does. const home = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-home-'))); +// Recorded for the end of the run, which removes the home if the afterAll +// below never runs (a file whose every test is skipped runs no hooks) or +// cannot (global-setup-home-sandbox.ts). +const registry = inject('homeSandboxRegistry'); +const registryEntry = registry ? path.join(registry, path.basename(home)) : undefined; +if (registryEntry) fs.writeFileSync(registryEntry, home); + process.env.HOME = home; process.env.USERPROFILE = home; if (process.platform === 'win32') { @@ -60,7 +67,8 @@ process.env.GIT_CONFIG_GLOBAL = gitConfig; afterAll(() => { try { fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 }); + if (registryEntry) fs.rmSync(registryEntry, { force: true }); } catch { - /* a straggling child still holding a file there; it's in the temp dir */ + /* a straggling child still holding a file there; the end of the run retries */ } }); diff --git a/__tests__/test-home-sandbox.test.ts b/__tests__/test-home-sandbox.test.ts index f5ebb7a718..0775578de7 100644 --- a/__tests__/test-home-sandbox.test.ts +++ b/__tests__/test-home-sandbox.test.ts @@ -3,13 +3,15 @@ * nothing a test runs — in-process or spawned — resolves the developer's real * home, Claude profile, daemon registry or git identity. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, inject } from 'vitest'; +import type { GlobalSetupContext } from 'vitest/node'; import { execFileSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { claudeTarget } from '../src/installer/targets/claude'; import { getRegistryDir } from '../src/mcp/daemon-registry'; +import setupHomeSandboxRun from './global-setup-home-sandbox'; const tmpRoot = fs.realpathSync(os.tmpdir()); const under = (p: string, dir: string): boolean => path.resolve(p).startsWith(dir + path.sep); @@ -62,4 +64,28 @@ describe('test home sandbox', () => { expect(under(process.env.APPDATA ?? '', home)).toBe(true); expect(under(process.env.LOCALAPPDATA ?? '', home)).toBe(true); }); + + // A file whose every test is skipped runs no hooks, so its own afterAll never + // removes its home: the end of the run removes every home still listed. + it("records this file's home in the run's registry", () => { + const home = sandboxHome(); + const entry = path.join(inject('homeSandboxRegistry'), path.basename(home)); + expect(fs.readFileSync(entry, 'utf8')).toBe(home); + }); + + it('the end of the run removes every home still listed, then the registry', () => { + let registry = ''; + const teardown = setupHomeSandboxRun({ + provide(_key: string, value: string) { registry = value; }, + } as unknown as GlobalSetupContext); + const homes = ['skipped', 'held'].map((label) => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), `codegraph-test-home-${label}-`)); + fs.writeFileSync(path.join(home, '.gitconfig'), ''); + fs.writeFileSync(path.join(registry, path.basename(home)), home); + return home; + }); + teardown(); + expect(homes.filter((home) => fs.existsSync(home))).toEqual([]); + expect(fs.existsSync(registry)).toBe(false); + }); }); diff --git a/__tests__/ts-field-classification.test.ts b/__tests__/ts-field-classification.test.ts index 82069a8d2b..39d6217f8f 100644 --- a/__tests__/ts-field-classification.test.ts +++ b/__tests__/ts-field-classification.test.ts @@ -99,7 +99,6 @@ describe('TS/JS class field classification (#808)', () => { expect(fontsProp.signature).toBe('Fonts fonts'); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -126,7 +125,6 @@ describe('TS/JS class field classification (#808)', () => { expect(cg.getNodesByName('onClick')[0]?.kind).toBe('method'); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -153,7 +151,6 @@ describe('TS/JS class field classification (#808)', () => { expect(fnRefs.length).toBeGreaterThan(0); } finally { cg.destroy(); - tmpDir = undefined; } }); }); diff --git a/vitest.config.mts b/vitest.config.mts index 96fb2573a6..89147faecc 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -12,8 +12,9 @@ export default defineConfig({ globals: true, environment: 'node', include: ['__tests__/**/*.test.ts'], - // Suites that spawn the built CLI need a current dist/ (#1879). - globalSetup: ['./__tests__/global-setup-dist.ts'], + // Suites that spawn the built CLI need a current dist/ (#1879), and the end + // of the run removes any home sandbox its own test file could not. + globalSetup: ['./__tests__/global-setup-dist.ts', './__tests__/global-setup-home-sandbox.ts'], // A throwaway home dir (and git global config) per test file, so nothing // the suite runs can write to the developer's real one (#2275). setupFiles: ['./__tests__/setup-home-sandbox.ts'], From a25706950fa4e98b2e0d2d49c1e849474893defb Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 16 Oct 2026 18:09:04 +0000 Subject: [PATCH 2/5] test: wait for what holds an MCP test's dir before removing it (#2624) Two MCP test files sometimes failed under full-suite load on Windows with EBUSY in their afterEach, and then left their temp dir behind: - mcp-status-freshness: every case leaves an index-freshness measurement worker still ending, and the deadline case's worker is terminated in the middle of its measurement. Until it exits, the worker can hold the index or the project dir, and the removal raced that exit (EBUSY on codegraph.db or on the dir, ENOTEMPTY on .codegraph). The afterEach now awaits endFreshnessMeasurements(), the server's own shutdown wait, before it closes the index and removes the dir. - mcp-initialize: after the killed server's 'close', the dir itself can stay busy a moment longer (EBUSY rmdir). It freed within about 80 ms, with no process left under the server. A bounded `maxRetries` doesn't help: on Node 22, fs.rmSync never retries EBUSY on a directory and throws at once. So removeWhenReleased() retries the removal itself, for up to 5 s, and a Windows-only test holds a dir as a child's cwd to prove the helper waits. Under CPU load (14 of 16 cores busy, two runs at once), the status teardown failed 20 times in 240 without the wait and 0 times in 240 with it. Alternating real runs of both files failed in 5 of 18 on main and 0 of 10 with the fix. Co-Authored-By: Claude Opus 5.5 --- __tests__/mcp-initialize.test.ts | 38 +++++++++++++++++++++++++- __tests__/mcp-status-freshness.test.ts | 8 ++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/__tests__/mcp-initialize.test.ts b/__tests__/mcp-initialize.test.ts index 7dc90ae2d7..f155ba9739 100644 --- a/__tests__/mcp-initialize.test.ts +++ b/__tests__/mcp-initialize.test.ts @@ -101,6 +101,26 @@ function waitFor( }); } +/** + * Remove `dir` once nothing holds it. Even after the killed server's 'close', + * Windows can keep the dir itself busy a moment longer under load, and + * `fs.rmSync`'s `maxRetries` never retries EBUSY on a directory: it throws at + * once. A dir still held at the deadline fails the test. + */ +async function removeWhenReleased(dir: string, timeoutMs = 5000): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code ?? ''; + if (!['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code) || Date.now() > deadline) throw error; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } +} + describe('MCP initialize handshake (issue #172)', () => { let tempDir: string; let child: ChildProcessWithoutNullStreams | null = null; @@ -118,7 +138,7 @@ describe('MCP initialize handshake (issue #172)', () => { await closed; } child = null; - fs.rmSync(tempDir, { recursive: true, force: true }); + await removeWhenReleased(tempDir); }); it('responds to initialize quickly when no .codegraph exists in cwd', async () => { @@ -187,3 +207,19 @@ describe('MCP initialize handshake (issue #172)', () => { expect(prompts.result.prompts).toEqual([]); }, 15000); }); + +describe('MCP initialize teardown', () => { + // On Windows a dir another process holds (here as its cwd) refuses removal + // with EBUSY; the teardown waits until it is let go. + it.runIf(process.platform === 'win32')('removes a dir once the process holding it has exited', async () => { + const held = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-init-held-')); + const holder = spawn(process.execPath, ['-e', "process.stdout.write('ready'); setTimeout(() => {}, 500)"], { + cwd: held, + stdio: ['ignore', 'pipe', 'ignore'], + }); + await once(holder.stdout, 'data'); // running, so its cwd is open + expect(() => fs.rmdirSync(held)).toThrow(/EBUSY/); + await removeWhenReleased(held); + expect(fs.existsSync(held)).toBe(false); + }); +}); diff --git a/__tests__/mcp-status-freshness.test.ts b/__tests__/mcp-status-freshness.test.ts index c78844db53..7ce72868a1 100644 --- a/__tests__/mcp-status-freshness.test.ts +++ b/__tests__/mcp-status-freshness.test.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import { execFileSync } from 'child_process'; import { Worker } from 'worker_threads'; import CodeGraph from '../src/index'; -import { measurePendingChanges } from '../src/mcp/index-freshness'; +import { endFreshnessMeasurements, measurePendingChanges } from '../src/mcp/index-freshness'; import { ToolHandler } from '../src/mcp/tools'; describe('MCP status freshness (#1959)', () => { @@ -28,7 +28,11 @@ describe('MCP status freshness (#1959)', () => { handler = new ToolHandler(cg); }); - afterEach(() => { + afterEach(async () => { + // Every case leaves a measurement worker still ending, which can still + // hold the index or the project dir: on Windows under load the removal + // then failed with EBUSY. Wait until they are gone. + await endFreshnessMeasurements(); try { cg.close(); } catch { /* ignore */ } fs.rmSync(root, { recursive: true, force: true }); }); From 6e884d7bce501b655013af21e2ae0f3211e9e52b Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 16 Oct 2026 20:39:28 +0000 Subject: [PATCH 3/5] =?UTF-8?q?fix(cpp):=20a=20call=20on=20a=20member=20re?= =?UTF-8?q?ached=20through=20other=20members,=20like=20`options.cf=5Fpaths?= =?UTF-8?q?.emplace=5Fback(=E2=80=A6)`=20on=20a=20`std::vector`,=20no=20lo?= =?UTF-8?q?nger=20links=20to=20a=20project=20method=20of=20that=20name=20(?= =?UTF-8?q?#2626)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The extractor records a call on a member chain (`a.b.m()`, `a->b->m()`, `this->p_.m()`) by its bare method name, so it went to whichever project method shared the name. A census of the cgstd kit found most such calls linked by name alone, and most of those links wrong. Above all, they were on members of standard-library types, whose methods (`size`, `reset`, `push_back`, `get`, `empty`) a large codebase has namesakes of. matchCppOutsideMemberChainCall reads the chain where the call is written (cppMemberChainReceiver) and types it with the existing cppMemberObject: - a data member is typed by its declared type in its class (cppDataMemberType); - `->` goes through a pointer, smart pointer, optional or iterator. It gives no edge when the object the method is called on is a library type (isCppLibraryType) or a type under a namespace the project declares nothing in (isCppOutsideQualifiedType). These are the call-chain resolver's rules. Unlike a call chain, the operator is read as written, so `.` on a `std::vector` or a `std::unique_ptr` is that type's own member, whatever `T` is. Anything else, a member of a project class included, goes to the name strategies as before. Kit A/B against main a2570695 (only removals; nothing added or retargeted): | repo | calls removed | |---|---| | rocksdb | 2,001 | | protobuf | 234 | | leveldb | 15 | | fmt | 12 | | redis | 0 (identical) | Every removed link was a bare-name guess. Of the 2,235 removed sites in rocksdb and protobuf, a judge reading the members' declarations across each repo rated: - 2,044 as wrong links (a library type's namesake, or an unrelated class); - 165 unknown; - 10 not counted as chains by its pattern (after a cast, like `(int)data->entries.size()`); - 16 right. These turned out to be name collisions on the judge's side, checked in the source (`std::deque logs_`, `std::shared_ptr decompressor`, `std::vector ids`). 24 random removals were also read by hand, and all were correct. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../cpp-member-chain-outside-calls.test.ts | 150 ++++++++++++++++++ src/resolution/name-matcher.ts | 60 +++++++ 3 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 __tests__/cpp-member-chain-outside-calls.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a6d968aab..0e6fa7945c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,7 +92,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a base class like `Message` in `class DynamicMessage final : public Message` now links to the class C++ itself would pick: the one declared in the namespaces around the class, or brought in by the file's `using` declarations and namespace aliases, and the class a `typedef` or `using` alias names. Before, a base class was matched by its name alone, so it could land on a class of the same name in another namespace or another language: in protocolbuffers/protobuf, `DynamicMessage` derived from the JSON parser's internal `Message` class, and in google/leveldb and RocksDB, iterators derived from an unrelated nested `Iterator` class such as the skip list's. The wrong base showed up in `codegraph_explore`'s type hierarchy and in impact, and tied the base's methods to the wrong overrides. A template parameter used as a base no longer links to a class that shares its name, and a base written from the global scope, like `public ::testing::Test`, now links to its class. Re-index C++ projects after upgrading. - In C and C++, a header or source file named like a test, such as protocolbuffers/protobuf's `conformance_test.h` and `test_runner.h`, now counts as part of the code that `#include`s it, directly or through other headers. Before, calls from that code into it were dropped, so protobuf's conformance suites' `suite_.ReportFailure(…)` and `RunValidInputTest(…)` calls, and its unit tests' calls to the shared `TestUtil` helpers, linked to nothing. A test file that only tests include is still kept out of reach of the rest of your code. Re-index C and C++ projects after upgrading. - In C++, a class, struct or method now keeps the namespaces and the class it is declared in when the parser misreads something earlier in the file, such as an unknown macro in front of a member or in a class header. Before, everything after the misread code could lose them, or land inside a class it isn't in: in protocolbuffers/protobuf, `FieldDescriptor` was indexed without `google::protobuf::`, and in RocksDB, the `Opts` struct declared inside a cache table class was indexed outside it, so finding a class's base classes and the methods called on its objects fell back to guessing by name. Methods that had come loose from their class are its members again, and classes the misread code had hidden, like leveldb's POSIX file and environment classes, are now indexed. Re-index C++ projects after upgrading. -- In C++, a call on a variable, parameter or member declared as a standard-library or other outside type with a lowercase name, like `std::string`, `std::vector`, `std::set` or `absl::flat_hash_set`, no longer links to a project method that only shares the method's name, even one of a project class in another namespace with the type's name, like your own `util::flat_hash_set`. A variable declared outside the calling function, like another function's or a file's `std::string text`, no longer links its calls to such a class either. In google/leveldb, `saved_key_.clear()` on a `std::string saved_key_;` reached `Slice::clear`, and in protocolbuffers/protobuf, `proto.append(…)` on a `std::string` reached an unrelated `LeftoverBuffer::append`. The same goes for `->` through a raw pointer like `std::string* out`, while `->` through a smart pointer, iterator or optional still links to the type it holds. Comments also no longer pass for a variable's declaration, so more calls now link to the method of the type the variable is really declared with. Re-index C++ projects after upgrading. +- In C++, a call on a variable, parameter or member declared as a standard-library or other outside type with a lowercase name, like `std::string`, `std::vector`, `std::set` or `absl::flat_hash_set`, no longer links to a project method that only shares the method's name, even one of a project class in another namespace with the type's name, like your own `util::flat_hash_set`. A variable declared outside the calling function, like another function's or a file's `std::string text`, no longer links its calls to such a class either. In google/leveldb, `saved_key_.clear()` on a `std::string saved_key_;` reached `Slice::clear`, and in protocolbuffers/protobuf, `proto.append(…)` on a `std::string` reached an unrelated `LeftoverBuffer::append`. The same goes for `->` through a raw pointer like `std::string* out`, while `->` through a smart pointer, iterator or optional still links to the type it holds. Comments also no longer pass for a variable's declaration, so more calls now link to the method of the type the variable is really declared with. A call on a member reached through other members, like facebook/rocksdb's `options.cf_paths.emplace_back(…)` on a `std::vector`, `props->rep.db_id.size()` on a `std::string` or `bbto.filter_policy.reset(…)` on a `std::shared_ptr`, no longer links to a project method of that name either: those had reached `autovector::emplace_back`, `BlobContents::size` and `FSWritableFilePtr::reset`. The same goes for a member of an outside type like `absl::Status`. Re-index C++ projects after upgrading. - In C++, a class written with attribute macros the parser can't read now keeps its members. That covers several macros between `class` and the class name, like protocolbuffers/protobuf's generated `class PROTOBUF_EXPORT PROTOBUF_FUTURE_ADD_EARLY_WARN_UNUSED Any final`, a macro with arguments there, like RocksDB's `struct ALIGN_AS(64U) HandleImpl`, or one before a template specialization, and macros on a member: in front of it (`PROTOBUF_FUTURE_ADD_EARLY_NODISCARD`), between a pointer and its name (`PROTOBUF_NONNULL`), after its parameters or its name (`ABSL_ATTRIBUTE_LIFETIME_BOUND`, google/leveldb's `LOCKS_EXCLUDED(mu_)` and `GUARDED_BY(mu_)`), or alone on a line above a comment ({fmt}'s `FMT_BEGIN_EXPORT`). Before, protobuf's generated message classes were missing and their members, like `GetDescriptor`, `GetReflection` and `unknown_fields`, were indexed as functions outside any class, and leveldb's annotated methods were indexed under the annotation's name, so calls to them linked to nothing. Re-index C++ projects after upgrading. - In C++, a call on a range-based `for` loop's variable, like `suite->SetVerbose(verbose)` inside `for (ConformanceTestSuite *suite : suites)`, now links to the method of the type the loop declares. Before, the loop's declaration was never read, so the call was matched by the variable's name alone: in RocksDB, `log->file()` on a `log::Writer` went to `BlobLogWriter::file`, and a loop variable named like a member of its class was taken for that member. The variable counts only inside its loop. Re-index C++ projects after upgrading. - In C++, a call made with `->` on a variable, parameter or member declared as a `std::unique_ptr`, `std::shared_ptr` or `std::optional`, like `iter->Valid()` on a `std::unique_ptr iter`, now links to that method of the type it holds, or of a class that type inherits from, with the type looked up the way C++ does it, so a class of the same name nested elsewhere is not mistaken for it. Before, the call went to whichever project method shared its name: in facebook/rocksdb, `iter->Valid()` reached `ArenaWrappedDBIter::Valid` and `fs->GetFileSize(…)` on a `std::shared_ptr` reached a test's stand-in file system, and calls like `it->Next()` were often not linked at all. When what it holds is a template parameter or a standard-library type, like `std::unique_ptr>`, or a class that has no method of that name and neither do the classes it derives from, the call no longer links to a project method; a class derived from something that can't be followed, like a `decltype(…)`, may still have it, so that call is matched as before. The same goes for `->` on a container's iterator, like `it->data()` on a `std::deque::const_iterator it` or `loc->path_size()` on protocolbuffers/protobuf's `RepeatedPtrField::iterator loc`: the call links to that method of the container's element type, and to no project method when the element is a standard-library type like `std::string`. That includes an iterator an `auto` variable gets from a standard-library container's `begin()`, `find()` and the like, or from a standard search like `std::lower_bound`, as in facebook/rocksdb's `auto it = logs_.begin(); it->IsSyncing()`. A call on a dereferenced iterator or pointer, like `(*it)->Visit()` over a `std::vector` or `(*it).Weight()` over a `std::vector`, and on a variable a dereference sets, like `auto first = *nodes.begin()`, links the same way, where before such a call went to whichever class had a method of that name. So does a call on the key or value a map's iterator points at, like `it->second.Flush()`, `it->second->Flush()` or a range-based `for` loop's `kv.second.Flush()`, and on the `first` or `second` of a `std::pair` the code declares. So does a call on an element read by subscript, like `statuses[i].IsNotFound()` over a `std::vector`, `buffers[key].Flush()` over a map, `iters[i]->Valid()` over owning pointers, or an element of an array or of a class's `operator[]`. So does a call on what a container's `front()`, `back()` or `at()` hands out, like facebook/rocksdb's `edits.back()->MarkAtomicGroup(…)` or `wals.at(log_number).HasSyncedSize()` over a map, on a `std::stack`'s `top()` or a `std::queue`'s `front()`, and on what a smart pointer's `get()` or an optional's or `absl::StatusOr`'s `value()` returns, like `mock_clock.get()->GetCurrentTime(…)`, where before such calls linked to nothing; an overloaded function given such a value, like `test::PerThreadDBPath(env_.get(), …)`, now takes the overload for its type. Re-index C++ projects after upgrading. diff --git a/__tests__/cpp-member-chain-outside-calls.test.ts b/__tests__/cpp-member-chain-outside-calls.test.ts new file mode 100644 index 0000000000..88d027ecff --- /dev/null +++ b/__tests__/cpp-member-chain-outside-calls.test.ts @@ -0,0 +1,150 @@ +/** + * A C++ call on a data member reached through a member chain, like rocksdb's + * `options.cf_paths.emplace_back(…)` on a `std::vector` or + * `props->rep.db_id.size()` on a `std::string`, calls the library type's own + * method, none of the project's. The extractor records such a call by its + * bare name, so it went to any project method of that name: rocksdb's + * `autovector::emplace_back`, `BlobContents::size`. + */ +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 of a type from outside the project', () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-')); + const write = (file: string, text: string): void => { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + }; + // The project's namesakes of library methods, which a call matched by its bare name lands on. + write('src/types.h', lines( + 'namespace rocksdb {', + 'template ', + 'class autovector {', + ' public:', + ' void emplace_back(T v) {}', + ' void clear() {}', + ' int size() const { return 0; }', + '};', + 'class BlobContents {', + ' public:', + ' int size() const { return 0; }', + '};', + '} // namespace rocksdb', + 'namespace proto {', + 'class Option {', + ' public:', + ' bool has_value() const { return false; }', + '};', + '} // namespace proto', + 'namespace util {', + 'class Status {', + ' public:', + ' bool ok() const { return true; }', + '};', + '} // namespace util', + 'class FilePtr {', + ' public:', + ' void reset() {}', + '};', + 'class Engine {', + ' public:', + ' void Run() {}', + '};', + )); + write('src/use.cc', lines( + '#include ', + '#include ', + '#include ', + '#include ', + '#include "absl/status/status.h"', + '#include "src/types.h"', + 'struct Options {', + ' std::vector cf_paths;', + ' std::string name;', + '};', + 'struct Rep {', + ' std::string db_id;', + '};', + 'struct Props {', + ' Rep rep;', + '};', + 'struct State {', + ' std::optional child_pid;', + ' std::shared_ptr engine;', + ' Engine plain;', + ' absl::Status status;', + '};', + 'void Library(Options& options) { options.cf_paths.emplace_back(1); options.name.size(); }', + 'void TwoHops(Props* props) { props->rep.db_id.size(); }', + 'void Optional(State* state) { state->child_pid.has_value(); }', + 'void HolderOwn(State& state) { state.engine.reset(); }', + 'void OutsideQualified(State& state) { state.status.ok(); }', + 'void HolderArrow(State& state) { state.engine->Run(); }', + 'void ProjectMember(State& state) { state.plain.Run(); }', + 'class Cache {', + ' public:', + ' void Touch() { this->keys_.clear(); this->engine_->Run(); }', + ' private:', + ' std::vector keys_;', + ' Engine* engine_;', + '};', + 'template ', + 'struct Box {', + ' std::vector items;', + ' std::unique_ptr owner;', + '};', + 'template ', + 'void Drain(Box& box) { box.items.clear(); box.owner.reset(); }', + )); + 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 or method `name`, as `qualifiedName`. */ + const calls = (name: string): string[] => { + const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')].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 library container's or string's method reached through a member", () => { + expect(calls('Library')).toEqual([]); + expect(calls('TwoHops')).toEqual([]); + }); + + it("an optional's method, and a smart pointer's own through `.`", () => { + expect(calls('Optional')).toEqual([]); + expect(calls('HolderOwn')).toEqual([]); + }); + + it('a type under a namespace the project declares nothing in', () => { + expect(calls('OutsideQualified')).toEqual([]); + }); + + it("`->` on a smart pointer, and a member of a project class's type, still call the project's", () => { + expect(calls('HolderArrow')).toEqual(['Engine::Run']); + expect(calls('ProjectMember')).toEqual(['Engine::Run']); + }); + + it('a chain through `this`', () => { + expect(calls('Touch')).toEqual(['Engine::Run']); + }); + + it("a library template of a template parameter's type is still the library's", () => { + expect(calls('Drain')).toEqual([]); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 71211c2452..6767756868 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -11025,6 +11025,62 @@ function cppUnspelledReceiver(ref: UnresolvedRef, context: ResolutionContext): { return index === null ? null : at(index); } +/** A member chain and the operator after it: `props->rep.db_id` and `.`, `this->mu_` and `->`. */ +const CPP_MEMBER_CHAIN = /^([A-Za-z_]\w*(?:\s*(?:\.|->)\s*[A-Za-z_]\w*)*)\s*(\.|->)\s*(?:template\s+)?/; + +/** + * The member chain a C++ call the extractor recorded by its bare name is made + * on, with the operator after it — `options.cf_paths` and `.` for + * `options.cf_paths.emplace_back(…)`, `props->rep.db_id` and `.` for + * `props->rep.db_id.size()` — read at the call's column, where the chain + * starts. Null for any other call, a receiver of one name among them (the + * extractor spells those). + */ +function cppMemberChainReceiver(ref: UnresolvedRef, context: ResolutionContext): { expression: string; op: string } | null { + const raw = (context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split(/\r?\n/))?.[ref.line - 1]; + const line = raw === undefined ? null : cppCodeOf(cppLiteralsBlanked(raw, ref.line, cppNonCodeLines(ref.filePath, context))); + if (!line) return null; + const at = (column: number): { expression: string; op: string } | null => { + const chain = CPP_MEMBER_CHAIN.exec(line.slice(column)); + if (!chain || !/\.|->/.test(chain[1]!)) return null; + const rest = line.slice(column + chain[0].length); + if (!rest.startsWith(ref.referenceName) || /[\w$]/.test(rest.charAt(ref.referenceName.length))) return null; + return cppArgumentListFollows(rest, ref.referenceName.length) ? { expression: chain[1]!, op: chain[2]! } : null; + }; + const found = at(Math.max(0, ref.column)); + if (found || !/[^\x00-\x7f]/.test(line)) return found; + const index = cppUtf8Column(line, ref.column); + return index === null ? null : at(index); +} + +/** + * A C++ call on a data member reached through a member chain, which the + * extractor records by its bare name, calls no project method when that + * member's type is from outside the project: a library type + * (isCppLibraryType) or one under a namespace the project declares nothing + * in (isCppOutsideQualifiedType). By its bare name the call went to any + * project method of that name: rocksdb's `options.cf_paths.emplace_back(…)` + * on a `std::vector` reached `autovector::emplace_back`, + * `props->rep.db_id.size()` on a `std::string` `BlobContents::size`, and + * protobuf's `state_->child_pid.has_value()` on a `std::optional` + * `Option::has_value`. Unlike a call chain's, a member chain's operator is + * read where it is written, so a library template of a template parameter is + * no matter: `.` calls a member of `std::vector` itself, and `->` on a + * `std::unique_ptr` one of `T`, which is none of the project's to tell. + * - null for such a member: no project method is called. + * - undefined for any other call, or a member whose type can't be read or is + * the project's: the name strategies decide, as before. + */ +function matchCppOutsideMemberChainCall(ref: UnresolvedRef, context: ResolutionContext): null | undefined { + const receiver = cppMemberChainReceiver(ref, context); + if (!receiver) return undefined; + const object = cppMemberObject(receiver.expression, receiver.op, ref, context, 0); + if (!object || isCppPointerType(object.text)) return undefined; + if (isCppLibraryType(object.text, context)) return null; + return cppClassWritten(object.text, object, ref, context) === undefined && + isCppOutsideQualifiedType(object.text, object, context) ? null : undefined; +} + /** * A C++ call the extractor recorded by its bare name on a receiver it drops * (cppUnspelledReceiver) — `(*it)->Visit()` over a `std::vector`, @@ -16970,6 +17026,10 @@ function matchReferenceInner( if (ref.language === 'cpp' && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*$/.test(ref.referenceName)) { const unspelled = nmTimedT('cppUnspelled', ref, () => matchCppUnspelledReceiverCall(ref, context)); if (unspelled !== undefined) return unspelled; + // So is one on a member reached through a chain, `options.cf_paths.emplace_back(…)`, + // when that member's type is from outside the project. + const outside = nmTimedT('cppOutsideMemberChain', ref, () => matchCppOutsideMemberChainCall(ref, context)); + if (outside !== undefined) return outside; } // Try strategies in order of confidence From e8ec7fd847d0b9d44507b5a9128488bb8724762f Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Fri, 16 Oct 2026 23:13:48 +0000 Subject: [PATCH 4/5] fix(cpp): a call on a member reached through other members, like `thread->rand.Next()` on a `Random64`, links to the method of the member's class (#2627) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 2 of member chains; #2626 was step 1. matchCppMemberChainCall (formerly matchCppOutsideMemberChainCall) now also resolves a chain whose member is of a project class, in the same steps matchCppUnspelledReceiverCall takes for `it->second.Flush()`: - the class C++ lookup finds for the member's type, where that type is written; - then its method, or one it inherits (cppMethodOf, with overloads picked by argument types); - or no edge when the class surely lacks it (cppLacksMethod). Before, such a call went by its bare name to whichever class had a method of that name. Inside a method it could also be taken for an implicit `this` call on the method's own class. A member is now read from the class node C++ lookup found (cppDataMemberIn), not from the first class of that qualified name that declares it (cppDataMemberType, removed). rocksdb's db_bench_tool.cc and db_stress_shared_state.h each declare a ROCKSDB_NAMESPACE::ThreadState, whose `rand` is a Random64 in one and a Random in the other. Kit A/B against main 6e884d7b: | repo | added | retargeted | same target, now typed | removed | |---|---|---|---|---| | rocksdb | 571 | 2,008 | 1,738 | 2 | | protobuf | 42 | 95 | 142 | 0 | | leveldb | 42 | 30 | 141 | 0 | | fmt | 0 | 56 | 20 | 0 | | redis | 0 | 0 | 0 | 0 (identical) | The most common rocksdb retargets are DBImpl -> DB (calls on a `DB*`), BlobContents -> Slice, BlobFileReader -> FileDescriptor and EnvWrapper -> Env. Hand-reading random samples found one wrong retarget (db_bench's `thread->rand.Next()`, the ThreadState namesake), which led to the member-lookup fix above. After that fix, all 76 sites sampled across the four repos were correct: - 30 retargets and additions, plus both removals, in rocksdb; - 20 in protobuf, 14 in leveldb and 10 in fmt. The two removed calls are `task_reader->rep_->GetCoroutine(…)` on a `SstFileReader::Rep`, which has no such method. They had linked to `CoroDB`'s. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../cpp-member-chain-project-calls.test.ts | 176 ++++++++++++++++++ src/resolution/name-matcher.ts | 91 ++++----- 3 files changed, 224 insertions(+), 45 deletions(-) create mode 100644 __tests__/cpp-member-chain-project-calls.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6fa7945c..17eefc2b56 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. 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 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-project-calls.test.ts b/__tests__/cpp-member-chain-project-calls.test.ts new file mode 100644 index 0000000000..48dd1950ad --- /dev/null +++ b/__tests__/cpp-member-chain-project-calls.test.ts @@ -0,0 +1,176 @@ +/** + * A C++ call on a data member reached through a member chain, like rocksdb's + * `thread->rnd.Next()` on a `Random rnd` or `meta.smallest.user_key()` on an + * `InternalKey smallest`, calls the method of the class the member is + * declared as, or of a class it derives from. The extractor records such a + * call by its bare name, so it went to whichever class had a method of that + * name: `ArenaWrappedDBIter::Next`, `ClippingIterator::user_key`, or the + * calling method's own class. + */ +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 of a project class', () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-project-')); + 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 ArenaWrappedDBIter {', + ' public:', + ' void Next() {}', + '};', + 'class Random {', + ' public:', + ' unsigned Next() { return 0; }', + '};', + 'class ClippingIterator {', + ' public:', + ' int user_key() const { return 1; }', + '};', + 'class InternalKey {', + ' public:', + ' int user_key() const { return 0; }', + '};', + 'class InMemoryEnv {', + ' public:', + ' void NewWritableFile() {}', + '};', + 'class Env {', + ' public:', + ' virtual void NewWritableFile() {}', + '};', + 'class Other {', + ' public:', + ' void Shared() {}', + '};', + 'class Base {', + ' public:', + ' void Shared() {}', + '};', + 'class Derived : public Base {};', + 'class Free {', + ' public:', + ' void Missing() {}', + '};', + 'class Lacking {', + ' public:', + ' void Something() {}', + '};', + 'class Cache {', + ' public:', + ' void Put(int k, int v) {}', + '};', + 'class Table {', + ' public:', + ' void Put(int k) {}', + ' void Put(int k, int v) {}', + '};', + 'class Random64 {', + ' public:', + ' unsigned Next() { return 1; }', + '};', + )); + // Two files each declare a `SharedState` of their own, with members of the same name. + write('src/a_shared.h', lines( + '#include "src/types.h"', + 'struct SharedState {', + ' Random rand;', + '};', + )); + write('src/bench.cc', lines( + '#include "src/types.h"', + 'struct SharedState {', + ' Random64 rand;', + '};', + 'void Bench(SharedState* shared) { shared->rand.Next(); }', + )); + write('src/use.cc', lines( + '#include "src/types.h"', + 'struct ThreadState {', + ' Random rnd;', + '};', + 'struct FileMeta {', + ' InternalKey smallest;', + '};', + 'struct Options {', + ' Env* env;', + '};', + 'struct Holder {', + ' Derived d;', + ' Lacking l;', + ' Table table;', + '};', + 'struct Outer {', + ' ThreadState inner;', + '};', + 'void Rand(ThreadState* thread) { thread->rnd.Next(); }', + 'void Key(FileMeta& meta) { meta.smallest.user_key(); }', + 'void EnvCall(Options& options) { options.env->NewWritableFile(); }', + 'void TwoHops(Outer* o) { o->inner.rnd.Next(); }', + 'void Inherited(Holder& h) { h.d.Shared(); }', + 'void Lacks(Holder& h) { h.l.Missing(); }', + 'void Overload(Holder& h) { h.table.Put(1, 2); }', + 'class Worker {', + ' public:', + ' void Next() {}', + ' void Step() { this->state_.rnd.Next(); }', + ' private:', + ' ThreadState state_;', + '};', + )); + 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 or method `name`, as `qualifiedName` (`:line` with `withLine`). */ + const calls = (name: string, withLine = false): string[] => { + const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')].find((n: Node) => n.name === name); + expect(caller, name).toBeDefined(); + return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls') + .map((r) => (withLine ? `${r.node.qualifiedName}:${r.node.startLine}` : r.node.qualifiedName)).sort(); + }; + + it("calls the method of the member's class, not a namesake's", () => { + expect(calls('Rand')).toEqual(['Random::Next']); + expect(calls('Key')).toEqual(['InternalKey::user_key']); + expect(calls('EnvCall')).toEqual(['Env::NewWritableFile']); + }); + + it('through two members', () => { + expect(calls('TwoHops')).toEqual(['Random::Next']); + }); + + it("a method the member's class inherits, and none when the class lacks it", () => { + expect(calls('Inherited')).toEqual(['Base::Shared']); + expect(calls('Lacks')).toEqual([]); + }); + + it("of the class's overloads, the one the arguments fit", () => { + expect(calls('Overload', true)).toEqual(['Table::Put:49']); + }); + + it("a chain through `this` is no call on the calling method's own class", () => { + expect(calls('Step')).toEqual(['Random::Next']); + }); + + it("a member of the file's own class, not of another file's class of that name", () => { + expect(calls('Bench')).toEqual(['Random64::Next']); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 6767756868..d87d84adf0 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -9620,31 +9620,23 @@ function cppWrittenClass(type: CppWrittenType, at: UnresolvedRef, context: Resol return found && CPP_CLASS_KINDS.has(found.node.kind) ? found.node : null; } -/** - * The type the C++ class `cls` declares its data member `name` with, as - * written, in the class's scope: `std::vector` for - * rocksdb's `CompactionState::sub_compact_states`. Data members are not - * indexed, so the class's own lines are read — one inside none of the - * functions or classes nested in it. Null when none of them declares it - * (a base class may). - */ -function cppDataMemberType(cls: string, name: string, context: ResolutionContext): CppWrittenType | null { - for (const decl of context.getNodesByQualifiedName(cls)) { - const member = cppDataMemberIn(decl, name, context); - if (member) return member; - } - return null; -} - const CPP_DATA_MEMBERS = new WeakMap>(); const CPP_NESTED_SPANS = new WeakMap>>(); const CPP_CLASS_PAREN_DEPTHS = new WeakMap>(); /** - * cppDataMemberType for one declaration of the class, `decl`. A declarator - * inside parentheses is a parameter of a method the class declares — which - * has no node of its own, so its lines are the class's — not a member: - * rocksdb's `DB` declares `CreateColumnFamilies(const + * The type the C++ class `decl` declares its data member `name` with, as + * written, in the class's scope: `std::vector` for + * rocksdb's `CompactionState::sub_compact_states`. Data members are not + * indexed, so the class's own lines are read — one inside none of the + * functions or classes nested in it. Null when none of them declares it + * (a base class may). Only `decl`'s own: a class of the same qualified name + * another file declares for itself is another class — rocksdb's + * `db_bench_tool.cc` and `db_stress_shared_state.h` each declare a + * `ThreadState`, whose `rand` is a `Random64` in one and a `Random` in the + * other. A declarator inside parentheses is a parameter of a method the + * class declares — which has no node of its own, so its lines are the + * class's — not a member: rocksdb's `DB` declares `CreateColumnFamilies(const * std::vector& column_families, …)`. Nor is a name * an argument list follows, a method. */ @@ -10031,7 +10023,7 @@ function cppExpressionType(expr: string, at: UnresolvedRef, context: ResolutionC const element = object && cppPairMember(object, field![3]!, at, context); if (element) return element; const cls = object && cppWrittenClass(object, at, context); - return cls ? cppDataMemberType(cls.qualifiedName, field![3]!, context) : null; + return cls ? cppDataMemberIn(cls, field![3]!, context) : null; } const open = cppOpeningParen(text, text.length - 1); const callee = open > 0 ? text.slice(0, open).trim() : ''; @@ -11055,30 +11047,42 @@ function cppMemberChainReceiver(ref: UnresolvedRef, context: ResolutionContext): /** * A C++ call on a data member reached through a member chain, which the - * extractor records by its bare name, calls no project method when that - * member's type is from outside the project: a library type - * (isCppLibraryType) or one under a namespace the project declares nothing - * in (isCppOutsideQualifiedType). By its bare name the call went to any - * project method of that name: rocksdb's `options.cf_paths.emplace_back(…)` - * on a `std::vector` reached `autovector::emplace_back`, - * `props->rep.db_id.size()` on a `std::string` `BlobContents::size`, and - * protobuf's `state_->child_pid.has_value()` on a `std::optional` - * `Option::has_value`. Unlike a call chain's, a member chain's operator is - * read where it is written, so a library template of a template parameter is - * no matter: `.` calls a member of `std::vector` itself, and `->` on a - * `std::unique_ptr` one of `T`, which is none of the project's to tell. - * - null for such a member: no project method is called. - * - undefined for any other call, or a member whose type can't be read or is - * the project's: the name strategies decide, as before. - */ -function matchCppOutsideMemberChainCall(ref: UnresolvedRef, context: ResolutionContext): null | undefined { + * extractor records by its bare name, calls a member of the class that + * member's type names, looked up as C++ looks it up where the type is + * written (cppMemberObject reads each member's declared type in its class). + * By its bare name the call went to whichever class had a method of that + * name, or, in a method, to the method's own class: rocksdb's + * `thread->rnd.Next()` on a `Random rnd` reached `ArenaWrappedDBIter::Next`, + * `meta.smallest.user_key()` on an `InternalKey` `ClippingIterator::user_key`, + * and `options.cf_paths.emplace_back(…)` on a `std::vector` + * `autovector::emplace_back`. Unlike a call chain's, a member chain's + * operator is read where it is written, so a library template of a template + * parameter is no matter: `.` calls a member of `std::vector` itself, and + * `->` on a `std::unique_ptr` one of `T`. + * - The member of that class, or of a class it derives from. + * - null when no project method is called: a type from outside the project, + * a library type (isCppLibraryType) or one under a namespace the project + * declares nothing in (isCppOutsideQualifiedType); a template parameter's + * type the calling function writes; or a class that surely lacks the + * method (cppLacksMethod). + * - undefined for any other call, or a member whose type can't be read or + * placed: the name strategies decide, as before. + */ +function matchCppMemberChainCall(ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null | undefined { const receiver = cppMemberChainReceiver(ref, context); if (!receiver) return undefined; const object = cppMemberObject(receiver.expression, receiver.op, ref, context, 0); if (!object || isCppPointerType(object.text)) return undefined; if (isCppLibraryType(object.text, context)) return null; - return cppClassWritten(object.text, object, ref, context) === undefined && - isCppOutsideQualifiedType(object.text, object, context) ? null : undefined; + const found = cppClassWritten(object.text, object, ref, context); + if (found === undefined) return isCppOutsideQualifiedType(object.text, object, context) ? null : undefined; + // (As for an unspelled receiver: a template parameter another template + // writes is what the call's specialization makes it.) + if (found === null) return isCppTemplateParameterType(object) && object.scope !== context.getNodeById?.(ref.fromNodeId)?.qualifiedName ? undefined : null; + if (!found.exact || !CPP_CLASS_KINDS.has(found.node.kind)) return undefined; + const method = cppMethodOf(found.node, ref.referenceName, ref, context); + if (method) return { original: ref, targetNodeId: method.id, confidence: 0.9, resolvedBy: 'instance-method' }; + return cppLacksMethod(found.node, context) ? null : undefined; } /** @@ -17026,10 +17030,9 @@ function matchReferenceInner( if (ref.language === 'cpp' && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*$/.test(ref.referenceName)) { const unspelled = nmTimedT('cppUnspelled', ref, () => matchCppUnspelledReceiverCall(ref, context)); if (unspelled !== undefined) return unspelled; - // So is one on a member reached through a chain, `options.cf_paths.emplace_back(…)`, - // when that member's type is from outside the project. - const outside = nmTimedT('cppOutsideMemberChain', ref, () => matchCppOutsideMemberChainCall(ref, context)); - if (outside !== undefined) return outside; + // So is one on a member reached through a chain, `thread->rnd.Next()`. + const chained = nmTimedT('cppMemberChain', ref, () => matchCppMemberChainCall(ref, context)); + if (chained !== undefined) return chained; } // Try strategies in order of confidence From ad3a552cb54d1588e9b2db6db4ee6a10e879408c Mon Sep 17 00:00:00 2001 From: bompus Date: Fri, 16 Oct 2026 18:05:55 -0600 Subject: [PATCH 5/5] docs: record the upstream e8ec7fd8 merge point --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index dffbacdf5a..fc7fdd1c3f 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 [`19813544`](https://github.com/colbymchenry/codegraph/commit/19813544) (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 [`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. 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).