diff --git a/CHANGELOG.md b/CHANGELOG.md index 2065e530e..6ae25d944 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -492,7 +492,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a call a method makes with no receiver, or through `this->`, now links to the method C++ itself would call: one of the method's own class, else one the class inherits from a base class, else a static member of a class it is nested in, which a nested class can name without an object. The same goes for a call in a lambda inside the method, and of a class's overloads the one the call's arguments fit is taken. Before, the call was matched by its name alone, so another class's method of that name could win because it was declared nearer or found first: in protocolbuffers/protobuf, every generated message's `Swap` and move assignment calling `InternalSwap(…)` reached `Any::InternalSwap`, and the `GetArena()` messages inherit from `MessageLite` reached an arena helper's `GetArena`, while googletest's `~linked_ptr() { depart(); }`, vendored in facebook/rocksdb, reached `linked_ptr_internal::depart`. Calls C++ doesn't look up in the class are matched as before: a free function's, a call to a parameter or local of that name, and a template's bare call into a base class that depends on its template parameters. Re-index C++ projects after upgrading. Thanks @danusha2345 for reporting the C++ case in #1952. - In C++, a call on the variable of a range-based `for` loop over `auto` elements, like `cfd->Ref()` inside facebook/rocksdb's `for (auto cfd : cfds)`, now links to the method of the type the loop goes over. That type is read from what the loop iterates: a `std::vector`, `std::set`, `absl::Span` or similar container, protocolbuffers/protobuf's `RepeatedPtrField` or RocksDB's `autovector`, whether a variable, parameter or member holds it, an `auto` variable is set to it or a function or method returns it, and a project class you loop over through its `begin()`, like RocksDB's `ColumnFamilySet`. Before, the call was matched by the variable's name, or by an earlier variable of that name: in RocksDB, `blob_file.GetBlobFileNumber()` over a list of `BlobFileGarbage` reached `BlobFileAddition::GetBlobFileNumber`, and in protobuf's Objective-C generator, calls over a list of `std::unique_ptr` reached other generators' methods. A call on an element of a standard-library type, like `file.find(…)` over a `std::vector`, no longer links to a project method, and `->` on an element held in a `std::unique_ptr` or `std::shared_ptr` reaches the type it holds. Re-index C++ projects after upgrading. - In this fork, the two C++ changes above run in the native resolver. A call on a variable whose type is a template parameter, like `T* iter`, stays unlinked rather than reaching a class that happens to share the parameter's name. A call stays linked as before when the class has no method of that name but one of its base classes comes from an outside library other than the standard library, such as googletest's `testing::Test`. -- In C++, a call on a data member of the calling method's class, like `versions_->GetColumnFamilySet()` in facebook/rocksdb's `db_impl_write.cc`, now links to the method of the member's type wherever the class is declared. Before, only the call's own file and the header of the same name were read for the member, and RocksDB declares `DBImpl` in `db_impl.h` while its methods are spread over `db_impl_write.cc`, `db_impl_compaction_flush.cc`, `db_impl_open.cc` and more, so such calls were guessed by the member's name. A member declared below an inline method that uses it, one a base class declares, and, in a gtest `TEST_F` or `TEST_P` body, one of the test's fixture, like RocksDB's `db_`, are found the same way, and a member of a class nested in the caller's class is no longer taken for the caller's own. A call on a member of a standard-library type, like `std::atomic shutting_down_`, no longer links to a project method that only shares the method's name. Re-index C++ projects after upgrading. +- In C++, a call on a data member of the calling method's class, like `versions_->GetColumnFamilySet()` in facebook/rocksdb's `db_impl_write.cc`, now links to the method of the member's type wherever the class is declared. Before, only the call's own file and the header of the same name were read for the member, and RocksDB declares `DBImpl` in `db_impl.h` while its methods are spread over `db_impl_write.cc`, `db_impl_compaction_flush.cc`, `db_impl_open.cc` and more, so such calls were guessed by the member's name. A member declared below an inline method that uses it, one a base class declares, and, in a gtest `TEST_F` or `TEST_P` body, one of the test's fixture, like RocksDB's `db_`, are found the same way, and a member of a class nested in the caller's class is no longer taken for the caller's own. A call on a member of a standard-library type, like `std::atomic shutting_down_`, no longer links to a project method that only shares the method's name. A call on a member reached through other members now links the same way, like RocksDB's `thread->rand.Next()` on a `Random64` or `meta->largest.user_key()` on an `InternalKey`. Before, it went to whichever class had a method of that name, like `ArenaWrappedDBIter::Next` or `ClippingIterator::user_key`, or, inside a method, to that method's own class. When the member's class has no such method, the call no longer links to another class's. The member is read from the class the calling file sees: RocksDB's `db_bench_tool.cc` and its stress tool each declare their own `ThreadState`, whose `rand` is a `Random64` in one and a `Random` in the other. Re-index C++ projects after upgrading. - 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. @@ -621,7 +621,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - 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 as a standard-library or other outside type with a lowercase name, like `std::string`, `std::vector`, `std::set` or `absl::flat_hash_set`, no longer links to a project method that only shares the method's name, even one of a project class in another namespace with the type's name, like your own `util::flat_hash_set`. A variable declared outside the calling function, like another function's or a file's `std::string text`, no longer links its calls to such a class either. In google/leveldb, `saved_key_.clear()` on a `std::string saved_key_;` reached `Slice::clear`, and in protocolbuffers/protobuf, `proto.append(…)` on a `std::string` reached an unrelated `LeftoverBuffer::append`. The same goes for `->` through a raw pointer like `std::string* out`, while `->` through a smart pointer, iterator or optional still links to the type it holds. Comments also no longer pass for a variable's declaration, so more calls now link to the method of the type the variable is really declared with. Re-index C++ projects after upgrading. +- In C++, a call on a variable, parameter or member declared as a standard-library or other outside type with a lowercase name, like `std::string`, `std::vector`, `std::set` or `absl::flat_hash_set`, no longer links to a project method that only shares the method's name, even one of a project class in another namespace with the type's name, like your own `util::flat_hash_set`. A variable declared outside the calling function, like another function's or a file's `std::string text`, no longer links its calls to such a class either. In google/leveldb, `saved_key_.clear()` on a `std::string saved_key_;` reached `Slice::clear`, and in protocolbuffers/protobuf, `proto.append(…)` on a `std::string` reached an unrelated `LeftoverBuffer::append`. The same goes for `->` through a raw pointer like `std::string* out`, while `->` through a smart pointer, iterator or optional still links to the type it holds. Comments also no longer pass for a variable's declaration, so more calls now link to the method of the type the variable is really declared with. A call on a member reached through other members, like facebook/rocksdb's `options.cf_paths.emplace_back(…)` on a `std::vector`, `props->rep.db_id.size()` on a `std::string` or `bbto.filter_policy.reset(…)` on a `std::shared_ptr`, no longer links to a project method of that name either: those had reached `autovector::emplace_back`, `BlobContents::size` and `FSWritableFilePtr::reset`. The same goes for a member of an outside type like `absl::Status`. Re-index C++ projects after upgrading. - In C++, a call made with `->` on a variable, parameter or member declared as a `std::unique_ptr`, `std::shared_ptr` or `std::optional`, like `iter->Valid()` on a `std::unique_ptr iter`, now links to that method of the type it holds, or of a class that type inherits from, with the type looked up the way C++ does it, so a class of the same name nested elsewhere is not mistaken for it. Before, the call went to whichever project method shared its name: in facebook/rocksdb, `iter->Valid()` reached `ArenaWrappedDBIter::Valid` and `fs->GetFileSize(…)` on a `std::shared_ptr` reached a test's stand-in file system, and calls like `it->Next()` were often not linked at all. When what it holds is a template parameter or a standard-library type, like `std::unique_ptr>`, or a class that has no method of that name and neither do the classes it derives from, the call no longer links to a project method; a class derived from something that can't be followed, like a `decltype(…)`, may still have it, so that call is left unlinked. The same goes for `->` on a container's iterator, like `it->data()` on a `std::deque::const_iterator it` or `loc->path_size()` on protocolbuffers/protobuf's `RepeatedPtrField::iterator loc`: the call links to that method of the container's element type, and to no project method when the element is a standard-library type like `std::string`. That includes an iterator an `auto` variable gets from a standard-library container's `begin()`, `find()` and the like, or from a standard search like `std::lower_bound`, as in facebook/rocksdb's `auto it = logs_.begin(); it->IsSyncing()`. A call on a dereferenced iterator or pointer, like `(*it)->Visit()` over a `std::vector` or `(*it).Weight()` over a `std::vector`, and on a variable a dereference sets, like `auto first = *nodes.begin()`, links the same way, where before such a call went to whichever class had a method of that name. So does a call on the key or value a map's iterator points at, like `it->second.Flush()`, `it->second->Flush()` or a range-based `for` loop's `kv.second.Flush()`, and on the `first` or `second` of a `std::pair` the code declares. So does a call on an element read by subscript, like `statuses[i].IsNotFound()` over a `std::vector`, `buffers[key].Flush()` over a map, `iters[i]->Valid()` over owning pointers, or an element of an array or of a class's `operator[]`. So does a call on what a container's `front()`, `back()` or `at()` hands out, like facebook/rocksdb's `edits.back()->MarkAtomicGroup(…)` or `wals.at(log_number).HasSyncedSize()` over a map, on a `std::stack`'s `top()` or a `std::queue`'s `front()`, and on what a smart pointer's `get()` or an optional's or `absl::StatusOr`'s `value()` returns, like `mock_clock.get()->GetCurrentTime(…)`, where before such calls linked to nothing; an overloaded function given such a value, like `test::PerThreadDBPath(env_.get(), …)`, now takes the overload for its type. Re-index C++ projects after upgrading. diff --git a/README.md b/README.md index dffbacdf5..fc7fdd1c3 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Follow [@getcodegraph](https://x.com/getcodegraph) on X for updates. ## About this fork -This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`19813544`](https://github.com/colbymchenry/codegraph/commit/19813544) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. +This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`e8ec7fd8`](https://github.com/colbymchenry/codegraph/commit/e8ec7fd8) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. The fork publishes no releases. The install scripts, npm package, badges and `codegraph upgrade` further down this page install **upstream's** releases. To run the fork, build it from source (below). diff --git a/__tests__/cpp-member-chain-outside-calls.test.ts b/__tests__/cpp-member-chain-outside-calls.test.ts new file mode 100644 index 000000000..88d027ecf --- /dev/null +++ b/__tests__/cpp-member-chain-outside-calls.test.ts @@ -0,0 +1,150 @@ +/** + * A C++ call on a data member reached through a member chain, like rocksdb's + * `options.cf_paths.emplace_back(…)` on a `std::vector` or + * `props->rep.db_id.size()` on a `std::string`, calls the library type's own + * method, none of the project's. The extractor records such a call by its + * bare name, so it went to any project method of that name: rocksdb's + * `autovector::emplace_back`, `BlobContents::size`. + */ +import { describe, it, expect, beforeAll, afterAll } 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 lines = (...l: string[]): string => [...l, ''].join('\n'); + +describe('a C++ call on a member chain of a type from outside the project', () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-')); + const write = (file: string, text: string): void => { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + }; + // The project's namesakes of library methods, which a call matched by its bare name lands on. + write('src/types.h', lines( + 'namespace rocksdb {', + 'template ', + 'class autovector {', + ' public:', + ' void emplace_back(T v) {}', + ' void clear() {}', + ' int size() const { return 0; }', + '};', + 'class BlobContents {', + ' public:', + ' int size() const { return 0; }', + '};', + '} // namespace rocksdb', + 'namespace proto {', + 'class Option {', + ' public:', + ' bool has_value() const { return false; }', + '};', + '} // namespace proto', + 'namespace util {', + 'class Status {', + ' public:', + ' bool ok() const { return true; }', + '};', + '} // namespace util', + 'class FilePtr {', + ' public:', + ' void reset() {}', + '};', + 'class Engine {', + ' public:', + ' void Run() {}', + '};', + )); + write('src/use.cc', lines( + '#include ', + '#include ', + '#include ', + '#include ', + '#include "absl/status/status.h"', + '#include "src/types.h"', + 'struct Options {', + ' std::vector cf_paths;', + ' std::string name;', + '};', + 'struct Rep {', + ' std::string db_id;', + '};', + 'struct Props {', + ' Rep rep;', + '};', + 'struct State {', + ' std::optional child_pid;', + ' std::shared_ptr engine;', + ' Engine plain;', + ' absl::Status status;', + '};', + 'void Library(Options& options) { options.cf_paths.emplace_back(1); options.name.size(); }', + 'void TwoHops(Props* props) { props->rep.db_id.size(); }', + 'void Optional(State* state) { state->child_pid.has_value(); }', + 'void HolderOwn(State& state) { state.engine.reset(); }', + 'void OutsideQualified(State& state) { state.status.ok(); }', + 'void HolderArrow(State& state) { state.engine->Run(); }', + 'void ProjectMember(State& state) { state.plain.Run(); }', + 'class Cache {', + ' public:', + ' void Touch() { this->keys_.clear(); this->engine_->Run(); }', + ' private:', + ' std::vector keys_;', + ' Engine* engine_;', + '};', + 'template ', + 'struct Box {', + ' std::vector items;', + ' std::unique_ptr owner;', + '};', + 'template ', + 'void Drain(Box& box) { box.items.clear(); box.owner.reset(); }', + )); + cg = await CodeGraph.init(root, { index: true }); + }, 120_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + /** `calls` callees of the function or method `name`, as `qualifiedName`. */ + const calls = (name: string): string[] => { + const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')].find((n: Node) => n.name === name); + expect(caller, name).toBeDefined(); + return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls').map((r) => r.node.qualifiedName).sort(); + }; + + it("a library container's or string's method reached through a member", () => { + expect(calls('Library')).toEqual([]); + expect(calls('TwoHops')).toEqual([]); + }); + + it("an optional's method, and a smart pointer's own through `.`", () => { + expect(calls('Optional')).toEqual([]); + expect(calls('HolderOwn')).toEqual([]); + }); + + it('a type under a namespace the project declares nothing in', () => { + expect(calls('OutsideQualified')).toEqual([]); + }); + + it("`->` on a smart pointer, and a member of a project class's type, still call the project's", () => { + expect(calls('HolderArrow')).toEqual(['Engine::Run']); + expect(calls('ProjectMember')).toEqual(['Engine::Run']); + }); + + it('a chain through `this`', () => { + expect(calls('Touch')).toEqual(['Engine::Run']); + }); + + it("a library template of a template parameter's type is still the library's", () => { + expect(calls('Drain')).toEqual([]); + }); +}); diff --git a/__tests__/cpp-member-chain-project-calls.test.ts b/__tests__/cpp-member-chain-project-calls.test.ts new file mode 100644 index 000000000..48dd1950a --- /dev/null +++ b/__tests__/cpp-member-chain-project-calls.test.ts @@ -0,0 +1,176 @@ +/** + * A C++ call on a data member reached through a member chain, like rocksdb's + * `thread->rnd.Next()` on a `Random rnd` or `meta.smallest.user_key()` on an + * `InternalKey smallest`, calls the method of the class the member is + * declared as, or of a class it derives from. The extractor records such a + * call by its bare name, so it went to whichever class had a method of that + * name: `ArenaWrappedDBIter::Next`, `ClippingIterator::user_key`, or the + * calling method's own class. + */ +import { describe, it, expect, beforeAll, afterAll } 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 lines = (...l: string[]): string => [...l, ''].join('\n'); + +describe('a C++ call on a member chain of a project class', () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-project-')); + const write = (file: string, text: string): void => { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + }; + // Each namesake comes first: a call matched by its bare name lands on it. + write('src/types.h', lines( + 'class ArenaWrappedDBIter {', + ' public:', + ' void Next() {}', + '};', + 'class Random {', + ' public:', + ' unsigned Next() { return 0; }', + '};', + 'class ClippingIterator {', + ' public:', + ' int user_key() const { return 1; }', + '};', + 'class InternalKey {', + ' public:', + ' int user_key() const { return 0; }', + '};', + 'class InMemoryEnv {', + ' public:', + ' void NewWritableFile() {}', + '};', + 'class Env {', + ' public:', + ' virtual void NewWritableFile() {}', + '};', + 'class Other {', + ' public:', + ' void Shared() {}', + '};', + 'class Base {', + ' public:', + ' void Shared() {}', + '};', + 'class Derived : public Base {};', + 'class Free {', + ' public:', + ' void Missing() {}', + '};', + 'class Lacking {', + ' public:', + ' void Something() {}', + '};', + 'class Cache {', + ' public:', + ' void Put(int k, int v) {}', + '};', + 'class Table {', + ' public:', + ' void Put(int k) {}', + ' void Put(int k, int v) {}', + '};', + 'class Random64 {', + ' public:', + ' unsigned Next() { return 1; }', + '};', + )); + // Two files each declare a `SharedState` of their own, with members of the same name. + write('src/a_shared.h', lines( + '#include "src/types.h"', + 'struct SharedState {', + ' Random rand;', + '};', + )); + write('src/bench.cc', lines( + '#include "src/types.h"', + 'struct SharedState {', + ' Random64 rand;', + '};', + 'void Bench(SharedState* shared) { shared->rand.Next(); }', + )); + write('src/use.cc', lines( + '#include "src/types.h"', + 'struct ThreadState {', + ' Random rnd;', + '};', + 'struct FileMeta {', + ' InternalKey smallest;', + '};', + 'struct Options {', + ' Env* env;', + '};', + 'struct Holder {', + ' Derived d;', + ' Lacking l;', + ' Table table;', + '};', + 'struct Outer {', + ' ThreadState inner;', + '};', + 'void Rand(ThreadState* thread) { thread->rnd.Next(); }', + 'void Key(FileMeta& meta) { meta.smallest.user_key(); }', + 'void EnvCall(Options& options) { options.env->NewWritableFile(); }', + 'void TwoHops(Outer* o) { o->inner.rnd.Next(); }', + 'void Inherited(Holder& h) { h.d.Shared(); }', + 'void Lacks(Holder& h) { h.l.Missing(); }', + 'void Overload(Holder& h) { h.table.Put(1, 2); }', + 'class Worker {', + ' public:', + ' void Next() {}', + ' void Step() { this->state_.rnd.Next(); }', + ' private:', + ' ThreadState state_;', + '};', + )); + cg = await CodeGraph.init(root, { index: true }); + }, 120_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + /** `calls` callees of the function or method `name`, as `qualifiedName` (`:line` with `withLine`). */ + const calls = (name: string, withLine = false): string[] => { + const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')].find((n: Node) => n.name === name); + expect(caller, name).toBeDefined(); + return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls') + .map((r) => (withLine ? `${r.node.qualifiedName}:${r.node.startLine}` : r.node.qualifiedName)).sort(); + }; + + it("calls the method of the member's class, not a namesake's", () => { + expect(calls('Rand')).toEqual(['Random::Next']); + expect(calls('Key')).toEqual(['InternalKey::user_key']); + expect(calls('EnvCall')).toEqual(['Env::NewWritableFile']); + }); + + it('through two members', () => { + expect(calls('TwoHops')).toEqual(['Random::Next']); + }); + + it("a method the member's class inherits, and none when the class lacks it", () => { + expect(calls('Inherited')).toEqual(['Base::Shared']); + expect(calls('Lacks')).toEqual([]); + }); + + it("of the class's overloads, the one the arguments fit", () => { + expect(calls('Overload', true)).toEqual(['Table::Put:49']); + }); + + it("a chain through `this` is no call on the calling method's own class", () => { + expect(calls('Step')).toEqual(['Random::Next']); + }); + + it("a member of the file's own class, not of another file's class of that name", () => { + expect(calls('Bench')).toEqual(['Random64::Next']); + }); +}); diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 984854a88..95879a88f 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -14288,6 +14288,10 @@ describe('Unsupported-language projects report what they skipped (#1502)', () => tempDir = createTempDir(); }); + afterEach(() => { + cleanupTempDir(tempDir); + }); + it('counts files it could not index, by extension, on the git path', async () => { const runGit = (...args: string[]) => execFileSync('git', args, { cwd: tempDir, stdio: 'pipe' }); diff --git a/__tests__/function-ref.test.ts b/__tests__/function-ref.test.ts index c09acac92..e3fb14a1c 100644 --- a/__tests__/function-ref.test.ts +++ b/__tests__/function-ref.test.ts @@ -110,7 +110,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, callEdges)).toEqual(['direct_caller']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -154,7 +153,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, fnRefEdgesInto(cg, 'handleClick'))).toEqual(['wire']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -184,7 +182,6 @@ describe('Function-as-value capture (#756)', () => { expect(target?.filePath.endsWith('handlers.ts')).toBe(true); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -209,7 +206,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, edges)).not.toContain('wire'); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -237,7 +233,6 @@ describe('Function-as-value capture (#756)', () => { expect(target?.filePath.endsWith('real.c')).toBe(true); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -262,7 +257,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRef).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -286,7 +280,6 @@ describe('Function-as-value capture (#756)', () => { expect(selfLoops).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -351,7 +344,6 @@ describe('Function-as-value capture (#756)', () => { } } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -408,7 +400,6 @@ describe('Function-as-value capture (#756)', () => { ]); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -458,7 +449,6 @@ describe('Function-as-value capture (#756)', () => { } } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -498,7 +488,6 @@ describe('Function-as-value capture (#756)', () => { ).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -545,7 +534,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, fnRefEdgesInto(cg, 'run0'))).toEqual(['thisRef']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -590,7 +578,6 @@ describe('Function-as-value capture (#756)', () => { expect(cg.getIncomingEdges(decoy.id).filter((e) => e.metadata?.fnRef === true)).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -619,7 +606,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, edges)).toEqual(['wire']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -663,7 +649,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRefEdgesInto(cg, 'dupCmd')).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -704,7 +689,6 @@ describe('Function-as-value capture (#756)', () => { expect(sourceNames(cg, fnRefEdgesInto(cg, 'load'))).toEqual(['sorter']); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -781,7 +765,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRefEdgesInto(cg, 'title')).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -861,7 +844,6 @@ describe('Function-as-value capture (#756)', () => { expect(callers.some((c) => c.node.name === 'get_serializer_class')).toBe(true); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -894,7 +876,6 @@ describe('Function-as-value capture (#756)', () => { expect(fnRefEdgesInto(cg, 'unknown_thing')).toHaveLength(0); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -2012,7 +1993,6 @@ func wire(c *Store, pool Pool) { expect(sourceNames(cg, fnRefEdgesInto(cg, 'cb_a'))).toEqual(['wire']); } finally { cg.destroy(); - tmpDir = undefined; } }); }); diff --git a/__tests__/global-setup-home-sandbox.ts b/__tests__/global-setup-home-sandbox.ts new file mode 100644 index 000000000..cd259aa81 --- /dev/null +++ b/__tests__/global-setup-home-sandbox.ts @@ -0,0 +1,42 @@ +/** + * Remove the home sandboxes `setup-home-sandbox.ts` could not. + * + * Each test file removes its own sandbox in an `afterAll`, but Vitest runs no + * hooks at all for a file whose every test is skipped — a platform-gated suite + * skips that way, and each one left its home dir behind on every run — and a + * straggling child can still hold a file there when `afterAll` runs. So every + * sandbox records its path in this run's registry dir, and the end of the run + * removes whatever is still listed. + */ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import type { GlobalSetupContext } from 'vitest/node'; + +declare module 'vitest' { + export interface ProvidedContext { + /** The dir each test file's home sandbox records its path in. */ + homeSandboxRegistry: string; + } +} + +export default function setup({ provide }: GlobalSetupContext): () => void { + const registry = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-homes-')); + provide('homeSandboxRegistry', registry); + return () => { + const held: string[] = []; + for (const entry of fs.readdirSync(registry)) { + const home = fs.readFileSync(path.join(registry, entry), 'utf8'); + try { + fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 }); + } catch { + held.push(home); + } + } + fs.rmSync(registry, { recursive: true, force: true }); + if (held.length > 0) { + process.stderr.write(`[test teardown] ${held.length} home sandbox dir(s) still in use, left behind:\n` + + held.map((home) => ` ${home}\n`).join('')); + } + }; +} diff --git a/__tests__/liveness-watchdog.test.ts b/__tests__/liveness-watchdog.test.ts index cbd9144f0..268632394 100644 --- a/__tests__/liveness-watchdog.test.ts +++ b/__tests__/liveness-watchdog.test.ts @@ -142,6 +142,19 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { // when the watched DB files keep advancing (a slow synchronous SQLite // statement on degraded storage). --- + // The dirs holding each case's watched file, removed after it. + const progressDirs: string[] = []; + afterEach(() => { + for (const dir of progressDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + /** A fresh path for a case's watchdog to watch, in its own temp dir. */ + function progressFile(): string { + const dir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')); + progressDirs.push(dir); + return path.join(dir, 'db-wal'); + } + /** Grow `file` every 150ms for `forMs`; resolves when done. */ function growFile(file: string, forMs: number): Promise { return new Promise((resolve) => { @@ -151,7 +164,7 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { } it('does NOT kill a blocked loop while the watched files advance (slow store, not a wedge)', async () => { - const tmp = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')), 'db-wal'); + const tmp = progressFile(); fs.writeFileSync(tmp, 'seed'); // Base timeout 500ms; the loop blocks for 2.5s (5 timeouts, under the 10× // cap) while the test process grows the watched file. Old behavior: killed @@ -170,7 +183,7 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { }, 15000); it('still kills a blocked loop when the watched files do NOT advance (a true wedge)', async () => { - const tmp = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')), 'db-wal'); + const tmp = progressFile(); fs.writeFileSync(tmp, 'seed'); // Same blocked loop, nobody grows the file: the base timeout kills it long // before its own exit(5) at 2.5s. @@ -184,7 +197,7 @@ describe('liveness watchdog (spawned, real watchdog process)', () => { }, 15000); it('kills at the hard cap even with ongoing file activity (bounded deferral)', async () => { - const tmp = path.join(fs.mkdtempSync(path.join(require('os').tmpdir(), 'cg-wd-')), 'db-wal'); + const tmp = progressFile(); fs.writeFileSync(tmp, 'seed'); // Base timeout 300ms ⇒ cap 3s. The loop blocks for 8s with continuous file // growth: deferral carries it past 300ms but the cap kills it around ~3s, diff --git a/__tests__/mcp-initialize.test.ts b/__tests__/mcp-initialize.test.ts index 0545e3835..4b54eeb81 100644 --- a/__tests__/mcp-initialize.test.ts +++ b/__tests__/mcp-initialize.test.ts @@ -16,7 +16,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; -import { rmTempDir } from './rm-temp'; +import { once } from 'events'; const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); @@ -100,6 +100,26 @@ function waitFor( }); } +/** + * Remove `dir` once nothing holds it. Even after the killed server's 'close', + * Windows can keep the dir itself busy a moment longer under load, and + * `fs.rmSync`'s `maxRetries` never retries EBUSY on a directory: it throws at + * once. A dir still held at the deadline fails the test. + */ +async function removeWhenReleased(dir: string, timeoutMs = 5000): Promise { + const deadline = Date.now() + timeoutMs; + for (;;) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code ?? ''; + if (!['EBUSY', 'ENOTEMPTY', 'EPERM'].includes(code) || Date.now() > deadline) throw error; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } +} + describe('MCP initialize handshake (issue #172)', () => { let tempDir: string; let child: ChildProcessWithoutNullStreams | null = null; @@ -120,7 +140,7 @@ describe('MCP initialize handshake (issue #172)', () => { } child = null; } - await rmTempDir(tempDir); + await removeWhenReleased(tempDir); }); it('responds to initialize quickly when no .codegraph exists in cwd', async () => { @@ -189,3 +209,19 @@ describe('MCP initialize handshake (issue #172)', () => { expect(prompts.result.prompts).toEqual([]); }, 15000); }); + +describe('MCP initialize teardown', () => { + // On Windows a dir another process holds (here as its cwd) refuses removal + // with EBUSY; the teardown waits until it is let go. + it.runIf(process.platform === 'win32')('removes a dir once the process holding it has exited', async () => { + const held = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-init-held-')); + const holder = spawn(process.execPath, ['-e', "process.stdout.write('ready'); setTimeout(() => {}, 500)"], { + cwd: held, + stdio: ['ignore', 'pipe', 'ignore'], + }); + await once(holder.stdout, 'data'); // running, so its cwd is open + expect(() => fs.rmdirSync(held)).toThrow(/EBUSY/); + await removeWhenReleased(held); + expect(fs.existsSync(held)).toBe(false); + }); +}); diff --git a/__tests__/mcp-status-freshness.test.ts b/__tests__/mcp-status-freshness.test.ts index c229b24cd..ff73bfcbd 100644 --- a/__tests__/mcp-status-freshness.test.ts +++ b/__tests__/mcp-status-freshness.test.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import { execFileSync } from 'child_process'; import { Worker } from 'worker_threads'; import CodeGraph from '../src/index'; -import { measurePendingChanges } from '../src/mcp/index-freshness'; +import { endFreshnessMeasurements, measurePendingChanges } from '../src/mcp/index-freshness'; import { ToolHandler } from '../src/mcp/tools'; describe('MCP status freshness (#1959)', () => { @@ -28,7 +28,11 @@ describe('MCP status freshness (#1959)', () => { handler = new ToolHandler(cg); }); - afterEach(() => { + afterEach(async () => { + // Every case leaves a measurement worker still ending, which can still + // hold the index or the project dir: on Windows under load the removal + // then failed with EBUSY. Wait until they are gone. + await endFreshnessMeasurements(); try { cg.close(); } catch { /* ignore */ } fs.rmSync(root, { recursive: true, force: true }); }); diff --git a/__tests__/npm-sdk.test.ts b/__tests__/npm-sdk.test.ts index c27bfc5fe..0a3385633 100644 --- a/__tests__/npm-sdk.test.ts +++ b/__tests__/npm-sdk.test.ts @@ -13,7 +13,7 @@ * hermetically with no real bundle, network, or registry. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, afterEach } from 'vitest'; import { spawnSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; @@ -23,8 +23,13 @@ const SDK_SRC = path.join(__dirname, '..', 'scripts', 'npm-sdk.js'); const target = `${process.platform}-${process.arch}`; const VERSION = '9.9.9-test'; +// Every temp dir a case makes, removed after it. +const tmpDirs: string[] = []; + function mkTmp(label: string): string { - return fs.mkdtempSync(path.join(os.tmpdir(), `cg-sdk-${label}-`)); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), `cg-sdk-${label}-`)); + tmpDirs.push(dir); + return dir; } // A temp node_modules with the main package (npm-sdk.js + package.json). The @@ -75,6 +80,10 @@ function requireSdk(mainPkg: string, env: Record = {}) { } describe('npm-sdk programmatic entry', () => { + afterEach(() => { + for (const dir of tmpDirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + it('re-exports the installed platform bundle library', () => { const { root, mainPkg } = makeConsumer(); installPlatformPackage(root, 'platform-lib'); diff --git a/__tests__/pr19-improvements.test.ts b/__tests__/pr19-improvements.test.ts index d163ced95..eb84caabc 100644 --- a/__tests__/pr19-improvements.test.ts +++ b/__tests__/pr19-improvements.test.ts @@ -238,12 +238,13 @@ describe('Schema v2 Migration', () => { // just makes every schema change edit this test (v9/#1500 was the latest). // A fresh database records the current version, so nothing is pending; // ask a version-0 database instead to see the full migration list. - const dbPath = path.join(createTempDir(), 'schema-version.db'); - const conn = DatabaseConnection.initialize(dbPath); + const dir = createTempDir(); + const conn = DatabaseConnection.initialize(path.join(dir, 'schema-version.db')); const raw = conn.getDb(); raw.prepare('DELETE FROM schema_versions').run(); const highest = Math.max(...getPendingMigrations(raw).map((m) => m.version)); conn.close(); + cleanupTempDir(dir); expect(CURRENT_SCHEMA_VERSION).toBe(highest); }); diff --git a/__tests__/setup-home-sandbox.ts b/__tests__/setup-home-sandbox.ts index 5acb47394..ba5c4fe3a 100644 --- a/__tests__/setup-home-sandbox.ts +++ b/__tests__/setup-home-sandbox.ts @@ -21,7 +21,7 @@ * Tests that redirect the home dir themselves (spying on `os.homedir`, or * setting HOME and restoring it) keep working: they restore to this sandbox. */ -import { afterAll } from 'vitest'; +import { afterAll, inject } from 'vitest'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -30,6 +30,13 @@ import * as path from 'path'; // `/private/var`) sees the same home everything else does. const home = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-test-home-'))); +// Recorded for the end of the run, which removes the home if the afterAll +// below never runs (a file whose every test is skipped runs no hooks) or +// cannot (global-setup-home-sandbox.ts). +const registry = inject('homeSandboxRegistry'); +const registryEntry = registry ? path.join(registry, path.basename(home)) : undefined; +if (registryEntry) fs.writeFileSync(registryEntry, home); + process.env.HOME = home; process.env.USERPROFILE = home; if (process.platform === 'win32') { @@ -60,7 +67,8 @@ process.env.GIT_CONFIG_GLOBAL = gitConfig; afterAll(() => { try { fs.rmSync(home, { recursive: true, force: true, maxRetries: 3 }); + if (registryEntry) fs.rmSync(registryEntry, { force: true }); } catch { - /* a straggling child still holding a file there; it's in the temp dir */ + /* a straggling child still holding a file there; the end of the run retries */ } }); diff --git a/__tests__/test-home-sandbox.test.ts b/__tests__/test-home-sandbox.test.ts index f5ebb7a71..0775578de 100644 --- a/__tests__/test-home-sandbox.test.ts +++ b/__tests__/test-home-sandbox.test.ts @@ -3,13 +3,15 @@ * nothing a test runs — in-process or spawned — resolves the developer's real * home, Claude profile, daemon registry or git identity. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, inject } from 'vitest'; +import type { GlobalSetupContext } from 'vitest/node'; import { execFileSync } from 'child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { claudeTarget } from '../src/installer/targets/claude'; import { getRegistryDir } from '../src/mcp/daemon-registry'; +import setupHomeSandboxRun from './global-setup-home-sandbox'; const tmpRoot = fs.realpathSync(os.tmpdir()); const under = (p: string, dir: string): boolean => path.resolve(p).startsWith(dir + path.sep); @@ -62,4 +64,28 @@ describe('test home sandbox', () => { expect(under(process.env.APPDATA ?? '', home)).toBe(true); expect(under(process.env.LOCALAPPDATA ?? '', home)).toBe(true); }); + + // A file whose every test is skipped runs no hooks, so its own afterAll never + // removes its home: the end of the run removes every home still listed. + it("records this file's home in the run's registry", () => { + const home = sandboxHome(); + const entry = path.join(inject('homeSandboxRegistry'), path.basename(home)); + expect(fs.readFileSync(entry, 'utf8')).toBe(home); + }); + + it('the end of the run removes every home still listed, then the registry', () => { + let registry = ''; + const teardown = setupHomeSandboxRun({ + provide(_key: string, value: string) { registry = value; }, + } as unknown as GlobalSetupContext); + const homes = ['skipped', 'held'].map((label) => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), `codegraph-test-home-${label}-`)); + fs.writeFileSync(path.join(home, '.gitconfig'), ''); + fs.writeFileSync(path.join(registry, path.basename(home)), home); + return home; + }); + teardown(); + expect(homes.filter((home) => fs.existsSync(home))).toEqual([]); + expect(fs.existsSync(registry)).toBe(false); + }); }); diff --git a/__tests__/ts-field-classification.test.ts b/__tests__/ts-field-classification.test.ts index 82069a8d2..39d6217f8 100644 --- a/__tests__/ts-field-classification.test.ts +++ b/__tests__/ts-field-classification.test.ts @@ -99,7 +99,6 @@ describe('TS/JS class field classification (#808)', () => { expect(fontsProp.signature).toBe('Fonts fonts'); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -126,7 +125,6 @@ describe('TS/JS class field classification (#808)', () => { expect(cg.getNodesByName('onClick')[0]?.kind).toBe('method'); } finally { cg.destroy(); - tmpDir = undefined; } }); @@ -153,7 +151,6 @@ describe('TS/JS class field classification (#808)', () => { expect(fnRefs.length).toBeGreaterThan(0); } finally { cg.destroy(); - tmpDir = undefined; } }); }); diff --git a/vitest.config.mts b/vitest.config.mts index 87a59fba5..7945f2898 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -23,8 +23,14 @@ export default defineConfig({ '__tests__/setup-home-sandbox.ts', ], // Suites need a kernel and a current dist/ (#1879). Every run's - // temp files go in one directory that is removed afterwards. - globalSetup: ['__tests__/kernel.global-setup.ts', '__tests__/global-setup-dist.ts', '__tests__/run-tmpdir.global-setup.ts'], + // temp files go in one directory that is removed afterwards, and the end + // of the run removes any home sandbox its own test file could not. + globalSetup: [ + '__tests__/kernel.global-setup.ts', + '__tests__/global-setup-dist.ts', + '__tests__/run-tmpdir.global-setup.ts', + '__tests__/global-setup-home-sandbox.ts', + ], env: { /** * The suite spawns real CLI/MCP processes; without this they would write