From 819a0e4e0f1aeb3ce0c676e2c718744b93a02188 Mon Sep 17 00:00:00 2001 From: bompus Date: Tue, 27 Oct 2026 10:33:30 -0500 Subject: [PATCH] fix(resolution): ties that decide a link break by code unit, not the host's locale Several resolution tie-breaks sorted paths or ids with a bare localeCompare, which follows the host's locale, so two machines could index the same repository differently: under Czech collation `ch/` sorts after `h/`, under Danish `aa` sorts after `z`, and locale collation puts `alpha/` before `Beta/` where code-unit order does the opposite. The PR fixes the nearest-declaration tie in tsFieldCall, for `this.field.method()` on a type declared in two equally near apps. On top, the same bug at the three other sites that decide resolution: - cpp-type-aliases: the declaration an alias resolves to when two headers declare it differently, for a C++ base class named through it; - frameworks/react-native: the Swift copy of a module's class that a JS-to-native call reaches; - go-func-field-synthesizer: the last tie-break of wiringOrder, which decides the values a capped channel keeps (its site comparison already names 'en' and stays). All compare by code unit now. Comparators that only order output (CLI, MCP and viewer listings, dead-code and hierarchy order, the context builder's copybook entries) are left alone. The CHANGELOG bullet moves from the released 1.6.1 section to Unreleased. Validation: a test each for the TS, C++ and React Native sites; all 3 fail on main and pass here. Indexing `ch/` vs `h/` fixtures for those three under a simulated English and a simulated Czech host (a localeCompare preload that reaches the worker threads), main links different targets in all three, this commit the same in both. A/B, main vs this commit on an English-locale host, 12 repos (excalidraw, zod, vben, bulletproof-react, jq, cobra, gin, etcd, prometheus, okhttp, with flask and spring-modulith as controls): every dump byte-identical. Indexed again under a simulated Danish host (the Go repos) and Czech host (the TS repos), both arms match their English dumps, so these repos reach no tie the locale decides; the fix matters where one does. Original PR #1983 by @bompus. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/cpp-base-class-lookup.test.ts | 30 +++++++++++++++++++++ __tests__/react-native-bridge.test.ts | 9 +++++++ __tests__/ts-this-field-call.test.ts | 21 +++++++++++++++ src/resolution/cpp-type-aliases.ts | 6 +++-- src/resolution/frameworks/react-native.ts | 3 ++- src/resolution/go-func-field-synthesizer.ts | 5 +++- src/resolution/name-matcher.ts | 6 ++++- 8 files changed, 76 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93523c1374..e422877584 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph_explore` and `codegraph context` now find your project's busiest file, and favor the code around it, even when twenty or more test or generated files are busier still, such as a package of protobuf stubs. Before, the search looked only at the 20 busiest files and then set the tests and generated ones aside, so it could find no file at all, or judge one far ahead of the next when it was not. `getTopRouteFile()` in the library counts every file's routes the same way. Thanks @inth3shadows for the report. (#2578) - In JavaScript and TypeScript, a call written as the body of an `if`, `while`, `for` or `do` without braces, like `if (e.key === "Escape") setOpen(false);`, no longer links to a same-named property or method in another file when the name is the file's own, such as a `setOpen` from `useState` (#2585). - In JavaScript and TypeScript, a method called on an element of a class field, like `this.handlers[type].handle()` or `this.items[0].save()`, now links to the method of the type the field is declared to hold, such as `Item[]` or `Record`, instead of to any project method that shares its name, often another class's, and is left unlinked when that type isn't written down, as in plain JavaScript. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows. (#2766) +- Indexes built on machines set to different languages now agree. When a TypeScript `this.field.method()` call finds its type declared in two equally near apps, a C++ class names its base through an alias two headers declare differently, or a React Native module's Swift class has a copy in two app targets, the tie is now broken the same way everywhere. Before, it followed the system's language settings, so two machines could link the same call to different code. Thanks @bompus. - `codegraph sync` no longer fails with "FOREIGN KEY constraint failed" in a project whose code lazy-loads a workspace package, like Supabase Studio's `dynamic(() => import('icons').then((mod) => mod.EdgeFunctions))` or vben's `() => import('@vben/layouts').then((m) => m.IFrameView)`. Before, every sync after an edit in such a project rolled back, so the index stopped following your changes until a full re-index. Thanks @bompus for the report and @inth3shadows. (#2565) - In VB.NET, every member of a `Structure` is now indexed, including its fields, properties, methods, constructors and nested enums. Before, only the first member was, so the rest could not be found and their callers looked empty. - In Delphi, a routine with an attribute on one of its parameters, like `const [ref] AValue: Integer` or DelphiMVCFramework's `const [MVCFromBody] Person: TPerson`, is now indexed with its calls, and so is every class member declared after it. Before, the routine was lost or its calls were credited to the file, the parameter showed up as a field of the class, and the members after it were missing. Thanks @stalexteam for the report. (#2545) diff --git a/__tests__/cpp-base-class-lookup.test.ts b/__tests__/cpp-base-class-lookup.test.ts index 5774eb6ba9..9fbbe5a638 100644 --- a/__tests__/cpp-base-class-lookup.test.ts +++ b/__tests__/cpp-base-class-lookup.test.ts @@ -511,3 +511,33 @@ describe('what the hierarchy feeds', () => { expect(ancestors('leveldb::DBIter', 'db/db_iter.cc')).toEqual(['extends leveldb::Iterator (include/leveldb/iterator.h)']); }); }); + +describe('an alias two headers declare differently', () => { + // rocksdb's `using DBWithTTLImplBase = …;` in each arm of an `#if`: the base + // is the first declaration, by file in code-unit order. localeCompare put + // `a/…` before `B/…` on an English host, and a host locale decides how it + // orders other paths, so two machines could link different bases. + let root: string; + let g: CodeGraph; + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cpp-alias-twins-')); + const write = (rel: string, lines: string[]) => { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), lines.join('\n') + '\n'); + }; + write('B/config_b.h', ['#pragma once', 'namespace n {', 'class FastStore { public: void flush() {} };', '#ifdef USE_FAST', 'typedef FastStore Store;', '#endif', '}']); + write('a/config_a.h', ['#pragma once', 'namespace n {', 'class SafeStore { public: void flush() {} };', '#ifndef USE_FAST', 'typedef SafeStore Store;', '#endif', '}']); + write('main.cpp', ['#include "B/config_b.h"', '#include "a/config_a.h"', 'namespace n {', 'class Derived : public Store {};', '}']); + g = await CodeGraph.init(root, { index: true }); + }); + afterAll(() => { + g?.close?.(); + fs.rmSync(root, { recursive: true, force: true, maxRetries: 5 }); + }); + + it('takes the first declaration by file in code-unit order, the same on every host', () => { + const derived = g.getNodesByName('Derived').find((n) => n.qualifiedName === 'n::Derived')!; + const bases = g.getOutgoingEdgesFrom([derived.id], ['extends']).map((e) => g.getNode(e.target)?.filePath); + expect(bases).toEqual(['B/config_b.h']); + }); +}); diff --git a/__tests__/react-native-bridge.test.ts b/__tests__/react-native-bridge.test.ts index 67b1ec52d2..00fa644873 100644 --- a/__tests__/react-native-bridge.test.ts +++ b/__tests__/react-native-bridge.test.ts @@ -469,4 +469,13 @@ describe('React Native bridge resolver — RCT_EXTERN (Swift) modules', () => { it('never redirects a native caller', () => { expect(reactNativeBridgeResolver.resolve(ref('captureView.finalizeCaptureSession', 'swift', 'ios/x.swift'), ctx)).toBeNull(); }); + + it('picks between two copies of the Swift class by file in code-unit order, the same on every host', () => { + // Two app targets' copies: `ios/Beta/…` sorts before `ios/alpha/…` by + // code unit, after it by the locale collation localeCompare used. + const alpha = swiftMethod('syncSettings', 'CaptureView', 'ios/alpha/CaptureView.swift', 40); + const beta = swiftMethod('syncSettings', 'CaptureView', 'ios/Beta/CaptureView.swift', 40); + const r = reactNativeBridgeResolver.resolve(ref('captureView.syncSettings', 'tsx', 'src/a.tsx'), makeContext([alpha, beta], files)); + expect(r?.targetNodeId).toBe(beta.id); + }); }); diff --git a/__tests__/ts-this-field-call.test.ts b/__tests__/ts-this-field-call.test.ts index e8fd233e83..817d4a1d5b 100644 --- a/__tests__/ts-this-field-call.test.ts +++ b/__tests__/ts-this-field-call.test.ts @@ -82,6 +82,20 @@ beforeAll(async () => { ' put(x: string): void { this.#items.add(x); }\n' + '}\n' ); + // Two apps declare the same service; the caller is equally near both. + // Code-unit order and locale collation disagree on `Beta` vs `alpha`. + for (const app of ['Beta', 'alpha']) { + fs.mkdirSync(path.join(dir, 'src', 'apps', app), { recursive: true }); + w(`apps/${app}/service.ts`, 'export class UserService {\n load(): string { return "x"; }\n}\n'); + } + fs.mkdirSync(path.join(dir, 'src', 'shared')); + w( + 'shared/hub.ts', + 'export class Hub {\n' + + ' constructor(private readonly users: UserService) {}\n' + + ' run(): string { return this.users.load(); }\n' + + '}\n' + ); cg = CodeGraph.initSync(dir); await cg.indexAll(); }); @@ -113,6 +127,13 @@ describe('this..() (#1496)', () => { expect(calleesOf('Notifier::push')).toEqual([]); }); + it('breaks an equal-proximity tie by code-unit path order, the same on every host locale', () => { + // localeCompare depends on the host's ICU locale (en puts `alpha` first); + // code-unit order puts `Beta` first everywhere. + const callee = cg.getCallees(method('Hub::run').id).map(({ node }) => node.filePath); + expect(callee).toEqual(['src/apps/Beta/service.ts']); + }); + it('resolves a field typed `typeof ` onto the literal\'s member', () => { // The members are functions inside the constant's extent (#1573), qualified // under it (#2300). diff --git a/src/resolution/cpp-type-aliases.ts b/src/resolution/cpp-type-aliases.ts index da410e2cd6..ae0bbc3580 100644 --- a/src/resolution/cpp-type-aliases.ts +++ b/src/resolution/cpp-type-aliases.ts @@ -669,8 +669,10 @@ export function cppClassNamed( if (typeof found !== 'object') return null; if ('cls' in found) return found; if ('ambiguous' in found) { - // An alias each `#if` arm declares differently: the first declaration. - const [first] = [...found.ambiguous].sort((a, b) => a.filePath.localeCompare(b.filePath) || a.startLine - b.startLine); + // An alias each `#if` arm declares differently: the first declaration, by + // file in code-unit order (localeCompare follows the host's locale). + const [first] = [...found.ambiguous].sort((a, b) => + (a.filePath < b.filePath ? -1 : a.filePath > b.filePath ? 1 : 0) || a.startLine - b.startLine); return next === name.names.length ? { alias: first! } : null; } const alias = found.alias; diff --git a/src/resolution/frameworks/react-native.ts b/src/resolution/frameworks/react-native.ts index a213806f67..d5e71f7ca7 100644 --- a/src/resolution/frameworks/react-native.ts +++ b/src/resolution/frameworks/react-native.ts @@ -423,7 +423,8 @@ function buildRNMaps(context: ResolutionContext): { byJsName: Map c.qualifiedName.split('::').includes(ext.className)) - .sort((a, b) => a.filePath.localeCompare(b.filePath) || a.startLine - b.startLine); + // By file in code-unit order: localeCompare follows the host's locale. + .sort((a, b) => (a.filePath < b.filePath ? -1 : a.filePath > b.filePath ? 1 : 0) || a.startLine - b.startLine); const node = candidates[0]; if (!node) continue; const entry: NativeMethod = { moduleName: ext.moduleName, jsName: ext.jsName, node }; diff --git a/src/resolution/go-func-field-synthesizer.ts b/src/resolution/go-func-field-synthesizer.ts index e07b6d56de..df81acb9a9 100644 --- a/src/resolution/go-func-field-synthesizer.ts +++ b/src/resolution/go-func-field-synthesizer.ts @@ -250,7 +250,10 @@ function pkgDir(filePath: string): string { * several cites. */ function wiringOrder(a: FieldValue, b: FieldValue): number { - return Number(a.test) - Number(b.test) || a.site.localeCompare(b.site, 'en', { numeric: true }) || a.target.id.localeCompare(b.target.id); + // The last tie-break is by code unit: a bare localeCompare follows the + // host's locale (Danish sorts an id's `aa` after `z`). + return Number(a.test) - Number(b.test) || a.site.localeCompare(b.site, 'en', { numeric: true }) || + (a.target.id < b.target.id ? -1 : a.target.id > b.target.id ? 1 : 0); } /** Store `value` under `key` in a memo that drops its oldest entry past `max`, so a whole-repo walk stays bounded. */ diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index e5b0d970c4..88b164918e 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -26005,7 +26005,11 @@ function tsFieldCall( while (i < dirs.length && i < callDirs.length && dirs[i] === callDirs[i]) i++; return i; }; - const nearest = [...declared].sort((a, b) => shared(b.filePath) - shared(a.filePath) || a.filePath.localeCompare(b.filePath))[0]!; + // Ties break by code-unit order, not localeCompare: ICU collation + // depends on the host locale, so the same index could pick a + // different target on another machine. + const nearest = [...declared].sort((a, b) => shared(b.filePath) - shared(a.filePath) || + (a.filePath < b.filePath ? -1 : a.filePath > b.filePath ? 1 : 0))[0]!; return { original: ref, targetNodeId: nearest.id, confidence: 0.85, resolvedBy: 'instance-method' }; } return resolveMethodOnType(typeName, methodName, ref, context, 0.85, 'instance-method', imported);