From 672a763ef30677e3bdc5cae9f69e2de6d4d74d00 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Wed, 14 Oct 2026 23:20:41 -0500 Subject: [PATCH] fix(cpp): an alias of a type written from the global scope names that global type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit protobuf's hpb declares `class Arena { … backend::Arena arena_; }`, where `namespace backend = hpb::internal::backend::upb;` and that namespace says `using Arena = ::upb::Arena;`. cppTypeName dropped the leading `::`, so the alias's target was looked up as `upb::Arena` from the alias's own scope, whose nearest `upb` is that very namespace: the alias named itself, the lookup ended on no class, and `arena_.Fuse(other.arena_)` fell back to the type's last name, linking `hpb::Arena::Fuse` (and `IsFused`, `RefArena`) to itself instead of `upb::Arena::Fuse`. TypeName now records a leading `::` (`global`), and a global name is looked up from the global scope alone: when an alias's target is followed (follow, and the scope expand looks the target up from and hands on), in a class's base clause (baseClassesOf), and in cppAliasedTypeWritten, which #2532's `auto` locals and chains walk alias by alias. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- __tests__/cpp-global-alias-target.test.ts | 216 ++++++++++++++++++++++ src/resolution/cpp-type-aliases.ts | 33 +++- 3 files changed, 241 insertions(+), 10 deletions(-) create mode 100644 __tests__/cpp-global-alias-target.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 325e1fd962..384ddbee00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,7 +87,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - An import in a Vue, Svelte or Astro component now links the component to the file it imports, as an import in a script does, and so does an import of a scoped package that a `tsconfig` path or a workspace package maps to a project file, like `import { debounce } from '@excalidraw/common'`. Before, only the names such an import binds were linked, so a barrel or a module imported for its side effects showed no dependency from those files in `codegraph affected`, and `codegraph sync` could not update them when a barrel they import through was deleted or added. `codegraph sync` and the file watcher also link such an import again, and one written as `.` or `..`, when the file it names comes back. Re-index JavaScript, TypeScript, Vue, Svelte and Astro projects after upgrading. - `codegraph sync` and the file watcher now move an import written without an extension, like `import { sizeOf } from './button'`, to a file added beside the one it found before when the new file comes first, such as `button.ts` next to `button.vue`. Before, the import and the names it binds stayed linked to `button.vue` until the importing file changed or the project was re-indexed. JavaScript, TypeScript, Vue, Svelte and Astro. - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading. -- In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading. +- In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. An alias of a type written from the global scope names that global type, also inside a namespace of the same name: protocolbuffers/protobuf's `using Arena = ::upb::Arena;` in `hpb::internal::backend::upb`, so hpb's `arena_.Fuse(…)` on a `backend::Arena` now reaches `upb::Arena::Fuse` instead of the calling method itself. Re-index C++ projects after upgrading. - In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector>` or `Striped>`, now links to the method of the class it is declared as. Before, the nested arguments hid that class: in RocksDB, `files_marked_for_compaction_.clear()` on such an `autovector` linked to an unrelated `CompactionInputFiles::clear` that only shares words with the variable's name, and `mutex_.Get(key)` on a `Striped` linked to nothing. Re-index C++ projects after upgrading. - 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. diff --git a/__tests__/cpp-global-alias-target.test.ts b/__tests__/cpp-global-alias-target.test.ts new file mode 100644 index 0000000000..5c6754c981 --- /dev/null +++ b/__tests__/cpp-global-alias-target.test.ts @@ -0,0 +1,216 @@ +/** + * A C++ type written from the global scope (`::upb::Arena`) is looked up there + * alone, also where an alias names it and where a class's base clause or a + * declaration writes it. + * + * protocolbuffers/protobuf's hpb declares `class Arena { … backend::Arena + * arena_; }`, where `namespace backend = hpb::internal::backend::upb;` and + * that namespace says `using Arena = ::upb::Arena;`. The alias was looked up + * as if written `upb::Arena`, from its own namespace, whose nearest `upb` is + * that namespace itself: the alias named itself, no class was found, and + * `arena_.Fuse(other.arena_)` fell back to the type's last name, landing on + * the calling method `hpb::Arena::Fuse`. + */ +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'; +import type { Node } from '../src/types'; + +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-global-alias-')); + 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 }); +} + +function callable(cg: CodeGraph, qualifiedName: string): Node { + const node = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName); + if (!node) throw new Error(`no function or method ${qualifiedName}`); + return node; +} + +/** `calls` callees of a function or method, as `qualifiedName`. */ +function calls(cg: CodeGraph, caller: string): string[] { + return cg + .getCallees(callable(cg, caller).id) + .filter((r) => r.edge.kind === 'calls') + .map((r) => r.node.qualifiedName) + .sort(); +} + +const lines = (...l: string[]): string => [...l, ''].join('\n'); + +const UPB_ARENA = lines( + 'namespace upb {', + 'class Arena {', + ' public:', + ' bool Fuse(Arena& other) { return true; }', + ' bool IsFused(Arena& other) const { return false; }', + ' void RefArena(const Arena& to) {}', + '};', + '} // namespace upb', +); + +describe('a C++ alias of a type written from the global scope', () => { + it('reproduction: protobuf hpb\'s `backend::Arena` is `upb::Arena`, not the calling method\'s own class', async () => { + const cg = await indexed({ + 'upb/mem/arena.hpp': UPB_ARENA, + 'hpb/multibackend.h': lines( + '#define HPB_INTERNAL_BACKEND_UPB 1', + '#define HPB_INTERNAL_BACKEND_CPP 2', + 'namespace hpb {', + 'namespace internal {', + 'namespace backend {', + 'namespace upb {}', + 'namespace cpp {}', + '} // namespace backend', + '} // namespace internal', + '#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB', + 'namespace backend = hpb::internal::backend::upb;', + '#elif HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_CPP', + 'namespace backend = hpb::internal::backend::cpp;', + '#endif', + '} // namespace hpb', + ), + 'hpb/backend/types.h': lines( + '#include "hpb/multibackend.h"', + '#include "upb/mem/arena.hpp"', + 'namespace hpb {', + 'namespace internal {', + 'namespace backend {', + '#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB', + 'namespace upb {', + 'using Arena = ::upb::Arena;', + '}', + '#elif HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_CPP', + 'namespace cpp {', + 'using Arena = google::protobuf::Arena;', + '}', + '#endif', + '} // namespace backend', + '} // namespace internal', + '} // namespace hpb', + ), + 'hpb/arena.h': lines( + '#include "hpb/backend/types.h"', + 'namespace hpb {', + 'class Arena {', + ' public:', + ' bool Fuse(Arena& other) { return arena_.Fuse(other.arena_); };', + ' bool IsFused(Arena& other) { return arena_.IsFused(other.arena_); };', + ' void RefArena(const Arena& to) { arena_.RefArena(to.arena_); }', + ' private:', + ' backend::Arena arena_;', + '};', + '} // namespace hpb', + ), + }); + try { + expect(calls(cg, 'hpb::Arena::Fuse')).toEqual(['upb::Arena::Fuse']); + expect(calls(cg, 'hpb::Arena::IsFused')).toEqual(['upb::Arena::IsFused']); + expect(calls(cg, 'hpb::Arena::RefArena')).toEqual(['upb::Arena::RefArena']); + } finally { + cg.close(); + } + }); + + it('names the global class, not a namesake in the scope around the alias', async () => { + const cg = await indexed({ + 'upb/mem/arena.hpp': UPB_ARENA, + 'hpb/arena.cc': lines( + '#include "upb/mem/arena.hpp"', + 'namespace hpb {', + 'namespace upb {', + 'class Arena {', + ' public:', + ' bool Fuse(Arena& other) { return false; }', + '};', + '} // namespace upb', + 'using GlobalArena = ::upb::Arena;', + 'bool Join(GlobalArena& a, GlobalArena& b) {', + ' return a.Fuse(b);', + '}', + '} // namespace hpb', + ), + }); + try { + expect(calls(cg, 'hpb::Join')).toEqual(['upb::Arena::Fuse']); + } finally { + cg.close(); + } + }); + + it('through an `auto` variable the alias types', async () => { + const cg = await indexed({ + 'upb/mem/arena.hpp': UPB_ARENA, + 'hpb/arena.cc': lines( + '#include "upb/mem/arena.hpp"', + 'namespace hpb {', + 'namespace upb {', + 'using Arena = ::upb::Arena;', + '} // namespace upb', + 'class Arena {', + ' public:', + ' bool Fuse(Arena& other) { return false; }', + '};', + 'upb::Arena MakeArena() { return upb::Arena(); }', + 'bool Join(upb::Arena& other) {', + ' auto made = MakeArena();', + ' return made.Fuse(other);', + '}', + '} // namespace hpb', + ), + }); + try { + expect(calls(cg, 'hpb::Join').filter((c) => c.endsWith('::Fuse'))).toEqual(['upb::Arena::Fuse']); + } finally { + cg.close(); + } + }); +}); + +describe('other C++ names written from the global scope', () => { + it('a base class: `::lib::Options`, not the namespace\'s own `lib::Options`', async () => { + const cg = await indexed({ + 'src/config.cc': lines( + 'namespace app {', + 'struct Inner {', + ' void Run() {}', + '};', + '} // namespace app', + 'namespace lib {', + 'struct Options {', + ' struct Inner {', + ' void Run() {}', + ' };', + '};', + '} // namespace lib', + 'namespace app {', + 'namespace lib {', + 'struct Options {};', + '} // namespace lib', + 'class Config : public ::lib::Options {};', + 'void Use() {', + ' Config::Inner inner;', + ' inner.Run();', + '}', + '} // namespace app', + ), + }); + try { + expect(calls(cg, 'app::Use')).toEqual(['lib::Options::Inner::Run']); + } finally { + cg.close(); + } + }); +}); diff --git a/src/resolution/cpp-type-aliases.ts b/src/resolution/cpp-type-aliases.ts index 08fa66d17b..a708e11ca4 100644 --- a/src/resolution/cpp-type-aliases.ts +++ b/src/resolution/cpp-type-aliases.ts @@ -62,6 +62,13 @@ export interface CppAliasedType { interface TypeName { names: string[]; templated: boolean[]; + /** + * Written from the global scope (`::upb::Arena`), so looked up there alone: + * protobuf's `namespace hpb::internal::backend::upb { using Arena = + * ::upb::Arena; }` would otherwise find its own namespace's `upb::Arena`, + * the alias itself. + */ + global?: boolean; } /** `SkipList>::Iterator` → `SkipList::Iterator`. */ @@ -105,13 +112,19 @@ function cppTypeName(raw: string): TypeName | null { } parts.push(part.trim()); templated.push(args); - if (parts[0] === '') { - // A leading `::` names the global scope. + // A leading `::` names the global scope. + const global = parts[0] === ''; + if (global) { parts.shift(); templated.shift(); } if (parts.length === 0 || !parts.every((p) => /^[A-Za-z_]\w*$/.test(p))) return null; - return { names: parts, templated }; + return global ? { names: parts, templated, global } : { names: parts, templated }; +} + +/** The scopes a written name is looked up from: the global scope alone for `::Name`. */ +function scopesFor(name: TypeName, scopes: readonly string[]): readonly string[] { + return name.global ? [''] : scopes; } /** Is a written type a pointer (`Table*`, `const Foo* const`), not counting its template arguments? */ @@ -430,7 +443,7 @@ function baseClassesOf(cls: string, context: ResolutionContext): string[] { for (const part of splitTopLevel(text.slice(colon + 1, brace))) { const name = cppTypeName(part.replace(/\b(?:public|protected|private|virtual)\b/g, ' ')); if (!name) continue; - const base = classNamed(name.names, scopesWithin(parentScope(cls)), context); + const base = classNamed(name.names, scopesFor(name, scopesWithin(parentScope(cls))), context); if (base && base !== cls && !bases.includes(base)) bases.push(base); } } @@ -621,7 +634,7 @@ export function cppAliasedTypeWritten( ): { text: string; alias: Node } | null | undefined { const name = cppTypeName(written); if (!name) return undefined; - const { found, next } = lookupSegments(name, scopes, ref, context); + const { found, next } = lookupSegments(name, scopesFor(name, scopes), ref, context); if (!found || typeof found !== 'object' || 'cls' in found) return undefined; if ('ambiguous' in found || next !== name.names.length || isAliasTemplate(found.alias, context)) return null; const text = declarationText(found.alias, context); @@ -689,7 +702,7 @@ function follow( context: ResolutionContext, hops: number, ): CppAliasedType | null | undefined { - const { found, next } = lookupSegments(name, scopesWithin(scope), ref, context); + const { found, next } = lookupSegments(name, scopesFor(name, scopesWithin(scope)), ref, context); if (!found || typeof found !== 'object' || !('alias' in found)) return undefined; const alias = found.alias; if (hops >= MAX_HOPS) return null; @@ -721,11 +734,13 @@ function expand( if (!target || parameters?.has(target.names[0]!)) return null; const pointer = isCppPointerType(written); const names = [...target.names, ...rest]; - const further = follow({ names, templated: [...target.templated, ...restTemplated] }, scope, ref, context, hops); + const further = follow({ names, templated: [...target.templated, ...restTemplated], global: target.global }, scope, ref, context, hops); if (further === null) return null; if (further) return { ...further, pointer: further.pointer || pointer }; - const cls = classNamed(names, scopesWithin(scope), context); + // `::upb::Arena` is looked up from the global scope, whatever scope writes it. + const from = target.global ? '' : scope; + const cls = classNamed(names, scopesWithin(from), context); const classTemplate = !!cls && context.getNodesByQualifiedName(cls) .some((n) => isCFamily(n) && CLASS_KINDS.has(n.kind) && templateParametersAround(n, context).size > 0); - return { target: target.names, rest: [...rest], scope, pointer, classTemplate }; + return { target: target.names, rest: [...rest], scope: from, pointer, classTemplate }; }