From 1753472a3f25a43e97bfcadc79fa8bd2d7eb32cc Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Wed, 14 Oct 2026 07:59:41 +0000 Subject: [PATCH] fix(cpp): a call of a member template with explicit template arguments has the type the template returns (#2556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A member call was read only when its callee ended in the method's name, so `reflection->GetRaw>(…)` and `factory->GetOptions()` gave the `auto` locals they initialize no type. Such a call now reaches the class's template overloads, and when each is declared to return the same one of its own template parameters (`template const Type& GetRaw(…)`), it returns the call's argument for it. A macro body's `\`-spliced lines are read as one statement. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/cpp-auto-local-receiver.test.ts | 5 +- __tests__/cpp-member-template-returns.test.ts | 189 ++++++++++++++++++ src/resolution/name-matcher.ts | 74 ++++++- 4 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 __tests__/cpp-member-template-returns.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a231a5d..2c3b4b79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -115,6 +115,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a call with a comparison or a shift among its arguments, like protocolbuffers/protobuf's `p->Emit({{"declare", has_bit_word_index < 0 ? … : ""}, …}, "…")` or `WriteVarint(num << 3, s)`, is now read with the right number of arguments, so it links to the overload that fits rather than one taking fewer. A parameter or variable whose type has template arguments is now read even after another such parameter on the same line, like facebook/rocksdb's `const autovector& cf_ids`, or with empty ones, like fmt's `parse_context<>& ctx`: calls on it link to that class's methods, and calls on a `std::vector` declared that way no longer link to a project class's method of the same name. Re-index C++ projects after upgrading. - In C++, a call passing `nullptr` now links to the overload that takes a `std::nullptr_t`, as C++ picks it, rather than one taking a pointer, and a call no longer links to an overload declared `= delete`, which no call that compiles can reach: protocolbuffers/protobuf's `ptr.reset(nullptr)` on an `Arena::UniquePtr` reaches `reset(std::nullptr_t)`, and `field.Add("foo")` on a `RepeatedPtrField` reaches `Add(Element&&)` instead of the deleted `Add(const Element&)`. Re-index C++ projects after upgrading. - In C++, a variable or parameter declared over more than one line, with its type begun on a line above or alone on the line above its name, now has its type read, so calls on it link to that class's methods: protocolbuffers/protobuf's `extension.number()` on a `const google::protobuf::internal::ExtensionIdentifier<` … `>& extension` parameter reaches protobuf's `ExtensionIdentifier`, not hpb's, and facebook/rocksdb's `cf_sv_pairs.size()` on an `autovector<…>` written above `cf_sv_pairs;` reaches `autovector`. Calls on a standard-library container declared that way no longer link to a project class's method of the same name. Re-index C++ projects after upgrading. +- In C++, a variable initialized from a call of a member function template with explicit template arguments now has the type that template returns for them, so calls on it link to that class's methods: protocolbuffers/protobuf's `const auto& rep = reflection->GetRaw>(…)` is a `RepeatedPtrField`, and facebook/rocksdb's `auto bbto = factory->GetOptions()` a `BlockBasedTableOptions`. Re-index C++ projects after upgrading. - Framework support now switches on when an app's `package.json` sits three or more folders deep, as with an ASP.NET solution's Angular or React app in `src/Web/ClientApp/`, or a React app in a workspace folder like `web/ui/mantine-ui/`. Before, only the project root and the first two folder levels were read, so frameworks such as Angular, React Router, Vue Router, SvelteKit and TanStack Router could go undetected there: an Angular app's routes, its `routerLink` and `router.navigate(…)` navigation and its template's event bindings were missing, so a handler used only from a `(click)` binding had no callers, and a React app's `navigate('/login')` calls didn't reach their route. Re-index projects laid out this way after upgrading. - In a repository that mixes languages, a web framework's naming conventions now link only code written in that framework's own languages. Before, Express's also linked Go and Python code: in etcd a Go method's `*zap.Logger` result type linked to the method itself, and in Harbor a Python `self.jaeger.validate()` call linked to another class's `validate`. Links that cross languages on purpose, like React Native and Expo native modules, Swift and Objective-C, Drupal's `routing.yml`, SvelteKit's `$lib/…` imports in `.ts` files and a Razor page's `@model`, are unchanged. - In C and C++, a call written from the global scope, like `::leveldb::RepairDB(…)` or `::hpb::CreateMessage(…)`, now links to the function of exactly that name, and one through a namespace alias, like protobuf's `::_pbi::…` calls, to the function in the aliased namespace. Before, these calls were dropped unless the repository also held Swift and Objective-C files, and even then a name spelled out in full, like `::google::protobuf::internal::TSanWrite(…)`, linked to nothing, while `::operator delete(…)` linked to one class's own `operator delete`. A global call never links to a function or method of the same name inside a namespace or class, such as a wrapper `open()` that calls the C library's `::open(…)`, and `::std::…` and `::testing::…` calls stay unlinked unless that library's source is part of your project. Re-index C and C++ projects after upgrading. diff --git a/__tests__/cpp-auto-local-receiver.test.ts b/__tests__/cpp-auto-local-receiver.test.ts index c179d179..c7aa6670 100644 --- a/__tests__/cpp-auto-local-receiver.test.ts +++ b/__tests__/cpp-auto-local-receiver.test.ts @@ -515,8 +515,9 @@ describe('what keeps the call as it was', () => { ), }); try { - // Not the shared_ptr member's type, which has no SetName. - expect(calls(cg, 'rocksdb::Rename').filter((c) => c.includes('SetName'))).toEqual(['rocksdb::test::ChanglingMergeOperator::SetName @0.7']); + // Not the shared_ptr member's type, which has no SetName: + // the type `CheckedCast()` returns. + expect(calls(cg, 'rocksdb::Rename').filter((c) => c.includes('SetName'))).toEqual(['rocksdb::test::ChanglingMergeOperator::SetName @0.9']); } finally { cg.close(); } diff --git a/__tests__/cpp-member-template-returns.test.ts b/__tests__/cpp-member-template-returns.test.ts new file mode 100644 index 00000000..73b08a72 --- /dev/null +++ b/__tests__/cpp-member-template-returns.test.ts @@ -0,0 +1,189 @@ +/** + * An `auto` local initialized from a call of a C++ member template with + * explicit template arguments has the type the template returns for them. + * A member call was read only when its callee ended in the method's name, so + * `reflection->GetRaw>(…)` was no call it could + * read; and a return type that is the template's own parameter (`template + * const Type& GetRaw(…)`) is what the arguments make it. + * protocolbuffers/protobuf's `rep.size()` on such a local, in a macro whose + * lines end in `\`, guessed `SooRep::size`; facebook/rocksdb's `auto bbto = + * factory->GetOptions();` gave `bbto` no type. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const roots: string[] = []; +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); + +async function indexed(files: Record): Promise { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-template-')); + roots.push(root); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + return CodeGraph.init(root, { index: true }); +} + +const lines = (...text: string[]): string => [...text, ''].join('\n'); + +/** What a function's calls of `method` reach, as `qualifiedName resolvedBy@confidence`. */ +function calls(cg: CodeGraph, qualifiedName: string, method: string): string[] { + const from = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName)!; + return cg.getOutgoingEdgesFrom([from.id], ['calls']) + .map((e) => { + const meta = e.metadata as { confidence?: number; resolvedBy?: string } | undefined; + return `${cg.getNode(e.target)?.qualifiedName} ${meta?.resolvedBy}@${meta?.confidence}`; + }) + .filter((t) => t.split(' ')[0]!.endsWith(`::${method}`)) + .sort(); +} + +const SHAPES = lines( + 'class Widget {', + ' public:', + ' void Draw() {}', + '};', + 'class Gadget {', + ' public:', + ' void Draw() {}', + '};', +); + +describe('C++ member templates called with explicit template arguments', () => { + it("types protobuf's `const auto& rep =` from GetRaw>, its line spliced with `\\`", async () => { + const cg = await indexed({ + 'src/google/protobuf/reflection.h': lines( + 'namespace google {', + 'namespace protobuf {', + 'class Message {};', + 'class FieldDescriptor {};', + 'template ', + 'class RepeatedPtrField {', + ' public:', + ' int size() const { return 0; }', + '};', + 'class SooRep {', + ' public:', + ' int size() const { return 0; }', + '};', + 'class Reflection {', + ' public:', + ' template ', + ' const Type& GetRaw(const Message& message, const FieldDescriptor* field) const;', + '};', + 'template ', + 'const Type& Reflection::GetRaw(const Message& message, const FieldDescriptor* field) const {', + ' static Type value;', + ' return value;', + '}', + '} // namespace protobuf', + '} // namespace google', + ), + 'src/google/protobuf/reflection_visit_fields.h': lines( + '#include "src/google/protobuf/reflection.h"', + 'namespace google {', + 'namespace protobuf {', + 'int VisitFields(const Reflection* reflection, const Message& message,', + ' const FieldDescriptor* field) {', + // protobuf writes this in a macro, whose lines end in `\`. + ' const auto& rep = \\', + ' reflection->GetRaw>(message, field); \\', + ' ;', + ' return rep.size();', + '}', + '} // namespace protobuf', + '} // namespace google', + ), + }); + try { + expect(calls(cg, 'google::protobuf::VisitFields', 'size')).toEqual(['google::protobuf::RepeatedPtrField::size instance-method@0.9']); + } finally { + cg.close(); + } + }); + + it("types rocksdb's `auto bbto =` from GetOptions(), const and not", async () => { + const cg = await indexed({ + 'include/rocksdb/configurable.h': lines( + 'namespace rocksdb {', + 'class Configurable {', + ' public:', + ' template ', + ' const T* GetOptions() const { return nullptr; }', + ' template ', + ' T* GetOptions() { return nullptr; }', + '};', + 'struct BlockBasedTableOptions {', + ' bool IsValid() const { return true; }', + '};', + 'struct PlainTableOptions {', + ' bool IsValid() const { return true; }', + '};', + '} // namespace rocksdb', + ), + 'db/check.cc': lines( + '#include "include/rocksdb/configurable.h"', + 'namespace rocksdb {', + 'bool Check(Configurable* factory) {', + ' auto bbto = factory->GetOptions();', + ' return bbto->IsValid();', + '}', + '} // namespace rocksdb', + ), + }); + try { + expect(calls(cg, 'rocksdb::Check', 'IsValid')).toEqual(['rocksdb::BlockBasedTableOptions::IsValid instance-method@0.9']); + } finally { + cg.close(); + } + }); + + it('reads a template overload only, a defaulted parameter by its name, and no guess between overloads', async () => { + const cg = await indexed({ + 'shapes.h': SHAPES, + 'store.cc': lines( + '#include "shapes.h"', + 'class Store {', + ' public:', + ' Widget* Find() { return nullptr; }', + ' template ', + ' T* Find() { return nullptr; }', + ' template ', + ' T* Make() { return nullptr; }', + ' template ', + ' A* Get() { return nullptr; }', + ' template ', + ' B* Get(int key) { return nullptr; }', + '};', + 'void Found(Store* store) {', + ' auto found = store->Find();', + ' found->Draw();', + '}', + 'void Made(Store* store) {', + ' auto made = store->Make();', + ' made->Draw();', + '}', + 'void Got(Store* store) {', + ' auto got = store->Get(1);', + ' got->Draw();', + '}', + ), + }); + try { + // Explicit template arguments call `T* Find()`, never `Widget* Find()`. + expect(calls(cg, 'Found', 'Draw')).toEqual(['Gadget::Draw instance-method@0.9']); + expect(calls(cg, 'Made', 'Draw')).toEqual(['Gadget::Draw instance-method@0.9']); + // The overloads return different parameters; which one the call takes + // isn't read, so neither is guessed. + expect(calls(cg, 'Got', 'Draw').filter((t) => t.endsWith('@0.9'))).toEqual([]); + } finally { + cg.close(); + } + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 09724c53..d3c99d01 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -9397,6 +9397,61 @@ function cppWrittenReturnType(fn: Node, context: ResolutionContext): string | nu return written; } +/** + * The names of the template parameters a C++ function template's own + * `template <…>` header declares, in order, without their defaults: + * `['Type']` for protobuf's `template const Type& GetRaw(…)`. + * A template's node starts at its declaration, so the header is the code + * right before it. Empty for a function that is no template. + */ +function cppOwnTemplateParameters(fn: Node, context: ResolutionContext): string[] { + const lines = context.getFileLines?.(fn.filePath) ?? context.readFile(fn.filePath)?.split(/\r?\n/); + if (!lines) return []; + const at = lines[fn.startLine - 1] ?? ''; + const before = [...lines.slice(Math.max(0, fn.startLine - 4), fn.startLine - 1), at.slice(0, fn.startColumn)] + .map((line) => cppCodeOf(line) ?? '').join(' ').trimEnd(); + let depth = 0; + for (let i = before.endsWith('>') ? before.length - 1 : -1; i >= 0; i--) { + if (before[i] === '>') depth++; + else if (before[i] === '<' && --depth === 0) { + if (!/\btemplate\s*$/.test(before.slice(0, i))) return []; + return splitCppTopLevel(before.slice(i + 1, -1)).map((p) => /([A-Za-z_]\w*)$/.exec(p.split('=')[0]!.trim())?.[1] ?? ''); + } + } + return []; +} + +/** + * What a call of the C++ member template whose overloads are `methods`, with + * the explicit template arguments `args` (`<…>` as written), returns when + * every overload is declared to return the same one of its own template + * parameters, perhaps const, a pointer or a reference: that argument, so + * qualified, as written at the call (`site`). protobuf's `template const Type& GetRaw(…)` called as `reflection->GetRaw>(…)` returns a `const RepeatedPtrField&`, and + * rocksdb's `T* GetOptions()` called as `GetOptions()` + * a `BlockBasedTableOptions*`. Undefined otherwise: then the return type is + * read as written. + */ +function cppMemberTemplateReturn( + methods: readonly Node[], + args: string, + site: Omit, + context: ResolutionContext, +): CppWrittenType | undefined { + let slot: { index: number; written: string; param: string } | undefined; + for (const fn of methods) { + const written = cppWrittenReturnType(fn, context); + if (!written) continue; + const param = written.replace(/\b(?:const|volatile)\b|[*&]/g, ' ').trim(); + const index = param ? cppOwnTemplateParameters(fn, context).indexOf(param) : -1; + if (index < 0 || (slot && slot.index !== index)) return undefined; + slot ??= { index, written, param }; + } + const arg = slot && splitCppTopLevel(args.slice(1, -1))[slot.index]; + return slot && arg ? { ...site, text: slot.written.replace(new RegExp(`\\b${slot.param}\\b`), arg) } : undefined; +} + /** What one of `fns` (overloads, a declaration and its definition) returns, as written where it is declared. */ function cppReturnTypeOf(fns: readonly Node[], context: ResolutionContext): CppWrittenType | null { for (const fn of fns) { @@ -9785,12 +9840,17 @@ function cppExpressionType(expr: string, at: UnresolvedRef, context: ResolutionC const returned = wrapper && cppWrapperReturnType(wrapper); if (returned) return returned; } - // `recv.method(…)`, `recv->method(…)`: the method of the receiver's class. - const member = /^(.+?)\s*(\.|->)\s*([A-Za-z_]\w*)$/.exec(callee); + // `recv.method(…)`, `recv->method(…)`, `recv->method(…)`: the method of the receiver's class. + const member = /^(.+?)\s*(\.|->)\s*([A-Za-z_]\w*)\s*(<[\s\S]*>)?$/.exec(callee); if (member) { const object = cppMemberObject(member[1]!, member[2]!, at, context, depth); const cls = object && cppWrittenClass(object, at, context); - return cls ? cppReturnTypeOf(cppMethodsOf(cls, member[3]!, at, context), context) : null; + if (!cls) return null; + const methods = cppMethodsOf(cls, member[3]!, at, context); + if (!member[4]) return cppReturnTypeOf(methods, context); + // Explicit template arguments call a template, never an overload that is none. + const templates = methods.filter((fn) => cppOwnTemplateParameters(fn, context).length > 0); + return cppMemberTemplateReturn(templates, member[4], callers, context) ?? cppReturnTypeOf(templates, context); } if (!/^(?:[A-Za-z_]\w*\s*::\s*)*[A-Za-z_]\w*(?:\s*<[\s\S]*>)?$/.test(callee)) return null; const spelled = stripCppTemplateArguments(callee).replace(/\s+/g, ''); @@ -11354,7 +11414,9 @@ function cppAutoInitializerStatement( /** * The C++ statement that `text`, the rest of `lines[index]`, begins, read on - * to the `;` that ends it. Null when no `;` ends it within a few lines. + * to the `;` that ends it. Null when no `;` ends it within a few lines. A + * macro body's lines end in `\`, which splices them: protobuf's `const auto& + * rep = \` goes on with `reflection->GetRaw<…>(message, field); \`. */ function cppStatementFrom( text: string, @@ -11362,11 +11424,13 @@ function cppStatementFrom( index: number, rawLines: ReturnType, ): string | null { + const spliced = (code: string): string => code.replace(/\\\s*$/, ' '); + text = spliced(text); for (let next = index + 1; ; next++) { const end = cppStatementEnd(text); if (end >= 0) return text.slice(0, end).trim() || null; if (next >= lines.length || next > index + 4) return null; - text += ` ${cppCodeOf(cppLiteralsBlanked(lines[next]!, next + 1, rawLines)) ?? ''}`; + text += ` ${spliced(cppCodeOf(cppLiteralsBlanked(lines[next]!, next + 1, rawLines)) ?? '')}`; } }