fix(cpp): resolve calls through singletons/factories/chained getters (colbymchenry#645) (colbymchenry#742)

colbymchenry · claude · web-flow · commit fd03f31b2cdf · 2026-06-08T20:18:17.000-04:00
A C++ method call whose receiver is another call's result — `Foo::instance().bar()`,
`WidgetFactory::create().draw()`, `openSession()-&gt;run()`, or the same stored in an
`auto` local first — lost the receiver's type during extraction. The callee degraded
to a bare method name, so when two classes shared a method name the call silently
resolved to whichever was indexed first (or not at all), corrupting callers / impact /
trace with a plausible-but-wrong edge.

Three parts:
- Capture C++ return types (new nodes.return_type column, schema v5): the
  function_definition's `type` field, normalized — smart-pointer pointee unwrapped,
  void/primitives dropped.
- Preserve the inner-call receiver in extraction: a C/C++ field_expression whose
  receiver is itself a call is encoded `inner().method` instead of dropping to the
  bare name. Other languages keep the existing behavior.
- New resolution strategy (matchCppCallChain): infer the receiver's class from the
  inner call's return type, then resolve AND validate the method on it. Handles
  singletons/accessors, factories returning a different type, free-function
  factories, make_unique/make_shared/new/direct construction, single-level member
  chains, and namespace-qualified inner calls. A wrong inference yields no edge,
  never a wrong one.

EXTRACTION_VERSION 2-&gt;3 (re-index to populate return types).

Validated on the issue repro + spdlog: node count stable (no explosion),
deterministic, and ~100 pre-existing wrong `.size()`-style edges removed.

Co-authored-by: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- C++ method calls made through a singleton, factory, or chained getter now resolve to the correct class. A call like `Foo::instance().bar()`, `WidgetFactory::create().draw()`, `openSession()->run()`, or the same stored in an `auto` local first, used to lose the receiver's type — so when two classes had a same-named method the call silently attached to whichever was indexed first (or didn't resolve at all), corrupting callers, impact, and trace. CodeGraph now infers the receiver's type from what the inner call returns (capturing C++ return types for the first time) and creates the edge only when that class genuinely has the method, so a wrong guess produces no edge instead of a misleading one. Covers singletons and self-returning accessors, factories that return a different type, free-function factories, `make_unique` / `make_shared` / `new` / direct construction, and single-level member chains. Existing C/C++ indexes should be re-indexed (`codegraph index -f`) to benefit. Thanks @stabey. (#645) (C/C++)
 - The shared background server no longer logs a scary-looking `[error] … undefined` line on every session start. Attaching to the shared daemon is normal, healthy behavior, but the informational message was being surfaced by MCP hosts (Claude Code and others) as an error; it's now silent by default — set `CODEGRAPH_MCP_LOG_ATTACH=1` to surface it when debugging daemon attach. Thanks @mturac. (#618)
 - On Windows, CodeGraph's background processes no longer pile up without bound and saturate CPU over a long session. When the editor or agent that launched CodeGraph exited, its helper process couldn't tell its parent had gone — Windows reports process lineage differently than macOS and Linux — so the helper kept running, the shared background server never saw the client disconnect, and its idle timer never fired to shut it down. CodeGraph now detects parent-process exit directly on Windows, so helpers and the idle background server wind down promptly, the same as they already did on macOS and Linux. (#692, #576, #680)
 - The shared background server has two further safeguards against ever lingering: it now drops a client the moment it detects that client's process is gone (even if the disconnect arrived uncleanly — a force-quit or a dropped connection that never closed the socket), and it won't stay running indefinitely with clients attached but no activity. Together these guarantee it always winds down, on every platform. (#692)
diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts
@@ -2369,6 +2369,41 @@ end
     });
   });
 
+  describe('C/C++ return type capture (#645)', () => {
+    it('captures the normalized return type of a C++ method/function', () => {
+      const code = `
+struct Widget { void draw(); };
+class Factory { public: static Widget create(); };
+Widget Factory::create() { return Widget(); }
+void doNothing() {}
+`;
+      const result = extractFromSource('f.cpp', code);
+
+      const create = result.nodes.find(
+        (n) => n.name === 'create' && (n.kind === 'method' || n.kind === 'function')
+      );
+      expect(create?.returnType).toBe('Widget');
+
+      // A `void` return records no type, so resolution never tries to resolve a
+      // method on it.
+      const doNothing = result.nodes.find((n) => n.name === 'doNothing');
+      expect(doNothing).toBeDefined();
+      expect(doNothing?.returnType).toBeUndefined();
+    });
+
+    it('unwraps a smart-pointer return type to its pointee', () => {
+      const code = `
+#include <memory>
+struct Widget {};
+std::unique_ptr<Widget> makeWidget() { return nullptr; }
+`;
+      const result = extractFromSource('f.cpp', code);
+
+      const make = result.nodes.find((n) => n.name === 'makeWidget');
+      expect(make?.returnType).toBe('Widget');
+    });
+  });
+
   describe('C/C++ imports', () => {
     it('should extract system include', () => {
       const code = `#include <iostream>`;
diff --git a/__tests__/foundation.test.ts b/__tests__/foundation.test.ts
@@ -242,7 +242,7 @@ describe('Database Connection', () => {
 
     const version = db.getSchemaVersion();
     expect(version).not.toBeNull();
-    expect(version?.version).toBe(4);
+    expect(version?.version).toBe(5);
 
     db.close();
   });
diff --git a/__tests__/pr19-improvements.test.ts b/__tests__/pr19-improvements.test.ts
@@ -299,7 +299,7 @@ describe('Best-Candidate Resolution', () => {
 describe('Schema v2 Migration', () => {
   it.skipIf(!HAS_SQLITE)('should have correct current schema version', async () => {
     const { CURRENT_SCHEMA_VERSION } = await import('../src/db/migrations');
-    expect(CURRENT_SCHEMA_VERSION).toBe(4);
+    expect(CURRENT_SCHEMA_VERSION).toBe(5);
   });
 
   it.skipIf(!HAS_SQLITE)('should have migration for version 2', async () => {
diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts
@@ -1918,4 +1918,112 @@ func main() {
       }
     });
   });
+
+  describe('C++ chained-call receiver resolution (#645)', () => {
+    async function indexCpp(files: Record<string, string>): Promise<void> {
+      for (const [name, content] of Object.entries(files)) {
+        fs.writeFileSync(path.join(tempDir, name), content);
+      }
+      cg = await CodeGraph.init(tempDir, { index: true });
+    }
+
+    function callerNamesOf(qualifiedName: string): string[] {
+      const target = cg.getNodesByKind('method').find((n) => n.qualifiedName === qualifiedName);
+      if (!target) return [];
+      const names = cg
+        .getIncomingEdges(target.id)
+        .filter((e) => e.kind === 'calls')
+        .map((e) => cg.getNode(e.source)?.name)
+        .filter((n): n is string => !!n);
+      return [...new Set(names)].sort();
+    }
+
+    it('resolves singleton chains and auto locals to the right class, never the first-sorted one', async () => {
+      // Two classes share writeLog; Logger sorts first so it wins any name-only
+      // tie. All three call forms target Metrics.
+      await indexCpp({
+        'logger.hpp': `#pragma once
+#include <string>
+class Logger  { public: static Logger&  instance(); void writeLog(const std::string&); };
+class Metrics { public: static Metrics& instance(); void writeLog(const std::string&); };
+`,
+        'impl.cpp': `#include "logger.hpp"
+Logger&  Logger::instance()  { static Logger l;  return l; }
+Metrics& Metrics::instance() { static Metrics m; return m; }
+void Logger::writeLog(const std::string&)  {}
+void Metrics::writeLog(const std::string&) {}
+`,
+        'app.cpp': `#include "logger.hpp"
+void a() { Metrics::instance().writeLog("x"); }              // chained singleton
+void b() { auto& m = Metrics::instance(); m.writeLog("x"); } // stored in auto
+void c() { Metrics& m = Metrics::instance(); m.writeLog("x"); } // explicit type
+`,
+      });
+
+      expect(callerNamesOf('Metrics::writeLog')).toEqual(['a', 'b', 'c']);
+      expect(callerNamesOf('Logger::writeLog')).toEqual([]);
+    });
+
+    it('resolves factories, free-function factories, and member chains via the inner call return type', async () => {
+      await indexCpp({
+        'types.hpp': `#pragma once
+#include <memory>
+struct Widget { void draw(); };
+struct Session { void run(); };
+struct View { void render(); };
+class WidgetFactory { public: static Widget create(); };
+class Manager { public: View view(); };
+Session* openSession();
+// Decoy that sorts first and has all three methods — must never win.
+struct Aaa { void draw(); void run(); void render(); };
+`,
+        'impl.cpp': `#include "types.hpp"
+void Widget::draw() {}
+void Session::run() {}
+void View::render() {}
+void Aaa::draw() {}
+void Aaa::run() {}
+void Aaa::render() {}
+Widget WidgetFactory::create() { return Widget(); }
+View Manager::view() { return View(); }
+Session* openSession() { return nullptr; }
+`,
+        'app.cpp': `#include "types.hpp"
+void factory()     { WidgetFactory::create().draw(); }   // -> Widget::draw
+void freefunc()    { openSession()->run(); }             // -> Session::run
+void member()      { Manager mgr; mgr.view().render(); }  // -> View::render
+void makeUnique()  { auto w = std::make_unique<Widget>(); w->draw(); } // -> Widget::draw
+`,
+      });
+
+      expect(callerNamesOf('Widget::draw')).toEqual(['factory', 'makeUnique']);
+      expect(callerNamesOf('Session::run')).toEqual(['freefunc']);
+      expect(callerNamesOf('View::render')).toEqual(['member']);
+      // The first-sorted decoy never captures any of them.
+      expect(callerNamesOf('Aaa::draw')).toEqual([]);
+      expect(callerNamesOf('Aaa::run')).toEqual([]);
+      expect(callerNamesOf('Aaa::render')).toEqual([]);
+    });
+
+    it('creates NO edge when the inferred type lacks the method (silent miss, not a wrong edge)', async () => {
+      await indexCpp({
+        'types.hpp': `#pragma once
+struct Widget { void draw(); };
+struct Other  { void onlyOther(); };
+class WidgetFactory { public: static Widget create(); };
+`,
+        'impl.cpp': `#include "types.hpp"
+void Widget::draw() {}
+void Other::onlyOther() {}
+Widget WidgetFactory::create() { return Widget(); }
+`,
+        'app.cpp': `#include "types.hpp"
+// Widget has no onlyOther() — must produce NO edge, never a wrong one to Other.
+void wrong() { WidgetFactory::create().onlyOther(); }
+`,
+      });
+
+      expect(callerNamesOf('Other::onlyOther')).toEqual([]);
+    });
+  });
 });
diff --git a/src/db/migrations.ts b/src/db/migrations.ts
@@ -9,7 +9,7 @@ import { SqliteDatabase } from './sqlite-adapter';
 /**
  * Current schema version
  */
-export const CURRENT_SCHEMA_VERSION = 4;
+export const CURRENT_SCHEMA_VERSION = 5;
 
 /**
  * Migration definition
@@ -65,6 +65,16 @@ const migrations: Migration[] = [
       `);
     },
   },
+  {
+    version: 5,
+    description:
+      'Add nodes.return_type — normalized return/result type for receiver-type inference (C++ singletons/factories, #645)',
+    up: (db) => {
+      db.exec(`
+        ALTER TABLE nodes ADD COLUMN return_type TEXT;
+      `);
+    },
+  },
 ];
 
 /**
diff --git a/src/db/queries.ts b/src/db/queries.ts
@@ -72,6 +72,7 @@ interface NodeRow {
   is_abstract: number;
   decorators: string | null;
   type_parameters: string | null;
+  return_type: string | null;
   updated_at: number;
 }
 
@@ -133,6 +134,7 @@ function rowToNode(row: NodeRow): Node {
     isAbstract: row.is_abstract === 1,
     decorators: row.decorators ? safeJsonParse(row.decorators, undefined) : undefined,
     typeParameters: row.type_parameters ? safeJsonParse(row.type_parameters, undefined) : undefined,
+    returnType: row.return_type ?? undefined,
     updatedAt: row.updated_at,
   };
 }
@@ -232,13 +234,13 @@ export class QueryBuilder {
           start_line, end_line, start_column, end_column,
           docstring, signature, visibility,
           is_exported, is_async, is_static, is_abstract,
-          decorators, type_parameters, updated_at
+          decorators, type_parameters, return_type, updated_at
         ) VALUES (
           @id, @kind, @name, @qualifiedName, @filePath, @language,
           @startLine, @endLine, @startColumn, @endColumn,
           @docstring, @signature, @visibility,
           @isExported, @isAsync, @isStatic, @isAbstract,
-          @decorators, @typeParameters, @updatedAt
+          @decorators, @typeParameters, @returnType, @updatedAt
         )
       `);
     }
@@ -281,6 +283,7 @@ export class QueryBuilder {
       isAbstract: node.isAbstract ? 1 : 0,
       decorators: node.decorators ? JSON.stringify(node.decorators) : null,
       typeParameters: node.typeParameters ? JSON.stringify(node.typeParameters) : null,
+      returnType: node.returnType ?? null,
       updatedAt: node.updatedAt ?? Date.now(),
     });
   }
@@ -321,6 +324,7 @@ export class QueryBuilder {
           is_abstract = @isAbstract,
           decorators = @decorators,
           type_parameters = @typeParameters,
+          return_type = @returnType,
           updated_at = @updatedAt
         WHERE id = @id
       `);
@@ -355,6 +359,7 @@ export class QueryBuilder {
       isAbstract: node.isAbstract ? 1 : 0,
       decorators: node.decorators ? JSON.stringify(node.decorators) : null,
       typeParameters: node.typeParameters ? JSON.stringify(node.typeParameters) : null,
+      returnType: node.returnType ?? null,
       updatedAt: node.updatedAt ?? Date.now(),
     });
   }
diff --git a/src/db/schema.sql b/src/db/schema.sql
@@ -37,6 +37,7 @@ CREATE TABLE IF NOT EXISTS nodes (
     is_abstract INTEGER DEFAULT 0,
     decorators TEXT, -- JSON array
     type_parameters TEXT, -- JSON array
+    return_type TEXT, -- normalized return/result type name (e.g. C++ method return, for receiver-type inference)
     updated_at INTEGER NOT NULL
 );
 
diff --git a/src/extraction/extraction-version.ts b/src/extraction/extraction-version.ts
@@ -21,4 +21,4 @@
  * turns the re-index hint into noise — keep it honest (see CLAUDE.md, "Honesty
  * in the product is load-bearing").
  */
-export const EXTRACTION_VERSION = 2;
+export const EXTRACTION_VERSION = 3;
diff --git a/src/extraction/languages/c-cpp.ts b/src/extraction/languages/c-cpp.ts
@@ -45,6 +45,56 @@ function extractCppReceiverType(node: SyntaxNode, source: string): string | unde
   return parts.length > 1 ? parts.slice(0, -1).join('::') : undefined;
 }
 
+/**
+ * Built-in / non-class return types that can never be a method receiver. We
+ * store no `returnType` for these so resolution never tries to resolve a method
+ * on `void` / `int` / etc.
+ */
+const CPP_NON_CLASS_RETURN = new Set([
+  'void', 'bool', 'char', 'short', 'int', 'long', 'float', 'double', 'unsigned',
+  'signed', 'size_t', 'ssize_t', 'auto', 'wchar_t', 'char8_t', 'char16_t',
+  'char32_t', 'int8_t', 'int16_t', 'int32_t', 'int64_t', 'uint8_t', 'uint16_t',
+  'uint32_t', 'uint64_t', 'intptr_t', 'uintptr_t', 'nullptr_t',
+]);
+
+/**
+ * Normalize a C++ return type to the bare class name a method could be called
+ * on. Unwraps smart-pointer / optional wrappers to their element type
+ * (`std::unique_ptr<Widget>` → `Widget`) so a factory's `->method()` resolves on
+ * the pointee. Strips cv-qualifiers, `&`/`*`, namespace qualifiers, and other
+ * template args. Returns undefined for primitives / void / `auto` / empty.
+ */
+export function normalizeCppReturnType(raw: string): string | undefined {
+  let t = raw.trim();
+  if (!t) return undefined;
+  // Unwrap smart pointers / optional to their pointee (the thing you call `->` on).
+  const wrapper = t.match(/\b(?:std\s*::\s*)?(?:unique_ptr|shared_ptr|weak_ptr|optional)\s*<\s*([^,>]+?)\s*>/);
+  if (wrapper && wrapper[1]) t = wrapper[1];
+  t = t
+    .replace(/\b(?:const|volatile|typename|struct|class|enum)\b/g, ' ')
+    .replace(/<[^>]*>/g, ' ')
+    .replace(/[*&]+/g, ' ')
+    .replace(/\s+/g, ' ')
+    .trim();
+  if (!t) return undefined;
+  const last = t.split('::').filter(Boolean).pop();
+  if (!last) return undefined;
+  if (CPP_NON_CLASS_RETURN.has(last)) return undefined;
+  if (!/^[A-Za-z_]\w*$/.test(last)) return undefined;
+  return last;
+}
+
+/**
+ * A function/method's return type lives in the `function_definition`'s `type`
+ * field (`Metrics& Metrics::instance()` → `Metrics`). Constructors, destructors,
+ * and conversion operators have no `type` field → undefined.
+ */
+function extractCppReturnType(node: SyntaxNode, source: string): string | undefined {
+  const typeNode = getChildByField(node, 'type');
+  if (!typeNode) return undefined;
+  return normalizeCppReturnType(getNodeText(typeNode, source));
+}
+
 export const cExtractor: LanguageExtractor = {
   functionTypes: ['function_definition'],
   classTypes: [],
@@ -60,6 +110,7 @@ export const cExtractor: LanguageExtractor = {
   nameField: 'declarator',
   bodyField: 'body',
   paramsField: 'parameters',
+  getReturnType: extractCppReturnType,
   resolveTypeAliasKind: (node, _source) => {
     // C typedef: `typedef enum { ... } name;` or `typedef struct { ... } name;`
     // The inner enum_specifier/struct_specifier is anonymous, but we want the typedef name
@@ -107,6 +158,7 @@ export const cppExtractor: LanguageExtractor = {
   paramsField: 'parameters',
   resolveName: extractCppQualifiedMethodName,
   getReceiverType: extractCppReceiverType,
+  getReturnType: extractCppReturnType,
   getVisibility: (node) => {
     // Check for access specifier in parent
     const parent = node.parent;
diff --git a/src/extraction/tree-sitter-types.ts b/src/extraction/tree-sitter-types.ts
@@ -205,6 +205,15 @@ export interface LanguageExtractor {
    */
   getReceiverType?: (node: SyntaxNode, source: string) => string | undefined;
 
+  /**
+   * Extract a function/method's normalized return type name (bare class name,
+   * smart-pointer pointee unwrapped), stored on the node as `returnType`. Used
+   * by C/C++ so resolution can infer a chained receiver's type from what the
+   * inner call returns (`Foo::instance().bar()` → resolve `bar` on `Foo`,
+   * issue #645). Return undefined for primitives / void / constructors.
+   */
+  getReturnType?: (node: SyntaxNode, source: string) => string | undefined;
+
   /**
    * Resolve the actual node kind for a type alias declaration.
    * Used by Go where `type_spec` is the named declaration wrapper for structs/interfaces:
diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts
diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts
diff --git a/src/types.ts b/src/types.ts
