fix(cpp): a nullptr argument picks the std::nullptr_t overload, and no call links to a deleted overload

CHANGELOG.md

@@ -113,6 +113,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++, 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.
 - 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.

__tests__/cpp-nullptr-deleted-overloads.test.ts

@@ -0,0 +1,131 @@
+/**
+ * Two ways a C++ call reached an overload no call that compiles reaches:
+ * - A `nullptr` argument fitted any pointer parameter as well as it could fit
+ *   anything, and a `std::nullptr_t` parameter no better than a class, so
+ *   protocolbuffers/protobuf's `ptr.reset(nullptr)` on an
+ *   `Arena::UniquePtr<int>` took `reset(T*)` over `reset(std::nullptr_t)`.
+ *   `nullptr` is a `std::nullptr_t` exactly, and a pointer by conversion.
+ * - That `reset(T*)` is declared `= delete`. A deleted overload is now taken
+ *   only when every overload is.
+ */
+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';
+
+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-nullptr-'));
+  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');
+
+/** What the calls at `line` of the function or method `qualifiedName` reach, as `target:its first line @confidence`. */
+function callsAt(cg: CodeGraph, qualifiedName: string, line: number): string[] {
+  const from = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName)!;
+  return cg.getOutgoingEdgesFrom([from.id], ['calls'])
+    .filter((e) => e.line === line)
+    .map((e) => {
+      const t = cg.getNode(e.target)!;
+      return `${t.qualifiedName}:${t.startLine} @${(e.metadata as { confidence?: number } | undefined)?.confidence}`;
+    })
+    .sort();
+}
+
+describe('C++ overloads for nullptr, and deleted ones', () => {
+  it("takes protobuf's reset(std::nullptr_t) for reset(nullptr), not the deleted reset(T*)", async () => {
+    const cg = await indexed({
+      'arena.h': lines(
+        '#include <cstddef>',
+        'namespace google {',
+        'namespace protobuf {',
+        'class Arena {',
+        ' public:',
+        '  template <typename T>',
+        '  class UniquePtr {',
+        '   public:',
+        '    void reset() {}',
+        '    void reset(std::nullptr_t) {}',
+        '    void reset(T* PROTOBUF_NULLABLE) = delete;',
+        '  };',
+        '};',
+        '}  // namespace protobuf',
+        '}  // namespace google',
+      ),
+      'arena_unittest.cc': lines(
+        '#include "arena.h"',
+        'namespace google {',
+        'namespace protobuf {',
+        'void Reset() {',
+        '  Arena::UniquePtr<int> ptr;',
+        '  ptr.reset(nullptr);',
+        '  ptr.reset();',
+        '}',
+        '}  // namespace protobuf',
+        '}  // namespace google',
+      ),
+    });
+    try {
+      expect(callsAt(cg, 'google::protobuf::Reset', 6)).toEqual(['google::protobuf::Arena::UniquePtr::reset:10 @0.9']);
+      expect(callsAt(cg, 'google::protobuf::Reset', 7)).toEqual(['google::protobuf::Arena::UniquePtr::reset:9 @0.9']);
+    } finally {
+      cg.close();
+    }
+  });
+
+  it('fits nullptr to std::nullptr_t before a pointer, and 0 to an int before both', async () => {
+    const cg = await indexed({
+      'canvas.h': lines(
+        '#include <cstddef>',
+        'class Widget {};',
+        'class Canvas {',
+        ' public:',
+        '  void Set(Widget* w) {}',
+        '  void Set(std::nullptr_t) {}',
+        '  void Set(int v) {}',
+        '  void Run() {',
+        '    Set(nullptr);',
+        '    Set(0);',
+        '  }',
+        '};',
+      ),
+    });
+    try {
+      expect(callsAt(cg, 'Canvas::Run', 9)).toEqual(['Canvas::Set:6 @0.9']);
+      expect(callsAt(cg, 'Canvas::Run', 10)).toEqual(['Canvas::Set:7 @0.9']);
+    } finally {
+      cg.close();
+    }
+  });
+
+  it('takes no deleted overload while another fits, reading each declaration to its own end', async () => {
+    const cg = await indexed({
+      'meter.h': lines(
+        'class Meter {',
+        ' public:',
+        '  void Scale(double factor) = delete;',
+        '  void Scale(int factor) {}',
+        '  void Scale(',
+        '      long factor) = delete;',
+        '  void Run() { Scale(2); }',
+        '};',
+      ),
+    });
+    try {
+      expect(callsAt(cg, 'Meter::Run', 7)).toEqual(['Meter::Scale:4 @0.9']);
+    } finally {
+      cg.close();
+    }
+  });
+});

src/resolution/name-matcher.ts

@@ -10637,9 +10637,13 @@ function cppScopeMethod(scope: CppClassScope, name: string, receiver: 'bare' | '
  * (cppOverloadFit), then, among overloads those fit alike, by the types the
  * arguments have where the call reads them (cppArgumentTypeFit). rocksdb's
  * `Insert(ch)` on a `char ch` is `Insert(char key, …)`, not `Insert(const
- * std::string& key, …)`.
+ * std::string& key, …)`. A deleted overload (`void reset(T*) = delete;`) is
+ * none a call that compiles reaches, so it is only taken when every one is.
  */
-function cppOverloadFor(overloads: Node[], name: string, ref: UnresolvedRef, context: ResolutionContext): Node {
+function cppOverloadFor(candidates: Node[], name: string, ref: UnresolvedRef, context: ResolutionContext): Node {
+  if (candidates.length === 1) return candidates[0]!;
+  const callable = candidates.filter((n) => !isCppDeletedFunction(n, name, context));
+  const overloads = callable.length > 0 ? callable : candidates;
   if (overloads.length === 1) return overloads[0]!;
   const args = cppCallArguments(ref, name, context);
   if (!args) return overloads[0]!;
@@ -10661,6 +10665,27 @@ function cppOverloadFor(overloads: Node[], name: string, ref: UnresolvedRef, con
   return best;
 }
 
+const CPP_DELETED_FUNCTIONS = new WeakMap<ResolutionContext, Map<string, boolean>>();
+
+/**
+ * Whether the C++ function or method `n`, named `name`, is declared deleted:
+ * from its name to the declaration's `;` or body, it says `= delete`
+ * (protobuf's `void reset(T* PROTOBUF_NULLABLE) = delete;`).
+ */
+function isCppDeletedFunction(n: Node, name: string, context: ResolutionContext): boolean {
+  let memo = CPP_DELETED_FUNCTIONS.get(context);
+  if (!memo) CPP_DELETED_FUNCTIONS.set(context, (memo = new Map()));
+  const hit = memo.get(n.id);
+  if (hit !== undefined) return hit;
+  const lines = context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/) ?? [];
+  const text = lines.slice(n.startLine - 1, Math.min(lines.length, n.startLine + 11)).join('\n');
+  const at = text.search(new RegExp(`\\b${name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*\\(`));
+  const declaration = at < 0 ? '' : text.slice(at).split(/[;{]/)[0]!;
+  const deleted = /=\s*delete\b/.test(declaration);
+  memo.set(n.id, deleted);
+  return deleted;
+}
+
 /** What a C++ call's argument is: a literal's kind, or the type a name or expression has as written. */
 type CppArgumentType = { kind: 'char' | 'number' | 'zero' | 'bool' | 'null' } | { kind: 'type'; text: string };
 
@@ -10701,7 +10726,7 @@ function cppArgumentType(arg: string, ref: UnresolvedRef, context: ResolutionCon
 }
 
 /** What a C++ type is, for an argument to fit a parameter of it. */
-type CppTypeCategory = 'string' | 'cstring' | 'char' | 'number' | 'bool' | 'pointer' | 'class';
+type CppTypeCategory = 'string' | 'cstring' | 'char' | 'number' | 'bool' | 'pointer' | 'null' | 'class';
 
 const CPP_CHAR_TYPES: ReadonlySet<string> = new Set(['char', 'wchar_t', 'char8_t', 'char16_t', 'char32_t']);
 const CPP_NUMBER_TYPES: ReadonlySet<string> = new Set([
@@ -10710,12 +10735,16 @@ const CPP_NUMBER_TYPES: ReadonlySet<string> = new Set([
 ]);
 const CPP_STRING_TYPES: ReadonlySet<string> = new Set(['string', 'wstring', 'string_view', 'wstring_view', 'basic_string', 'basic_string_view']);
 
-/** The category of the written C++ type `type`: a pointer's, a string's, a character's, a number's, a class's. */
+/**
+ * The category of the written C++ type `type`: a pointer's, a string's, a
+ * character's, a number's, `std::nullptr_t`'s, a class's.
+ */
 function cppTypeCategory(type: string): CppTypeCategory {
   const flat = stripCppTemplateArguments(type).replace(/\b(?:const|volatile|struct|class|typename|enum)\b/g, ' ').replace(/&+/g, ' ').trim();
   const words = flat.replace(/\*/g, ' ').trim().split(/\s+/);
   const last = cppLastSegment(words[words.length - 1] ?? '');
   if (flat.includes('*')) return CPP_CHAR_TYPES.has(last) && (flat.match(/\*/g) ?? []).length === 1 ? 'cstring' : 'pointer';
+  if (last === 'nullptr_t') return 'null';
   if (last === 'bool') return 'bool';
   if (CPP_CHAR_TYPES.has(last) && words.every((w) => CPP_CHAR_TYPES.has(w) || w === 'signed' || w === 'unsigned')) return 'char';
   if (CPP_NUMBER_TYPES.has(last) || words.every((w) => CPP_NUMBER_TYPES.has(w))) return 'number';
@@ -10744,13 +10773,17 @@ function cppArgumentTypeFit(n: Node, name: string, types: Array<CppArgumentType
   return score;
 }
 
-/** How a literal fits a parameter of each category. */
+/**
+ * How a literal fits a parameter of each category. `nullptr` is a
+ * `std::nullptr_t` exactly, and a pointer only by conversion: protobuf's
+ * `ptr.reset(nullptr)` is `reset(std::nullptr_t)`, not `reset(T*)`.
+ */
 const CPP_LITERAL_FIT: Record<'char' | 'number' | 'zero' | 'bool' | 'null', Record<CppTypeCategory, number>> = {
-  char: { char: 3, number: 1, bool: 0, string: -3, cstring: -3, pointer: -3, class: 0 },
-  number: { number: 2, char: 1, bool: 0, string: -3, cstring: -3, pointer: -3, class: 0 },
-  zero: { number: 2, char: 1, bool: 0, pointer: 1, cstring: 1, string: -3, class: 0 },
-  bool: { bool: 3, number: 1, char: 0, string: -3, cstring: -3, pointer: -3, class: 0 },
-  null: { pointer: 3, cstring: 3, class: 1, string: -3, char: -3, number: -3, bool: -1 },
+  char: { char: 3, number: 1, bool: 0, string: -3, cstring: -3, pointer: -3, null: -3, class: 0 },
+  number: { number: 2, char: 1, bool: 0, string: -3, cstring: -3, pointer: -3, null: -3, class: 0 },
+  zero: { number: 2, char: 1, bool: 0, pointer: 1, cstring: 1, null: 1, string: -3, class: 0 },
+  bool: { bool: 3, number: 1, char: 0, string: -3, cstring: -3, pointer: -3, null: -3, class: 0 },
+  null: { null: 4, pointer: 3, cstring: 3, class: 1, string: -3, char: -3, number: -3, bool: -1 },
 };
 
 /** How an argument written as type `arg` fits a parameter of type `param` (of category `category`). */
@@ -11957,6 +11990,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void {
   CPP_USING_MEMBERS.delete(context);
   CPP_MEMBER_MACRO_ARGS.delete(context);
   CPP_OVERLOAD_PARAMETERS.delete(context);
+  CPP_DELETED_FUNCTIONS.delete(context);
   CPP_LOOP_ELEMENTS.delete(context);
   CPP_AUTO_LOCALS.delete(context);
   CPP_SCOPE_NAMES.delete(context);