test(scope-resolution): gate the edges #2881 moved but nothing watched

Every widened-shape test in the branch used a one-file corpus, so not one of the
149 first-child reselections was pinned — the tier that commits to `children[0]`
unfiltered had no test that could see which file it commits to. Kotlin and Java
now pin that choice absolutely, in both insertion orders, for the member path
(tier 3, both members) and the wildcard path (tier 1, one file) separately,
saying plainly that both candidates are valid members and the only tie-break is
file-set iteration order.

The tier-3-preempts-tier-4 class gets its first gate, with a control that makes
it a transition rather than a fact. The bench corpus holds zero instances, so
this case is the only thing standing between that behaviour and a silent
revert.

C# gains three absolute arms, because its differential harness cannot see any of
them — the legacy copy was edited in lockstep with production, which the file's
own header admits. One pins the empty-`dirPrefix` filter the branch calls
load-bearing and which nothing defended: deleting the guard leaves the whole
suite green but changes the answer, so the arm was verified to fail with the
guard removed and pass with it restored. Java gains the negative control Kotlin
already had.

`kotlin-index-internals.test.ts` stops implying coverage it does not have. The
mutation matrix is recorded in its header: deleting the memo passes every arm
(it is output-identical by construction), deleting the compaction's `slice()`
passes every arm (a JS array's capacity has no reflective surface), while
mis-keying the memo fails three and compacting-but-never-storing fails two. Four
arms were added that do fail under those mutations. V8's growth steps were
re-measured — 1, 19, 46, 86 with growth at lengths 2, 20, 47, 87 — so the old
1/17/41 model, which under-counted the slack at 40 files by 6x, is gone.

`go-package-resolve.test.ts` drops four `as never` casts that were hiding
nothing (`GoModuleConfig` is structurally satisfied), and pins vendor/, testdata/
and nested-go.mod directories, which merge into the importing package — a
pre-existing unmodelled gap, verified present before #2881 and documented as
such rather than blamed on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kim7UK3kPhF1ZRZDuuvnus
This commit is contained in:
Gergo Magyar 2026-08-12 08:23:52 +00:00
parent d1e70eca5b
commit 3d2ad21893
5 changed files with 473 additions and 64 deletions

View file

@ -14,8 +14,8 @@
* SEGMENT-aligned directory suffixes; step 3 runs an UNANCHORED
* `normalized.lastIndexOf(dirPrefix + '/')` (`indexOf` before #2881, which is
* the first-occurrence rule that issue removed; the empty-prefix case still
* takes `indexOf` — see the guard in both copies). Step 3 therefore answers
* strictly more:
* takes `indexOf` — see `directChildIdx`, which both copies call). Step 3
* therefore answers strictly more:
*
* - `dirPrefix = 'ubModels'` matches `src/SubModels/` (character suffix of a
* segment, not a segment);
@ -61,6 +61,26 @@ import type {
// `SuffixIndex`) are imported from production because this PR does not touch
// them; only the function below changed.
/**
* The direct-child probe the frozen copies below run — four call sites, two in
* each copy, that have to move together.
*
* Direct child of a directory ENDING with `dirPrefix` — since #2881, minus
* "…and that occurrence is the FIRST". Empty `dirPrefix` keeps `indexOf`: its
* needle is a bare '/', and step 3 answers that query from the
* one-directory-deep set, which only the first occurrence expresses.
*
* Local to this file on purpose. A parity harness has to stay independent of
* PRODUCTION — that independence is the whole instrument, and importing this
* expression from `csharp.ts` would make the differential compare production
* against itself. But all four copies live inside the harness, so one local
* helper keeps the independence while removing three sites that could silently
* drift apart from each other.
*/
function directChildIdx(normalized: string, dirPrefix: string, dirTrail: string): number {
return dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
}
function legacyResolveCSharpImportInternal(
importPath: string,
csharpConfigs: CSharpProjectConfig[],
@ -112,13 +132,7 @@ function legacyResolveCSharpImportInternal(
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
for (const f of dirFiles) {
const normalized = f.replace(/\\/g, '/');
// Direct child of a directory ENDING with `dirPrefix` — since #2881,
// minus "…and that occurrence is the FIRST". Empty `dirPrefix` keeps
// `indexOf`: its needle is a bare '/', and step 3 answers that query
// from the one-directory-deep set, which only the first occurrence
// expresses.
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
if (!afterDir.includes('/')) {
@ -133,8 +147,7 @@ function legacyResolveCSharpImportInternal(
for (let i = 0; i < normalizedFileList.length; i++) {
const normalized = normalizedFileList[i];
if (!normalized.endsWith('.cs')) continue;
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirTrail.length);
if (!afterDir.includes('/')) {
@ -204,8 +217,7 @@ function skipStep3WhenIndexed(
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
for (const f of dirFiles) {
const normalized = f.replace(/\\/g, '/');
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
if (!afterDir.includes('/')) {
@ -219,8 +231,7 @@ function skipStep3WhenIndexed(
for (let i = 0; i < normalizedFileList.length; i++) {
const normalized = normalizedFileList[i];
if (!normalized.endsWith('.cs')) continue;
const prefixIdx =
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
if (prefixIdx < 0) continue;
const afterDir = normalized.substring(prefixIdx + dirTrail.length);
if (!afterDir.includes('/')) {
@ -646,3 +657,101 @@ describe('C# csproj leg — the directory index is built once per file set (#290
);
});
});
/**
* ABSOLUTE arms, deliberately not differential.
*
* The harness above is blind to everything in this block. Its frozen legacy copy
* carries the same `dirPrefix === '' ? indexOf : lastIndexOf` rule production
* does (see `directChildIdx`, and the header's note that #2881's edit landed in
* BOTH), so where #2881 moved step 2 the two sides moved together and the
* differential stays green by construction. Only stated expectations can see
* these, so each arm below names the exact line it gates.
*/
describe('C# csproj leg — where step 2 stops and step 3 begins (absolute)', () => {
function corpus(raw: readonly string[]): { paths: ReadonlySet<string>; index: SuffixIndex } {
const all = [...raw];
const normalized = all.map((f) => f.replace(/\\/g, '/'));
return { paths: new Set(all), index: buildSuffixIndex(normalized, all) };
}
const ROOT_NS_ONLY: CSharpProjectConfig[] = [{ rootNamespace: 'App', projectDir: '' }];
it("step 2's empty-`dirPrefix` filter rejects a doubled slash, which is NOT one directory deep", () => {
// Gates the five-line `if (dirPrefix === '')` guard in step 2 of
// `resolveCSharpImportInternal`. Delete it and this arm is the only thing in
// the suite that fails.
//
// `getDirMap` keys a file under every suffix of its DIRECTORY, so the empty
// key holds every path whose directory's last component is empty. That is a
// leading '/' on a root-level file (`/Root.cs`, directory ''), but ALSO a
// doubled slash immediately before the file name (`a//Doubled.cs`, directory
// 'a/'). Only the first is one directory deep — the query an empty
// `dirPrefix` is asking, and the one step 3 answers from `singleSegmentDirs`
// — so without the guard step 2 and step 3 disagree.
const { paths, index } = corpus([
'/Root.cs',
'a//Doubled.cs',
'Top.cs',
'one/Deep.cs',
'a/b/Deeper.cs',
]);
// Not vacuous: `a//Doubled.cs` really is in the bucket step 2 filters, so
// this arm fails by ADDING it rather than by finding nothing to reject.
expect(index.getFilesInDir('', '.cs')).toEqual(['/Root.cs', 'a//Doubled.cs']);
expect(resolveCSharpImportInternal('App', ROOT_NS_ONLY, paths, index)).toEqual(['/Root.cs']);
});
it('step 2 answering a query it used to miss also PREEMPTS step 3', () => {
// #2881 widened step 2 from "the FIRST `/<dirPrefix>/`" to "the directory
// ENDS with `dirPrefix`". The justification reasons about step 2's own
// bucket and is right there — but step 2 returns as soon as it pushes
// anything, so a query it used to answer with nothing now also suppresses
// step 3, whose unanchored match set is a strict SUPERSET of step 2's.
//
// Pinned in both directions: step 2's answer with an index, and step 3's own
// answer with none. The gap between them is the suppression.
const { paths, index } = corpus([
'nest/src/SubModels/F0.cs',
'SubModels/Models/F1.cs',
'F2.cs',
'SubModels/F3.cs',
]);
// Step 2 alone: `SubModels/Models` is the only SEGMENT-aligned `Models`.
expect(resolveCSharpImportInternal('App.Models', ROOT_NS_ONLY, paths, index)).toEqual([
'SubModels/Models/F1.cs',
]);
// Step 3 alone: every directory whose path merely ENDS with `Models`, which
// is the segment-aligned hit plus both `SubModels` character-suffix ones.
// Before #2881 step 2 rejected here and this was the answer; the widened
// step 2 now returns first, and the narrower, more precise answer above is
// the one that reaches the graph.
expect(resolveCSharpImportInternal('App.Models', ROOT_NS_ONLY, paths, undefined)).toEqual([
'nest/src/SubModels/F0.cs',
'SubModels/Models/F1.cs',
'SubModels/F3.cs',
]);
});
it('a name that is not a suffix of the PARENT directory stays out of the bucket', () => {
// The negative control for the widened rule, matching the one Kotlin's
// parity test carries. The rule is "the file's parent directory ENDS with
// `dirPrefix`", not "`dirPrefix` appears anywhere in the path" — dropping
// the first-occurrence half must not widen it that far. Both positions the
// old `indexOf` distinguished are covered: leading, and mid-path.
//
// Asserted through both legs, because they run different predicates on
// different indexes and either one alone could widen without the other.
const { paths, index } = corpus([
'Models/sub/Leading.cs',
'top/Models/mid/Middle.cs',
'Models/Direct.cs',
]);
expect(resolveCSharpImportInternal('App.Models', ROOT_NS_ONLY, paths, index)).toEqual([
'Models/Direct.cs',
]);
expect(resolveCSharpImportInternal('App.Models', ROOT_NS_ONLY, paths, undefined)).toEqual([
'Models/Direct.cs',
]);
});
});

View file

@ -13,25 +13,37 @@
* here rather than silently moving Go IMPORTS edges in every repository that
* nests a package name inside itself (`internal/…/internal` is the shape Go
* actually produces).
*
* One caveat on "the LanguageProvider leg", recorded in #2929 review: nothing
* in production reads that field today. `configs/go.ts` reaches this function
* through `createImportResolver` → `LanguageProvider.importResolver`, and the
* only readers of `importResolver` are `import-target-adapter.ts:74-75`, whose
* two exports have no importer outside their own unit test. So the leg is wired
* but unreached, and this file is the only thing exercising it.
*/
import { describe, expect, it } from 'vitest';
import {
resolveGoPackage,
resolveGoPackageDir,
} from '../../../src/core/ingestion/import-resolvers/go.js';
import type { GoModuleConfig } from '../../../src/core/ingestion/language-config.js';
import { resolveGoImportTarget } from '../../../src/core/ingestion/languages/go/import-target.js';
const MOD = { modulePath: 'example.com/mod' } as const;
const MOD: GoModuleConfig = { modulePath: 'example.com/mod' };
function resolve(files: readonly string[], importPath: string): string[] {
const normalized = files.map((f) => f.replace(/\\/g, '/'));
return resolveGoPackage(importPath, MOD as never, normalized, files);
return resolveGoPackage(importPath, MOD, normalized, files);
}
/** The indexed leg, for the agreement arm. */
function indexed(files: readonly string[], importPath: string): readonly string[] {
const got = resolveGoImportTarget(importPath, 'main.go', new Set(files), MOD);
return got === null ? [] : Array.isArray(got) ? got : [got];
// `typeof got === 'string'`, not `Array.isArray(got)`: `Array.isArray` narrows
// to `any[]`, which does not subsume `readonly string[]`, so the false branch
// kept the array member and `[got]` did not typecheck (TS2322 under
// `tsconfig.test.json`). Runtime behaviour is identical.
return got === null ? [] : typeof got === 'string' ? [got] : got;
}
describe('resolveGoPackage', () => {
@ -92,10 +104,40 @@ describe('resolveGoPackage', () => {
expect(resolve(['internal/auth/x.go'], 'github.com/other/repo/internal/auth')).toEqual([]);
// The root package is the caller's `findRootPackageFiles` leg, not this one.
expect(resolve(['main.go'], 'example.com/mod')).toEqual([]);
expect(resolveGoPackageDir('example.com/mod', MOD as never)).toBeNull();
expect(resolveGoPackageDir('example.com/mod/internal/auth', MOD as never)).toBe(
'/internal/auth/',
);
expect(resolveGoPackageDir('example.com/mod', MOD)).toBeNull();
expect(resolveGoPackageDir('example.com/mod/internal/auth', MOD)).toBe('/internal/auth/');
});
it('vendor/, testdata/ and nested-module directories all merge in (unmodelled)', () => {
// Go excludes all three from a package: `vendor/` is a dependency tree
// resolved against the vendoring module, the go tool ignores `testdata/`
// entirely, and a directory carrying its own `go.mod` is a separate module
// whose packages this module's import paths never name.
//
// This resolver models NONE of that — it matches on the parent directory's
// path suffix alone. That is unchanged by #2881: the pre-#2881 rule
// (first `/<pkg>/`, nothing but a filename after it) accepted all three
// shapes too. These assertions record what the function actually does so
// the gap is visible and a change to it is deliberate; they document the
// behaviour rather than endorse it.
const files = [
'go.mod',
'internal/auth/service.go',
'vendor/example.com/dep/internal/auth/vendored.go',
'testdata/internal/auth/fixture.go',
'sub/go.mod', // `sub/` is its own module; its packages are not ours
'sub/internal/auth/other_module.go',
];
expect(resolve(files, 'example.com/mod/internal/auth')).toEqual([
'internal/auth/service.go',
'vendor/example.com/dep/internal/auth/vendored.go',
'testdata/internal/auth/fixture.go',
'sub/internal/auth/other_module.go',
]);
// A `go.mod` beside the files changes nothing — it is not read here.
expect(resolve(['sub/go.mod', 'sub/pkg/x.go'], 'example.com/mod/pkg')).toEqual([
'sub/pkg/x.go',
]);
});
it('agrees with the indexed leg on the repeated-name shapes', () => {

View file

@ -352,6 +352,30 @@ const HAND_CASES: readonly Case[] = [
files: ['com/example/model/User.java'],
target: 'java.util.List',
},
// ── negative control for the widening (#2881), matching Kotlin's ──────────
// Everything above pins the widened leg from the POSITIVE side: a file whose
// package directory repeats higher in its own path now answers. Nothing here
// pinned the other half — that dropping the first-occurrence rule did not
// widen "the parent directory ends with `pathLike`" into "`pathLike` appears
// somewhere in the path". These three are the same trio
// `kotlin-import-target-parity.test.ts` carries: the two positions the old
// `atRoot`/`indexOf` pair distinguished, plus the positive control that keeps
// the pair from passing by refusing everything.
{
label: 'not-the-parent-directory-stays-out-leading',
files: ['com/example/sub/Repo.java'],
target: 'com.example',
},
{
label: 'not-the-parent-directory-stays-out-mid-path',
files: ['top/com/example/mid/Repo.java'],
target: 'com.example',
},
{
label: 'the-parent-directory-itself-does-answer',
files: ['top/com/example/Repo.java'],
target: 'com.example',
},
];
/** Absolute pre-change behaviour, so the differential cannot pass vacuously. */
@ -389,6 +413,12 @@ const HAND_EXPECTED: readonly string[] = [
'directory-named-like-a-java-file => null',
'jdk-import-strips-into-a-local-lookalike => src/main/java/util/List.java',
'jdk-import-with-no-lookalike-resolves-to-nothing => null',
// `com/example/sub` ends with `example/sub`, not with `com/example`, and the
// stripping loop's `example` level does not reach it either — so the file is
// in no bucket the query can name.
'not-the-parent-directory-stays-out-leading => null',
'not-the-parent-directory-stays-out-mid-path => null',
'the-parent-directory-itself-does-answer => top/com/example/Repo.java',
];
// ─── generated corpus ────────────────────────────────────────────────────────
@ -617,6 +647,55 @@ describe('Java import target — index hoist parity (#2908)', () => {
]);
});
it('pins WHICH of two competing package directories the first-child leg takes', () => {
// `firstFileDirectlyInPkgDir` commits to ONE file with no downstream
// filter, and #2881 widened the set it chooses from. Every widened-shape
// case in HAND_CASES uses a ONE-FILE corpus, so the widened bucket has a
// single member and the choice is not exercised anywhere — yet a different
// first child is the largest class of movement the change produced.
//
// Both files below are legitimate members of the widened `com/example`
// set: `com/example/legacy/com/example` ends with `com/example` (it is the
// self-nested shape #2881 admitted) and `src/main/java/com/example` ends
// with it too. Nothing in the resolver prefers one over the other. The
// tie-break is FILE-SET ITERATION ORDER — the insertion order of the Set
// the caller passes — so reversing the corpus reverses the answer. That is
// the property these assertions pin, and the one nothing else watches:
// membership is already pinned above, the WINNER was not.
const set = (files: readonly string[]): WorkspaceIndex =>
({ fromFile: FROM_FILE, allFilePaths: new Set(files) }) as WorkspaceIndex;
const nestedFirst = [
'com/example/legacy/com/example/Old.java',
'src/main/java/com/example/App.java',
];
const conventionalFirst = [...nestedFirst].reverse();
expect(resolveJavaImportTarget(javaImport('com.example'), set(nestedFirst))).toBe(
'com/example/legacy/com/example/Old.java',
);
expect(resolveJavaImportTarget(javaImport('com.example'), set(conventionalFirst))).toBe(
'src/main/java/com/example/App.java',
);
// A wildcard drops its `.*` before any of that, so it lands on the same leg
// and moves with it. Spelled out because `com.example.*` is how real Java
// source reaches this tier.
expect(resolveJavaImportTarget(javaImport('com.example.*'), set(nestedFirst))).toBe(
'com/example/legacy/com/example/Old.java',
);
expect(resolveJavaImportTarget(javaImport('com.example.*'), set(conventionalFirst))).toBe(
'src/main/java/com/example/App.java',
);
// The pre-change copy agrees on both orders, which places the movement in
// #2881's removal of the first-occurrence rule rather than in #2908's
// index hoist: the hoist did not touch the tie-break, it inherited it.
expect(legacyResolveJavaImportTarget(javaImport('com.example'), set(nestedFirst))).toBe(
'com/example/legacy/com/example/Old.java',
);
expect(legacyResolveJavaImportTarget(javaImport('com.example'), set(conventionalFirst))).toBe(
'src/main/java/com/example/App.java',
);
});
it('builds each index once per file set rather than once per import', () => {
const files = new CountingSet(generatedFiles());
const ws = { fromFile: FROM_FILE, allFilePaths: files };

View file

@ -13,6 +13,16 @@
* first-occurrence restriction on `dirChildren`, so a file whose package
* directory name repeats higher in its path is now a child of that package.
* The cases carrying it say so and name the issue.
*
* That removal has two consequences a widened-shape case built on a ONE-FILE
* corpus cannot express, and both are pinned at the bottom of this file:
*
* - a bucket with TWO members makes `children[0]` a CHOICE. The wildcard tier
* commits to it with no downstream filter, so widening the bucket moves
* which file an already-resolving `import data.*` binds to.
* - tier 3 sits in front of tier 4, so a bucket the removed guards used to
* leave empty no longer lets tier 4 run — which can turn a bound answer
* into a candidate list that does not carry the symbol.
*/
import { describe, it, expect } from 'vitest';
import { resolveKotlinImportTarget } from '../../../../src/core/ingestion/languages/kotlin/import-target.js';
@ -184,4 +194,70 @@ describe('resolveKotlinImportTarget — index parity', () => {
it('an unknown target resolves to null', () => {
expect(resolve(['pkg/A.kt'], 'nowhere.Thing')).toBeNull();
});
it('two competing `data` directories: WHICH one the first-child tier picks (#2881)', () => {
// The gap every other widened-shape case above leaves open. Each of them
// uses a ONE-FILE corpus, so the widened bucket has exactly one member and
// `findKotlinDirectoryChild`'s `children[0]` has nothing to choose between.
// #2881 moved 149 of the census's 235 records to a DIFFERENT first child,
// and not one of those 149 is a shape any single-file case can express.
//
// Both files below are legitimate members of the widened `data` bucket:
// each is a direct child of a directory named `data`. Neither is "the right
// answer" — the resolver has no rule that prefers one, and the ONLY thing
// deciding it is FILE-SET ITERATION ORDER, i.e. the insertion order of the
// Set the caller hands in. That is what these assertions pin: not that the
// bucket contains both (the cases above already pin membership) but WHICH
// member wins, which is the property nothing else in the repo watches.
const files = ['top/data/mid/data/Wrong.kt', 'src/data/Correct.kt'];
const reversed = ['src/data/Correct.kt', 'top/data/mid/data/Wrong.kt'];
// Tier 3, the member path: `data.helper` strips to `data`, misses the file
// tiers and fans the WHOLE bucket out. Order is preserved but nothing is
// dropped, so this path commits to nothing on its own — the finalize pass
// still gets to pick by `localDefs` (#1759).
expect(resolve(files, 'data.helper')).toEqual([
'top/data/mid/data/Wrong.kt',
'src/data/Correct.kt',
]);
expect(resolve(reversed, 'data.helper')).toEqual([
'src/data/Correct.kt',
'top/data/mid/data/Wrong.kt',
]);
// Tier 1, the wildcard path: `data.*` strips to `data`, which IS the whole
// `pathLike`, so `findKotlinFile` answers with `children[0]` — one file,
// unfiltered, no later tier and no downstream narrowing. This is the leg
// where the widening changes a bound answer rather than adding a candidate,
// and it flips with insertion order alone.
expect(resolve(files, 'data.*')).toBe('top/data/mid/data/Wrong.kt');
expect(resolve(reversed, 'data.*')).toBe('src/data/Correct.kt');
});
it('tier 3 preempts tier 4: a widened bucket replaces a BOUND answer (#2881)', () => {
// The class the published `54 / 149 / 32` census has no bucket for, because
// that taxonomy is shape-preserving (null→resolved, wider array, different
// first child) and this one is not. `findKotlinPackageFiles` runs BEFORE
// `findByProgressivePrefixStrip`, so a bucket the removed guards used to
// leave empty returned null and let tier 4 run; a now-populated bucket
// stops tier 4 from running at all.
//
// Read the two assertions together. The answer here is no longer a bound
// file — it is a CANDIDATE LIST, and the only file in it does not carry
// `helper`. `common/helper.kt`, the file tier 4 used to bind, is not in the
// list at all: it is not a child of any `data` directory, so no widening of
// the bucket can ever reach it. A resolved answer became an unresolved one.
// The bench corpus contains ZERO instances of this class, which is why it
// ships ungated — `bench/kotlin-import-target`'s own generator at 4000
// repositories hits it only 4-12 times per seed. This case is the gate.
expect(
resolve(['data/src/main/kotlin/com/example/data/Repo.kt', 'common/helper.kt'], 'data.helper'),
).toEqual(['data/src/main/kotlin/com/example/data/Repo.kt']);
// The control that makes the arm above a transition rather than a fact:
// drop the `data` directory and tier 3 has nothing, so tier 4 runs and
// binds the same import to the file that actually holds `helper`. This is
// what the first assertion returned before #2881.
expect(resolve(['common/helper.kt'], 'data.helper')).toBe('common/helper.kt');
});
});

View file

@ -2,25 +2,52 @@
* Structural guard for the two `getKotlinFileIndex` optimizations added in
* #2881 — the per-directory key memo and the bucket compaction.
*
* Both are output-identical by construction, which is why they exist and also
* why nothing else in the repo watches them. `bench/kotlin-import-target`'s
* correctness fingerprint observes the index only through the four resolver
* tiers, so it cannot see a Map key-ORDER move, and reverting either
* optimization leaves every arm of both benches green (the heap ceiling is a
* 1.5x bound, not an equality). Without this file the memo's safety argument —
* "the key list is a pure function of `dir`, so interning it per directory
* cannot change the key set, the key insertion order or any bucket's order" —
* is asserted nowhere.
* ## What this file can and cannot see
*
* Read this before adding a case here, and before citing this file as coverage
* for either optimization. Both claims below were checked by re-running every
* arm in this file against a mutated COPY of the resolver:
*
* - **the key memo (`dirKeys`) is not observable.** Deleting it outright —
* cutting the component-suffix key list once per FILE, the way the code did
* before — leaves every arm here green. That is not a hole to be plugged: it
* is the optimization's safety argument restated, since the key list is a
* pure function of `dir` and interning it cannot change the key set, the key
* insertion order or any bucket's order. In particular
* `expect(first).toBe(second)` proves nothing about it — bucket identity
* across calls comes from the OUTER `perFileSet` memo on the Set's identity,
* a different cache, guarded in
* `test/integration/kotlin-import-index-reuse.test.ts`. What the arms below
* pin is the OUTPUT INVARIANT the memo has to preserve, and the one
* plausible way to get the memo wrong — keying it on the last directory
* segment instead of the whole `dir`, which hands `x/pkg`'s key list to
* `y/pkg` and merges them — fails three of them. So: a MIS-KEYED memo is
* caught here, a DELETED one is not.
*
* - **the compaction (`bucket.slice()`) is not observable either.** Deleting
* the slice and freezing the grown bucket in place leaves every arm green,
* `Object.isFrozen` included. A JS array's backing-store capacity has no
* reflective surface, so no assertion through any API can see the reclaimed
* slack. The only instrument that can is `heap_ceiling_bytes.kotlin` in
* `bench/import-target/baselines.json` — a CEILING, not a floor: compaction
* reclaims, so deleting it makes the retained reading GROW (measured
* +12.57%, 42 805 256 -> 48 184 784 B), which no floor could see.
* `Object.isFrozen` is still worth
* asserting for what it DOES catch: no freeze at all, and "compact, freeze
* the copy, forget to `set` it back" — which hands out the original,
* unfrozen bucket, and which fails the arm (verified).
*
* ## What the arms are for
*
* The index is module-private, so these assertions run through the resolver's
* observable surface and reconstruct what they need:
*
* - bucket CONTENTS and ORDER come from the fan-out tier, which hands out the
* bucket array itself;
* - bucket IDENTITY across calls proves the memoization, and `Object.isFrozen`
* proves the compaction pass ran over the array actually handed out;
* - the memo HIT path is reached only by a second file in the same directory,
* which is the case a single-file corpus never exercises.
* - the FIRST-CHILD tier reads `[0]` of that same array, so the two tiers
* agreeing is what makes the freeze load-bearing rather than decorative;
* - a second file in the same directory is what reaches the memo's hit path
* at all, which a single-file corpus never exercises.
*
* A key-order assertion is deliberately absent: `dirChildren` is only ever read
* by `.get(key)`, so Map key order has no consumer and pinning it would assert
@ -30,9 +57,17 @@ import { describe, expect, it } from 'vitest';
import type { ParsedImport } from 'gitnexus-shared';
import { resolveKotlinImportTarget } from '../../../../src/core/ingestion/languages/kotlin/import-target.js';
function bucket(files: readonly string[], targetRaw: string, set = new Set(files)) {
/**
* Takes the file Set directly. It used to take `(files, targetRaw, set = new
* Set(files))`, which three call sites drove as `bucket([], 'pkg.fn', set)` —
* an empty first argument that reads as "no files" in the one place the corpus
* matters most. Callers now spell `new Set(...)`, which also makes it visible
* where a Set is REUSED across calls (the `perFileSet` cache hit) and where a
* fresh one is built.
*/
function bucket(files: ReadonlySet<string>, targetRaw: string) {
const parsed = { kind: 'named', localName: 'X', importedName: 'X', targetRaw } as ParsedImport;
return resolveKotlinImportTarget(parsed, { fromFile: 'App.kt', allFilePaths: set } as never);
return resolveKotlinImportTarget(parsed, { fromFile: 'App.kt', allFilePaths: files } as never);
}
describe('getKotlinFileIndex internals (#2881)', () => {
@ -41,9 +76,10 @@ describe('getKotlinFileIndex internals (#2881)', () => {
// between the two paths shows up as a missing or reordered member. The
// per-file form and the memoized form must agree element for element.
const files = ['a/b/pkg/One.kt', 'a/b/pkg/Two.kt', 'a/b/pkg/Three.kt', 'a/b/pkg/Four.kts'];
expect(bucket(files, 'pkg.someTopLevelFun')).toEqual(files);
expect(bucket(files, 'b.pkg.someTopLevelFun')).toEqual(files);
expect(bucket(files, 'a.b.pkg.someTopLevelFun')).toEqual(files);
const set = new Set(files);
expect(bucket(set, 'pkg.someTopLevelFun')).toEqual(files);
expect(bucket(set, 'b.pkg.someTopLevelFun')).toEqual(files);
expect(bucket(set, 'a.b.pkg.someTopLevelFun')).toEqual(files);
});
it('two directories sharing a component-suffix keep separate buckets', () => {
@ -51,40 +87,112 @@ describe('getKotlinFileIndex internals (#2881)', () => {
// one way this optimization can move an answer — would hand `x/pkg`'s key
// list to `y/pkg` and merge them.
const files = ['x/pkg/One.kt', 'y/pkg/Two.kt'];
expect(bucket(files, 'x.pkg.fn')).toEqual(['x/pkg/One.kt']);
expect(bucket(files, 'y.pkg.fn')).toEqual(['y/pkg/Two.kt']);
const set = new Set(files);
expect(bucket(set, 'x.pkg.fn')).toEqual(['x/pkg/One.kt']);
expect(bucket(set, 'y.pkg.fn')).toEqual(['y/pkg/Two.kt']);
// `pkg` alone is a component-suffix of both, so it legitimately holds both,
// in file-set iteration order.
expect(bucket(files, 'pkg.fn')).toEqual(files);
expect(bucket(set, 'pkg.fn')).toEqual(files);
});
it('the bucket handed out is frozen, compacted and the same object every call', () => {
// `slice()` in the freeze loop must not break the caching contract: the
// fan-out tier hands out the cached array, and the first-child tier reads
// `[0]` from the same one, so a downstream `sort()` would reorder the index
// itself. Freezing makes that a loud TypeError.
it('a shorter key list cached first does not truncate a longer one', () => {
// The case above has two directories of EQUAL depth, so a mis-keyed memo
// merges two lists of the same length and only the bucket contents move.
// Here the first directory seen (`pkg`) contributes one key and the second
// (`a/pkg`) contributes two, so reusing the first's list by last segment
// loses the `a/pkg` key entirely — a lookup that resolves today returning
// null. Different failure, same mis-keying.
const set = new Set(['pkg/One.kt', 'a/pkg/Two.kt']);
expect(bucket(set, 'pkg.fn')).toEqual(['pkg/One.kt', 'a/pkg/Two.kt']);
expect(bucket(set, 'a.pkg.fn')).toEqual(['a/pkg/Two.kt']);
});
it('directories sharing a MULTI-segment suffix keep separate buckets', () => {
// `q/pkg` is a shared suffix of both directories and `pkg` is a shared
// suffix of that, so the two files collide on two keys and stay apart on a
// third. A memo keyed on anything shorter than the whole `dir` merges the
// third as well.
const set = new Set(['p/q/pkg/One.kt', 'r/q/pkg/Two.kt']);
expect(bucket(set, 'p.q.pkg.fn')).toEqual(['p/q/pkg/One.kt']);
expect(bucket(set, 'r.q.pkg.fn')).toEqual(['r/q/pkg/Two.kt']);
expect(bucket(set, 'q.pkg.fn')).toEqual(['p/q/pkg/One.kt', 'r/q/pkg/Two.kt']);
expect(bucket(set, 'pkg.fn')).toEqual(['p/q/pkg/One.kt', 'r/q/pkg/Two.kt']);
});
it('the bucket handed out is frozen and the same object every call', () => {
// What this pins is the FREEZE, not the compaction (see the header): the
// array the fan-out tier hands out must be the one stored in the index and
// must be immutable. The finalize pass normalizes with `Array.isArray(t) ?
// t : [t]`, whose `arg is any[]` predicate widens the true branch, so
// `tsc --strict` accepts a `.sort()` or `.push()` there — and a sort would
// permanently reorder the cached bucket and flip the first-child tier's
// answer for every later import in the run. Freezing makes that a loud
// TypeError. It also fails if the compacted copy is frozen but never
// written back, since the array handed out is then the original.
const set = new Set(['pkg/One.kt', 'pkg/Two.kt']);
const first = bucket([], 'pkg.fn', set) as readonly string[];
const second = bucket([], 'pkg.fn', set) as readonly string[];
const first = bucket(set, 'pkg.fn') as readonly string[];
const second = bucket(set, 'pkg.fn') as readonly string[];
expect(first).toBe(second);
expect(Object.isFrozen(first)).toBe(true);
expect(() => (first as string[]).push('pkg/Three.kt')).toThrow(TypeError);
});
it('the first-child tier reads position 0 of the SAME bucket the fan-out returns', () => {
// The reason the freeze above matters, made observable. `import pkg.*`
// strips to `pkg`, which is the whole `pathLike`, so it answers from
// `findKotlinDirectoryChild`'s `children[0]`; `import pkg.fn` strips to
// `pkg` and fans the bucket out. One array, two tiers — so any reordering
// of the fan-out array moves the wildcard's single answer with it.
const set = new Set(['pkg/One.kt', 'pkg/Two.kt']);
const fanOut = bucket(set, 'pkg.fn') as readonly string[];
expect(fanOut).toEqual(['pkg/One.kt', 'pkg/Two.kt']);
expect(bucket(set, 'pkg.*')).toBe(fanOut[0]);
});
it('every key of one directory hands out its own frozen array', () => {
// The compaction loop walks EVERY key, and a file's directory contributes
// one key per component-suffix. Checking a single key would leave a loop
// that freezes only the first entry — or that interns one array across the
// keys, which would make a future in-place edit of one bucket visible
// through all of them — passing.
const set = new Set(['a/b/pkg/One.kt', 'a/b/pkg/Two.kt']);
const full = bucket(set, 'a.b.pkg.fn') as readonly string[];
const mid = bucket(set, 'b.pkg.fn') as readonly string[];
const leaf = bucket(set, 'pkg.fn') as readonly string[];
expect(Object.isFrozen(full)).toBe(true);
expect(Object.isFrozen(mid)).toBe(true);
expect(Object.isFrozen(leaf)).toBe(true);
expect(full).not.toBe(mid);
expect(mid).not.toBe(leaf);
expect(full).toEqual(leaf);
});
it('a single-child bucket is frozen too, on the length === 1 skip path', () => {
// Compaction skips `slice()` for a bucket that never grew. That branch must
// still freeze, or exactly the packages with one file stay mutable.
const set = new Set(['solo/One.kt']);
const only = bucket([], 'solo.fn', set) as readonly string[];
const only = bucket(new Set(['solo/One.kt']), 'solo.fn') as readonly string[];
expect(only).toEqual(['solo/One.kt']);
expect(Object.isFrozen(only)).toBe(true);
});
it('a package larger than V8 s first growth step keeps every member in order', () => {
// 40 files takes the backing store through 1 -> 17 -> 41, so this is the
// shape compaction actually reclaims. Nothing may be lost or reordered.
it('a package larger than V8 s first growth steps keeps every member in order', () => {
// Measured on this repo's Node (v22.18.0, x64, 8 bytes per element slot),
// by allocating 40 000 push-grown arrays per length and reading retained
// heap against the same arrays rebuilt at exact length: a bucket minted as
// `[raw]` and pushed into takes its backing store through
//
// capacity 1 -> 19 -> 46 -> 86 -> 146
// growing at lengths 2, 20, 47, 87
//
// so 40 files sits inside the 46-slot store with 6 slots — 48 bytes — of
// retained slack, which is what compaction reclaims. (The older `1 -> 17 ->
// 41` note in this comment described a capacity that never appears here and
// under-counted that slack by 6x. The arm is unaffected either way: 40 is
// past a growth step under both models. The exact steps are a V8 detail and
// may move with the Node floor — the assertion deliberately depends only on
// there BEING slack, not on how much.)
const files = Array.from({ length: 40 }, (_, i) => `big/Item${i}.kt`);
expect(bucket(files, 'big.fn')).toEqual(files);
expect(bucket(new Set(files), 'big.fn')).toEqual(files);
});
it('the memo keys on the NORMALIZED directory while storing raw paths', () => {
@ -93,15 +201,10 @@ describe('getKotlinFileIndex internals (#2881)', () => {
// separator is the same character at the same index — but only the KEY is
// normalized; the memo must not leak that into the stored value, which
// stays the raw path the file set holds.
expect(bucket(['win\\pkg\\A.kt', 'win\\pkg\\B.kt'], 'win.pkg.fn')).toEqual([
'win\\pkg\\A.kt',
'win\\pkg\\B.kt',
]);
const set = new Set(['win\\pkg\\A.kt', 'win\\pkg\\B.kt']);
expect(bucket(set, 'win.pkg.fn')).toEqual(['win\\pkg\\A.kt', 'win\\pkg\\B.kt']);
// Same directory reached by its component-suffix, i.e. through the memo's
// second and later keys rather than the full-dir key.
expect(bucket(['win\\pkg\\A.kt', 'win\\pkg\\B.kt'], 'pkg.fn')).toEqual([
'win\\pkg\\A.kt',
'win\\pkg\\B.kt',
]);
expect(bucket(set, 'pkg.fn')).toEqual(['win\\pkg\\A.kt', 'win\\pkg\\B.kt']);
});
});