fix(cpp): a bare call in a gtest TEST_F or TEST_P body reaches the fixture's method

[colbymchenry]

Summary

The index holds a gtest TEST_F(Fixture, Name) or TEST_P body as a function named TEST_F, but gtest makes it a method of a class it derives from Fixture. C++ looks a name the body calls up in that class, then in the fixture and the classes it derives from, before the namespace.

#2533 gave such a body the fixture's data members (cppTestFixture), but not its methods. facebook/rocksdb's 5,000 test bodies call DBTestBase's Put(…), Flush(), Key(i) and NewIterator(…) bare, and those calls were guessed by name:

• most at 0.4, and often to a namesake in another class (ColumnFamilyTestBase::Put, WriteBatch::Put);
• auto iter = NewIterator(…) gave iter no type, so iter->SeekToFirst() reached the nested MemTableRep::Iterator.

protocolbuffers/protobuf's command-line tests sent ExpectNoErrors() to the unrelated EncodeDecodeTest::ExpectNoErrors.

What changed (name-matcher.ts)

• cppImplicitThisMethod. This is fix(cpp): a call a method makes with no receiver links to its class's own or inherited method #2479's lookup taken out of matchCppImplicitThisCall: the caller's class, the classes it derives from, then the classes around it. A gtest body's owner is its fixture (cppTestFixture).
  • The lookup keeps the declaration of the class in the caller's own file: rocksdb declares BestEffortsRecoverIncompleteVersionTest and LRUCacheTest in two test files each, with different bases.
  • matchCppImplicitThisCall keeps fix(cpp): a call a method makes with no receiver links to its class's own or inherited method #2479's local-shadowing check for the link.
• Typing bare calls. cppExpressionType types a bare call (auto iter = NewIterator(…)) with the same lookup, made as on this, rather than in the class's first declaration.
  • So of two classes sharing a name, it reads the caller's own.
  • As before, it doesn't skip a template's dependent base: a bare call into one compiles only through a using-declaration, like protobuf's using RepeatedFieldWrapper<T>::MutableRepeatedField;.
  • this in a gtest body is a pointer to the fixture.
• -> on a value of an outside type. A -> call on a value of a type from outside the project no longer takes that type's last name for the class (isCppArrowThroughOutsideType); the name strategies decide.
  • Typing JsonTest's auto m = ToProto<Any>(…) as absl::StatusOr<Any> had sent m->value() to protobuf's own hpb::StatusOr::value at 0.9.
  • The operator check returns before fix(cpp): a call on an auto variable or a chained call reaches the class C++ finds, not a namesake #2532's outside-type gate for -> on a value, so that gate never saw these calls. On main, a declared absl::StatusOr<Any> parsed gets the same; these repos just declare none whose method hpb::StatusOr also has.

Validation

Site-keyed edge diffs (diff-sites.mjs), main df3ce972 vs this branch, both built from the same tree, kernel included:

repo	removed	added	retargeted	metadata-only
google/leveldb	0	53	51	1,003
fmtlib/fmt	0	0	0	2
facebook/rocksdb	68	3,784	22,203	12,246
protocolbuffers/protobuf	14	9	826	3,124
redis/redis (C, control)	0	0	0	0

Nodes and files are identical between the arms, and every changed site is in a gtest TEST_F / TEST_P body: no edge outside one moved.

Retargeted, every group read. A script checked every retargeted or added bare call in a gtest body: it reaches the fixture the body's header names, or a class the fixture derives from.

• rocksdb: all 22,456. In 17 of them the fixture's name is declared in two test files with different bases, and the class in the body's own file is the one used.
• protobuf: all 826. One goes through using FeaturesBaseTest = ValidationErrorTest;.
• leveldb: all 51.

The large groups, by the class the calls left:

• rocksdb:
  • 7,967 calls like Flush() and Put(…) go from ColumnFamilyTestBase's namesakes at 0.4 to DBTestBase at 0.9;
  • 3,133 go from a namespace function of the same name, like Key(i), to DBTestBase's static method, which the class scope finds first;
  • 1,797 had reached CompactionJobStatsTest and 1,549 CuckooTableDBTest (DestroyAndReopen, CurrentOptions, …);
  • 58 iter->Seek(…) / iter->SeekToFirst() calls leave MemTableRep::Iterator for IteratorBase.
• protobuf:
  • 219 ExpectNoErrors() calls go to the CommandLineInterfaceTester that CommandLineInterfaceTest derives from;
  • 131 BuildFile(…) calls go to ValidationErrorTest, no longer AllowUnknownDependenciesTest;
  • test bodies' AddToDatabase calls go to the fixture's method, which hides the namespace function of that name.
• leveldb: LogTest::Read, not the nested StringSource::Read; DBTest::Put, not ModelDB::Put; Harness::Add.

Overload choices within one class (rocksdb 520, protobuf 437) were sorted by argument count against each overload's parameters. 809 fit better and none fit worse. 135 tie on arity, and 13 have raw-string arguments the count can't read:

• Most ties go to the overload the arguments' types call for: string literals to const char* / std::string, Get(0, "key0") to Get(int cf, …), braced lists to std::initializer_list.
• 20 go to the wrong one. fix(cpp): a call a method makes with no receiver links to its class's own or inherited method #2479's cppOverloadFor keeps the first-written overload when arguments tie, and here only the argument's type tells them apart:
  • protobuf's BuildFile(X::descriptor()->file()) (18) now reaches BuildFile(absl::string_view), not BuildFile(const FileDescriptor*), which main's 0.7 guess had;
  • rocksdb's Insert(ch) on a char (2).

Removed, every site read. rocksdb's 68 and protobuf's 14 were wrong guesses on library types the fixture's methods now type:

• 45 compaction_stats.size() and 10 blob_files_initial.size() on returned std::vectors, which had reached CompactionInputFiles / BlobContents at 0.65;
• protobuf's arena.get() on a std::unique_ptr<Arena>, which had reached ArenaHolder::get.

Added:

• chains through the fixture's methods: leveldb's dbfull()->TEST_CompactMemTable(), rocksdb's dbfull()->TEST_WaitForCompact();
• calls on locals those methods type: iter->Seek(…), precise_cache->GetUsage() on NewCache(…)'s std::shared_ptr<Cache>.

Metadata-only: the same target, now found by C++ lookup at 0.9, no longer guessed.

Tests: __tests__/cpp-test-fixture-call.test.ts, 9 cases:

• 5 fail on main:
  • the fixture's own and inherited methods;
  • auto iter = NewIterator();
  • TEST_P and this->;
  • two test files' fixtures sharing a name;
  • -> on an absl::StatusOr value, which main sends to hpb::StatusOr for a declared parameter.
• 4 guard behavior that must not change:
  • a TEST body has no fixture;
  • a local shadows a fixture's method;
  • a bare call's type in a method, past a nested class's constructor parameter of its name (leveldb's SpecialEnv, which an earlier version of this change broke);
  • a template's using-declared dependent base (protobuf's RepeatedFieldPrimitiveAccessor).
• 7 mutations, one per guard or branch, each fail a test.
• Full suite: 6,980 passed, 0 failed, 82 skipped.

Together with #2536

• fix(cpp): a declaration with a qualifier or annotation around its name gives its type to calls on it #2536 reads declarations with qualifiers or annotations around the name (Iterator* const iter). The two branches touch different parts of name-matcher.ts and conflict only in CHANGELOG, where both bullets go after fix(cpp): a call on a data member of the caller's class reaches its type's method, wherever the class is declared #2533's: keep both.
• Both applied to df3ce972 pass the 28 C/C++ test files (335 tests).
• Indexed with both, rocksdb's calls into MemTableRep::Iterator go from 111 to 53, the same as with this PR alone. 44 of the 53 are right: memtable.cc's own 42, and 2 on a static_cast<MemTableRep::Iterator*>. The other 9 have a receiver typed through a std::function<Iterator*()> call or a structured binding (auto& [cfh, iter]).

Not in this PR

• Overloads of one arity are told apart only by string literals (cppOverloadFit), not by the arguments' types: the 20 ties above.
• A bare call into a template's dependent base that a using-declaration names is still not linked by fix(cpp): a call a method makes with no receiver links to its class's own or inherited method #2479's lookup, only typed.
• absl::StatusOr<T>'s -> isn't followed to T the way std::unique_ptr's and std::optional's are, so resolver->MergeFeatures(…) stays a 0.7 guess.

🤖 Generated with Claude Code