diff --git a/CHANGELOG.md b/CHANGELOG.md index 9641befea4..7060940d77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,6 +104,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a variable, parameter or member whose declaration puts a qualifier or an annotation around its name is now read for its type, so calls on it link to the declared class's methods: a `const` after the `*` or `&`, like facebook/rocksdb's `Iterator* const iter = iter_guard.get();` or google/leveldb's `Env* const env_;`, a type written before its `const`, like `Slice const& key`, a nullability annotation after the `*`, like protocolbuffers/protobuf's `io::EpsCopyOutputStream* PROTOBUF_NONNULL stream`, and a thread-safety annotation after the name, like LevelDB's `MemTable* imm_ GUARDED_BY(mutex_);`. Before, such a declaration was skipped, or the annotation was taken for its type, so the call took the type of an earlier variable of the same name or was guessed by the method's name: RocksDB's `iter->SeekToFirst()` reached the unrelated `ArenaWrappedDBIter`, and LevelDB's `env_->NewWritableFile(…)` reached `InMemoryEnv`. Re-index C++ projects after upgrading. - In C++, a call on a name a structured binding declares now links to the method of the type C++ gives it. That covers a range-based `for` over pairs or a map, like facebook/rocksdb's `for (auto& [cfh, iter] : cfh_iter_pairs_) { iter->Valid(); }`, and a declaration like protocolbuffers/protobuf's `auto [dst_ext, is_new] = Insert(…);`. A call on what a `std::function` or another function wrapper returns, like `create_iterator()->Valid()` on a `std::function`, links the same way, and so does a call on a class member whose type is written on the line above its name. Before, these calls were guessed by the method's name, and calls on a library container declared that way, like a `std::priority_queue` member's `pop()`, reached a project class's method of that name. Re-index C++ projects after upgrading. - In C++, a call written without a receiver in a gtest `TEST_F` or `TEST_P` body now links to the test fixture's method, its own or one it inherits, as C++ finds it: facebook/rocksdb's tests call `DBTestBase`'s `Put(…)`, `Flush()` and `NewIterator(…)` this way. A variable such a call initializes, like `auto iter = NewIterator(…)`, takes the type the method returns, so `iter->SeekToFirst()` reaches the iterator class. Before, those calls were guessed by name and often reached a namesake in another class, like RocksDB's `ColumnFamilyTestBase::Put` or protocolbuffers/protobuf's `EncodeDecodeTest::ExpectNoErrors`, and `iter->SeekToFirst()` reached the unrelated `MemTableRep::Iterator`. A `->` call on a value of a type from outside the project, like protobuf's `m->value()` on an `absl::StatusOr`, no longer links to a class of the project that shares the type's name, like protobuf's own `hpb::StatusOr`. Re-index C++ projects after upgrading. +- In C++, a call made with `->` on an `absl::StatusOr`, an `absl::optional` or an `absl::NoDestructor`, like protocolbuffers/protobuf's `resolver->MergeFeatures(…)` on an `absl::StatusOr`, now links to the method of the class it holds, as one on a `std::unique_ptr` or `std::optional` does. A call written without a receiver in a class template now also finds a member of a base that depends on the template's parameters when the class brings it in with a using-declaration, like facebook/rocksdb's `using BasicTypedCacheHelperFns::DownCastValue;`, and so does a call on such a member. 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-absl-holder-receiver.test.ts b/__tests__/cpp-absl-holder-receiver.test.ts new file mode 100644 index 0000000000..d656db6922 --- /dev/null +++ b/__tests__/cpp-absl-holder-receiver.test.ts @@ -0,0 +1,213 @@ +/** + * A C++ `->` call on an `absl::StatusOr`, an `absl::optional` or an + * `absl::NoDestructor` calls a member of the type it holds, as one on a + * `std::unique_ptr` or `std::optional` does; and a bare call in a class + * template finds a member its dependent base declares when the class brings + * the name in with a using-declaration. + * + * In protocolbuffers/protobuf, `absl::StatusOr resolver = + * FeatureResolver::Create(…); resolver->MergeFeatures(…)` was guessed by the + * method's name: the declared type's last name, `StatusOr`, is no class of + * the project's. In facebook/rocksdb's typed_cache.h, `using + * BasicTypedCacheHelperFns::DownCastValue;` makes a bare + * `DownCastValue(v)` the base's, which C++ doesn't look in otherwise: the + * base depends on the class's template parameters. + */ +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-absl-held-')); + 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 @confidence`. */ +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} @${(r.edge.metadata as { confidence?: number } | undefined)?.confidence}`) + .sort(); +} + +const lines = (...text: string[]): string => [...text, ''].join('\n'); + +// Two classes with a method of each name, so a guess by the name can miss. +const RESOLVERS = lines( + 'namespace google {', + 'namespace protobuf {', + 'class FeatureResolver {', + ' public:', + ' int MergeFeatures(int parent, int child) const { return parent; }', + '};', + 'class FeatureSetResolver {', + ' public:', + ' int MergeFeatures(int parent, int child) const { return child; }', + '};', + '} // namespace protobuf', + '} // namespace google', +); + +describe('C++ absl holders and using-declared members of dependent bases', () => { + it("is the held class's method for `->` on an absl::StatusOr, absl::optional or absl::NoDestructor", async () => { + const cg = await indexed({ + 'src/feature_resolver.h': RESOLVERS, + 'src/feature_resolver_test.cc': lines( + '#include "src/feature_resolver.h"', + 'namespace google {', + 'namespace protobuf {', + 'int Merge(absl::StatusOr resolver) {', + ' return resolver->MergeFeatures(1, 2);', + '}', + 'int MergeOptional(absl::optional resolver) {', + ' return resolver->MergeFeatures(1, 2);', + '}', + 'int MergeStatic() {', + ' static absl::NoDestructor resolver;', + ' return resolver->MergeFeatures(1, 2);', + '}', + '} // namespace protobuf', + '} // namespace google', + ), + }); + try { + for (const caller of ['Merge', 'MergeOptional', 'MergeStatic']) { + expect(calls(cg, `google::protobuf::${caller}`)).toEqual(['google::protobuf::FeatureResolver::MergeFeatures @0.9']); + } + } finally { + cg.close(); + } + }); + + it("is not a project's own StatusOr, nor what a StatusOr of a pointer holds", async () => { + const cg = await indexed({ + 'src/feature_resolver.h': RESOLVERS, + // protobuf's hpb declares a `StatusOr` of its own. + 'hpb/status.h': lines( + 'namespace absl {', + 'template ', + 'class StatusOr {', + ' public:', + ' T value() const;', + '};', + '} // namespace absl', + ), + 'src/use.cc': lines( + '#include "src/feature_resolver.h"', + '#include "hpb/status.h"', + 'namespace google {', + 'namespace protobuf {', + 'int Merge(absl::StatusOr resolver) {', + ' return resolver->MergeFeatures(1, 2);', + '}', + '} // namespace protobuf', + '} // namespace google', + ), + }); + try { + expect(calls(cg, 'google::protobuf::Merge').filter((c) => c.startsWith('google::protobuf::FeatureResolver::') && c.endsWith('@0.9'))).toEqual([]); + } finally { + cg.close(); + } + }); + + it('is the member of the base the using-declaration names, of two dependent bases', async () => { + const cg = await indexed({ + 'util/runner.h': lines( + 'namespace rocksdb {', + 'template ', + 'class Second {', + ' public:', + ' void Run() {}', + '};', + 'template ', + 'class First {', + ' public:', + ' void Run() {}', + '};', + 'template ', + 'class Both : public Second, public First {', + ' public:', + ' using First::Run;', + ' void Go() { Run(); }', + '};', + '} // namespace rocksdb', + ), + }); + try { + expect(calls(cg, 'rocksdb::Both::Go')).toEqual(['rocksdb::First::Run @0.9']); + } finally { + cg.close(); + } + }); + + it('is the dependent base member a using-declaration brings in, for a bare call and a data member', async () => { + const cg = await indexed({ + // (The base in a header of its own, where the derived class's file doesn't write its member.) + 'cache/typed_cache_base.h': lines( + 'namespace rocksdb {', + 'class Cache {', + ' public:', + ' int Insert(int key) { return key; }', + '};', + 'class Other {', + ' public:', + ' int Insert(int key) { return 0; }', + '};', + 'template ', + 'class BasicTypedCacheHelperFns {', + ' public:', + ' static TValue* DownCastValue(void* v) { return nullptr; }', + ' protected:', + ' Cache* cache_;', + '};', + '} // namespace rocksdb', + ), + 'cache/typed_cache.h': lines( + '#include "cache/typed_cache_base.h"', + 'namespace rocksdb {', + 'inline int* DownCastValue(void* v) { return nullptr; }', + 'template ', + 'class FullTypedCacheHelperFns : public BasicTypedCacheHelperFns {', + ' public:', + ' using BasicTypedCacheHelperFns::DownCastValue;', + ' using BasicTypedCacheHelperFns::cache_;', + ' static void Delete(void* v) {', + ' DownCastValue(v);', + ' }', + ' int Store(int key) {', + ' return cache_->Insert(key);', + ' }', + '};', + '} // namespace rocksdb', + ), + }); + try { + // (Without the using-declarations C++ finds the namespace's `DownCastValue`, as before.) + expect(calls(cg, 'rocksdb::FullTypedCacheHelperFns::Delete')).toEqual(['rocksdb::BasicTypedCacheHelperFns::DownCastValue @0.9']); + expect(calls(cg, 'rocksdb::FullTypedCacheHelperFns::Store')).toEqual(['rocksdb::Cache::Insert @0.9']); + } finally { + cg.close(); + } + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index fa0f6498e9..abd407c770 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -9374,7 +9374,8 @@ function cppInheritedDataMember(decls: readonly Node[], name: string, context: R if (depth >= 4 || !context.getSupertypeNodes) return null; for (const decl of decls) { for (const base of context.getSupertypeNodes(decl.id)) { - if (!CPP_CLASS_KINDS.has(base.kind) || seen.has(base.qualifiedName) || isCppDependentBase(decl, base, context)) continue; + if (!CPP_CLASS_KINDS.has(base.kind) || seen.has(base.qualifiedName)) continue; + if (isCppDependentBase(decl, base, context) && !cppUsingDeclares(decl, base, name, context)) continue; seen.add(base.qualifiedName); const inherited = cppInheritedDataMember([base], name, context, depth + 1, seen); if (inherited) return inherited; @@ -9402,7 +9403,7 @@ function cppTestFixture(caller: Node, ref: UnresolvedRef, context: ResolutionCon /** * The type `*x` has when `x` has the type `type`: what a pointer points at, - * or what a standard smart pointer or optional holds. Null for any other type. + * or what a smart pointer, an optional or an `absl::StatusOr` holds (cppHeldType). Null for any other type. */ function cppPointee(type: CppWrittenType): CppWrittenType | null { const text = type.text.replace(/&+\s*$/, '').trim(); @@ -9413,7 +9414,7 @@ function cppPointee(type: CppWrittenType): CppWrittenType | null { else if (ch === '<') depth--; else if (ch === '*' && depth === 0) return { ...type, text: text.slice(0, i).trim() }; } - const held = cppStdHeldType(text); + const held = cppHeldType(text); return held ? { ...type, text: held } : null; } @@ -9531,8 +9532,8 @@ function cppExpressionType(expr: string, at: UnresolvedRef, context: ResolutionC // `std::make_unique(…)`: the smart pointer it makes. const made = /^(?:::\s*)?(?:std|absl)\s*::\s*make_(unique|shared)\s*<([\s\S]+)>$/.exec(callee); if (made) return { ...callers, text: `std::${made[1]}_ptr<${made[2]!.trim()}>` }; - // `std::unique_ptr(…)`: the smart pointer or optional it makes. - if (cppStdHeldType(callee)) return { ...callers, text: callee }; + // `std::unique_ptr(…)`: the holder it makes (cppHeldType). + if (cppHeldType(callee)) return { ...callers, text: callee }; // `create_iterator()` on a `std::function create_iterator`: what its function type returns. if (/^[A-Za-z_]\w*$/.test(callee)) { const wrapper = cppExpressionType(callee, at, context, depth + 1); @@ -9648,7 +9649,7 @@ function cppAutoLocalType( function isCppTemplateDependentType(type: CppWrittenType, context: ResolutionContext): boolean { if (type.parameters.size === 0) return false; if (isCppTemplateParameterType(type)) return true; - const held = cppStdHeldType(type.text); + const held = cppHeldType(type.text); if (held) return isCppTemplateDependentType({ ...type, text: held }, context); // (A trait of one, `std::conditional_t`, too: it names another type.) return (isCppLibraryType(type.text, context) || cppTypeSegments(type.text)?.[0] === 'std') && @@ -9899,19 +9900,31 @@ function isCppLibraryType(raw: string, context: ResolutionContext): boolean { return !declared.some((n) => n.qualifiedName === spelled || n.qualifiedName.endsWith(`::${spelled}`)); } -/** Standard-library types whose `->` reaches the value they hold. */ -const CPP_STD_HOLDERS: ReadonlySet = new Set(['unique_ptr', 'shared_ptr', 'optional']); +/** Library types whose `->` reaches the value they hold. */ +const CPP_HOLDERS: ReadonlySet = new Set(['std::unique_ptr', 'std::shared_ptr', 'std::optional', 'absl::optional', 'absl::StatusOr', 'absl::NoDestructor']); -/** The type a standard smart pointer or optional written as `raw` holds (`Iterator` for `std::unique_ptr`), or null. */ -function cppStdHeldType(raw: string): string | null { +/** + * The type a standard smart pointer, an optional, an `absl::StatusOr` or an + * `absl::NoDestructor` written as `raw` holds (`Iterator` for + * `std::unique_ptr`), or null. + */ +function cppHeldType(raw: string): string | null { const holder = cppTypeSegments(raw); - if (holder?.length !== 2 || holder[0] !== 'std' || !CPP_STD_HOLDERS.has(holder[1]!)) return null; + if (holder?.length !== 2 || !CPP_HOLDERS.has(holder.join('::'))) return null; return cppTemplateArguments(raw)?.[0] ?? null; } +/** Whether the holder `raw` names (cppHeldType) is the library's: no type of the project's has its name. */ +function isCppLibraryHolder(raw: string, context: ResolutionContext): boolean { + const spelled = cppTypeSegments(raw)!.join('::'); + return !cppTypesNamed(cppLastSegment(spelled), context).some((n) => n.qualifiedName === spelled || n.qualifiedName.endsWith(`::${spelled}`)); +} + /** - * A `->` call on a C++ receiver declared as a standard smart pointer or - * optional (`std::unique_ptr iter; iter->Valid()`) calls a member of + * A `->` call on a C++ receiver declared as a standard smart pointer, an + * optional or an `absl::StatusOr` (`std::unique_ptr iter; + * iter->Valid()`, protobuf's `absl::StatusOr resolver; + * resolver->MergeFeatures(…)`) calls a member of * the type it holds. That type is looked up as C++ looks it up where the call * is written (cpp-supertypes.ts): the scopes around the call, the file's * `using`s, and aliases followed. Looking it up by its last name instead @@ -9929,8 +9942,8 @@ function cppStdHeldType(raw: string): string | null { * doesn't name): the call goes on as before. */ function matchCppHeldTypeCall(raw: string, methodName: string, ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null | undefined { - const held = cppStdHeldType(raw); - if (!held || !isCppLibraryType(raw, context)) return undefined; + const held = cppHeldType(raw); + if (!held || !isCppLibraryHolder(raw, context)) return undefined; const caller = context.getNodeById?.(ref.fromNodeId); if (!caller || isCppPointerType(held)) return undefined; if (isCppLibraryType(held, context)) return null; @@ -10029,8 +10042,9 @@ function matchCppDeclaredTypeCall( * type from outside the project that nothing the file sees declares * (isCppOutsideQualifiedType)? Its `operator->` hands the call to a type the * declared one only wraps (`absl::StatusOr`'s `Any`), so the declared - * type's last name names no class the method is in. (A standard smart - * pointer or optional is matchCppHeldTypeCall's.) + * type's last name names no class the method is in. (A smart pointer, an + * optional or an `absl::StatusOr` holding one of the project's classes is + * matchCppHeldTypeCall's.) */ function isCppArrowThroughOutsideType(receiver: string, decl: CppReceiverDeclaration, ref: UnresolvedRef, context: ResolutionContext): boolean { if (!decl.callers || decl.shadowed || !decl.raw || decl.pointer || decl.aliased) return false; @@ -10375,7 +10389,7 @@ function cppScopeMethod(scope: CppClassScope, name: string, receiver: 'bare' | ' for (const decl of scope.decls) { for (const base of context.getSupertypeNodes(decl.id)) { if (!CPP_CLASS_KINDS.has(base.kind)) continue; - if (receiver === 'bare' && isCppDependentBase(decl, base, context)) continue; + if (receiver === 'bare' && isCppDependentBase(decl, base, context) && !cppUsingDeclares(decl, base, name, context)) continue; const inherited = cppMethodOf(base, name, ref, context, 1, new Set([scope.qualifiedName])); if (!inherited) continue; // Its class's other overloads, which the arguments may fit better @@ -10463,10 +10477,37 @@ function cppBaseSpecifiers(decl: Node, context: ResolutionContext): string[] { return specs; } +/** Per context: a C++ class declaration's id → the `using Base::name;` declarations its body writes, as [base, name]. */ +const CPP_USING_MEMBERS = new WeakMap>>(); + +/** + * Whether the C++ class declaration `decl` brings `name` in from its base + * `base` with a using-declaration: rocksdb's `FullTypedCacheHelperFns` + * writes `using BasicTypedCacheHelperFns::DownCastValue;`, and a bare + * `DownCastValue(v)` in it then finds the base's, though the base depends on + * the class's template parameters (isCppDependentBase). + */ +function cppUsingDeclares(decl: Node, base: Node, name: string, context: ResolutionContext): boolean { + let memo = CPP_USING_MEMBERS.get(context); + if (!memo) CPP_USING_MEMBERS.set(context, (memo = new Map())); + let usings = memo.get(decl.id); + if (!usings) { + usings = []; + const lines = context.getFileLines?.(decl.filePath) ?? context.readFile(decl.filePath)?.split(/\r?\n/) ?? []; + const body = lines.slice(decl.startLine - 1, decl.endLine ?? decl.startLine).map((l) => cppCodeOf(l) ?? '').join('\n'); + for (const m of body.matchAll(/\busing\s+(?!namespace\b)((?:::\s*)?[A-Za-z_][\w:]*(?:\s*<[^;{}]*>)?)\s*::\s*([A-Za-z_]\w*)\s*;/g)) { + usings.push([cppLastSegment(stripCppTemplateArguments(m[1]!).trim()), m[2]!]); + } + memo.set(decl.id, usings); + } + return usings.some(([of, member]) => of === base.name && member === name); +} + /** * Whether `base`, a base of the C++ class declaration `decl`, depends on the * template parameters of `decl` or a class around it: `template - * class Foo : public Base`. A bare name is not looked up in such a base. + * class Foo : public Base`. A bare name is not looked up in such a base, + * unless the class brings it in with a using-declaration (cppUsingDeclares). */ function isCppDependentBase(decl: Node, base: Node, context: ResolutionContext): boolean { const parameters = cppTemplateParameters(decl, context); @@ -11406,6 +11447,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { clearCppRawStringMemos(context); CPP_CLASSES_IN_FILE.delete(context); CPP_BASE_SPECS.delete(context); + CPP_USING_MEMBERS.delete(context); CPP_MEMBER_MACRO_ARGS.delete(context); CPP_LOOP_ELEMENTS.delete(context); CPP_AUTO_LOCALS.delete(context); @@ -12712,8 +12754,9 @@ export function matchMethodCall( // followed. if (isUndeclaredTypeName(cppDecl.written!, ref, context)) return null; } - // A `->` call on a standard smart pointer or optional the calling - // function or its class declares is a call on the type it holds: + // A `->` call on a smart pointer, an optional or an `absl::StatusOr` + // (cppHeldType) the calling function or its class declares is a call on + // the type it holds: // rocksdb's `std::unique_ptr iter; iter->Valid()` reached // `ArenaWrappedDBIter::Valid` by the method's name. if (ref.language === 'cpp' && cppDecl.callers && !cppDecl.aliased && !cppDecl.shadowed && !cppDecl.pointer &&