fix(cpp): an alias of a type written from the global scope names that global type

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<const char*, KeyComparator> 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<const char*, KeyComparator> 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<std::pair<int, FileMetaData*>>` or `Striped<CacheAlignedWrapper<port::Mutex>>`, 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.

__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<string, string>): Promise<CodeGraph> {
+  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();
+    }
+  });
+});

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<const char*, Cmp<int>>::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 };
 }