mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 13:53:01 +08:00
fix(cpp): a gtest TEST_F or TEST_P body finds its fixture's types, and a function's local class is seen only from that function (#2551)
A TEST_F / TEST_P body is a member function of a class derived from its fixture, so its unqualified names find the fixture's nested types (its bases' included) before the enclosing namespaces'. And since the index names a function's local class after the function, every TEST_F body's local `FakeWF` or `MyManager` shared one qualified name: a class local to a function is now visible only from inside that function, and a namesake class's methods are its own body's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -110,6 +110,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In C++, a nested class defined outside the class it belongs to, like google/leveldb's `struct SkipList<Key, Comparator>::Node { … }` or facebook/rocksdb's `struct BlockBasedTable::Rep { … }`, is now indexed under its own name, `SkipList::Node`, with its methods and members. Before, it was indexed as an unnamed class, so it was missing from search and `codegraph_explore`, its methods belonged to no class you could name, and a call like `x->Next(level)` on a `Node*` was matched by the method's name alone. Creating an object, as in `new log::Writer(…)` or `Writer w(…)`, now links to the class C++ itself finds for the type as written there: leveldb's `new log::Writer(…)` in `DBImpl` methods no longer links to `DBImpl`'s own `Writer`, RocksDB's tests' `Random rnd(301)` links to RocksDB's `Random` instead of googletest's, an object created through an alias links to the class the alias names, and one links to its class rather than to the class's constructor. An object of a template parameter's type, or of a type the function names with its own `using`, like protocolbuffers/protobuf's `using Map = std::map<…>`, no longer links to a project class that only shares the name. Re-index C++ projects after upgrading.
|
||||
- In C++, a call whose receiver's type can't be read no longer links to a method of a class nested in the class the receiver's name suggests, or of a class whose name only contains that name. Before, such a guess took any method of the call's name whose qualified name contained the class's name: protocolbuffers/protobuf's `descriptor->file()` in a template reached `Descriptor::ExtensionRange::file`, and facebook/rocksdb's `CompositeEnv::IsInstanceOf(name)` reached `CompositeEnvWrapper::IsInstanceOf`. A call written through a class, like `PessimisticTransaction::PopSavePoint()` in an override, now links to the method that class inherits, here `TransactionBaseImpl::PopSavePoint`. Re-index C++ projects after upgrading.
|
||||
- In C++, an alias that names a class through a class derived from the one that declares it, like protocolbuffers/protobuf's `using ConformanceRequestSetting = BinaryAndJsonConformanceSuite::ConformanceRequestSetting;`, now leads to that class, `ConformanceTestSuite::ConformanceRequestSetting`. Before, an alias was followed only to a class declared under exactly the name it spells, so objects created through this one linked to the alias rather than the class. Re-index C++ projects after upgrading.
|
||||
- In C++, a name written in a googletest `TEST_F` or `TEST_P` body now finds the test fixture's nested types, those of its base classes included, before anything in the surrounding namespace, as C++ does for the member function such a body is: facebook/rocksdb's `new TestUserDefinedIndexFactory()` links to the class nested in the fixture's base. A class declared inside a test body, or inside any function, is now only found from that function, so the local classes many RocksDB tests each declare under one name, like `FakeWF` or `MyManager`, no longer stand in for one another, and a call like `fwf->SetIOError(true)` reaches the `FakeWF` its own test declares. 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.
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
/**
|
||||
* A gtest `TEST_F(Fixture, Name) { … }` or `TEST_P` body is a member function
|
||||
* of a class derived from its fixture, so an unqualified name in it finds the
|
||||
* body's own declarations, then the fixture's members, its bases' included,
|
||||
* then the enclosing namespaces'. C++ lookup from the body started at the
|
||||
* function and went straight to the namespaces, so facebook/rocksdb's
|
||||
* `new TestUserDefinedIndexFactory()`, a class nested in the fixture's base,
|
||||
* was found only as the one class of that name the file could see (0.7), and
|
||||
* a namespace-level class of the same name won over the fixture's.
|
||||
*/
|
||||
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-fixture-scope-'));
|
||||
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');
|
||||
|
||||
/** The `instantiates` edges of every function named `name`, by its first line, as `startLine -> qualifiedName @confidence`. */
|
||||
function made(cg: CodeGraph, name: string): string[] {
|
||||
return cg.getNodesByKind('function').filter((n: Node) => n.name === name).sort((a, b) => a.startLine - b.startLine)
|
||||
.flatMap((fn) => cg.getOutgoingEdgesFrom([fn.id], ['instantiates'])
|
||||
.map((e) => `${fn.startLine} -> ${cg.getNode(e.target)?.qualifiedName} @${(e.metadata as { confidence?: number } | undefined)?.confidence}`));
|
||||
}
|
||||
|
||||
describe('C++ names in a gtest TEST_F or TEST_P body', () => {
|
||||
it("find the fixture's nested types, its bases' included", async () => {
|
||||
const cg = await indexed({
|
||||
'table/table_test.cc': lines(
|
||||
'namespace rocksdb {',
|
||||
'class UserDefinedIndexTestBase : public testing::Test {',
|
||||
' public:',
|
||||
' class TestUserDefinedIndexFactory {',
|
||||
' public:',
|
||||
' TestUserDefinedIndexFactory() {}',
|
||||
' };',
|
||||
'};',
|
||||
'class UserDefinedIndexTest : public UserDefinedIndexTestBase {};',
|
||||
'TEST_F(UserDefinedIndexTest, Basic) {',
|
||||
' auto factory = new TestUserDefinedIndexFactory();',
|
||||
'}',
|
||||
'TEST_P(UserDefinedIndexTest, Params) {',
|
||||
' auto factory = new TestUserDefinedIndexFactory();',
|
||||
'}',
|
||||
'} // namespace rocksdb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
const nested = 'rocksdb::UserDefinedIndexTestBase::TestUserDefinedIndexFactory @0.9';
|
||||
expect(made(cg, 'TEST_F')).toEqual([`10 -> ${nested}`]);
|
||||
expect(made(cg, 'TEST_P')).toEqual([`13 -> ${nested}`]);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("find the fixture's type before a namespace's, and the body's own before both", async () => {
|
||||
const cg = await indexed({
|
||||
'db/db_test.cc': lines(
|
||||
'namespace rocksdb {',
|
||||
'class Listener {',
|
||||
' public:',
|
||||
' explicit Listener(int n) {}',
|
||||
'};',
|
||||
'class DBTest : public testing::Test {',
|
||||
' public:',
|
||||
' class Listener {',
|
||||
' public:',
|
||||
' explicit Listener(int n) {}',
|
||||
' };',
|
||||
'};',
|
||||
'TEST_F(DBTest, Fixture) {',
|
||||
' Listener listener(1);',
|
||||
'}',
|
||||
'TEST_F(DBTest, Local) {',
|
||||
' class Listener {',
|
||||
' public:',
|
||||
' explicit Listener(int n) {}',
|
||||
' };',
|
||||
' Listener listener(2);',
|
||||
'}',
|
||||
'TEST(DBPlainTest, NoFixture) {',
|
||||
' Listener listener(3);',
|
||||
'}',
|
||||
'void Helper() { Listener listener(4); }',
|
||||
'} // namespace rocksdb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(made(cg, 'TEST_F')).toEqual(['13 -> rocksdb::DBTest::Listener @0.9', '16 -> rocksdb::TEST_F::Listener @0.9']);
|
||||
expect(made(cg, 'TEST')).toEqual(['23 -> rocksdb::Listener @0.9']);
|
||||
expect(made(cg, 'Helper')).toEqual(['26 -> rocksdb::Listener @0.9']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("call the methods of the body's own local class, not another body's", async () => {
|
||||
const cg = await indexed({
|
||||
'db/listener_test.cc': lines(
|
||||
'namespace rocksdb {',
|
||||
'class ListenerTest : public testing::Test {};',
|
||||
'TEST_F(ListenerTest, First) {',
|
||||
' class Listener {',
|
||||
' public:',
|
||||
' void Fire() {}',
|
||||
' };',
|
||||
' Listener listener;',
|
||||
' listener.Fire();',
|
||||
'}',
|
||||
'TEST_F(ListenerTest, Second) {',
|
||||
' class Listener {',
|
||||
' public:',
|
||||
' void Fire() {}',
|
||||
' };',
|
||||
' Listener listener;',
|
||||
' listener.Fire();',
|
||||
'}',
|
||||
'} // namespace rocksdb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
const fired = cg.getNodesByKind('function').filter((n: Node) => n.name === 'TEST_F').sort((a, b) => a.startLine - b.startLine)
|
||||
.map((fn) => cg.getOutgoingEdgesFrom([fn.id], ['calls'])
|
||||
.map((e) => `${fn.startLine} -> ${cg.getNode(e.target)?.qualifiedName}:${cg.getNode(e.target)?.startLine} @${(e.metadata as { confidence?: number } | undefined)?.confidence}`));
|
||||
expect(fired).toEqual([['3 -> rocksdb::TEST_F::Listener::Fire:6 @0.9'], ['11 -> rocksdb::TEST_F::Listener::Fire:14 @0.9']]);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("find the body's own local class from a method of that class", async () => {
|
||||
// rocksdb's env_test.cc: two bodies each declare a `CB`, and a static
|
||||
// method of the second's calls `cb->Run()` on a `CB*`.
|
||||
const cg = await indexed({
|
||||
'env/env_test.cc': lines(
|
||||
'namespace rocksdb {',
|
||||
'class EnvTest : public testing::Test {};',
|
||||
'TEST_F(EnvTest, RunMany) {',
|
||||
' struct CB {',
|
||||
' void Run() {}',
|
||||
' };',
|
||||
'}',
|
||||
'TEST_F(EnvTest, TwoPools) {',
|
||||
' class CB {',
|
||||
' public:',
|
||||
' static void Trampoline(void* v) {',
|
||||
' CB* cb = static_cast<CB*>(v);',
|
||||
' cb->Run();',
|
||||
' }',
|
||||
' void Run() {}',
|
||||
' };',
|
||||
'}',
|
||||
'} // namespace rocksdb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
const from = cg.getNodesByKind('method').find((n: Node) => n.name === 'Trampoline')!;
|
||||
const calls = cg.getOutgoingEdgesFrom([from.id], ['calls'])
|
||||
.map((e) => `${cg.getNode(e.target)?.qualifiedName}:${cg.getNode(e.target)?.startLine} @${(e.metadata as { confidence?: number } | undefined)?.confidence}`);
|
||||
expect(calls).toEqual(['rocksdb::TEST_F::CB::Run:15 @0.9']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -75,6 +75,42 @@ function isCFamily(n: Node): boolean {
|
||||
return n.language === 'cpp' || n.language === 'c';
|
||||
}
|
||||
|
||||
/**
|
||||
* The declarations code at `site` can see (visibleFrom), where a class local
|
||||
* to a function counts only in that function's lines: the index names it
|
||||
* after the function, and every gtest `TEST_F` body in a namespace is
|
||||
* `ns::TEST_F`, so one body's local `Listener` passed for any other's — in
|
||||
* the body itself, and in the methods of a local class of its own.
|
||||
*/
|
||||
function seenFrom(site: CppNameSite, context: ResolutionContext): (n: Node) => boolean {
|
||||
const visible = visibleFrom(site.file);
|
||||
return (n) => {
|
||||
if (!visible(n)) return false;
|
||||
const fn = cppFunctionAround(n, context);
|
||||
return !fn || (fn.filePath === site.file && site.line >= fn.startLine && site.line <= fn.endLine);
|
||||
};
|
||||
}
|
||||
|
||||
const LOCAL_TO = new WeakMap<ResolutionContext, Map<string, Node | null>>();
|
||||
|
||||
/**
|
||||
* The function a C or C++ class is local to: the one of the scope its
|
||||
* qualified name is in whose lines hold it. Null for a class of a namespace
|
||||
* or of another class.
|
||||
*/
|
||||
function cppFunctionAround(cls: Node, context: ResolutionContext): Node | null {
|
||||
let memo = LOCAL_TO.get(context);
|
||||
if (!memo) LOCAL_TO.set(context, (memo = new Map()));
|
||||
const hit = memo.get(cls.id);
|
||||
if (hit !== undefined) return hit;
|
||||
const scope = cppParentScope(cls.qualifiedName);
|
||||
const fn = (scope ? context.getNodesByQualifiedName(scope) : []).find((f) =>
|
||||
(f.kind === 'function' || f.kind === 'method') && f.filePath === cls.filePath &&
|
||||
f.startLine <= cls.startLine && f.endLine >= cls.startLine) ?? null;
|
||||
memo.set(cls.id, fn);
|
||||
return fn;
|
||||
}
|
||||
|
||||
/** The declarations code in `file` can see: a header's, and the file's own. */
|
||||
function visibleFrom(file: string): (n: Node) => boolean {
|
||||
return (n) => n.filePath === file || !SOURCE_FILE.test(n.filePath);
|
||||
@@ -110,6 +146,13 @@ export interface CppNameSite {
|
||||
parameters: ReadonlySet<string>;
|
||||
/** A class the name cannot be: the deriving class, in its own base clause. */
|
||||
self?: Node;
|
||||
/**
|
||||
* A class looked in right after `scope` itself, its bases included: a gtest
|
||||
* `TEST_F` body is a member function of a class derived from its fixture,
|
||||
* so the fixture's nested types are what its unqualified names find
|
||||
* before the enclosing namespaces'.
|
||||
*/
|
||||
classScope?: string;
|
||||
}
|
||||
|
||||
/** The class a C or C++ type name names. */
|
||||
@@ -133,8 +176,9 @@ export function cppClassWritten(written: string, site: CppNameSite, ref: Unresol
|
||||
const global = written.startsWith('::');
|
||||
if (!global && site.parameters.has(segments[0]!)) return null;
|
||||
|
||||
const visible = visibleFrom(site.file);
|
||||
const visible = seenFrom(site, context);
|
||||
const scopes = cppScopesWithin(site.scope);
|
||||
if (site.classScope) scopes.splice(1, 0, site.classScope);
|
||||
const resolved = (node: Node): CppWrittenClass => ({ node, exact: true });
|
||||
|
||||
// The scopes around the name short of the global one, or only the global one for `::Base`.
|
||||
@@ -234,7 +278,7 @@ function classNode(named: CppNamedClass, site: CppNameSite, context: ResolutionC
|
||||
.filter((n) => isCFamily(n) && CLASS_KINDS.has(n.kind) && n.id !== site.self?.id);
|
||||
// A lookup that ended on the class itself (its namespace lost) found nothing.
|
||||
if (classes.length === 0) return undefined;
|
||||
const visible = classes.filter(visibleFrom(site.file));
|
||||
const visible = classes.filter(seenFrom(site, context));
|
||||
return pick(visible.length > 0 ? visible : classes, site.file);
|
||||
}
|
||||
|
||||
@@ -288,9 +332,10 @@ interface CppUsing {
|
||||
|
||||
const USINGS = new WeakMap<ResolutionContext, Map<string, CppUsing[]>>();
|
||||
|
||||
/** Drop the per-file `using` memo (see ReferenceResolver.clearCaches). */
|
||||
/** Drop the per-file `using` memo and the local-class memo (see ReferenceResolver.clearCaches). */
|
||||
export function clearCppSupertypeMemos(context: ResolutionContext): void {
|
||||
USINGS.delete(context);
|
||||
LOCAL_TO.delete(context);
|
||||
}
|
||||
|
||||
const PATH = String.raw`(?:::\s*)?[A-Za-z_]\w*(?:\s*::\s*[A-Za-z_]\w*)*`;
|
||||
|
||||
@@ -10163,13 +10163,21 @@ function matchCppHeldTypeCall(raw: string, methodName: string, ref: UnresolvedRe
|
||||
|
||||
const CPP_CLASS_KINDS: ReadonlySet<string> = new Set(['class', 'struct', 'union']);
|
||||
|
||||
/** Where a type the calling function writes is looked up from: the function itself, so a class local to it is found first. */
|
||||
/**
|
||||
* Where a type the calling function writes is looked up from: the function
|
||||
* itself, so a class local to it is found first, then — in a gtest `TEST_F`
|
||||
* or `TEST_P` body — the test's fixture (cppTestFixture) and its bases, as
|
||||
* for the member function of a class derived from it that the body is:
|
||||
* rocksdb's `new TestUserDefinedIndexFactory()` is the fixture base's own.
|
||||
*/
|
||||
function cppCallSite(caller: Node, ref: UnresolvedRef, context: ResolutionContext): CppNameSite {
|
||||
const fixture = cppTestFixture(caller, ref, context);
|
||||
return {
|
||||
scope: caller.kind === 'file' ? '' : caller.qualifiedName,
|
||||
file: ref.filePath,
|
||||
line: ref.line,
|
||||
parameters: cppTemplateParameters(caller, context),
|
||||
...(fixture ? { classScope: fixture } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -10333,10 +10341,12 @@ function cppBasesKnown(cls: Node, context: ResolutionContext, depth = 0, seen =
|
||||
* call's arguments fit (cppOverloadFor) — else the nearest one of a class
|
||||
* it derives from, through the base edges every declaration of `cls` has.
|
||||
* When other classes share `cls`'s qualified name, its own are the ones
|
||||
* defined where it is: leveldb's db_bench.cc and db_bench_sqlite3.cc each
|
||||
* declare a `RandomGenerator`, with a `Generate` that takes a `size_t` and
|
||||
* an `int`. (A chained call `inner(…).name(…)` keeps the first: the
|
||||
* arguments at its position are the inner call's.)
|
||||
* defined in its body, else where it is: the local classes of gtest bodies
|
||||
* in one namespace are each `ns::TEST_F::Listener`, and leveldb's
|
||||
* db_bench.cc and db_bench_sqlite3.cc each declare a `RandomGenerator`, with
|
||||
* a `Generate` that takes a `size_t` and an `int`. (A chained call
|
||||
* `inner(…).name(…)` keeps the first: the arguments at its position are the
|
||||
* inner call's.)
|
||||
*/
|
||||
function cppMethodOf(cls: Node, name: string, ref: UnresolvedRef, context: ResolutionContext, depth = 0, seen = new Set<string>()): Node | null {
|
||||
const want = `${cls.qualifiedName}::${name}`;
|
||||
@@ -10346,7 +10356,11 @@ function cppMethodOf(cls: Node, name: string, ref: UnresolvedRef, context: Resol
|
||||
if (own.length > 0) {
|
||||
const ordered = preferCallSiteFile(cppVisibleFirst(own, ref, context), ref.filePath);
|
||||
if (ref.referenceName.includes('().')) return ordered[0]!;
|
||||
const defined = cppClassDecls(cls.qualifiedName, context).length > 1
|
||||
const namesakes = cppClassDecls(cls.qualifiedName, context).length > 1;
|
||||
const inBody = namesakes
|
||||
? ordered.filter((n) => n.filePath === cls.filePath && n.startLine >= cls.startLine && n.endLine <= cls.endLine)
|
||||
: [];
|
||||
const defined = inBody.length > 0 ? inBody : namesakes
|
||||
? ordered.filter((n) => n.filePath === cls.filePath || cppIncluders(cls.filePath, context).has(n.filePath))
|
||||
: ordered;
|
||||
return cppOverloadFor(defined.length > 0 ? defined : ordered, name, ref, context);
|
||||
|
||||
Reference in New Issue
Block a user