mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-04 02:31:36 +00:00
fix(csharp): cover global-namespace workspaceFqnBindings path + doc + using-static perf
Addresses the production-readiness review of the namespace-siblings OOM fix. - Add a unit test proving global-(default-)namespace C# types route to indexes.workspaceFqnBindings (one entry per simple name) with ZERO bindingAugmentations — pinning the O(D) invariant behind the #1871 Unity-scale OOM fix and guarding against a revert to per-scope O(scopes x defs) augmentation. (The csharp-hooks mock now supplies workspaceFqnBindings, which the global fast path reads directly.) - Correct the workspaceFqnBindings doc comment: it is shared by PHP (backslash-FQN keys) and C# (global-namespace simple-name keys); the two key formats are disjoint. - Pre-index parsedFiles by path before the `using static` member-injection loop, replacing an O(files) find-per-import with an O(1) Map lookup. Verified: tsc --noEmit clean; csharp-hooks + csharp-namespace-extraction suites pass (38 tests); prettier clean; eslint 0 errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9540a958ec
commit
f0381b2616
3 changed files with 75 additions and 6 deletions
|
|
@ -318,6 +318,9 @@ export function populateCsharpNamespaceSiblings(
|
|||
// scope, so `Record(...)` (without `Logger.` qualifier) resolves
|
||||
// to `Logger.Record`. AST walk above captured these (including
|
||||
// `global using static` and aliased forms).
|
||||
// Pre-index files by path once: the member-injection lookup below would
|
||||
// otherwise be an O(files) scan per `using static` import.
|
||||
const fileByPath = new Map<string, ParsedFile>(parsedFiles.map((p) => [p.filePath, p]));
|
||||
for (const parsed of parsedFiles) {
|
||||
const struct = structureByFile.get(parsed.filePath);
|
||||
if (struct === undefined) continue;
|
||||
|
|
@ -343,7 +346,7 @@ export function populateCsharpNamespaceSiblings(
|
|||
// Inject the class's member methods into the importer's module
|
||||
// scope. `memberByOwner` wasn't built yet here, so we walk the
|
||||
// file's localDefs to find members with `ownerId === targetDef.nodeId`.
|
||||
const targetFile = parsedFiles.find((p) => p.filePath === targetDef.filePath);
|
||||
const targetFile = fileByPath.get(targetDef.filePath);
|
||||
if (targetFile === undefined) continue;
|
||||
for (const memberDef of targetFile.localDefs) {
|
||||
if ((memberDef as { ownerId?: string }).ownerId !== targetDef.nodeId) continue;
|
||||
|
|
|
|||
|
|
@ -77,11 +77,15 @@ export interface ScopeResolutionIndexes {
|
|||
* are returned first and win duplicate `def.nodeId` metadata, with
|
||||
* unique augmentations appended after. See I8. */
|
||||
readonly bindingAugmentations: ReadonlyMap<ScopeId, ReadonlyMap<string, readonly BindingRef[]>>;
|
||||
/** Workspace-level FQN binding lookup. Populated by PHP namespace-
|
||||
* siblings Step 3b as a shared map instead of per-scope duplication.
|
||||
* Consulted by `lookupBindingsAt` as a third source after finalized
|
||||
* and per-scope augmented bindings. Keys are backslash-separated FQNs
|
||||
* (e.g. `App\Models\User`). */
|
||||
/** Workspace-level binding lookup, shared instead of per-scope
|
||||
* duplication. Consulted by `lookupBindingsAt` as a third source after
|
||||
* finalized and per-scope augmented bindings. Two languages populate it
|
||||
* with disjoint key formats that never collide:
|
||||
* - PHP namespace-siblings (Step 3b): backslash-separated FQNs
|
||||
* (e.g. `App\Models\User`).
|
||||
* - C# namespace-siblings: simple names for global-(default-)namespace
|
||||
* types (e.g. `User`), which are visible from every file — one entry
|
||||
* per simple name instead of O(scopes × defs) per-scope augmentation. */
|
||||
readonly workspaceFqnBindings: ReadonlyMap<string, readonly BindingRef[]>;
|
||||
/** Pre-resolution usage facts; consumed by the resolution phase. */
|
||||
readonly referenceSites: readonly ReferenceSite[];
|
||||
|
|
|
|||
|
|
@ -322,6 +322,68 @@ describe('populateCsharpNamespaceSiblings', () => {
|
|||
|
||||
expect(bindingAugmentations.get(moduleA.id)?.get('B')?.[0]?.def.nodeId).toBe('def:b.B');
|
||||
});
|
||||
|
||||
it('routes global-namespace types to workspaceFqnBindings, not per-scope augmentations (#1871 OOM guard)', () => {
|
||||
// Types declared with NO `namespace` (the global/default namespace) are
|
||||
// visible from every C# file, so the hook writes ONE workspace-level entry
|
||||
// per simple name instead of O(scopes x defs) per-scope augmentations —
|
||||
// the fix for the #1871 Unity-scale OOM. This pins both halves of that
|
||||
// contract: global types are reachable via `workspaceFqnBindings`, and the
|
||||
// per-scope augmentation channel stays empty for them.
|
||||
//
|
||||
// Note: the mock MUST supply `workspaceFqnBindings` — the global fast path
|
||||
// reads `indexes.workspaceFqnBindings` directly, so omitting it (as the
|
||||
// other tests in this suite do) would throw.
|
||||
const defA = classDef('def:a.A', 'a.cs', 'A'); // simple name => global namespace
|
||||
const defB = classDef('def:b.B', 'b.cs', 'B');
|
||||
const moduleA = scope('scope:a:module', 'Module', 'a.cs');
|
||||
const classA = scope('scope:a:class', 'Class', 'a.cs', moduleA.id, [defA]);
|
||||
const moduleB = scope('scope:b:module', 'Module', 'b.cs');
|
||||
const classB = scope('scope:b:class', 'Class', 'b.cs', moduleB.id, [defB]);
|
||||
const parsedFiles: ParsedFile[] = [
|
||||
{
|
||||
filePath: 'a.cs',
|
||||
moduleScope: moduleA.id,
|
||||
scopes: Object.freeze([moduleA, classA]),
|
||||
parsedImports: Object.freeze([]),
|
||||
localDefs: Object.freeze([defA]),
|
||||
referenceSites: Object.freeze([]),
|
||||
} as ParsedFile,
|
||||
{
|
||||
filePath: 'b.cs',
|
||||
moduleScope: moduleB.id,
|
||||
scopes: Object.freeze([moduleB, classB]),
|
||||
parsedImports: Object.freeze([]),
|
||||
localDefs: Object.freeze([defB]),
|
||||
referenceSites: Object.freeze([]),
|
||||
} as ParsedFile,
|
||||
];
|
||||
const bindingAugmentations = new Map<ScopeId, ReadonlyMap<string, readonly BindingRef[]>>();
|
||||
const workspaceFqnBindings = new Map<string, readonly BindingRef[]>();
|
||||
|
||||
populateCsharpNamespaceSiblings(
|
||||
parsedFiles,
|
||||
{
|
||||
bindings: new Map(),
|
||||
bindingAugmentations,
|
||||
workspaceFqnBindings,
|
||||
} as unknown as ScopeResolutionIndexes,
|
||||
{
|
||||
fileContents: new Map([
|
||||
['a.cs', 'class A { }\n'], // no `namespace` => global
|
||||
['b.cs', 'class B { }\n'],
|
||||
]),
|
||||
},
|
||||
);
|
||||
|
||||
// Global types are reachable workspace-wide via simple-name keys.
|
||||
expect(workspaceFqnBindings.get('A')?.map((b) => b.def.nodeId)).toEqual(['def:a.A']);
|
||||
expect(workspaceFqnBindings.get('B')?.map((b) => b.def.nodeId)).toEqual(['def:b.B']);
|
||||
// O(D) invariant: one entry per unique simple name, never scopes x defs.
|
||||
expect(workspaceFqnBindings.size).toBe(2);
|
||||
// The whole point of the fast path: no per-scope augmentation explosion.
|
||||
expect(bindingAugmentations.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('csharpReceiverBinding', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue