fix(c,cpp): a source implements the header beside it that it reaches through other headers

CHANGELOG.md

@@ -132,6 +132,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - 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.
 - In C and C++ tests, a name that several files declare now links to the one the test's own includes declare, when there is one: protocolbuffers/protobuf's C# generator tests call the `TestSourceDir()` that `googletest.h` declares, not another test helper's they never include; its upb tests call `upb_Arena_New()` and the rest of upb's C API in upb's own files, not the copies bundled for PHP and Ruby, which had left those calls unlinked; and facebook/rocksdb's C API test reaches `rocksdb_open` and its siblings in `db/c.cc`. Re-index C and C++ projects after upgrading.
 - In C and C++ code outside tests too, a name that several files declare now links to the one the file's own includes declare, when there is one: protocolbuffers/protobuf's Python extension calls functions like `upb_Encode()` in upb's own files rather than the copies bundled for PHP, and facebook/rocksdb's stress tool calls the `StringToHex()` its own header declares rather than `LDBCommand::StringToHex`. Re-index C and C++ projects after upgrading.
+- In C and C++, a call of a function a header declares now links to its definition even when the file that defines it reaches that header only through other headers: protocolbuffers/protobuf's Python extension calls upb's reflection functions, like `upb_FieldDef_Number()`, in upb's own files rather than the copies bundled for PHP. Re-index C and C++ projects after upgrading.
 - In C++, a declaration continued on a line that starts with `*`, like facebook/rocksdb's `const auto& el =` over `*reinterpret_cast<std::shared_ptr<EventListener>*>(jhandle);`, is now read, so calls such as `el->OnFlushCompleted(…)` link to `EventListener`'s methods; such a line used to be skipped as part of a comment. Code commented out in a `/* … */` block, and the text of a string literal, like protocolbuffers/protobuf's `[[deprecated("Please use SpaceUsedLong() instead")]]`, are no longer read as declarations, which had sent a call to the wrong method or to none. Re-index C++ projects after upgrading.
 - In C++, a call with a comparison or a shift among its arguments, like protocolbuffers/protobuf's `p->Emit({{"declare", has_bit_word_index < 0 ? … : ""}, …}, "…")` or `WriteVarint(num << 3, s)`, is now read with the right number of arguments, so it links to the overload that fits rather than one taking fewer. A parameter or variable whose type has template arguments is now read even after another such parameter on the same line, like facebook/rocksdb's `const autovector<uint32_t>& cf_ids`, or with empty ones, like fmt's `parse_context<>& ctx`: calls on it link to that class's methods, and calls on a `std::vector` declared that way no longer link to a project class's method of the same name. Re-index C++ projects after upgrading.
 - In C++, a call passing `nullptr` now links to the overload that takes a `std::nullptr_t`, as C++ picks it, rather than one taking a pointer, and a call no longer links to an overload declared `= delete`, which no call that compiles can reach: protocolbuffers/protobuf's `ptr.reset(nullptr)` on an `Arena::UniquePtr` reaches `reset(std::nullptr_t)`, and `field.Add("foo")` on a `RepeatedPtrField` reaches `Add(Element&&)` instead of the deleted `Add(const Element&)`. Re-index C++ projects after upgrading.

__tests__/cpp-translation-unit.test.ts

@@ -7,7 +7,10 @@
  * helpers.h declares, not `ImmutableFieldGenerator::HasHasbit`, a method
  * beside the caller. A source implements the header beside it (the
  * Python extension's pyext/message.cc is no implementation of
- * google/protobuf/message.h), a header's inline overload reaches the
+ * google/protobuf/message.h), even one it reaches only through other
+ * headers (upb's reflection/field_def.c includes
+ * reflection/internal/field_def.h, which reaches reflection/field_def.h
+ * through def.h), a header's inline overload reaches the
  * definition its own source holds, and a call keeps a function outside the
  * unit that takes its arguments over one inside that can't (glog's
  * three-argument `GetStackTrace(…)` is not utilities.h's `GetStackTrace()`).
@@ -44,13 +47,38 @@ int upb_Encode(const void* msg, int options, char** buf, size_t* size) {
     'php/ext/google/protobuf/php-upb.h': `#include <stddef.h>
 
 int upb_Encode(const void* msg, int options, char** buf, size_t* size);
+int upb_FieldDef_Number(const void* f);
 `,
     'php/ext/google/protobuf/php-upb.c': `#include "php-upb.h"
 
 int upb_Encode(const void* msg, int options, char** buf, size_t* size) {
   *size = 0;
   return 0;
 }
+
+int upb_FieldDef_Number(const void* f) { return 0; }
+`,
+    // upb's reflection/field_def.c implements reflection/field_def.h, which
+    // it reaches only through its internal header and def.h.
+    'upb/reflection/field_def.h': `typedef struct upb_FieldDef upb_FieldDef;
+
+int upb_FieldDef_Number(const upb_FieldDef* f);
+`,
+    'upb/reflection/def.h': `#include "upb/reflection/field_def.h"
+`,
+    'upb/reflection/internal/field_def.h': `#include "upb/reflection/def.h"
+
+int _upb_FieldDef_Index(const upb_FieldDef* f);
+`,
+    'upb/reflection/field_def.c': `#include "upb/reflection/internal/field_def.h"
+
+int upb_FieldDef_Number(const upb_FieldDef* f) { return 1; }
+`,
+    'python/convert.c': `#include "upb/reflection/def.h"
+
+int PyUpb_FieldNumber(const upb_FieldDef* f) {
+  return upb_FieldDef_Number(f);
+}
 `,
     'python/message.c': `#include "upb/wire/encode.h"
 
@@ -344,6 +372,12 @@ describe('a C/C++ reference links what its translation unit declares', () => {
     expect(calls.some((c) => c.includes('php-upb'))).toBe(false);
   });
 
+  it('reads a source as implementing the header beside it that it reaches through others', () => {
+    const calls = callsFrom('python/convert.c');
+    expect(calls).toContain('upb_FieldDef_Number (upb/reflection/field_def.c)');
+    expect(calls.some((c) => c.includes('php-upb'))).toBe(false);
+  });
+
   it('calls the free function its includes declare, not a class\'s method beside it', () => {
     const calls = callsFrom('src/google/protobuf/compiler/java/full/enum_field.cc');
     expect(calls).toContain('google::protobuf::compiler::java::HasHasbit (src/google/protobuf/compiler/java/helpers.h)');

src/resolution/cpp-includers.ts

@@ -33,6 +33,8 @@ interface Include {
 interface Memo {
   /** Every C / C++ `#include`, by the file name it spells (`a/b.h` → `b.h`). */
   byName: Map<string, Include[]> | null;
+  /** The same, by the stem of that name (`a/b.h` → `b`). */
+  byStem: Map<string, Include[]> | null;
   /** Import node id → the indexed file its `#include` names (null: none). */
   targets: Map<string, string | null>;
   /** File → the files that include it, directly or through other includes. */
@@ -47,12 +49,17 @@ export function clearCppIncluderMemos(context: ResolutionContext): void {
 
 function memoFor(context: ResolutionContext): Memo {
   let memo = MEMO.get(context);
-  if (!memo) MEMO.set(context, (memo = { byName: null, targets: new Map(), includers: new Map() }));
+  if (!memo) MEMO.set(context, (memo = { byName: null, byStem: null, targets: new Map(), includers: new Map() }));
   return memo;
 }
 
 const isCFamily = (language: string): boolean => language === 'c' || language === 'cpp';
 const fileName = (filePath: string): string => filePath.slice(Math.max(filePath.lastIndexOf('/'), filePath.lastIndexOf('\\')) + 1);
+const stemOf = (name: string): string => {
+  const dot = name.lastIndexOf('.');
+  return dot > 0 ? name.slice(0, dot) : name;
+};
+const dirOf = (filePath: string): string => filePath.slice(0, filePath.lastIndexOf('/') + 1);
 
 const toInclude = (n: Pick<Node, 'id' | 'name' | 'filePath' | 'language' | 'startLine' | 'startColumn'>): Include =>
   ({ id: n.id, spec: n.name, file: n.filePath, language: n.language, line: n.startLine, column: n.startColumn });
@@ -91,6 +98,37 @@ function includesByName(context: ResolutionContext, memo: Memo): Map<string, Inc
   return byName;
 }
 
+function includesByStem(context: ResolutionContext, memo: Memo): Map<string, Include[]> {
+  if (memo.byStem) return memo.byStem;
+  const byStem = new Map<string, Include[]>();
+  for (const [name, includes] of includesByName(context, memo)) {
+    const stem = stemOf(name);
+    const list = byStem.get(stem);
+    if (list) list.push(...includes);
+    else byStem.set(stem, [...includes]);
+  }
+  memo.byStem = byStem;
+  return byStem;
+}
+
+/**
+ * The headers in `filePath`'s folder named like it (`field_def.c` →
+ * `field_def.h`) that it includes, directly or through other headers. upb's
+ * reflection/field_def.c includes only reflection/internal/field_def.h, which
+ * reaches the reflection/field_def.h it implements through def.h.
+ */
+export function cppIncludedHeadersBeside(filePath: string, context: ResolutionContext): string[] {
+  const memo = memoFor(context);
+  const dir = dirOf(filePath);
+  const found: string[] = [];
+  for (const include of includesByStem(context, memo).get(stemOf(fileName(filePath))) ?? []) {
+    const target = includedFile(include, context, memo);
+    if (!target || target === filePath || found.includes(target) || dirOf(target) !== dir) continue;
+    if (cppIncluders(target, context).has(filePath)) found.push(target);
+  }
+  return found;
+}
+
 /** The indexed file the `#include` an import node records names, or null. */
 export function cppIncludedFile(include: Node, context: ResolutionContext): string | null {
   return include.kind === 'import' && isCFamily(include.language) ? includedFile(toInclude(include), context, memoFor(context)) : null;

src/resolution/name-matcher.ts

@@ -18,7 +18,7 @@ import { cppAliasedClass, cppAliasedTypeName, cppAliasedTypeWritten, cppDeclares
 import { clearCppNamespaceMemos, cppMacroNamespaceFrames, cppMacroNamespaceHeads, cppNameMacros, cppNamespaceAliases } from './cpp-namespaces';
 import { clearCppRawStringMemos, cppLiteralsBlanked, cppNonCodeLines } from './cpp-raw-strings';
 import { cppClassWritten, type CppNameSite } from './cpp-supertypes';
-import { cppIncludedFile, cppIncluders } from './cpp-includers';
+import { cppIncludedFile, cppIncludedHeadersBeside, cppIncluders } from './cpp-includers';
 import { isCFeatureVariant, isOffByDefault } from './c-feature-variants';
 import { isTestPath } from '../search/query-utils';
 import { isMinifiedContent } from '../extraction/generated-detection';
@@ -1620,7 +1620,10 @@ const CPP_STEM_HEADERS = new WeakMap<ResolutionContext, Map<string, string[]>>()
  * definitions implement. One beside it, when it includes one, is the only
  * such header: protobuf's pyext/message.cc implements pyext/message.h, not
  * the google/protobuf/message.h it also includes; fmt's src/format.cc, with
- * none beside it, implements include/fmt/format.h.
+ * none beside it, implements include/fmt/format.h. It may include that one
+ * through other headers only: upb's reflection/field_def.c includes
+ * reflection/internal/field_def.h, which reaches reflection/field_def.h
+ * through def.h (cppIncludedHeadersBeside).
  */
 function cppSameStemHeaders(filePath: string, context: ResolutionContext): string[] {
   let memo = CPP_STEM_HEADERS.get(context);
@@ -1635,7 +1638,8 @@ function cppSameStemHeaders(filePath: string, context: ResolutionContext): strin
       if (header && header !== filePath && fileStem(header) === stem) found.push(header);
     }
     const dir = filePath.slice(0, filePath.lastIndexOf('/') + 1);
-    const beside = found.filter((header) => header.slice(0, header.lastIndexOf('/') + 1) === dir);
+    let beside = found.filter((header) => header.slice(0, header.lastIndexOf('/') + 1) === dir);
+    if (beside.length === 0) beside = cppIncludedHeadersBeside(filePath, context);
     headers = beside.length > 0 ? beside : found;
     memo.set(filePath, headers);
   }