Просмотр исходного кода

fix(extraction): index C++ pure virtual methods as nodes (#1727) (#1758)

Pure-virtual declarations (`virtual int read(int key) = 0;`) parse as
field_declaration, not function_definition, so they minted no method node —
calls through an abstract base and cpp-override synthesis had nothing to
attach to. Mirror Java interface methods: mint the node (TS + kernel), mark
isAbstract, and cover with extraction/e2e/parity fixtures.

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 6 часов назад
Родитель
Сommit
2f1a99d34c

+ 68 - 0
__tests__/extraction.test.ts

@@ -5868,6 +5868,74 @@ end
   });
 });
 
+describe('C++ pure-virtual method nodes (#1727)', () => {
+  // Pure-virtual methods are field_declarations (`virtual int read(int key) = 0;`),
+  // not function_definitions — they previously minted no method node, so calls
+  // through an abstract base and cpp-override synthesis had nothing to attach to.
+  // Java interface methods already get nodes; C++ should behave similarly.
+  it('indexes Store::read from the issue fixture and records the call', () => {
+    const code = `
+class Store {
+public:
+    virtual ~Store() {}
+    virtual int read(int key) = 0;
+};
+
+class DiskStore : public Store {
+public:
+    int read(int key) override { return key + 1; }
+};
+
+class MemStore : public Store {
+public:
+    int read(int key) override { return key + 2; }
+};
+
+int fetch(Store* s, int k) {
+    return s->read(k);
+}
+`;
+    const result = extractFromSource('store.cc', code);
+    const methods = result.nodes.filter((n) => n.kind === 'method').map((n) => n.qualifiedName);
+    expect(methods).toContain('Store::read');
+    expect(methods).toContain('DiskStore::read');
+    expect(methods).toContain('MemStore::read');
+
+    const baseRead = result.nodes.find((n) => n.qualifiedName === 'Store::read');
+    expect(baseRead?.isAbstract).toBe(true);
+
+    // Call site unresolved ref targets the method name (resolver types the receiver).
+    expect(
+      result.unresolvedReferences.some(
+        (r) => r.referenceKind === 'calls' && (r.referenceName === 'read' || r.referenceName.endsWith('.read') || r.referenceName.endsWith('->read') || r.referenceName === 's.read')
+      )
+    ).toBe(true);
+  });
+
+  it('indexes pure virtuals with pointer/reference return types and operators', () => {
+    const code = `
+class Cloneable {
+public:
+    virtual Cloneable* clone() = 0;
+    virtual const Foo& get() = 0;
+    virtual Cloneable& operator=(const Cloneable&) = 0;
+    int notPure(int x);
+    int data = 0;
+};
+`;
+    const result = extractFromSource('clone.hpp', code);
+    const methods = result.nodes.filter((n) => n.kind === 'method').map((n) => n.name);
+    expect(methods).toContain('clone');
+    expect(methods).toContain('get');
+    expect(methods).toContain('operator=');
+    // Non-pure prototype and data member must NOT become methods here.
+    expect(methods).not.toContain('notPure');
+    expect(methods).not.toContain('data');
+    expect(result.nodes.find((n) => n.name === 'clone')?.isAbstract).toBe(true);
+  });
+
+});
+
 describe('C++ free-function name extraction', () => {
   let tempDir: string;
   let cg: CodeGraph;

+ 2 - 0
__tests__/fixtures/kernel-parity/torture.cpp

@@ -39,6 +39,8 @@ class Session {
 public:
   void open();
   virtual ~Session() {}
+  // #1727 — pure virtual must mint a method node (parity between wasm + kernel).
+  virtual int read(int key) = 0;
 };
 void Session::open() {}
 }  // namespace app::net

+ 55 - 0
__tests__/frameworks-integration.test.ts

@@ -353,6 +353,61 @@ describe('C++ end-to-end — virtual override synthesis', () => {
 
     cg.close();
   });
+
+  it('indexes pure-virtual base methods and bridges overrides (#1727)', async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cpp-pure-'));
+    fs.writeFileSync(
+      path.join(tmpDir, 'store.cc'),
+      'class Store {\n' +
+        'public:\n' +
+        '    virtual ~Store() {}\n' +
+        '    virtual int read(int key) = 0;\n' +
+        '};\n' +
+        'class DiskStore : public Store {\n' +
+        'public:\n' +
+        '    int read(int key) override { return key + 1; }\n' +
+        '};\n' +
+        'class MemStore : public Store {\n' +
+        'public:\n' +
+        '    int read(int key) override { return key + 2; }\n' +
+        '};\n' +
+        'int fetch(Store* s, int k) {\n' +
+        '    return s->read(k);\n' +
+        '}\n'
+    );
+
+    const cg = CodeGraph.initSync(tmpDir);
+    await cg.indexAll();
+
+    const storeRead = cg
+      .getNodesByKind('method')
+      .find((n) => n.qualifiedName === 'Store::read');
+    expect(storeRead, 'Store::read pure virtual must be a method node').toBeDefined();
+    expect(storeRead!.isAbstract).toBe(true);
+
+    const diskRead = cg
+      .getNodesByKind('method')
+      .find((n) => n.qualifiedName === 'DiskStore::read');
+    const memRead = cg
+      .getNodesByKind('method')
+      .find((n) => n.qualifiedName === 'MemStore::read');
+    expect(diskRead).toBeDefined();
+    expect(memRead).toBeDefined();
+
+    // cpp-override synthesis: base pure virtual → each override
+    const out = cg.getOutgoingEdges(storeRead!.id).filter((e) => e.kind === 'calls');
+    const targets = out.map((e) => e.target);
+    expect(targets).toContain(diskRead!.id);
+    expect(targets).toContain(memRead!.id);
+
+    // Call through abstract base resolves onto Store::read
+    const fetch = cg.getNodesByKind('function').find((n) => n.name === 'fetch');
+    expect(fetch).toBeDefined();
+    const callees = cg.getCallees(fetch!.id).map((c) => c.node.qualifiedName);
+    expect(callees).toContain('Store::read');
+
+    cg.close();
+  });
 });
 
 describe('Java end-to-end — field-injected bean trace (issue #389)', () => {

+ 55 - 1
codegraph-kernel/src/ccpp/mod.rs

@@ -52,6 +52,10 @@
 //!    per-caller targets (insertion-ordered, branch reassignments accumulate);
 //!    a later bare `k(args)` emits one `calls` ref PER target and suppresses
 //!    the local name. Template args stripped like base-class refs (#1043).
+//!  - pure-virtual methods (#1727): cpp in-class `virtual T f(...) = 0;` is a
+//!    `field_declaration` (not `function_definition`); mint a method node so
+//!    abstract-base calls and cpp-override synthesis have a target. Mirrors
+//!    TS `methodTypes` + `classifyMethodNode` / `isAbstract`.
 //!  - stack construction (#1035): cpp `declaration` with class-like named
 //!    `type` and an init_declarator whose value is argument_list /
 //!    initializer_list → `instantiates` (most-vexing-parse excluded).
@@ -66,7 +70,7 @@
 
 use crate::buffers::{
     build_meta, edge_kind_index, node_kind_index, Arena, BoolFlags, EdgeRow, EmitOut, NodeRow,
-    RefRow, StrRef, Tables, FLAG_IS_EXPORTED, FUNCTION_REF_CODE, NONE, NONE_STR,
+    RefRow, StrRef, Tables, FLAG_IS_ABSTRACT, FLAG_IS_EXPORTED, FUNCTION_REF_CODE, NONE, NONE_STR,
 };
 use crate::docstring::preceding_docstring;
 use crate::ids;
@@ -289,6 +293,7 @@ struct Extra {
     signature: Option<String>,
     visibility: Option<u8>,
     is_exported: Option<bool>,
+    is_abstract: Option<bool>,
     return_type: Option<String>,
     qualified_name: Option<String>,
 }
@@ -508,6 +513,9 @@ impl<'t> Walker<'t> {
         if let Some(v) = extra.is_exported {
             flags.set(FLAG_IS_EXPORTED, v);
         }
+        if let Some(v) = extra.is_abstract {
+            flags.set(FLAG_IS_ABSTRACT, v);
+        }
         let name_ref = self.arena.put(name);
         let qn_ref = self.arena.put(&qualified);
         let id_ref = self.arena.put(&id);
@@ -740,6 +748,36 @@ impl<'t> Walker<'t> {
             .any(|c| c.kind() == "type_qualifier" && self.text(c) == "const")
     }
 
+    /// `#1727`: C++ pure-virtual method declaration (`virtual int read(int key) = 0;`).
+    /// tree-sitter-cpp shapes these as `field_declaration` whose declarator unwraps
+    /// to a `function_declarator`, with the pure-virtual `= 0` as a DIRECT
+    /// `number_literal` "0" child (default-arg `= 0` lives inside
+    /// `parameter_declaration` and must not match).
+    fn is_cpp_pure_virtual_method_decl(&self, node: Node<'_>) -> bool {
+        if node.kind() != "field_declaration" {
+            return false;
+        }
+        let Some(mut declarator) = node.child_by_field_name("declarator") else {
+            return false;
+        };
+        while matches!(declarator.kind(), "pointer_declarator" | "reference_declarator") {
+            let inner = declarator
+                .child_by_field_name("declarator")
+                .or_else(|| declarator.named_child(0));
+            let Some(inner) = inner else {
+                return false;
+            };
+            declarator = inner;
+        }
+        if declarator.kind() != "function_declarator" {
+            return false;
+        }
+        (0..node.named_child_count())
+            .filter_map(|i| node.named_child(i))
+            .any(|c| c.kind() == "number_literal" && self.text(c) == "0")
+    }
+
+
     /// cppExtractor.isMisparsedFunction (languages/c-cpp.ts:811). cpp only.
     fn is_misparsed_function(&self, name: &str, node: Node) -> bool {
         if self.variant != Variant::Cpp {
@@ -830,6 +868,17 @@ impl<'t> Walker<'t> {
             self.extract_variable(node);
             self.scan_fn_ref_subtree(node, 0);
             skip_children = true;
+        } else if self.variant == Variant::Cpp
+            && kind == "field_declaration"
+            && self.inside_class_like()
+            && self.is_cpp_pure_virtual_method_decl(node)
+        {
+            // Pure-virtual methods have no `function_definition` body — mint the
+            // method node so calls through the abstract base and cpp-override
+            // synthesis have a target (#1727). Non-pure field_declarations fall
+            // through to the children walk (data members / prototypes).
+            self.extract_method(node);
+            skip_children = true;
         } else if kind == "preproc_include" {
             self.extract_import(node);
         } else if kind == "call_expression" {
@@ -914,6 +963,11 @@ impl<'t> Walker<'t> {
         let extra = Extra {
             docstring: preceding_docstring(node, self.src),
             visibility: if self.variant == Variant::Cpp { self.visibility_of(node) } else { None },
+            is_abstract: if self.variant == Variant::Cpp && self.is_cpp_pure_virtual_method_decl(node) {
+                Some(true)
+            } else {
+                None
+            },
             return_type: self.return_type_of(node),
             qualified_name: receiver_type
                 .as_ref()

+ 40 - 1
src/extraction/languages/c-cpp.ts

@@ -166,6 +166,35 @@ export function stripCppTemplateArgs(name: string): string {
   return out.trim();
 }
 
+
+/**
+ * Is this C++ `field_declaration` a pure-virtual method (`virtual int read(int key) = 0;`)?
+ * tree-sitter-cpp shapes those as a field_declaration whose declarator unwraps to a
+ * `function_declarator`, with the pure-virtual `= 0` as a DIRECT `number_literal` "0"
+ * child of the field_declaration (default-arg `= 0` lives inside parameter_declaration
+ * and must not match). Bodiless method prototypes (`int foo();`) and data members
+ * (`int x = 0;`) are excluded — prototypes usually have an out-of-line definition that
+ * already mints the method node; pure virtuals never do (#1727).
+ */
+export function isCppPureVirtualMethodDecl(node: SyntaxNode): boolean {
+  if (node.type !== 'field_declaration') return false;
+  let declarator: SyntaxNode | null = getChildByField(node, 'declarator');
+  if (!declarator) return false;
+  while (
+    declarator.type === 'pointer_declarator' ||
+    declarator.type === 'reference_declarator'
+  ) {
+    const inner: SyntaxNode | null =
+      getChildByField(declarator, 'declarator') || declarator.namedChild(0);
+    if (!inner) return false;
+    declarator = inner;
+  }
+  if (declarator.type !== 'function_declarator') return false;
+  return node.namedChildren.some(
+    (c: SyntaxNode) => c.type === 'number_literal' && c.text === '0'
+  );
+}
+
 /**
  * A function/method's return type lives in the `function_definition`'s `type`
  * field (`Metrics& Metrics::instance()` → `Metrics`). Constructors, destructors,
@@ -1607,7 +1636,17 @@ export const cppExtractor: LanguageExtractor = {
   // get picked as the blast-radius representative over — the single real
   // definition, exactly as bodiless struct/enum specifiers are already skipped. (#1093)
   skipBodilessClass: true,
-  methodTypes: ['function_definition'],
+  // `function_definition` covers inline / out-of-line bodies; `field_declaration`
+  // covers pure-virtual methods (`virtual int read(int key) = 0;`), which have no
+  // body and would otherwise mint no node — so calls through the abstract base and
+  // cpp-override synthesis had nothing to attach to (#1727). classifyMethodNode
+  // keeps ordinary data members / prototypes on the children-walk path.
+  methodTypes: ['function_definition', 'field_declaration'],
+  classifyMethodNode: (node) => {
+    if (node.type !== 'field_declaration') return 'method';
+    return isCppPureVirtualMethodDecl(node) ? 'method' : 'skip';
+  },
+  isAbstract: (node) => (isCppPureVirtualMethodDecl(node) ? true : undefined),
   interfaceTypes: [],
   structTypes: ['struct_specifier'],
   // C++ unions additionally carry member functions, which extract through the

+ 8 - 2
src/extraction/tree-sitter-types.ts

@@ -160,6 +160,8 @@ export interface LanguageExtractor {
   isAsync?: (node: SyntaxNode) => boolean;
   /** Check if node is static */
   isStatic?: (node: SyntaxNode) => boolean;
+  /** Check if a method/class is abstract (C++ pure virtual, Java abstract, …). Return true to set; undefined/false leaves the flag unset. */
+  isAbstract?: (node: SyntaxNode) => boolean | undefined;
   /** Check if variable declaration is a constant (const vs let/var) */
   isConst?: (node: SyntaxNode) => boolean;
   /**
@@ -221,9 +223,13 @@ export interface LanguageExtractor {
    * both callable and data members (#808): TS/JS class FIELDS
    * (`public_field_definition` / `field_definition`) are methods only when
    * their value is callable (`onClick = () => {}`); a plain field
-   * (`public fonts: Fonts;`, `count = 0`) is a property. Default: 'method'.
+   * (`public fonts: Fonts;`, `count = 0`) is a property. C++ also lists
+   * `field_declaration` in methodTypes so pure-virtual methods (`= 0`) can
+   * mint nodes (#1727); non-callable field_declarations return `'skip'` so
+   * the walker still descends (data-member initializers keep their call
+   * edges). Default: 'method'.
    */
-  classifyMethodNode?: (node: SyntaxNode) => 'method' | 'property';
+  classifyMethodNode?: (node: SyntaxNode) => 'method' | 'property' | 'skip';
 
   /**
    * Resolve the body node for a function/method/class when it's not a child field.

+ 12 - 2
src/extraction/tree-sitter.ts

@@ -1041,8 +1041,14 @@ export class TreeSitterExtractor {
     else if (this.extractor.methodTypes.includes(nodeType)) {
       // TS/JS class fields parse as a methodTypes node; only function-valued
       // fields are methods — a plain field (`public fonts: Fonts;`) is a
-      // property (#808). classifyMethodNode is absent for other languages.
-      if (this.extractor.classifyMethodNode?.(node) === 'property') {
+      // property (#808). C++ lists `field_declaration` so pure-virtual methods
+      // mint nodes (#1727); non-callable ones return 'skip' and fall through to
+      // the children walk. classifyMethodNode is absent for other languages.
+      const methodClass = this.extractor.classifyMethodNode?.(node) ?? 'method';
+      if (methodClass === 'skip') {
+        // Not a method — leave skipChildren false so data-member initializers
+        // still contribute call/instantiation edges under the enclosing class.
+      } else if (methodClass === 'property') {
         const propNode = this.extractProperty(node);
         // Walk the initializer so its calls/instantiations attribute to the
         // property (`history = createHistory()` → history calls
@@ -1798,6 +1804,9 @@ export class TreeSitterExtractor {
     const visibility = this.extractor.getVisibility?.(node);
     const isAsync = this.extractor.isAsync?.(node);
     const isStatic = this.extractor.isStatic?.(node);
+    // Only persist abstract when true — a false return must not mint `isAbstract: false`
+    // on every ordinary method (breaks kernel↔wasm parity JSON equality).
+    const isAbstract = this.extractor.isAbstract?.(node) ? true : undefined;
     const returnType = this.extractor.getReturnType?.(node, this.source);
     const extraProps: Partial<Node> = {
       docstring,
@@ -1805,6 +1814,7 @@ export class TreeSitterExtractor {
       visibility,
       isAsync,
       isStatic,
+      isAbstract,
       returnType,
     };
     if (receiverType) {