mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(scope-resolution): resolve a package whose directory name repeats higher in the path (#2881) (#2929)
* fix(kotlin): resolve a root-level package whose name repeats higher in the path `getKotlinFileIndex` built its `dirChildren` buckets under two guards inherited from the pre-index per-import scan rather than from anything Kotlin requires: a `startsWith` test that skipped the bucket when the path began with the package name, and an `indexOf` equality that demanded the parent be the FIRST occurrence of `/<name>/` in the path. `s` is taken as `dir.slice(i + 1)` at each `/`, so `dir` ends with `/s` by construction and the file always IS a direct child of a directory named `s`. The guards therefore dropped legitimate buckets: data/src/main/kotlin/com/example/data/Repo.kt (leading, startsWith) top/data/mid/data/Repo.kt (mid-path, indexOf) `import data.helper` resolved to null against both. Only the fan-out tier was affected — `data.Repo` answers from `suffixByStem`, which carries no such guard — which is why the shape looked narrow enough for #2872 to preserve rather than change inside a performance PR. Both guards are removed. The rule stays "the parent directory is named `s`" — a name that appears in the path without being the parent (`top/data/mid/Repo.kt` for `data.something`) is still not a child, and a new case pins that. Widening is filtered downstream for the fan-out tier, which hands the finalize pass a candidate list (#1759), but NOT for the tier-1 fallback, which commits to `children[0]` unfiltered — and that is where most of the change lands: 149 of the 235 moved corpus records are a different first child against 32 wider arrays. Both are deliberate. A narrower bucket for the first-child tier alone would keep its answers identical and would also leave `import data.*` — a wildcard, which strips to `data` and lands on exactly that tier — resolving to null on the very shape this fixes. Both Kotlin benches are re-baselined deliberately, with the drift measured rather than accepted: - bench/kotlin-import-target: 235 of 19968 distinct records moved. 54 null -> resolved (the fix, and exactly the +54 in non_null), 181 answers that changed within a now-larger bucket. Zero buckets lost a member, zero results were dropped, and every reselected answer's parent directory is the queried package segment. The corpus is untouched, so `cases` is unchanged and the fingerprint covers the same surface as the value it replaces. - bench/import-target: the collide arm needed a corpus edit beside the new numbers. Its `d % 7` slice imported `com.example.vendor{d}`, a package that exists nowhere, purely to mirror the unique arm's nested-slice MISS; with that slice now resolving, leaving it would have left collide at 1100 against small's 1153 and broken the same-workload invariant the arm is built on. That assertion is what caught it. The gate controls were re-run against the new baseline, including one the fix makes newly plausible: a HALF fix that drops only `startsWith` and keeps the `indexOf` check still fails the fingerprint, so a partial fix cannot land quietly. Two gates moved with the code rather than being left behind: - kotlin `heap_reading_bytes` and `heap_ceiling_bytes` are re-recorded together as `_heap_reading_note` requires (48073096 -> 48200224, +0.264%, ceiling still 1.5x). The note says why that is small: the heap corpus is built with HEAP_PAD 8, so no path can begin with a suffix of its own directory and the leading-segment half of the old rule is invisible to that arm. - `depth_budget` 2.4 -> 2.2. Deleting two string comparisons per directory component is per-depth work, so the depth band fell from 1.44-1.51 to 1.27-1.40; left at 2.4 the gate's headroom would have drifted from ~1.6x to ~1.8x without anyone deciding to loosen it. `package-dir-index.ts` documents the same first-occurrence rule as universal, and it is not any more: Go, Java and C# still carry it and still have the shape. Fixing them means re-baselining three languages and editing the verbatim pre-change scans that import-target-index-parity.test.ts keeps as the specification, so it is a separate change — the comment now says so instead of describing a rule one of its readers no longer follows. Fixes #2881. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8QYxYvd5ntikRUvpLZD5e * fix(scope-resolution): drop the first-occurrence directory rule for Java, Go and C# too #2881 was reported against Kotlin, but the rule it removed was never Kotlin's. It is what the pre-index per-import scan happened to compute — `indexOf` for the package directory, then "nothing after the match holds a slash" — and every resolver built to reproduce that scan inherited it. Three still had it, and all three reproduced the reported defect: java data/src/main/java/com/example/data/Repo.java `import data.*` -> null java top/data/mid/data/Repo.java `import data.*` -> null csharp Models/src/App/Models/User.cs `using Models;` -> null csharp a/Models/b/Models/User.cs `using Models;` -> null go a/internal/auth/b/internal/auth/svc.go import "internal/auth" -> null Controls (`top/data/Repo.java`, `a/b/internal/auth/svc.go`) resolve, so these are the rule firing rather than an unrelated miss. Four sites, all reduced to "the file's parent directory ends with the queried path": - `package-dir-index.ts` `matchingDirs` (Go, Java, C# without csproj): the `indexOf` equality becomes `endsWith`, which also subsumes the length guard it needed — a shorter haystack is false instead of comparing -1 to -1. - `csharp.ts` `matchingDirPositions` (csproj step 3): same, and still deliberately UNANCHORED, so `src/SubModels` keeps answering `Models`. - `csharp.ts` csproj step 2: `indexOf` -> `lastIndexOf`, EXCEPT for an empty `dirPrefix`, which must keep `indexOf`. Its needle is a bare '/', and step 3 answers that query from `singleSegmentDirs` ("exactly one directory deep"), which only the first occurrence expresses; with `lastIndexOf` there, step 2 accepts every `.cs` in any directory and diverges from step 3. The csproj parity test catches it. - `go.ts` `resolveGoPackage`: `indexOf` -> `lastIndexOf`. No production caller, but the parity harness copies it verbatim as its spec. The two C# csproj sites must move together. Fixing only step 3 makes `Lib.Models` return step 3's superset instead of step 2's segment-aligned answer. Risk is not symmetric across the three. Go's consumer is a fan-out list and the finalize pass materializes one IMPORTS edge per element, so widening only ADDS edges. Java and C#-without-csproj commit to a single file through `firstFileDirectlyInPkgDir` with no downstream filter, so a widened bucket can also change which file an already-resolving import binds to — java's collide fingerprints moved while its resolved count did not, which is exactly that. C#'s leg is additionally gated by `csharpSuffixFallbackAllowed` (#1881) before resolution runs. Gates: - Twenty fingerprints re-baselined across go, csharp and java (five arms plus the top-level alias each). resolved 979 -> 1153 small, 4064 -> 4681 large for go and csharp; 1100 -> 1153 / 4456 -> 4681 for java. No `distinct_outcomes` moved. - csharp and java hit the same collide-arm trap Kotlin did: both sent their `d % 7` slice to a namespace that exists nowhere purely to mirror the unique arm's nested-slice MISS, so once that became a hit the arms resolved fewer imports than `small` and the same-workload assertion failed. Both now use their arm's ordinary spelling. - GO WAS NOT GATED AT ALL and the corpus had to change to make it so. Its nested slice repeated only the last segment (`src/pkg{d}/internal/ pkg{d}`) while a Go query addresses the whole package path, so the directory never ended with the query and the rule was never reached — every go arm sat unchanged through the resolver fix. `uniqueDir` and `collideDir` now repeat the shape at the granularity Go queries. `languages.go.heap.path_segments` 13 -> 14 follows from that. - `csharp_csproj`'s heap reading moved -0.79% (stable across runs) and is re-recorded with its ceiling: the step-2 filter decides which lazy `getFilesInDir` maps the probe forces. Everything else stayed within +/-0.03%, which is this box's jitter — `_heap_reading_note`'s claim that the readings reproduce to the byte across processes did not hold here, and the note now says so. The three parity harnesses keep VERBATIM copies of the pre-change scans as their specification, so each copy was updated with the resolver and the cases that pinned the rule now pin its removal. Two of them left the `mustBeNull` set in the shared harness — they resolve now, which holds them to the stronger "pin a winner" bar the rest of that arm uses. Refs #2881. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8QYxYvd5ntikRUvpLZD5e * perf(kotlin): intern dirChildren keys per directory and compact the buckets Two optimizations to `getKotlinFileIndex`, both output-identical, kept because they were measured and a third was dropped because it was not. 1. PER-DIRECTORY KEY MEMO. The component walk over `dir` cut one `slice` per component per FILE, and every slice after the first file of a directory is a freshly allocated string that hashes to a key the map already holds and is then dropped. The key list is a pure function of `dir`, so it is interned once per DIRECTORY. Measured -18.4% to -21.7% of the build at 32 000 files; zero retained cost, the memo dies with the frame. 2. BUCKET COMPACTION at the freeze loop. `addChild` mints `[raw]` and pushes, and V8 grows a backing store by `old + old/2 + 16`, so the SECOND child takes a 1-slot store to 17 and every bucket then retains its overshoot. 61 144 buckets at 32 000 files, 52.9% of their slots empty, 88 B each. `slice()` on freeze: -5 397 768 B, -11.20%, and the predicted 5 382 507 B lands within 0.03% of it. Same fix and the same accounting as the python `byBasename` note this repo already carries. `length === 1` is skipped deliberately. A bucket that never grew is already exact, so slicing it allocates a second array to save nothing — on a corpus of single-file packages the unguarded form costs 31% of the build for zero bytes. DROPPED: merging the `dirChildren` walk into the `suffixByStem` walk. It measures -0.10% at 32k, +0.23% at 100k and +0.40% at one file per directory, all inside a base-vs-identical-copy noise floor of -2.3% to +3.1%, and it does not compose usefully with the memo — the second scan it deletes is exactly the scan the memo makes rare. Only its provably free half is kept: `stem.lastIndexOf('/')` in place of `norm.lastIndexOf`, one backwards scan instead of two, exact because an extension carries no '/'. Neither optimization is visible to the correctness fingerprint, which is the point and also the risk: it observes the index only through the four resolver tiers, so a key-order move no corpus query reaches would survive it. Correctness therefore rests on a structural comparison of all three maps — key insertion order, values, bucket contents in order, frozen-ness — over 1234 corpora in both iteration orders, 14 808 comparisons, zero failures. The fingerprint, `cases` and `non_null` are unchanged and MUST NOT be re-baselined by this commit. Gates that did move, both because a reading and its budget move with the code rather than when CI goes red: - `heap_reading_bytes.kotlin` 48 200 224 -> 42 802 456 with its ceiling at 1.5x. A memory WIN passes every arm, so nothing forced this. - `depth_budget` 2.2 -> 2.0. The memo turns a per-file component walk into a per-directory one, which is precisely the per-depth work this arm exists to see: the band went 1.27-1.40 -> 1.20-1.26, and 2.2 held over it would have drifted from ~1.6x headroom to ~1.9x. The gate controls were re-run against the optimized builder, including one this change makes newly plausible: keying the memo on the directory's LAST SEGMENT instead of its full path drifts the fingerprint (36a4e9dad313, non_null 13310 -> 13305). That is the memo's whole safety argument stated as a test — its key decides which key set a directory contributes — and it is the one way this optimization could move an answer. The bucket-cap control was re-run too, since compaction now rewrites the same buckets. Also recorded, from measuring a reuse this repo had been invited to make: replacing `dirChildren` with the shared `package-dir-index` is output-identical (0 divergences over 107 948 answers) and passes every arm of the kotlin bench at 1.37x-1.50x — while costing 409x per fan-out and 8114x on `import data.*` at 200 matching directories on a corpus this bench does not carry. `_blind_spot` in the kotlin baselines now says so, with the memory the trade would have bought (26.2%, 12.18 MiB) and the corpus arm that would have to exist first. Refs #2881. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8QYxYvd5ntikRUvpLZD5e * fix(scope-resolution): close the gaps a four-lens review found in the #2881 change Correctness review found no defect in the shipped resolvers — the `endsWith` rewrites, the C# empty-prefix guard, the memo's purity, the `stem` vs `norm` derivation, the Map re-`set` during iteration and Go's `substring` arithmetic were each attacked with running code and each held. Everything below is a gap in what the change ASSERTS, measures or claims. UNGATED BEHAVIOUR, now covered: - `import-resolvers/go.ts` had no test at all. Its rule changed, the shared bench drives the indexed leg rather than this one, and a revert was caught by nothing. `go-package-resolve.test.ts` pins the membership rule and, more usefully, pins that Go's two independent legs agree on it — they disagreed before #2881 and a divergence here means the LanguageProvider hook and the ScopeResolver hook hold different views of a package. - The memo and the compaction are output-identical, so no fingerprint sees them and reverting either leaves both benches green. `kotlin-index-internals .test.ts` asserts them directly: the memo hit path against the miss path, two directories sharing a component-suffix keeping separate buckets (the one way a coarser memo key could move an answer), and that the bucket handed out is the cached, frozen, compacted array on both the sliced and the skipped path. The comment claiming this was "asserted structurally" previously pointed at nothing in the repo. GATES: - Six ratio budgets in `bench/import-target` were slack: the measurements they bound got faster and the numbers were left alone. kotlin depth 3.4 -> 2.8, go 1.6 -> 1.4, csharp 2.2 -> 2.0, java 2.2 -> 2.1, kotlin collide_scaling 1.8 -> 1.65, go 5.5 -> 5.1, each holding the headroom the old value expressed. The absolute ms ceilings are deliberately untouched: they carry runner-contention headroom, and a ratio is runner-speed-invariant where a millisecond is not. This is the failure the branch already fixed one directory over and missed here. - The `csharp_csproj` heap re-baseline is REVERTED. Base and branch both measure ~73.10e6 three runs each; the recorded 73703384 was simply not reproducible, and re-recording it would have dropped that language's derived floor 0.8% for no reason belonging to this change. - kotlin's collide arm was blind to the rule it was re-baselined for — a full revert of the Kotlin guards left both its fingerprints unmoved, because `com/example/models` is not a suffix of `…/models/inner/models`. Deepened to repeat the whole queried path; those two fingerprints are the only ones that moved for it. The same deepening on the java and kotlin UNIQUE arms was measured and REVERTED: ten more fingerprints, java's heap reading up 43%, and no coverage gained, because progressive stripping lands those queries on the same file either way. SIMPLIFICATION: - `go.ts` now states the predicate as ends-with like its three siblings, instead of keeping the `indexOf` shape with `lastIndexOf` swapped in. - C# csproj step 2's direct-child filter is dead for a non-empty prefix — `getFilesInDir`'s keys ARE segment-aligned directory suffixes, so it cannot reject, and measurement agrees over 12 008 pairs. Only the empty-prefix case does work, and only that case remains. - `addChild` had one call site left; inlined. The memo's double read of its own lookup is gone. The V8 byte accounting duplicated verbatim between the resolver comment and the baselines note now lives only in the note. - Four copies of the same ternary in the csproj parity harness collapse onto one hoisted `dirTrail`; two locals in the java harness were named for the branch that was deleted. CLAIMS THAT WERE WRONG: - `package-dir-index.ts` said "the four resolvers agree again". It is six, and the sixth is the evidence: `import-resolvers/jvm.ts` has answered the same question with `lastIndexOf` since #488, so before #2881 Java's and Kotlin's LanguageProvider hook and their ScopeResolver hook disagreed about which files a package holds. - The `uniqueDir` docblock claimed the last segment IS the query granularity for csharp/java/kotlin. They query the whole dotted path first and reach the tail only through stripping — which is why the partial-revert control fires on the go arm alone, now stated instead of implied. - Three parity harnesses described themselves as verbatim copies of the pre-change implementations; they were edited by this branch, so they are re-derivations of the current spec, a weaker claim their headers now make. - The shared harness header still listed the removed rule as current, the `DIRS` docblock still justified shapes by a divergence that no longer exists, and `measure.mjs`'s tier-two docblock plus `_heap_bound_note` still counted nine bounded languages when `HEAP_BOUNDED` derives to three — this branch had dutifully updated a kotlin bound in a list no gate reads. - `_blind_spot` told the next reader to build a repeated-leaf arm that already exists in the sibling bench, with a budget that already fails the swap. Both baselines are also re-serialized to preserve each note's original escaping, undoing ~20 KB of no-op churn an earlier revision introduced by round-tripping the JSON. Refs #2881. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W8QYxYvd5ntikRUvpLZD5e * perf(scope-resolution): drop the string each membership test built per candidate The three `endsWith` membership tests each minted a decorated copy of the directory once per candidate, per import. The decoration cancels: ('/' + D + '/').endsWith('/' + P + '/') <=> D === P || D.endsWith('/' + P) (D + '/').endsWith(P + '/') <=> D.endsWith(P) Verified exhaustively rather than argued — every pair of strings up to length 5 over `{a, b, /}` including the empty string, 132496 pairs, 0 divergences, with the match count reported beside it because two predicates that agree on `false` everywhere also show 0 divergences. `matchingDirs` 32.58 -> 8.22 ns/candidate (3.96x), `matchingDirPositions` 64.9 -> 18.4 ns. C#'s deliberate unanchoredness survives verbatim: `src/SubModels` still answers `Models`. `resolveGoPackage` was the opposite of a win — the rewrite in this branch left the `'/' + path` cons the old `includes` guard used to short-circuit, and the first `endsWith` forces V8 to flatten it once per file. Working on the raw path with an explicit start index is 4.8x faster than that and 1.78x faster than the code before this branch. It also now reuses `resolveGoPackageDir` instead of re-deriving six of its lines. Three claims these files make are corrected while they are open: - `package-dir-index.ts` argued the rule was accidental because a sixth implementation never had it, "wired as `importResolver` by `languages/{java,kotlin}.ts`" and therefore live. It is wired and not read: `provider.importResolver` is consumed only at `import-target-adapter.ts:74-75`, and that module's exports have no importer outside their own unit test, while its docblock claims it is threaded through `finalizeScopeModel`. The argument survives on the pre-index-scan derivation; `jvm.ts` is evidence about how the predicate was written, not about live behaviour. Whether those resolvers should be deleted or wired is left as an open question. - `csharp.ts` derived the empty `dirPrefix` case from "any path whose first slash is its last", which is wrong in both directions: `src/X.cs` satisfies it and emits no empty key, `a//X.cs` violates it and does. The conclusion stands and the filter stays — it is what rejects `a//X.cs`. - Step 2 returns on its first push, so widening it also suppresses step 3's unanchored leg. The narrower answer is the more precise one, but it was an unstated output change. `SuffixIndex.getFilesInDir` now states the segment-alignment its callers rely on, bounded as a guarantee about what may be RETURNED — php's root-anchored index answers only the equality arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kim7UK3kPhF1ZRZDuuvnus * fix(scope-resolution): say what the widened bucket actually does downstream The comment justifying the widening claimed a bucket that is too wide is "filtered downstream" by the finalize pass. It is not, for the edge that matters. `finalize-algorithm.ts` mints one draft per candidate, each keeping its own `targetFile`, and the File->File emitter in `graph-bridge/imports-to-edges.ts` tests only `targetFile === null` and `targetFile === sourceFile` before adding an `IMPORTS` relationship at confidence 1.0 — it never reads `linkStatus`. The `localDefs` filter from #1759 constrains `targetDefId` and the `BindingRef`; every extra bucket member is an unconditional file-level edge regardless. Measured on an Android-shaped layout, one `import data.load` goes from 5 to 6 edges, all six unresolved. No filtering is added here. Whether an unresolved candidate should produce that edge at all is a design question about the graph bridge, not about this bucket. The published drift census — 149 first-child reselections, 32 wider arrays, 54 null -> resolved — has no bucket for a fourth class this change introduces. Tier 3 precedes tier 4, so a bucket the guards used to leave empty returned null and let the progressive strip run; a populated bucket stops tier 4 entirely, turning a bound answer into a candidate list that need not carry the symbol. Re-running the census with a shape classifier finds that class ZERO times over the corpus, and the zero is the finding: the shape reproduces by hand, and this bench's own generator at 4000 repositories hits it 4-12 times per seed. The fingerprint cannot gate what the corpus cannot express — the same blindness the go arm carried until #2881 widened it. Two further claims are brought back in line with what shipped. The memo's docblock said `kotlin-index-internals.test.ts` asserts the key set, key insertion order and bucket order "over the built maps"; that file says it works through the resolver's observable surface and omits key order deliberately. The mutation matrix bounds it honestly: a mis-keyed memo is caught, a deleted one is not, and the compaction's only instrument is the bench heap ceiling. `findKotlinDirectoryChild` no longer claims to return "the same file the scan used to return" — that is precisely what moved. Structural, no behaviour: `let keys` sits with its consumer instead of 33 lines above it, the archaeology moves to the docblock, `tight` -> `compacted`, `dirEnd` -> `lastSlash` (the name three sibling builders use), and the one-use `MutableDirChildren` alias goes with the `addChild` it existed for. `finalize-algorithm.ts` annotates `targetFiles` as `readonly string[]` so `Array.isArray`'s `any[]` predicate can no longer widen a frozen cached bucket into something `.sort()` compiles against. The runtime freeze stays; it is the backstop for every other call site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kim7UK3kPhF1ZRZDuuvnus * 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 * test(bench): gate the bucket compaction, and publish the whole drift taxonomy The compaction shipped with no gate anywhere. Deleting `bucket.slice()` while keeping the freeze moves no fingerprint, no count and no test — only retained heap, 42805256 -> 48184784 B (+12.57%), byte-identical across three runs. Note the direction: compaction reclaims, so losing it makes the reading GROW, which no floor can see. `heap_ceiling_bytes.kotlin` tightens 64203684 -> 46000000 (1.5x -> 1.0747x of the reading), leaving the regression 4.8% clear above the ceiling and the reading 7.5% below it. The band is derived from first principles in `_heap_compaction_gate` (~61000 buckets x 11 spare slots at Node 22's 1->19 step) so it can be re-checked rather than trusted, and the note carries the triage rule: heapUsed accounting drift moves every arm, so kotlin alone over its ceiling is a lost compaction. `_gate_controls` claimed the two optimizations rest on a structural comparison over 1234 corpora in both iteration orders. No such probe exists in the tree. It now names the test that does exist and lists what it actually pins, and says key insertion order is unasserted by design. `_provenance` gains the full shape classification behind the 235 moved records: 149 string -> string, 38 null -> string, 16 null -> array, 32 array grew, and zero of every other transition — including `string -> array`, the resolved-becomes-unresolved class the old taxonomy had no bucket for. The harness was validated byte-exactly first: driven over this corpus the base resolver reproduces ebf1790bf1 / 13256 and head reproduces d91110bee3 / 13310. `measure.mjs` loses a paragraph asserting the C# unique slice repeats the whole queried path, directly above the paragraph explaining it is leaf-only deliberately and the code that makes it so. Acting on the deleted half resolves the csproj arm to zero. While measuring: the csharp collide arm is NOT blind — its fingerprint already moves across #2881 — but both csharp_csproj arms are, because `getFilesInDir` keys on segment-aligned suffixes and neither nested slice is one. Closing that needs a corpus redesign and four re-baselines; recorded, not attempted. One number changes in either baselines file, and it tightens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kim7UK3kPhF1ZRZDuuvnus --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergő Magyar
GitHub
parent
0fa547ccdc
commit
054641cafa
17 changed files with 1449 additions and 355 deletions
|
|
@ -392,7 +392,13 @@ function makeEdgeDrafts(
|
|||
// and resolved-dynamic imports are terminal at the file level — no
|
||||
// `targetDefId` needed since they materialize no `BindingRef`. Pre-
|
||||
// finalize them here so the fixpoint loop skips them entirely.
|
||||
const targetFiles = Array.isArray(targetFile) ? targetFile : [targetFile];
|
||||
// Annotated rather than inferred: `isArray`'s `arg is any[]` predicate widens
|
||||
// the true branch to a MUTABLE array, and a resolver may hand back a cached,
|
||||
// frozen candidate list (Kotlin's `dirChildren` buckets do). Only `.map` is
|
||||
// wanted here, so pinning `readonly` makes an in-place `.sort()`/`.push()` —
|
||||
// which would reorder that resolver's index for the rest of the run — a
|
||||
// compile error rather than a runtime TypeError.
|
||||
const targetFiles: readonly string[] = Array.isArray(targetFile) ? targetFile : [targetFile];
|
||||
const isFileLevelTerminal = parsed.kind === 'side-effect' || parsed.kind === 'dynamic-resolved';
|
||||
return targetFiles.map((tf) => {
|
||||
const base: ImportEdge = {
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -270,9 +270,12 @@
|
|||
* of eight, so go, dart and kotlin were excluded silently. All three
|
||||
* retain a real per-pass structure: go's `PackageDirIndex` reads
|
||||
* 2 998 464 B, dart's basename buckets 7 834 200 B, and kotlin's
|
||||
* `suffixByStem` cascade 48 073 096 B (45.85 MiB) — the second-largest
|
||||
* reading in this file, above ruby's 39.12 and java's 33.34, both of which
|
||||
* carry a full budget.
|
||||
* `suffixByStem` cascade 42 802 456 B (40.82 MiB) — above ruby's 39.12 and
|
||||
* java's 33.34, both of which carry a full budget. (Read 48 073 096 B when
|
||||
* this paragraph was written and described as "the second-largest reading
|
||||
* in this file", which it was not even then: csharp_csproj and php both
|
||||
* read higher. #2881 then compacted kotlin's `dirChildren` buckets and
|
||||
* took 11% off it.)
|
||||
* 2. TWO OF THE STATED REASONS NO LONGER HOLD. swift was excluded as "below
|
||||
* its own noise floor" on 0.98 MB at 8000 files against 0.29 MB at 32 000;
|
||||
* it now reads 969 120 B and 3 449 216 B, growing the right way. COBOL was
|
||||
|
|
@ -560,9 +563,20 @@ const HEAP_BUDGETED = [
|
|||
// per-pass structure and each grows LINEARLY with the file count (ratio
|
||||
// 0.996-1.004 against a 1.25 budget over 8000 -> 32000 files), so each can
|
||||
// carry the full ceiling + floor + ratio set rather than a bound alone.
|
||||
// kotlin's 45.85 MiB is the second-largest reading in this file — larger than
|
||||
// ruby's and java's, both of which were budgeted from the start — and it had
|
||||
// no stated exclusion reason at all.
|
||||
// kotlin's 40.82 MiB is larger than ruby's and java's, both of which were
|
||||
// budgeted from the start, and it had no stated exclusion reason at all.
|
||||
//
|
||||
// Its ceiling is also the one TIGHT ceiling in this file — 1.0747x its
|
||||
// reading where every other is 1.5x — because it is the only one gating a
|
||||
// size REDUCTION being preserved rather than a footprint not growing.
|
||||
// #2881 compacts `dirChildren`'s buckets, and deleting that `slice()` is
|
||||
// invisible to every other instrument in the repository: output-identical,
|
||||
// so no fingerprint moves; capacity has no reflective surface, so no unit
|
||||
// assertion moves; and both heap scales grow together, so `heap_ratio_budget`
|
||||
// divides it out. It shows up here and nowhere else, at +12.57%. See
|
||||
// `_heap_compaction_gate` in baselines.json for the measurement, the
|
||||
// arithmetic behind the 5.4 MB, and how to tell a lost compaction from a
|
||||
// runner's heapUsed accounting moving under the whole file.
|
||||
'kotlin',
|
||||
'dart',
|
||||
'go',
|
||||
|
|
@ -726,12 +740,33 @@ function unwiredLanguage(where, lang) {
|
|||
* UNIQUE-LEAF layout: one directory name per index, so no two directories share
|
||||
* a last segment and no two files share a basename. Every index bucket holds
|
||||
* exactly one entry. A nested same-name directory in one repo slice is the
|
||||
* shape whose handling the first-`indexOf` tie-break decides (see
|
||||
* package-dir-index.ts), and the shape Kotlin's `dirChildren` resolves the same
|
||||
* way.
|
||||
* shape the first-`indexOf` tie-break used to reject (see package-dir-index.ts);
|
||||
* #2881 removed that tie-break from every resolver that had it, so the go,
|
||||
* csharp, java and kotlin arms all resolve their `d % 7` slice now.
|
||||
*
|
||||
* A repeat the query cannot ask about leaves the arm blind, which is why go's
|
||||
* slice repeats the WHOLE package path: a Go import addresses `src/pkg{d}`, and
|
||||
* `…/internal/pkg{d}` does not end with that, so the old rule was never even
|
||||
* reached and every go arm sat still through the fix. Java, C# and Kotlin query
|
||||
* the whole dotted path FIRST and only fall back to the tail through
|
||||
* progressive stripping, so their slices — which repeat the last segment only —
|
||||
* move through that fallback rather than the primary query. The consequence is
|
||||
* measured and worth knowing: a partial revert that reinstates first-occurrence
|
||||
* only for multi-segment package paths is caught on the go arm alone.
|
||||
*/
|
||||
function uniqueDir(lang, d, i) {
|
||||
if (lang === 'go') return d % 7 === 0 ? `src/pkg${d}/internal/pkg${d}` : `src/pkg${d}`;
|
||||
// Go's nested slice repeats the WHOLE queried path (`src/pkg{d}`), not just
|
||||
// its last segment. `src/pkg{d}/internal/pkg{d}` repeated only `pkg{d}`, so
|
||||
// the query `src/pkg{d}` failed on "the directory ends with the package path"
|
||||
// and never reached the first-occurrence rule at all — Go's arms did not move
|
||||
// when #2881 removed that rule, which would have shipped a widened bucket
|
||||
// with no bench coverage while C# and Java were re-baselined for it.
|
||||
if (lang === 'go') return d % 7 === 0 ? `src/pkg${d}/internal/src/pkg${d}` : `src/pkg${d}`;
|
||||
// Leaf-only repeat, deliberately: this layout is shared with the
|
||||
// `csharp_csproj` arm, whose configs mint `dirPrefix` against `src/Ns{d}`, so
|
||||
// deepening it to the full `App/Ns{d}` query path resolves that arm to ZERO
|
||||
// and breaks its same-workload invariant. C# therefore exercises the removed
|
||||
// rule through progressive stripping rather than through its primary query.
|
||||
if (lang === 'csharp') return d % 7 === 0 ? `src/Ns${d}/Sub/Ns${d}` : `src/Ns${d}`;
|
||||
if (lang === 'dart') return d % 3 === 0 ? `lib/feature${d}` : `pkg/feature${d}`;
|
||||
if (lang === 'kotlin') {
|
||||
|
|
@ -782,14 +817,52 @@ function uniqueDir(lang, d, i) {
|
|||
*/
|
||||
function collideDir(lang, d, i) {
|
||||
if (lang === 'go') {
|
||||
if (d % 7 === 0) return `svc${d}/internal/sub/internal`;
|
||||
// `…/sub/internal` repeats only the last segment, which the ends-with test
|
||||
// answers on its own; `…/internal/sub/svc{d}/internal` is the shape the
|
||||
// removed first-occurrence rule used to reject (see `uniqueDir`).
|
||||
if (d % 7 === 0) return `svc${d}/internal/sub/svc${d}/internal`;
|
||||
return d % 5 === 1 ? `svc${d}/internal/shared` : `svc${d}/internal`;
|
||||
}
|
||||
// Leaf-only repeat here too, and unlike the kotlin arm below that is not a
|
||||
// blind spot — measured, base against head over this exact corpus. C#'s match
|
||||
// test is an unanchored ends-with and its cascade strips leading segments, so
|
||||
// `App.Src{d}.Models` reaches `Models` after two strips and finds
|
||||
// `Src{d}/Models/Inner/Models`, whose FIRST `/Models/` is not its last: the
|
||||
// removed first-occurrence rule rejected it and the current one takes it. The
|
||||
// `csharp` collide fingerprint therefore moves across #2881 (03c9afe33276
|
||||
// head, 89d0a054b617 base) with the resolved count unchanged at 1153 — the
|
||||
// arm sees the change, it just sees it as different ANSWERS rather than more
|
||||
// of them. Deepening the slice to `Src{d}/Models/Inner/Src{d}/Models` only
|
||||
// moves which strip level finds it; both layouts move base -> head, so it
|
||||
// buys nothing here.
|
||||
//
|
||||
// And it costs, because the `csharp_csproj` constraint binds this arm too —
|
||||
// differently from the way it binds `uniqueDir`. There, deepening resolves
|
||||
// that arm to ZERO. Here it resolves MORE: `Lib` has `projectDir: ''`, so its
|
||||
// `dirPrefix` is `Src{d}/Models`, which is not a segment suffix of
|
||||
// `…/Inner/Models` and is one of `…/Inner/Src{d}/Models`. Measured, the
|
||||
// csproj arm's collide `resolved` goes 979 -> 1153 against its `small` 979,
|
||||
// which is the same-workload invariant `--check` asserts. (Worth recording
|
||||
// while it is measured: with the shipped layout BOTH csproj arms are blind to
|
||||
// #2881 — unique and collide fingerprints identical base and head — because
|
||||
// `getFilesInDir` is keyed on segment-aligned directory SUFFIXES and neither
|
||||
// nested slice is one. Closing that is the deepening plus a mirrored miss for
|
||||
// the csproj arm's `d % 7` slice, i.e. a corpus redesign and four
|
||||
// re-baselines, not this edit.)
|
||||
if (lang === 'csharp') return d % 7 === 0 ? `Src${d}/Models/Inner/Models` : `Src${d}/Models`;
|
||||
if (lang === 'dart') return `pkg${d}/lib/src`;
|
||||
if (lang === 'kotlin') {
|
||||
return d % 7 === 0
|
||||
? `mod${d}/src/main/kotlin/com/example/models/inner/models`
|
||||
? // Repeats the WHOLE queried path (`com.example.models`), not just the
|
||||
// `models` leaf. With a leaf-only repeat this arm was structurally
|
||||
// blind to the #2881 rule: a full revert of the Kotlin guards left both
|
||||
// collide fingerprints unmoved, because `com/example/models` is not a
|
||||
// suffix of `…/models/inner/models` and the query never reached the
|
||||
// rule. Deepening it is the only corpus edit in this file that buys
|
||||
// coverage — the same deepening applied to the java and kotlin UNIQUE
|
||||
// arms was measured and reverted, because progressive stripping lands
|
||||
// those queries on the same file either way.
|
||||
`mod${d}/src/main/kotlin/com/example/models/inner/com/example/models`
|
||||
: `mod${d}/src/main/kotlin/com/example/models`;
|
||||
}
|
||||
if (lang === 'php') return `svc${d}/src/Models`;
|
||||
|
|
@ -1203,11 +1276,13 @@ function collideTarget(lang, { local, r, d, j, dirs }) {
|
|||
}
|
||||
if (lang === 'csharp') {
|
||||
return local
|
||||
? // `Vendor` has no directory anywhere, mirroring the unique arm's
|
||||
// nested-same-name slice, which also resolves to nothing.
|
||||
d % 7 === 0
|
||||
? `App.Src${d}.Vendor`
|
||||
: `App.Src${d}.Models`
|
||||
? // This used to send the `d % 7` slice to `App.Src{d}.Vendor`, a
|
||||
// namespace with no directory anywhere, to mirror the unique arm's
|
||||
// nested-same-name slice, which also resolved to nothing. #2881 made
|
||||
// that slice resolve, so the mirror has to as well — otherwise this arm
|
||||
// stops resolving as many imports as `small`, which is the invariant
|
||||
// that makes the two timings comparable and is asserted below.
|
||||
`App.Src${d}.Models`
|
||||
: (r >>> 3) % 2 === 0
|
||||
? ['System', 'System.Threading.Tasks', 'System.Collections.Generic'][(r >>> 4) % 3]
|
||||
: `Ghost${(r >>> 4) % 97}.Deep.Missing`;
|
||||
|
|
@ -1246,13 +1321,18 @@ function collideTarget(lang, { local, r, d, j, dirs }) {
|
|||
`package:ext${(r >>> 4) % 97}/other/mod${(r >>> 4) % 8}.dart`;
|
||||
}
|
||||
if (lang === 'kotlin') {
|
||||
// Same wildcard share as the unique arm; `vendor${d}` is the collide
|
||||
// layout's spelling of a package that exists nowhere.
|
||||
// Same wildcard share as the unique arm. This used to send the `d % 7`
|
||||
// nested slice to `com.example.vendor${d}`, a package that exists nowhere,
|
||||
// to mirror the unique arm's nested slice — which missed, because
|
||||
// `dirChildren` required the parent to be the FIRST occurrence of its own
|
||||
// name and `…/com/example/pkg${d}/inner/pkg${d}` therefore did not belong to
|
||||
// `pkg${d}`. #2881 removed that rule, so the unique arm's nested wildcards
|
||||
// resolve and the mirror has to as well, or this arm stops resolving as
|
||||
// many imports as `small` — which is the invariant that makes the two
|
||||
// timings comparable, and it is asserted below.
|
||||
return local
|
||||
? (r >>> 3) % 3 === 0
|
||||
? d % 7 === 0
|
||||
? `com.example.vendor${d}.*`
|
||||
: `com.example.models.*`
|
||||
? `com.example.models.*`
|
||||
: `com.example.models.File${j}`
|
||||
: (r >>> 3) % 2 === 0
|
||||
? ['java.util.List', 'kotlin.collections.Map', 'kotlinx.coroutines.flow.Flow'][
|
||||
|
|
@ -1282,13 +1362,13 @@ function collideTarget(lang, { local, r, d, j, dirs }) {
|
|||
// every directory now ends in, so `firstFileDirectlyInPkgDir` walks the
|
||||
// whole `model` bucket twice — at the direct match and again after the
|
||||
// first strip — before the third strip finds `model` on its own. That walk
|
||||
// is the non-constant term this arm exists to measure. `vendor` buckets to
|
||||
// nothing, mirroring the unique arm's nested slice, which also misses.
|
||||
// is the non-constant term this arm exists to measure. The `d % 7` slice
|
||||
// used to import `com.svc{d}.vendor`, which buckets to nothing, mirroring
|
||||
// the unique arm's nested slice — which missed until #2881 and resolves
|
||||
// now, so the mirror follows it or the same-workload invariant below breaks.
|
||||
return local
|
||||
? (r >>> 3) % 3 === 0
|
||||
? d % 7 === 0
|
||||
? `com.svc${d}.vendor.*`
|
||||
: `com.svc${d}.model.*`
|
||||
? `com.svc${d}.model.*`
|
||||
: `com.example.model.File${j}`
|
||||
: (r >>> 3) % 2 === 0
|
||||
? ['java.util.List', 'java.io.IOException', 'java.util.concurrent.ConcurrentHashMap'][
|
||||
|
|
@ -1820,8 +1900,9 @@ const HEAP_PROBE_TARGET = {
|
|||
javascript: 'vendor0/lib/missing',
|
||||
python: 'vendor0.deep.missing',
|
||||
c: 'vendor0/missing.h',
|
||||
// The nine below are the BOUNDED tier — see `HEAP_BOUNDED`. Same rule as the
|
||||
// eight above: a spelling `uniqueTarget` already mints for that language, and
|
||||
// The entries below cover the BOUNDED tier — see `HEAP_BOUNDED`, which
|
||||
// derives to cobol, swift and rust; the rest were promoted. Same rule as the
|
||||
// budgeted ones above: a spelling `uniqueTarget` already mints for that language, and
|
||||
// one that MISSES, so the reading is the index and the cascade runs to the
|
||||
// end. Chosen from the miss family that reaches furthest into each cascade:
|
||||
// - `go` takes the GOPATH fallback, one `filesDirectlyInPkgDir` per path
|
||||
|
|
@ -2578,9 +2659,12 @@ for (const lang of HEAP_BUDGETED) {
|
|||
* TIER TWO, the bounded arms: ONE comparison, and what it is a comparison FOR.
|
||||
*
|
||||
* `heap_bound_bytes` is the "exclusion still holds" bound. It does not claim
|
||||
* these nine indexes are small enough, which is what a ceiling claims about a
|
||||
* these indexes are small enough, which is what a ceiling claims about a
|
||||
* budgeted one; it claims each is still the SIZE the decision to leave it out
|
||||
* was taken on. The re-entry condition the MEMORY section states — "if any of
|
||||
* was taken on. `HEAP_BOUNDED` derives to THREE today — cobol, swift, rust.
|
||||
* The prose below still counts nine because six were promoted to tier one
|
||||
* after it was written; read the counts as history, and `HEAP_BOUNDED` itself
|
||||
* as the answer. The re-entry condition the MEMORY section states — "if any of
|
||||
* the four ever diverges in what it ASKS, it earns an arm the same way" — is a
|
||||
* claim about growth, and this is the only thing in the file that can see it.
|
||||
*
|
||||
|
|
@ -2588,13 +2672,12 @@ for (const lang of HEAP_BUDGETED) {
|
|||
* because it builds nothing, so any floor at all would be a floor on noise and
|
||||
* `1.5 x 0 B` is 0 — its bound is ABSOLUTE (1 MiB) for the same reason: a
|
||||
* multiplier on 16 B fails on the first byte of anything. The other eight are
|
||||
* stable enough today to floor (0.24% peak-to-peak at worst over five runs) and
|
||||
* two of them — kotlin at 45.85 MiB and dart at 7.47 — are larger than budgeted
|
||||
* arms, so a floor there would be worth having. That is a promotion to tier one,
|
||||
* with a ceiling and a recorded reading, and it is not this change: a floor
|
||||
* without them would assert "still measuring" against a number nothing else
|
||||
* bounds. What this tier is NOT is a weaker version of tier one — it is the
|
||||
* different question, asked of every language instead of eight.
|
||||
* stable enough today to floor (0.24% peak-to-peak at worst over five runs).
|
||||
* The two this paragraph named as floor candidates, kotlin and dart, TOOK that
|
||||
* promotion: both now carry a ceiling and a recorded reading in tier one, which
|
||||
* is what the paragraph said the promotion had to be. What this tier is NOT is a
|
||||
* weaker version of tier one — it is a different question, asked of the
|
||||
* languages tier one does not ask it of.
|
||||
*/
|
||||
const heapBoundScope =
|
||||
`That leaves the arm bounded by nothing, which is the state all nine of these were in before ` +
|
||||
|
|
|
|||
|
|
@ -1,14 +1,15 @@
|
|||
{
|
||||
"_comment": "Baselines for bench/kotlin-import-target/measure.mjs --check. `fingerprint` is a sha256 over every `fileSet | fromFile | targetRaw -> result` record the correctness corpus resolves, in BOTH file-set iteration orders; it is a CORRECTNESS gate, so drift means Kotlin import resolution started returning a different file set and IMPORTS/CALLS edges moved in every Kotlin repository. Explain it, never re-baseline to make CI green. `cases` and `non_null` are asserted beside it because a shrunken or hollowed corpus produces a perfectly valid fingerprint over a smaller surface — all three are one re-baseline, never separate ones. `scaling_budget`, `depth_budget` and `small_ms_ceiling` are timing gates and carry deliberate headroom for shared CI runners.",
|
||||
"_provenance": "This fingerprint is the value the PRE-INDEX implementation produces. It was not read off the new code: the same corpus was run against `git show <pre-index>:gitnexus/src/core/ingestion/languages/kotlin/import-target.ts` — the four-tier per-import scan — and against the index that replaced it. Both print ebf1790bf1d42dad483a51f2cbdeb2351e493b9e8236e4eedeef592dd81e2c5c over 20106 cases, 13256 of them non-null. That is what makes the index change a performance change rather than a behaviour change, and it is reproducible: swap the module specifier at the top of measure.mjs for the old file and re-run. The corpus deliberately includes the shapes where the two could have diverged — repeated directory names whose FIRST occurrence is not the parent (`data/src/main/kotlin/com/example/data/Repo.kt` is NOT a child of `data`, because the old scan tested startsWith and then used indexOf), doubly nested same-name directories, an exact match appearing after a suffix match in iteration order, `.kt`/`.kts` stem collisions, backslash paths, repo-root files, wildcard `.*` targets landing on the single-file tier rather than fanning out, and non-Kotlin noise.",
|
||||
"_gate_controls": "The gate is only worth its baseline if a plausible regression moves it, so each arm was checked against the mutation it exists to catch, with the resolver otherwise untouched. Caught, all with the corpus below: capping suffixByStem key depth at 7 (fingerprint a0e6eb98f9…); skipping the dirChildren suffix loop above depth 8 (d53182ebbc…, non_null 13256 -> 12746); capping a dirChildren bucket at 17 entries (ed3ea85c59…). Also caught, with the RESOLVER untouched and only the corpus edited: dropping the competing file from the exact-beats-earlier-suffix case and emptying the repeated-directory negative case (44df5093ee…). All four passed silently before this corpus carried deep paths, packages above 16 files, queries against suffix keys deeper than 7, and the file set inside the hashed record. Re-check them after any corpus edit — a corpus that stops spanning an axis takes the gate with it.",
|
||||
"fingerprint": "ebf1790bf1d42dad483a51f2cbdeb2351e493b9e8236e4eedeef592dd81e2c5c",
|
||||
"_provenance": "RE-BASELINED ONCE, DELIBERATELY, IN #2881. The previous value ebf1790bf1d42dad483a51f2cbdeb2351e493b9e8236e4eedeef592dd81e2c5c (13256 non-null) was the PRE-INDEX implementation's, and the index that replaced it in #2872 reproduced it byte for byte — that is what made #2872 a performance change. #2881 changes resolution on purpose: `getKotlinFileIndex` no longer requires the parent directory to be the FIRST occurrence of that name in the path, so a file whose package directory name repeats higher in its own path is now a child of that package (`data/src/main/kotlin/com/example/data/Repo.kt` IS a child of `data`, and `import data.helper` resolves instead of returning null). The drift was not read off the new code and accepted; the corpus was dumped from both implementations and diffed record by record. That census was RE-RUN with a shape classifier after review found its taxonomy — 54 NULL -> resolved, 149 reselections, 32 wider fan-outs — was entirely SHAPE-PRESERVING and so had no bucket for a class this change introduces. Both ends of the re-run are validated against numbers this file already publishes, so the census is provably over the surface they describe: driven over this bench's own corpus, the BASE resolver reproduces ebf1790bf1… at 13256 non_null and the HEAD resolver reproduces d91110bee3… at 13310, across 20106 records of which 19968 are distinct. 235 distinct records moved, classified by SHAPE rather than by null-ness: null -> string 38 and null -> array 16, which together are the first census's '54 NULL -> resolved' and exactly the +54 in non_null; string -> string (a different member of a now-wider bucket) 149; array -> array (the fan-out grew) 32; string -> array 0; and ZERO of every other transition — nothing went string -> null, array -> null or array -> string, and no array shrank or reordered. So the old three buckets reappear inside the shape taxonomy exactly, and its two structural claims hold when checked directly instead of inferred: all 32 growths are order-preserving SUPERSETS of the base answer, no record lost a member, and in all 149 reselections the new answer's parent directory is named by a segment of the import and carries the same directory NAME the base answer's parent did. WHAT THE OLD TAXONOMY HAD NO BUCKET FOR is `string -> array`, and it is the one class here that is not shape-preserving: it is a RESOLVED -> UNRESOLVED transition. Tier 3 (`findKotlinPackageFiles`) runs before tier 4 (`findByProgressivePrefixStrip`), so a bucket the removed guards left empty returned null and let tier 4 answer with a single BOUND file; a now-populated bucket stops tier 4 running at all and hands back a fan-out array that need not contain the imported name at all. Two files reproduce it, in both iteration orders: ['data/src/main/kotlin/com/example/data/Repo.kt', 'common/helper.kt'] with `import data.helper` answers 'common/helper.kt' at base and ['data/src/main/kotlin/com/example/data/Repo.kt'] at head. ITS COUNT OVER THIS CORPUS IS 0, AND THAT IS A FACT ABOUT THE CORPUS RATHER THAN ABOUT THE CLASS. This file's own fuzz generator, run at ten times the repositories (4000, ~198600 distinct records), hits the class 12, 4, 10 and 10 times over four seeds — ~5e-5 per record, an expectation of about ONE over the 19968 records here — so 0 is this corpus being an order of magnitude too small to reach it, not the shape being unreachable. The consequence is worth stating plainly: the fingerprint below is blind to a resolved -> unresolved class this change introduces, by corpus SIZE and not by construction, and no arm in this bench gates it today. Adding a hand-written case for it is a deliberate fingerprint move and a fourth re-baseline of this file; it is worth doing and it is not this change. The corpus itself is untouched, which is why `cases` is unchanged at 20106 — the fingerprint is over the same surface as the value it replaces.",
|
||||
"_gate_controls": "The gate is only worth its baseline if a plausible regression moves it, so each arm was checked against the mutation it exists to catch, with the resolver otherwise untouched. All values below are against the CURRENT baseline (#2881, guards removed + per-directory key memo + bucket compaction). Caught: skipping the dirChildren component walk above depth 8 (fingerprint 41bb550b76d4…, non_null 13310 -> 12800); capping a dirChildren bucket at 17 entries (a7681945b752…, non_null UNCHANGED — the fingerprint is the only arm that sees it, and note the compaction pass now rewrites those same buckets, so this control was re-run after it); capping suffixByStem key depth at 7 (d24b8a2bd822…, non_null unchanged); and a HALF fix that drops only the `startsWith` guard while keeping the `indexOf` first-occurrence check (836977b83bf0…, non_null 13310 -> 13282), which leaves every mid-path repeat such as `top/data/mid/data/Repo.kt` broken and is the mutation #2881 itself makes plausible. Added with the memo: keying `dirKeys` on the directory's LAST SEGMENT instead of its full path (36a4e9dad313…, non_null 13310 -> 13305) — the memo's whole safety argument is that its key determines the key SET a directory contributes, so a coarser key silently hands one directory another's bucket list, and that is the one way this optimization can move an answer. Also caught, with the RESOLVER untouched and only the corpus edited: dropping the competing file from the exact-beats-earlier-suffix case and emptying the repeated-directory case (44df5093ee…). All of these passed silently before this corpus carried deep paths, packages above 16 files, queries against suffix keys deeper than 7, and the file set inside the hashed record. Re-check them after any corpus edit — a corpus that stops spanning an axis takes the gate with it. NOTE what no fingerprint control here can catch: the memo and the compaction are both invisible to this bench by design (identical output), so no arm in this file gates either one, and the honest version of where they ARE gated is narrower than a claim about comparing the three maps would suggest. The memo's gate is test/unit/scope-resolution/kotlin/kotlin-index-internals.test.ts, which drives the resolver's OBSERVABLE SURFACE rather than the built index — the index is module-private — and reconstructs what it needs from the tiers. It pins: bucket CONTENTS and ORDER, read back from the fan-out tier, which hands out the bucket array itself; that the first-child tier reads position 0 of that SAME array; bucket IDENTITY across two calls on one Set, which is what proves the memo's hit path ran at all, since only a second file in the same directory reaches it; the frozen state of the array actually handed out, on the multi-child path, on the `length === 1` skip path, and once per key of a multi-key directory; that the memo keys on the NORMALIZED directory while storing the raw path; and the one mutation that can move an answer — keying `dirKeys` on the directory's last segment instead of the whole `dir` — which fails three of its arms. KEY INSERTION ORDER is unasserted there BY DESIGN and not by omission: `dirChildren` is only ever read by `.get(key)`, so key order has no consumer, and that file says so. The COMPACTION is unasserted there too and cannot be asserted there at all — a JS array's backing-store capacity has no reflective surface, so deleting `bucket.slice()` and freezing the grown bucket in place leaves every arm in that file green, `Object.isFrozen` included. Its only instrument is the retained-heap arm in bench/import-target, whose kotlin ceiling was tightened to 1.0747x its recorded reading precisely so that the +12.57% the slice reclaims fails `--check`; see `_heap_compaction_gate` in bench/import-target/baselines.json for the measurement and for how to tell that failure apart from a runner's heapUsed accounting moving under the whole file.",
|
||||
"fingerprint": "d91110bee389891c313811c5b4bae61d909561156e1458d38d487be969f0059c",
|
||||
"cases": 20106,
|
||||
"non_null": 13256,
|
||||
"non_null": 13310,
|
||||
"scaling_budget": 1.6,
|
||||
"depth_budget": 2.4,
|
||||
"depth_budget": 2.0,
|
||||
"small_ms_ceiling": 40,
|
||||
"_scaling_note": "(t_large/t_small)/(1600/400). ~1.0 is linear. OBSERVED BAND: 0.99-1.04 on a 12-core dev box, small arm ~6 ms. Read that band as a floor, not a spec — independent runs on other hardware during review came out 0.954-1.014, 0.965-1.036 and ~0.95-1.08, so a 1.2 reading is noise and should be re-run, not investigated. IMPORTS_PER_FILE is sized so the small arm lands in the ms rather than the ~2 ms a first revision measured, where timer granularity and JIT warm-up, not scaling, set the number; bench/cpp-qualified-ns documents the same artifact. TRIAGE: every timing arm here is a TIMING signal — RE-RUN IT on an idle machine before investigating; runner contention dominates. The fingerprint arm is the opposite: deterministic, a re-run never changes it, and it must never be wished away. FLOOR CHECK: the pre-index implementation — i.e. exactly the regression this gate exists to catch — measures ratio 3.737 on this corpus (2207.8 ms small, 33003.5 ms large, one cold run) against ~1.0 for the index. Independent review runs measured its floor at 3.905-4.297. Treat the absolute times as an order of magnitude only: the floor arm is one cold run because best-of-seven against a quadratic implementation costs minutes, while the index arm is best-of-seven after two warmups.",
|
||||
"_depth_note": "deep_ms/shallow_ms at a FIXED file count, paths 24 components against 8. scaling_ratio divides the file count out, so it is scale-invariant and structurally cannot see a cost that grows with path depth instead — and both loops this change added are depth loops (one suffixByStem entry per '/' in a stem, one dirChildren pass per component of dir). OBSERVED BAND: 1.44-1.51 over four unloaded runs. It sits above 1.0 legitimately: 3x the depth is 3x the suffix keys per file, so the build genuinely does more work; what the budget of 2.4 forbids is that growing faster than the depth ratio itself.",
|
||||
"_ceiling_note": "small_ms_ceiling is an ABSOLUTE bound, because scaling_ratio is a ratio and a constant-factor regression that grows both arms equally passes it. Measured during review: a full workspace scan reintroduced on 1-in-16 imports is caught by the ratio (1.814), but at 1-in-32 it passes at 1.490 while running 2.8x slower in absolute terms. 40 ms against an observed 5.9-6.1 ms leaves ~6x of headroom for a loaded shared runner while still catching that shape."
|
||||
"_depth_note": "deep_ms/shallow_ms at a FIXED file count, paths 24 components against 8. scaling_ratio divides the file count out, so it is scale-invariant and structurally cannot see a cost that grows with path depth instead — and the two loops the index is built from are depth loops (one suffixByStem entry per '/' in a stem, one dirChildren pass per component of dir). OBSERVED BAND, five runs each on one box: 1.44-1.51 before #2881; 1.27-1.40 after its guard removal, which deleted two string comparisons per component of every dir; 1.20-1.26 after the same issue's per-directory key memo, which turns that whole component walk from once-per-FILE into once-per-DIRECTORY. Both movements are per-depth work, which is why this arm sees them and the file-count arm does not. The BUDGET moved with the band both times — 2.4 -> 2.2 -> 2.0 — holding the ~1.6x headroom over the band's top that 2.4 expressed against the original; left at 2.4 it would quietly have become 1.9x, which is how a gate goes slack without anyone deciding to loosen it. Note what this budget is NOT for: a revert of #2881 scores ~1.5 and passes at any of those numbers, and that is correct — reverting it restores a resolution bug, which is the FINGERPRINT's job to catch, not a timing arm's. It sits above 1.0 legitimately: 3x the depth is 3x the suffix keys per file, so the build genuinely does more work; what the budget forbids is that growing faster than the depth ratio itself.",
|
||||
"_ceiling_note": "small_ms_ceiling is an ABSOLUTE bound, because scaling_ratio is a ratio and a constant-factor regression that grows both arms equally passes it. Measured during review: a full workspace scan reintroduced on 1-in-16 imports is caught by the ratio (1.814), but at 1-in-32 it passes at 1.490 while running 2.8x slower in absolute terms. 40 ms against an observed 5.9-6.1 ms leaves ~6x of headroom for a loaded shared runner while still catching that shape.",
|
||||
"_blind_spot": "WHAT THIS BENCH CANNOT SEE, measured rather than guessed. Its scaling corpus gives every module a UNIQUE package leaf (`com/example/mod{N}`), so a `dirChildren` query matches exactly one directory. That makes it blind to any cost that grows with the number of DIRECTORIES sharing a queried segment — the shape a real Kotlin monorepo has, where 200 modules each hold `data`, `ui` and `domain`. Established by building the reuse this file's memo argues against: swapping `dirChildren` for the shared `import-resolvers/package-dir-index.ts` (with its first-occurrence rule off) is OUTPUT-IDENTICAL — same fingerprint, same cases, same non_null, 0 divergences over 107948 answers — and on THIS corpus it costs only 1.37x-1.50x and passes every arm here. On a repeated-leaf corpus the same swap measures 13.5x per first-child query, 409x per fan-out, and 8114x on `import data.*` at 200 matching directories (it merges and SORTS every candidate, per import), for 3.1x-5.7x end to end and a bench-style scaling_ratio of 3.465 against this file's 1.6 budget — i.e. back to the pre-index quadratic floor of 3.737. A change that regresses this resolver to the very shape the bench exists to catch would go GREEN here. The trade it buys is real and also measured: 26.2% less retained memory, 12.18 MiB at 32000 files. If that memory is ever wanted, the shape to build is per-suffix keys -> DIRECTORY lists plus files-per-directory (8.29 MiB against 15.87 measured, single-directory query still one hash lookup) — and the repeated-leaf arm to measure it against already exists one directory over: bench/import-target's kotlin `collide` layout puts `com/example/models` under 200 modules at the 1600-file scale, with `collide_scaling_budget` 1.8 against a measured 1.081. The swap scores 3.465 there. So the gate for this decision is that arm, not a new one here; what this file lacks is only a repeated-leaf arm of its own, which would be duplicated coverage."
|
||||
}
|
||||
|
|
|
|||
|
|
@ -60,13 +60,16 @@
|
|||
* Set-iteration order — "first suffix match wins", and the two stem maps
|
||||
* keeping the FIRST path inserted per key. A single-order corpus scores an
|
||||
* implementation that keeps the LAST match identically.
|
||||
* 2. **The correctness corpus contains repeated directory names where the
|
||||
* first occurrence is not the parent** (`data/src/main/kotlin/com/example/
|
||||
* data/Repo.kt`). The pre-index scan tested `startsWith` and then used
|
||||
* `indexOf`, so it only ever considered the FIRST `/dir/`; that file is
|
||||
* therefore NOT a child of `data`. The index reproduces it deliberately.
|
||||
* Without these shapes the fingerprint cannot tell the preserved rule from
|
||||
* the intuitive one.
|
||||
* 2. **The correctness corpus contains repeated directory names at BOTH the
|
||||
* leading and the mid-path position** (`data/src/main/kotlin/com/example/
|
||||
* data/Repo.kt` and `top/data/mid/data/Repo.kt`). Until #2881 the resolver
|
||||
* required a file's package directory to be the FIRST occurrence of that
|
||||
* name in its own path, so neither file was a child of `data`; both are
|
||||
* now, and that is what the fingerprint pins. Two positions, not one,
|
||||
* because the old rule was two guards and a half fix that drops only the
|
||||
* leading-position one still leaves the mid-path shape broken — see
|
||||
* `_gate_controls` in baselines.json. Without these shapes the fingerprint
|
||||
* cannot tell the current rule from either predecessor.
|
||||
* 3. **~40% of the scaling corpus's imports are unresolvable.** The old cost
|
||||
* was worst when nothing matched, because only then did all four tiers
|
||||
* run. A corpus where every import hits tier 1 exits after one pass and
|
||||
|
|
@ -141,12 +144,7 @@ function record(files, targetRaw, fromFile = 'App.kt') {
|
|||
if (r !== null) nonNull++;
|
||||
const rendered = r === null ? 'NULL' : Array.isArray(r) ? `[${r.join(',')}]` : r;
|
||||
// The FILE SET is part of the hashed record, not just the query and the
|
||||
// result — see header property 4. Without it a corpus edit that changes
|
||||
// which workspace a case runs against, while leaving the result string
|
||||
// alone, is invisible: dropping the competing file from the
|
||||
// "exact beats an earlier suffix" case, or emptying the repeated-directory
|
||||
// negative case, both leave `cases`, `non_null` and the fingerprint
|
||||
// byte-identical.
|
||||
// result — see header property 4.
|
||||
lines.push(`${order}\t${list.join('|')}\t${fromFile}\t${targetRaw}\t${rendered}`);
|
||||
}
|
||||
}
|
||||
|
|
@ -187,7 +185,8 @@ record(['win\\pkg\\A.kt', 'win\\pkg\\B.kt'], 'win.pkg.someFunction');
|
|||
record(['pkg/A.java', 'pkg/A.md', 'pkg/A.kt.txt'], 'pkg.A');
|
||||
// Kotlin file alongside non-Kotlin noise of the same stem.
|
||||
record(['pkg/A.java', 'pkg/A.kt'], 'pkg.A');
|
||||
// Header property 2: repeated directory name, first occurrence is not the parent.
|
||||
// Header property 2: repeated directory name — a child of the repeated package
|
||||
// since #2881, at the leading position here and mid-path below.
|
||||
record(['data/src/main/kotlin/com/example/data/Repo.kt'], 'data.something');
|
||||
record(['data/src/main/kotlin/com/example/data/Repo.kt'], 'data.Repo');
|
||||
record(['a/c/b/c/File.kt'], 'c.X');
|
||||
|
|
|
|||
|
|
@ -25,19 +25,31 @@ import { csharpSuffixFallbackAllowed } from '../csharp-namespace-gate.js';
|
|||
* normalized directory of a `.cs` file and `dirPrefix` for the query:
|
||||
*
|
||||
* let H = D + '/', P = dirPrefix + '/'
|
||||
* match ⟺ H.length >= P.length && H.indexOf(P) === H.length - P.length
|
||||
* match ⟺ H.endsWith(P)
|
||||
*
|
||||
* Derivation, because both halves are load-bearing:
|
||||
* Derivation:
|
||||
* - the scan keeps a file only when nothing after the matched occurrence holds
|
||||
* a slash, so the occurrence's trailing '/' must be the file's LAST slash —
|
||||
* i.e. `H` ends with `P`;
|
||||
* - it uses `indexOf`, the FIRST occurrence, so `a/Models/b/Models/x.cs` does
|
||||
* NOT answer `Models`: the first `Models/` is found and `b/Models/x.cs`
|
||||
* still contains a slash. Dropping that half moves edges in every repo that
|
||||
* nests a directory name inside itself.
|
||||
* - it used `indexOf`, the FIRST occurrence, so `a/Models/b/Models/x.cs` did
|
||||
* NOT answer `Models`. That half was removed in #2881: it was an artifact of
|
||||
* how the pre-index scan was written, not a rule about C# namespaces, and it
|
||||
* dropped every repository that nests a directory name inside itself. The
|
||||
* same removal landed in `package-dir-index.ts` and in step 2 below, which
|
||||
* have to move together — see the note at step 3.
|
||||
* - the needle ends with '/', so every occurrence of it lies wholly inside
|
||||
* `D + '/'` and never reaches into the file name — which is what lets the
|
||||
* whole test be evaluated on `D` alone.
|
||||
* whole test be evaluated on `D` alone;
|
||||
* - and then the '/' cancels. `(D + '/').endsWith(P + '/')` IS `D.endsWith(P)`:
|
||||
* the appended character only ever matches itself, so it decides nothing and
|
||||
* the comparison of everything before it is unchanged. The predicate the code
|
||||
* actually runs is therefore
|
||||
*
|
||||
* match ⟺ D.endsWith(dirPrefix)
|
||||
*
|
||||
* with no concatenation on either side. Verified rather than argued: over
|
||||
* every ordered pair of strings up to length 5 over `{a, b, '/'}` including
|
||||
* the empty string — 132 496 pairs — the two forms disagreed 0 times.
|
||||
*
|
||||
* NOT the same query as `package-dir-index.ts`, and the difference is exactly
|
||||
* one character on each side: that module tests `'/'+D+'/'` against
|
||||
|
|
@ -50,6 +62,11 @@ import { csharpSuffixFallbackAllowed } from '../csharp-namespace-gate.js';
|
|||
* "cleaned up" into a reuse of `filesDirectlyInPkgDir` — see
|
||||
* `test/unit/import-resolvers/csharp-csproj-parity.test.ts`.
|
||||
*
|
||||
* That one character is also why the cancellation above empties this predicate
|
||||
* out but not that one: the decoration is one term per side here (`D + '/'`) and
|
||||
* two there (`'/' + D + '/'`), and only the TRAILING '/' cancels. Here nothing
|
||||
* is left to concatenate; there the leading segment anchor has to stay.
|
||||
*
|
||||
* Candidates are narrowed by the directory's LAST segment, the same
|
||||
* O(directories) bucket `package-dir-index.ts` uses instead of an
|
||||
* O(files × depth) suffix map (#2649).
|
||||
|
|
@ -176,14 +193,32 @@ function* matchingDirPositions(
|
|||
index: CsharpNamespaceDirIndex,
|
||||
dirPrefix: string,
|
||||
): Generator<readonly number[]> {
|
||||
const needle = dirPrefix + '/';
|
||||
for (const dir of candidateDirs(index, dirPrefix)) {
|
||||
const haystack = dir + '/';
|
||||
// The length guard is not redundant: for a shorter `haystack`, `indexOf`
|
||||
// returns -1 and `haystack.length - needle.length` can also be -1, which
|
||||
// would report a bogus match.
|
||||
if (haystack.length < needle.length) continue;
|
||||
if (haystack.indexOf(needle) !== haystack.length - needle.length) continue;
|
||||
// `(dir + '/').endsWith(dirPrefix + '/')` IS `dir.endsWith(dirPrefix)` — the
|
||||
// appended '/' only ever matches itself, so it decides nothing and BOTH
|
||||
// concatenations go. Exhaustively verified, not assumed: 0 disagreements
|
||||
// over every ordered pair of strings up to length 5 over `{a, b, '/'}`
|
||||
// including '' (132 496 pairs). Measured 64.9 ns -> 18.4 ns per candidate
|
||||
// (Node 22.18); the `dir + '/'` was paid once per candidate, on every sweep
|
||||
// of the last-segment keys.
|
||||
//
|
||||
// Still deliberately UNANCHORED (no leading '/'), so `src/SubModels` keeps
|
||||
// answering `Models` — see the derivation above. That is also exactly why
|
||||
// the reduction empties this predicate out while `package-dir-index.ts`
|
||||
// keeps its concatenations: one decorating term per side here, two there,
|
||||
// and only the trailing one cancels.
|
||||
//
|
||||
// `endsWith` subsumes the length guard the `indexOf` form needed: a shorter
|
||||
// `dir` is simply false, where `indexOf` returned -1 and
|
||||
// `haystack.length - needle.length` could also be -1 and report a bogus
|
||||
// match.
|
||||
//
|
||||
// Do NOT "finish the job" with the two-argument overload. `endsWith(search,
|
||||
// endPosition)` measured 8.8-11.8 ns against 9.5-14.9 ns for the
|
||||
// one-argument form across seven call-site shapes (Node 22.18) — a wash —
|
||||
// and `dir.endsWith(dirPrefix, dir.length)` is character-for-character this
|
||||
// same test anyway. There is nothing left here to win.
|
||||
if (!dir.endsWith(dirPrefix)) continue;
|
||||
const positions = index.positionsByDir.get(dir);
|
||||
if (positions !== undefined) yield positions;
|
||||
}
|
||||
|
|
@ -284,15 +319,48 @@ export function resolveCSharpImportInternal(
|
|||
// 2. Try as directory: all .cs files directly inside (namespace import)
|
||||
if (index) {
|
||||
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
|
||||
// `getFilesInDir` already answers "directly inside a directory `D` where
|
||||
// `D === dirPrefix || D.endsWith('/' + dirPrefix)`" — its keys ARE
|
||||
// segment-aligned directory suffixes. So for a non-empty `dirPrefix` the
|
||||
// direct-child re-check this loop used to run cannot reject anything, and
|
||||
// measurement agrees: zero rejections over 12 008 (prefix, candidate)
|
||||
// pairs. It rejected before #2881 only because it asked `indexOf` for the
|
||||
// FIRST `/<dirPrefix>/`, which is the rule that issue removed.
|
||||
//
|
||||
// That widening does not stay inside step 2's own bucket. This step
|
||||
// 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: over `SubModels/Models/F1.cs` + `SubModels/F3.cs`,
|
||||
// `using App.Models` answered both through step 3 and now answers only the
|
||||
// first through step 2. The new answer is the more precise one — a
|
||||
// directory literally named `Models` beating a character-suffix hit on
|
||||
// `SubModels` — and it is what this module's step-2-before-step-3 layering
|
||||
// asks for, so it is kept rather than worked around. Pinned absolutely by
|
||||
// the parity test, which is differentially blind to it (its frozen legacy
|
||||
// copy moved in lockstep with this line).
|
||||
//
|
||||
// The empty prefix is the exception and keeps a real filter. `getDirMap`
|
||||
// keys a file under every suffix of its DIRECTORY, so it emits the EMPTY
|
||||
// one exactly when that directory's last component is empty: a leading '/'
|
||||
// on a root-level file, or a doubled slash immediately before the file
|
||||
// name. Probed against `getDirMap`'s own key emission:
|
||||
//
|
||||
// src/X.cs -> ['src:.cs'] no empty key
|
||||
// /X.cs -> [':.cs'] empty key
|
||||
// a//X.cs -> [':.cs', 'a/:.cs'] empty key
|
||||
// /a/b/X.cs -> ['b:.cs', 'a/b:.cs', '/a/b:.cs'] no empty key
|
||||
//
|
||||
// So the `''` bucket is not "one directory deep" on its own — `a//X.cs`
|
||||
// sits in it two components down — while step 3 answers that same query
|
||||
// from `singleSegmentDirs`, which is. Filtering on `D` holding no slash is
|
||||
// what rejects `a//X.cs` and keeps steps 2 and 3 in agreement.
|
||||
for (const f of dirFiles) {
|
||||
const normalized = f.replace(/\\/g, '/');
|
||||
// Check it's a direct child by finding the dirPrefix and ensuring no deeper slashes
|
||||
const prefixIdx = normalized.indexOf(dirPrefix + '/');
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
|
||||
if (!afterDir.includes('/')) {
|
||||
results.push(f);
|
||||
if (dirPrefix === '') {
|
||||
const normalized = f.replace(/\\/g, '/');
|
||||
const lastSlash = normalized.lastIndexOf('/');
|
||||
if (lastSlash < 0 || normalized.slice(0, lastSlash).includes('/')) continue;
|
||||
}
|
||||
results.push(f);
|
||||
}
|
||||
if (results.length > 0) return results;
|
||||
}
|
||||
|
|
@ -301,7 +369,7 @@ export function resolveCSharpImportInternal(
|
|||
//
|
||||
// Not redundant with step 2, and not skippable when `index` is present:
|
||||
// `getFilesInDir` is keyed on SEGMENT suffixes of a directory, while this
|
||||
// leg's predicate is an unanchored substring one, so it additionally
|
||||
// leg's predicate is an unanchored ends-with one, so it additionally
|
||||
// answers `Models` with `src/SubModels/` and `src/Models` with
|
||||
// `vendor/mysrc/Models/`. It is also the only leg that answers an empty
|
||||
// `dirPrefix` — the `relative = ''` branch above (the import IS the root
|
||||
|
|
|
|||
|
|
@ -3,10 +3,23 @@
|
|||
*
|
||||
* Strategy lives in configs/go.ts.
|
||||
* This file contains the shared helpers used by the strategy.
|
||||
*
|
||||
* **Reachability, as of #2929:** nothing in production calls either export
|
||||
* today. The only path in is `configs/go.ts` → `createImportResolver` →
|
||||
* the `importResolver` field on Go's `LanguageProvider`, and that field is
|
||||
* read at exactly two lines — `import-target-adapter.ts:74-75` — whose two
|
||||
* exports (`buildImportTargetWorkspace`,
|
||||
* `resolveImportTargetAcrossLanguages`) have no importer anywhere but their
|
||||
* own unit test. So this is a live-looking but currently unwired leg; the
|
||||
* tests in `test/unit/import-resolvers/go-package-resolve.test.ts` are the
|
||||
* only thing watching it.
|
||||
*/
|
||||
|
||||
import type { GoModuleConfig } from '../language-config.js';
|
||||
|
||||
/** `'/'`, for the parent-directory boundary check in `resolveGoPackage`. */
|
||||
const SLASH_CODE = 47;
|
||||
|
||||
/**
|
||||
* Extract the package directory suffix from a Go import path.
|
||||
* Returns the suffix string (e.g., "/internal/auth/") or null if invalid.
|
||||
|
|
@ -28,29 +41,39 @@ export function resolveGoPackage(
|
|||
normalizedFileList: readonly string[],
|
||||
allFileList: readonly string[],
|
||||
): string[] {
|
||||
if (!importPath.startsWith(goModule.modulePath)) return [];
|
||||
// Identical to the six lines this used to re-derive; `resolveGoPackageDir`
|
||||
// returns the '/'-wrapped form and the scan wants the bare path, so unwrap.
|
||||
const pkgDir = resolveGoPackageDir(importPath, goModule);
|
||||
if (pkgDir === null) return [];
|
||||
const relativePkg = pkgDir.slice(1, -1); // "/internal/auth/" → "internal/auth"
|
||||
|
||||
// Strip module path to get relative package path
|
||||
const relativePkg = importPath.slice(goModule.modulePath.length + 1); // e.g., "internal/auth"
|
||||
if (!relativePkg) return [];
|
||||
|
||||
const pkgSuffix = '/' + relativePkg + '/';
|
||||
const pkgLen = relativePkg.length; // >= 1: `resolveGoPackageDir` rejects empty
|
||||
const matches: string[] = [];
|
||||
|
||||
for (let i = 0; i < normalizedFileList.length; i++) {
|
||||
// Prepend '/' so paths like "internal/auth/service.go" match suffix "/internal/auth/"
|
||||
const normalized = '/' + normalizedFileList[i];
|
||||
// File must be directly in the package directory (not a subdirectory)
|
||||
if (
|
||||
normalized.includes(pkgSuffix) &&
|
||||
normalized.endsWith('.go') &&
|
||||
!normalized.endsWith('_test.go')
|
||||
) {
|
||||
const afterPkg = normalized.substring(normalized.indexOf(pkgSuffix) + pkgSuffix.length);
|
||||
if (!afterPkg.includes('/')) {
|
||||
matches.push(allFileList[i]);
|
||||
}
|
||||
}
|
||||
const normalized = normalizedFileList[i];
|
||||
if (!normalized.endsWith('.go') || normalized.endsWith('_test.go')) continue;
|
||||
// The file's PARENT directory ends with the package path — the same
|
||||
// predicate `package-dir-index.ts` states. This used to ask `indexOf` for
|
||||
// the FIRST `/<pkg>/` and then check that nothing after it held a slash,
|
||||
// which made `a/pkg/b/pkg/x.go` not a member of `pkg` (#2881).
|
||||
//
|
||||
// Expressed as "`relativePkg` sits immediately before the last slash, on a
|
||||
// segment boundary". The boundary is either the start of the path (an
|
||||
// import matching from index 0, `internal/auth/x.go`) or a `/` — which is
|
||||
// what the old `'/' + path` cons bought, at the price of a per-file
|
||||
// concatenation the first `endsWith` forced V8 to flatten (#2929).
|
||||
//
|
||||
// Rewriting this as `endsWith(relativePkg, lastSlash)` buys nothing: the
|
||||
// two-argument overload measured a wash against `startsWith(needle, pos)`
|
||||
// here (10.28 ns vs 9.82 ns), so it trades the clarity of an explicit start
|
||||
// index for no gain. A "the 2-arg overload leaves V8's fast path, 20x"
|
||||
// claim from review did not reproduce on Node 22.18 — its baseline was a
|
||||
// one-argument call that early-exited on the length precheck.
|
||||
const start = normalized.lastIndexOf('/') - pkgLen; // < 0 when there is no parent dir
|
||||
if (start < 0 || !normalized.startsWith(relativePkg, start)) continue;
|
||||
if (start > 0 && normalized.charCodeAt(start - 1) !== SLASH_CODE) continue;
|
||||
matches.push(allFileList[i]);
|
||||
}
|
||||
|
||||
return matches;
|
||||
|
|
|
|||
|
|
@ -12,15 +12,51 @@
|
|||
*
|
||||
* let D = '/' + <normalized dir of the file> + '/'
|
||||
* let P = '/' + pkgPath + '/'
|
||||
* match ⟺ D.endsWith(P)
|
||||
*
|
||||
* It used to say one more thing, and #2881 removed it:
|
||||
*
|
||||
* match ⟺ D.length >= P.length && D.indexOf(P) === D.length - P.length
|
||||
*
|
||||
* The right-hand side says two things at once, and BOTH are load-bearing:
|
||||
* 1. `D` ends with `P` — the file's directory ends with `pkgPath`;
|
||||
* 2. that trailing occurrence is the FIRST one — so `a/pkg/b/pkg/x.go` does
|
||||
* NOT answer `pkg`, because the original `indexOf` found the earlier `/pkg/`
|
||||
* and `b/pkg/x.go` still contained a slash. Dropping condition 2 looks like
|
||||
* a cleanup and moves edges in every repository that nests a directory name
|
||||
* inside itself (`internal/…/internal`, `Models/…/Models`).
|
||||
* — i.e. `D` ends with `P` AND that trailing occurrence is the FIRST one, so
|
||||
* `a/pkg/b/pkg/x.go` did NOT answer `pkg`. The second half was never a rule
|
||||
* anyone chose. It is what the pre-index per-import scan happened to compute
|
||||
* (it called `indexOf`, then checked that nothing after the match contained a
|
||||
* slash), and the index was built to reproduce that scan byte for byte. It
|
||||
* dropped exactly the repositories that nest a directory name inside itself:
|
||||
* `internal/…/internal`, `Models/…/Models`, and the reported shape
|
||||
* `data/src/main/kotlin/com/example/data/Repo.kt`, where `import data.helper`
|
||||
* resolved to null. Kotlin was fixed first, in its own `dirChildren`
|
||||
* (`languages/kotlin/import-target.ts`); this index, the C# csproj index and
|
||||
* the legacy `go.ts` scan followed.
|
||||
*
|
||||
* The strongest evidence that the rule was accidental is that a sixth
|
||||
* implementation of the same question never had it. `import-resolvers/jvm.ts`
|
||||
* answers "files directly inside a directory ending with <packagePath>" for
|
||||
* Java and Kotlin wildcard imports, and has used `lastIndexOf` since #488.
|
||||
*
|
||||
* That is evidence about how the predicate was WRITTEN, not about live
|
||||
* behaviour, and the distinction matters enough to spell out. `jvm.ts` is
|
||||
* reached only through `provider.importResolver`, which `languages/java.ts` and
|
||||
* `languages/kotlin.ts` do wire — but that field currently has no production
|
||||
* READER. Its only reader anywhere is `import-target-adapter.ts`, whose own
|
||||
* docblock says it is "threaded through `finalizeScopeModel`"; nothing threads
|
||||
* it, and neither that module nor its two exports
|
||||
* (`buildImportTargetWorkspace`, `resolveImportTargetAcrossLanguages`) is
|
||||
* referenced outside its own unit test. So `jvm.ts`'s `resolveJvmWildcard` and
|
||||
* `import-resolvers/go.ts`'s `resolveGoPackage` are dormant, while THIS index,
|
||||
* `csharp.ts`'s `resolveCSharpImportInternal` and Kotlin's `dirChildren` are
|
||||
* the ones that run. Whether those two dormant resolvers should be deleted or
|
||||
* actually wired up is an open question and wants its own issue; it is not
|
||||
* settled here.
|
||||
*
|
||||
* The argument survives that correction intact, because it never needed the
|
||||
* resolvers to be live: an independent implementation of the same question,
|
||||
* written without reference to the pre-index scan, reached for `lastIndexOf`.
|
||||
* The extra clause was never a rule anyone chose. All six spellings now agree.
|
||||
*
|
||||
* The length guard the `indexOf` form needed is gone with it: `endsWith` is
|
||||
* false for a shorter `D` instead of comparing -1 to -1.
|
||||
*
|
||||
* Candidates are narrowed by the directory's LAST segment rather than by
|
||||
* indexing every directory suffix: a suffix map costs O(files × depth) entries,
|
||||
|
|
@ -120,14 +156,40 @@ function* matchingDirs(index: PackageDirIndex, pkgPath: string): Generator<reado
|
|||
const lastSegment = pkgPath.slice(pkgPath.lastIndexOf('/') + 1);
|
||||
const dirs = index.dirsByLastSegment.get(lastSegment);
|
||||
if (dirs === undefined) return;
|
||||
const needle = `/${pkgPath}/`;
|
||||
// `('/' + D + '/').endsWith('/' + P + '/')` ⟺ `D === P || D.endsWith('/' + P)`,
|
||||
// which is the same predicate without the two strings per candidate the
|
||||
// wrapped form built: 32.58 ns → 8.22 ns per candidate, 3.96x, over 2001
|
||||
// directories of which 668 match (Node 22.18.0, best-of-80 after 300 warmup
|
||||
// passes). Verified exhaustively rather than argued, over every pair of
|
||||
// strings up to length 5 over `{a, b, /}` including the empty string —
|
||||
// 132 496 pairs, 911 of them matching: 0 divergences. The match count is
|
||||
// reported beside the timing on purpose: two predicates that agree on `false`
|
||||
// everywhere also show 0 divergences.
|
||||
//
|
||||
// Worth the care because this loop is genuinely hot: Go's GOPATH fallback
|
||||
// calls `matchingDirs` once per import-path segment over the bucket holding
|
||||
// EVERY directory that shares the queried last segment (every service's
|
||||
// `internal`). At 1000 services × 100 000 unresolved imports × 4 segments
|
||||
// that is ~34.6 s against ~14.0 s, and ~0.5 GB of transient garbage not
|
||||
// allocated.
|
||||
//
|
||||
// There is nothing further to win by reaching for the two-argument
|
||||
// `endsWith(search, endPosition)` or for `startsWith(needle, pos)`: on the
|
||||
// same data all three land together — 8.15 ns one-argument, 8.55 ns
|
||||
// two-argument, 8.54 ns `startsWith` — and against the unwrapped string the
|
||||
// two-argument form is character-for-character the same test. A "the 2-arg
|
||||
// overload leaves V8's fast path, 20x" claim was measured during review and
|
||||
// did NOT reproduce here or in two independent re-runs; its 1.87 ns baseline
|
||||
// was a one-argument call whose needle failed the length/last-char precheck
|
||||
// and early-exited without comparing. Recorded because the retraction is the
|
||||
// useful part: compare forms that do the same work and report the hit count.
|
||||
//
|
||||
// The equality arm also carries the length guard the `indexOf` form needed: a
|
||||
// shorter `dir` is simply false, where `indexOf` returned -1 and
|
||||
// `haystack.length - needle.length` could also be -1 and report a bogus match.
|
||||
const suffix = `/${pkgPath}`;
|
||||
for (const dir of dirs) {
|
||||
const haystack = `/${dir}/`;
|
||||
// The length guard is not redundant: for a shorter `haystack`,
|
||||
// `indexOf` returns -1 and `haystack.length - needle.length` can also be
|
||||
// -1, which would report a bogus match.
|
||||
if (haystack.length < needle.length) continue;
|
||||
if (haystack.indexOf(needle) !== haystack.length - needle.length) continue;
|
||||
if (dir !== pkgPath && !dir.endsWith(suffix)) continue;
|
||||
const files = index.filesByDir.get(dir);
|
||||
if (files !== undefined) yield files;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -97,6 +97,15 @@ export interface SuffixIndex {
|
|||
/**
|
||||
* Get all files in a directory suffix.
|
||||
*
|
||||
* `dirSuffix` is matched as a SEGMENT-aligned directory suffix — every
|
||||
* returned file is a direct child of a directory `D` with
|
||||
* `D === dirSuffix || D.endsWith('/' + dirSuffix)`. Callers may rely on this
|
||||
* and skip a direct-child re-check; `import-resolvers/csharp.ts` step 2 does
|
||||
* exactly that. It bounds what may be RETURNED, not what must be found: an
|
||||
* implementation is free to answer with fewer files, and the root-anchored
|
||||
* index in `languages/php/import-target.ts` answers only the `D === dirSuffix`
|
||||
* arm.
|
||||
*
|
||||
* `readonly` is the CONTRACT, and it is the contract for every implementation
|
||||
* of this interface, not a description of any one of them: an implementation
|
||||
* is free to return its own bucket by reference, so callers must treat the
|
||||
|
|
|
|||
|
|
@ -42,10 +42,26 @@
|
|||
* only a `.java` file can carry a `…/<name>.java` suffix key, so the
|
||||
* extension filter is implied on the file/suffix legs and explicit in the
|
||||
* directory index's `accept`.
|
||||
* 5. The directory-child leg matched on the FIRST `'/' + pathLike + '/'`
|
||||
* occurrence, so `com/example/com/example/Deep.java` does NOT answer
|
||||
* `com.example`. `firstFileDirectlyInPkgDir` encodes exactly that rule (see
|
||||
* the header of `import-resolvers/package-dir-index.ts`).
|
||||
* 5. The directory-child leg used to match on the FIRST `'/' + pathLike + '/'`
|
||||
* occurrence, so `com/example/com/example/Deep.java` did NOT answer
|
||||
* `com.example`. #2881 removed that: the rule came from how the pre-index
|
||||
* scan was written, not from Java, and it made a package whose name repeats
|
||||
* higher in the path unresolvable. `firstFileDirectlyInPkgDir` now answers
|
||||
* plain "the parent directory ends with `pathLike`" (see the header of
|
||||
* `import-resolvers/package-dir-index.ts`). This leg commits to ONE file
|
||||
* with no downstream filter, so widening it can change which file an
|
||||
* already-resolving import binds to, not only turn a null into a hit.
|
||||
* WHICH file it binds to is decided by nothing in this resolver: it is
|
||||
* `allFilePaths` iteration order, i.e. the insertion order of the Set built
|
||||
* from `parsedFiles` in `scope-resolution/pipeline/run.ts`, which for a full
|
||||
* scan is the canonical sorted path order `filesystem-walker.ts` imposes on
|
||||
* its unsorted recursive-`glob` result. So the widened set's winner is a
|
||||
* property of the file list, not of the import — pinned explicitly, in both
|
||||
* insertion orders, by "pins WHICH of two competing package directories the
|
||||
* first-child leg takes" in
|
||||
* `test/unit/scope-resolution/java-import-target-parity.test.ts` (Kotlin's
|
||||
* twin, which has the same unfiltered first-child leg, is in
|
||||
* `test/unit/scope-resolution/kotlin/kotlin-import-target-parity.test.ts`).
|
||||
*/
|
||||
|
||||
import type { ParsedImport, WorkspaceIndex } from 'gitnexus-shared';
|
||||
|
|
|
|||
|
|
@ -88,8 +88,10 @@ function findKotlinDirectoryChild(index: KotlinFileIndex, pathLike: string): str
|
|||
if (pathLike === '') return null;
|
||||
const children = index.dirChildren.get(pathLike);
|
||||
// "First" is first in `allFilePaths` iteration order, which the index
|
||||
// preserves by appending as it walks the set — the same file the scan
|
||||
// used to return.
|
||||
// preserves by appending as it walks the set. Since #2881 that can be an
|
||||
// EARLIER file than the pre-index scan returned, never a later one: the
|
||||
// guards that fell take members away from no bucket, so a bucket only ever
|
||||
// gains, and a gained member lands wherever set iteration puts it.
|
||||
return children === undefined ? null : (children[0] ?? null);
|
||||
}
|
||||
|
||||
|
|
@ -156,22 +158,89 @@ function findByProgressivePrefixStrip(index: KotlinFileIndex, pathLike: string):
|
|||
* The shared `buildSuffixIndex` (`import-resolvers/utils.ts`, used by C#, Ruby,
|
||||
* Vue and TypeScript) is deliberately NOT reused — the same call Python
|
||||
* documents at `python/import-target.ts`. Run side by side against this
|
||||
* resolver, four probes out of five diverge:
|
||||
* resolver, three probes out of five diverge:
|
||||
*
|
||||
* - `['deep/util/User.kt', 'util/User.kt']` for `util.User` — it conflates
|
||||
* exact and proper-suffix matches in one map, so the deep path wins where
|
||||
* the scan returned the exact one;
|
||||
* - `['deep/util/User.kt', 'util/User.kts']` for `util.User` — its keys carry
|
||||
* the extension, so a `.kt` SUFFIX beats a `.kts` EXACT;
|
||||
* - `['data/src/…/data/Repo.kt']` for `data.getRepo` — it indexes every
|
||||
* directory suffix with no first-occurrence rule, so it fans out where the
|
||||
* scan returned null;
|
||||
* - `['models/A.kts', 'models/B.kt']` for `models.getThing` — it splits the
|
||||
* package into `:kt` and `:kts` buckets instead of returning both in set
|
||||
* order.
|
||||
*
|
||||
* Each divergence is an edge that would move in every Kotlin repository, so
|
||||
* A fourth probe — `['data/src/…/data/Repo.kt']` for `data.getRepo`, where the
|
||||
* shared index fanned out and this one returned null — stopped diverging in
|
||||
* #2881, which removed the first-occurrence rule that caused it. The remaining
|
||||
* three are still edges that would move in every Kotlin repository, so
|
||||
* consolidating the two is a behaviour change, not a cleanup.
|
||||
*
|
||||
* `dirChildren` is likewise NOT the shared `import-resolvers/package-dir-index.ts`
|
||||
* — the consolidation a maintainer will actually propose, since Java routes
|
||||
* through exactly it (`languages/java/import-target.ts`). Measured, that swap is
|
||||
* output-identical for 26.2% less retained memory, and costs 8114x on
|
||||
* `import data.*` at 200 matching directories. `bench/kotlin-import-target`
|
||||
* CANNOT see that regression — its corpus gives every module a unique package
|
||||
* leaf — and the arm that can is `bench/import-target`'s kotlin `collide`. See
|
||||
* `_blind_spot` in `bench/kotlin-import-target/baselines.json`.
|
||||
*
|
||||
* `dirChildren`'s bucket rule, and what #2881 removed
|
||||
* ---------------------------------------------------
|
||||
* A file is a child of its own directory and of every component-suffix of that
|
||||
* directory, unguarded. The suffix half used to carry two guards inherited from
|
||||
* the pre-index per-import scan rather than from anything Kotlin requires (the
|
||||
* same generation of code put the `indexOf` half into
|
||||
* `import-resolvers/package-dir-index.ts` and `import-resolvers/csharp.ts`,
|
||||
* where it was removed under the same issue): `startsWith(s + '/')` skipped the
|
||||
* bucket outright, and an `indexOf` equality demanded that the parent be the
|
||||
* FIRST `/s/` in the path. Between them they dropped the bucket whenever the
|
||||
* package name repeated higher up the tree, so
|
||||
* `data/src/main/kotlin/com/example/data/Repo.kt` was not a child of `data`
|
||||
* (leading segment, `startsWith`) and neither was `top/data/mid/data/Repo.kt`
|
||||
* (mid-path, `indexOf`). `import data.helper` resolved to null in both. Only
|
||||
* the fan-out tier looked affected — `data.Repo` answers from `suffixByStem`,
|
||||
* which never had such a guard — which is why the shape looked narrow enough to
|
||||
* preserve.
|
||||
*
|
||||
* Nothing downstream narrows a widened bucket back at the FILE level. The
|
||||
* `localDefs` filter of #1759 constrains `targetDefId`/`BindingRef` ONLY: the
|
||||
* finalize pass mints one draft PER CANDIDATE, each keeping its own
|
||||
* `targetFile` (`gitnexus-shared/src/scope-resolution/finalize-algorithm.ts`),
|
||||
* and the one File→File filter downstream
|
||||
* (`scope-resolution/graph-bridge/imports-to-edges.ts`) tests `targetFile`
|
||||
* against `null` and against the source file, never reads `linkStatus`, and
|
||||
* emits `IMPORTS` at confidence 1.0. So every extra bucket member becomes an
|
||||
* unconditional File→File `IMPORTS` edge: one `import data.load` on an
|
||||
* Android-style layout measured 5 → 6 edges, the added one `unresolved`. That
|
||||
* is a real cost, paid deliberately — a MISSING bucket is unrecoverable, and
|
||||
* there is no version of the bucket that is right for one consumer and wrong
|
||||
* for the other. Narrowing the File→File side, if it is ever wanted, is a
|
||||
* downstream filter and a separate change.
|
||||
*
|
||||
* What moved, over the corpus: of the 235 records the published census counted,
|
||||
* 149 are a different first child, 32 are a wider fan-out array, and 54 are
|
||||
* null → resolved — none of which turns a bound answer into an unbound one.
|
||||
* That taxonomy has no bucket for a fourth outcome class this change
|
||||
* introduces, and did not count it. Tier 3
|
||||
* (`findKotlinPackageFiles`) precedes tier 4 (`findByProgressivePrefixStrip`),
|
||||
* so a bucket the guards used to leave empty returned null and let tier 4 run;
|
||||
* a now-populated bucket stops tier 4 from running at all, which turns a
|
||||
* resolved answer into an unresolved one and a `string` into an array:
|
||||
*
|
||||
* ['data/src/main/kotlin/com/example/data/Repo.kt', 'common/helper.kt']
|
||||
* with `import data.helper`
|
||||
* before → 'common/helper.kt' (bound)
|
||||
* after → ['data/src/main/kotlin/com/example/data/Repo.kt'] (no `helper`)
|
||||
*
|
||||
* Over the census corpus that class is ZERO records — and the zero is the
|
||||
* point, not a reprieve. The shape above is real and reproduces by hand in
|
||||
* both iteration orders; running `bench/kotlin-import-target`'s own generator
|
||||
* at 10x (4000 repositories, ~198 600 distinct records) hits it 4-12 times per
|
||||
* seed, i.e. an expectation of about ONE over this corpus's 19 968. So the
|
||||
* fingerprint does not gate this class: it is the same blindness the go arm had
|
||||
* before #2881 widened its corpus — a gate cannot catch a shape its corpus
|
||||
* cannot express. Adding a case is a deliberate fingerprint move and belongs in
|
||||
* its own change, with the re-baseline that implies.
|
||||
*/
|
||||
interface KotlinFileIndex {
|
||||
readonly exactByStem: Map<string, string>;
|
||||
|
|
@ -187,7 +256,44 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
|
|||
|
||||
const exactByStem = new Map<string, string>();
|
||||
const suffixByStem = new Map<string, string>();
|
||||
const dirChildren: MutableDirChildren = new Map();
|
||||
const dirChildren = new Map<string, string[]>();
|
||||
/**
|
||||
* BUILD-LOCAL: `dir` -> every `dirChildren` key a file in that directory
|
||||
* contributes to. That list is a pure function of `dir`, and a package
|
||||
* directory holds many files, so without this the walk below cuts one `slice`
|
||||
* per component of the SAME directory once per FILE — and every slice after
|
||||
* the first file's is a freshly allocated string that hashes to a key the map
|
||||
* already holds and is then dropped. Interning them once per DIRECTORY
|
||||
* instead of once per FILE is ~21% of the build at 32 000 files.
|
||||
*
|
||||
* It cannot move an answer. The array is filled on the first file of a
|
||||
* directory, in the order the per-file walk produced, and every later file in
|
||||
* that directory finds those keys already present — so the key set, the Map's
|
||||
* key insertion order and every bucket's order are what the per-file form
|
||||
* produced. `kotlin-index-internals.test.ts` pins the part of that a consumer
|
||||
* can observe, and does it through the resolver's own surface rather than over
|
||||
* the built maps: bucket CONTENTS and ORDER (from the fan-out tier, which
|
||||
* hands out the bucket array itself), bucket IDENTITY across calls, and that
|
||||
* the array handed out is FROZEN. Be precise about the limits, because the
|
||||
* mutation matrix in that file's header measured them: a MIS-KEYED memo is
|
||||
* caught, a DELETED one is not — the memo is output-identical by construction,
|
||||
* so nothing observable can prove it ran. Likewise `Object.isFrozen` catches a
|
||||
* missing freeze and a compacted-but-never-stored copy, but NOT a deleted
|
||||
* `slice()`: a JS array's backing-store capacity has no reflective surface, so
|
||||
* the compaction's only instrument is `heap_ceiling_bytes.kotlin` in
|
||||
* `bench/import-target/baselines.json` — a CEILING, because compaction
|
||||
* reclaims, so losing it makes the retained reading grow (+12.57% measured).
|
||||
* Map key insertion ORDER is
|
||||
* unasserted BY DESIGN:
|
||||
* `dirChildren` is only ever read by `.get(key)`, so key order has no
|
||||
* consumer and pinning it would assert an implementation detail nothing
|
||||
* depends on. Nothing else watches it either — the correctness fingerprint
|
||||
* sees this index only through the four tiers, so no fingerprint could catch
|
||||
* a key-order move.
|
||||
*
|
||||
* Dropped with this frame, so it costs nothing retained.
|
||||
*/
|
||||
const dirKeys = new Map<string, string[]>();
|
||||
|
||||
for (const raw of allFilePaths) {
|
||||
const norm = raw.replace(/\\/g, '/');
|
||||
|
|
@ -206,60 +312,62 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
|
|||
if (!suffixByStem.has(suffix)) suffixByStem.set(suffix, raw);
|
||||
}
|
||||
|
||||
const lastSlash = norm.lastIndexOf('/');
|
||||
// From `stem`, not `norm`: an extension carries no '/', so the last '/' of
|
||||
// the two is the same character at the same index, and `stem.slice(0,
|
||||
// lastSlash)` IS the string `norm.slice(0, norm.lastIndexOf('/'))` was. One
|
||||
// backwards scan instead of two, over the string this loop already walked.
|
||||
const lastSlash = stem.lastIndexOf('/');
|
||||
if (lastSlash < 0) continue; // repo-root file has no package directory
|
||||
const dir = norm.slice(0, lastSlash);
|
||||
const dir = stem.slice(0, lastSlash);
|
||||
|
||||
// The file's own directory always qualifies: the old scan's `atRoot` branch
|
||||
// matched `norm.startsWith(dir + '/')` and found no '/' after it.
|
||||
addChild(dirChildren, dir, raw);
|
||||
|
||||
// A component-suffix of the directory also qualifies — but only under the
|
||||
// rule the scan actually implemented, which is narrower than "the parent
|
||||
// directory is named `s`":
|
||||
//
|
||||
// - `atRoot` was tested FIRST, so if the path *starts* with `s + '/'` the
|
||||
// scan used index 0 and the remainder still contained '/', i.e. no
|
||||
// match — even when a later directory is also named `s`.
|
||||
// - otherwise it used `indexOf`, the FIRST occurrence of `/s/`. A path
|
||||
// like `data/src/main/kotlin/com/example/data/Repo.kt` therefore does
|
||||
// NOT count as a child of `data`: the first `/data/` is not the parent,
|
||||
// and the scan never looked for a second one.
|
||||
//
|
||||
// Preserving that exactly keeps this a pure performance change. It is
|
||||
// arguably a bug — the file IS a direct child of a `data` directory — but
|
||||
// fixing it here would silently move edges in every Kotlin repository,
|
||||
// which belongs in its own change with its own fixtures.
|
||||
for (let i = 0; i < dir.length; i++) {
|
||||
if (dir[i] !== '/') continue;
|
||||
const suffix = dir.slice(i + 1);
|
||||
if (norm.startsWith(`${suffix}/`)) continue;
|
||||
if (norm.indexOf(`/${suffix}/`) === dir.length - suffix.length - 1) {
|
||||
addChild(dirChildren, suffix, raw);
|
||||
// The keys this file's directory contributes to, unguarded: `dir` itself,
|
||||
// plus every component-suffix of it. A suffix `s` starts just after a '/',
|
||||
// so `dir` ends with `/s` by construction and the file IS a direct child of
|
||||
// a directory named `s`. The absence of a narrowing guard is deliberate —
|
||||
// see the `dirChildren` section on `KotlinFileIndex` for the two guards
|
||||
// #2881 dropped and for what the resulting width costs downstream.
|
||||
let keys = dirKeys.get(dir);
|
||||
if (keys === undefined) {
|
||||
keys = [dir];
|
||||
for (let i = 0; i < lastSlash; i++) {
|
||||
if (dir[i] === '/') keys.push(dir.slice(i + 1));
|
||||
}
|
||||
dirKeys.set(dir, keys);
|
||||
}
|
||||
for (const key of keys) {
|
||||
const bucket = dirChildren.get(key);
|
||||
if (bucket === undefined) dirChildren.set(key, [raw]);
|
||||
else bucket.push(raw);
|
||||
}
|
||||
}
|
||||
|
||||
// Buckets are mutable only while this function runs; the index type hands
|
||||
// them out `readonly` and they are frozen here, before it is cached.
|
||||
// `findKotlinPackageFiles` hands a bucket straight out of the index — the
|
||||
// same array `findKotlinDirectoryChild` reads `children[0]` from. The
|
||||
// `readonly string[]` return type does not survive the caller: the finalize
|
||||
// pass normalizes with `Array.isArray(t) ? t : [t]`, and `isArray`'s
|
||||
// `arg is any[]` predicate widens the true branch, so `tsc --strict` accepts
|
||||
// a `.sort()` or `.push()` there. A downstream sort would permanently
|
||||
// reorder the cached bucket and flip the FIRST-child tier's answer for every
|
||||
// later import in the run. Freezing makes the contract true at runtime, so a
|
||||
// future mutation is a loud TypeError instead of a silent edge move.
|
||||
for (const bucket of dirChildren.values()) Object.freeze(bucket);
|
||||
// same array `findKotlinDirectoryChild` reads `children[0]` from — so a
|
||||
// downstream sort would permanently reorder the cached bucket and flip the
|
||||
// FIRST-child tier's answer for every later import in the run. The finalize
|
||||
// pass normalizes with `Array.isArray(t) ? t : [t]` and `isArray`'s
|
||||
// `arg is any[]` predicate widens the true branch; that one call site now
|
||||
// carries an explicit `readonly string[]` annotation, but the annotation is
|
||||
// one deletion away and covers only that site. Freezing makes the contract
|
||||
// true at runtime, so a future mutation is a loud TypeError, not a silent
|
||||
// edge move.
|
||||
//
|
||||
// COMPACTED as they are frozen: buckets grow by `push`, so V8's growth
|
||||
// overshoot stays retained for the life of the index. `length === 1` never
|
||||
// grew and is skipped — slicing it saves zero bytes and costs 31% of the
|
||||
// build on a corpus of single-file packages. The byte accounting lives once,
|
||||
// in `bench/import-target/baselines.json`.
|
||||
for (const [key, bucket] of dirChildren) {
|
||||
if (bucket.length === 1) {
|
||||
Object.freeze(bucket);
|
||||
continue;
|
||||
}
|
||||
const compacted = bucket.slice();
|
||||
Object.freeze(compacted);
|
||||
dirChildren.set(key, compacted);
|
||||
}
|
||||
|
||||
return { exactByStem, suffixByStem, dirChildren };
|
||||
});
|
||||
|
||||
function addChild(dirChildren: Map<string, string[]>, dir: string, raw: string): void {
|
||||
const bucket = dirChildren.get(dir);
|
||||
if (bucket === undefined) dirChildren.set(dir, [raw]);
|
||||
else bucket.push(raw);
|
||||
}
|
||||
|
||||
/** Mutable view of the buckets, used only while building — the index exposes
|
||||
* them as `readonly` and freezes them before it is cached. */
|
||||
type MutableDirChildren = Map<string, string[]>;
|
||||
|
|
|
|||
|
|
@ -12,7 +12,10 @@
|
|||
* That is wrong, and this file is the proof. Step 2 filters
|
||||
* `index.getFilesInDir(dirPrefix, '.cs')`, whose buckets are keyed on
|
||||
* SEGMENT-aligned directory suffixes; step 3 runs an UNANCHORED
|
||||
* `normalized.indexOf(dirPrefix + '/')`. Step 3 therefore answers strictly more:
|
||||
* `normalized.lastIndexOf(dirPrefix + '/')` (`indexOf` before #2881, which is
|
||||
* the first-occurrence rule that issue removed; the empty-prefix case still
|
||||
* 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);
|
||||
|
|
@ -58,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[],
|
||||
|
|
@ -99,13 +122,17 @@ function legacyResolveCSharpImportInternal(
|
|||
if (suffixResult) return [suffixResult];
|
||||
}
|
||||
|
||||
// Shared by steps 2 and 3 — the same needle, and since #2881 the same
|
||||
// `indexOf`-only-when-empty rule, so it is declared once rather than
|
||||
// re-derived per step.
|
||||
const dirTrail = dirPrefix + '/';
|
||||
|
||||
// 2. Try as directory: all .cs files directly inside (namespace import)
|
||||
if (index) {
|
||||
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
|
||||
for (const f of dirFiles) {
|
||||
const normalized = f.replace(/\\/g, '/');
|
||||
// Check it's a direct child by finding the dirPrefix and ensuring no deeper slashes
|
||||
const prefixIdx = normalized.indexOf(dirPrefix + '/');
|
||||
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -117,11 +144,10 @@ function legacyResolveCSharpImportInternal(
|
|||
|
||||
// 3. Linear scan fallback for directory matching
|
||||
if (results.length === 0) {
|
||||
const dirTrail = dirPrefix + '/';
|
||||
for (let i = 0; i < normalizedFileList.length; i++) {
|
||||
const normalized = normalizedFileList[i];
|
||||
if (!normalized.endsWith('.cs')) continue;
|
||||
const prefixIdx = normalized.indexOf(dirTrail);
|
||||
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirTrail.length);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -185,11 +211,13 @@ function skipStep3WhenIndexed(
|
|||
if (suffixResult) return [suffixResult];
|
||||
}
|
||||
|
||||
const dirTrail = dirPrefix + '/';
|
||||
|
||||
if (index) {
|
||||
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
|
||||
for (const f of dirFiles) {
|
||||
const normalized = f.replace(/\\/g, '/');
|
||||
const prefixIdx = normalized.indexOf(dirPrefix + '/');
|
||||
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -200,11 +228,10 @@ function skipStep3WhenIndexed(
|
|||
continue;
|
||||
}
|
||||
|
||||
const dirTrail = dirPrefix + '/';
|
||||
for (let i = 0; i < normalizedFileList.length; i++) {
|
||||
const normalized = normalizedFileList[i];
|
||||
if (!normalized.endsWith('.cs')) continue;
|
||||
const prefixIdx = normalized.indexOf(dirTrail);
|
||||
const prefixIdx = directChildIdx(normalized, dirPrefix, dirTrail);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirTrail.length);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -252,8 +279,10 @@ const RAW_FILES: readonly string[] = [
|
|||
// Character suffix across a segment boundary: answers `rc/Models`.
|
||||
'vendor/mysrc/Models/Vendored.cs',
|
||||
'src/Models/Late.cs',
|
||||
// `Models` nested inside `Models`: the FIRST `indexOf` occurrence is the
|
||||
// outer one, whose remainder still holds a slash, so this answers nothing.
|
||||
// `Models` nested inside `Models`. Answered nothing until #2881, because the
|
||||
// FIRST `indexOf` occurrence was the outer one and its remainder still held a
|
||||
// slash; the predicate now asks whether the file's DIRECTORY ends with the
|
||||
// prefix, which the inner `Models` satisfies.
|
||||
'nest/Models/inner/Models/Ignored.cs',
|
||||
// Single-segment directory, so it answers the empty `dirPrefix`.
|
||||
'Models/TopLevel.cs',
|
||||
|
|
@ -483,9 +512,14 @@ describe('C# csproj leg — the answers only step 3 can give (#2902)', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
it('keeps the FIRST-occurrence tie-break: a directory nested inside a same-named one loses', () => {
|
||||
// `nest/Models/inner/Models/Ignored.cs` is absent: `indexOf('odels/')` finds
|
||||
// the outer `Models/`, and `inner/Models/Ignored.cs` still has a slash.
|
||||
it('a directory nested inside a same-named one now answers too (#2881)', () => {
|
||||
// `nest/Models/inner/Models/Ignored.cs` used to be absent: `indexOf('odels/')`
|
||||
// found the OUTER `Models/`, and `inner/Models/Ignored.cs` still had a
|
||||
// slash. The predicate is now "the file's directory ends with the prefix",
|
||||
// which the inner `Models` satisfies. Note this arm queries `App.odels` —
|
||||
// the UNANCHORED half — so it also pins that removing the first-occurrence
|
||||
// rule did not accidentally anchor the match to a segment boundary:
|
||||
// `src/SubModels/Widget.cs` is still here.
|
||||
expect(withIndex([{ rootNamespace: 'App', projectDir: '' }], 'App.odels')).toEqual([
|
||||
'src/Models/User.cs',
|
||||
'src/Models/Order.cs',
|
||||
|
|
@ -493,6 +527,7 @@ describe('C# csproj leg — the answers only step 3 can give (#2902)', () => {
|
|||
'other/Models/Thing.cs',
|
||||
'vendor/mysrc/Models/Vendored.cs',
|
||||
'src/Models/Late.cs',
|
||||
'nest/Models/inner/Models/Ignored.cs',
|
||||
'Models/TopLevel.cs',
|
||||
'win\\Models\\Win.cs',
|
||||
]);
|
||||
|
|
@ -506,15 +541,19 @@ describe('C# csproj leg — the answers only step 3 can give (#2902)', () => {
|
|||
|
||||
it('a leading-slash dirPrefix cannot bogus-match a shorter directory', () => {
|
||||
// `dirPrefix = '/Models'` (projectDir used verbatim, since the import IS
|
||||
// the root namespace): `'Models/'` is SHORTER than `'/Models/'`, and both
|
||||
// `indexOf` and `haystack.length - needle.length` come out -1 without a
|
||||
// length guard, so `Models/TopLevel.cs` would join the answer.
|
||||
// the root namespace): `'Models/'` is SHORTER than `'/Models/'`, so
|
||||
// `Models/TopLevel.cs` must not join the answer. The `indexOf` form needed
|
||||
// an explicit length guard for this, because `indexOf` and
|
||||
// `haystack.length - needle.length` both came out -1; `endsWith` is simply
|
||||
// false on a shorter haystack, so the property now holds without one, and
|
||||
// this case is what proves the guard's removal was safe.
|
||||
expect(withIndex([{ rootNamespace: 'App', projectDir: '/Models' }], 'App')).toEqual([
|
||||
'src/Models/User.cs',
|
||||
'src/Models/Order.cs',
|
||||
'other/Models/Thing.cs',
|
||||
'vendor/mysrc/Models/Vendored.cs',
|
||||
'src/Models/Late.cs',
|
||||
'nest/Models/inner/Models/Ignored.cs',
|
||||
'win\\Models\\Win.cs',
|
||||
]);
|
||||
});
|
||||
|
|
@ -618,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',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
164
gitnexus/test/unit/import-resolvers/go-package-resolve.test.ts
Normal file
164
gitnexus/test/unit/import-resolvers/go-package-resolve.test.ts
Normal file
|
|
@ -0,0 +1,164 @@
|
|||
/**
|
||||
* Coverage for `resolveGoPackage` (`import-resolvers/go.ts`), which had none.
|
||||
*
|
||||
* Go resolves package imports through two independent legs. The ScopeResolver
|
||||
* leg (`languages/go/import-target.ts`) answers from `buildPackageDirIndex`;
|
||||
* this one is the LanguageProvider leg, wired through `configs/go.ts`, and it
|
||||
* is still a per-import scan. #2881 changed its membership rule — a directory
|
||||
* whose name repeats higher in the path is now a member — and review found the
|
||||
* change reached production with nothing watching it: `bench/import-target`'s
|
||||
* go arm drives the indexed leg only, and no test called this function.
|
||||
*
|
||||
* These cases pin the rule and the two legs' agreement on it, so a revert fails
|
||||
* 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: 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, 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);
|
||||
// `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', () => {
|
||||
it('returns every .go file directly inside the package directory', () => {
|
||||
const files = ['internal/auth/service.go', 'internal/auth/token.go', 'internal/auth/sub/x.go'];
|
||||
expect(resolve(files, 'example.com/mod/internal/auth')).toEqual([
|
||||
'internal/auth/service.go',
|
||||
'internal/auth/token.go',
|
||||
]);
|
||||
});
|
||||
|
||||
it('a package directory nested inside a same-named one IS a member (#2881)', () => {
|
||||
// The scan asked `indexOf` for the FIRST `/pkg/` and then required nothing
|
||||
// after it to hold a slash, so this resolved to nothing. Both halves of the
|
||||
// shape: the repeat leading the path, and the repeat mid-path.
|
||||
expect(resolve(['pkg/src/go/pkg/repo.go'], 'example.com/mod/pkg')).toEqual([
|
||||
'pkg/src/go/pkg/repo.go',
|
||||
]);
|
||||
expect(resolve(['a/pkg/b/pkg/x.go'], 'example.com/mod/pkg')).toEqual(['a/pkg/b/pkg/x.go']);
|
||||
expect(resolve(['svc/internal/sub/internal/x.go'], 'example.com/mod/internal')).toEqual([
|
||||
'svc/internal/sub/internal/x.go',
|
||||
]);
|
||||
});
|
||||
|
||||
it('a repeated name that is not the parent directory is still not a member', () => {
|
||||
// The rule is "the parent directory ends with the package path", not
|
||||
// "the package path appears anywhere".
|
||||
expect(resolve(['a/pkg/b/x.go'], 'example.com/mod/pkg')).toEqual([]);
|
||||
expect(resolve(['internal/auth/sub/x.go'], 'example.com/mod/internal/auth')).toEqual([]);
|
||||
});
|
||||
|
||||
it('multi-segment package paths match on the whole run, not the last segment', () => {
|
||||
const files = ['a/internal/models/b/internal/models/user.go', 'a/models/other.go'];
|
||||
expect(resolve(files, 'example.com/mod/internal/models')).toEqual([
|
||||
'a/internal/models/b/internal/models/user.go',
|
||||
]);
|
||||
});
|
||||
|
||||
it('_test.go files are a different package and never match', () => {
|
||||
expect(
|
||||
resolve(
|
||||
['internal/auth/service.go', 'internal/auth/service_test.go'],
|
||||
'example.com/mod/internal/auth',
|
||||
),
|
||||
).toEqual(['internal/auth/service.go']);
|
||||
});
|
||||
|
||||
it('non-.go files never match, and the RAW path is returned for backslashes', () => {
|
||||
expect(
|
||||
resolve(['internal/auth/README.md', 'internal/auth/x.go'], 'example.com/mod/internal/auth'),
|
||||
).toEqual(['internal/auth/x.go']);
|
||||
expect(resolve(['internal\\auth\\x.go'], 'example.com/mod/internal/auth')).toEqual([
|
||||
'internal\\auth\\x.go',
|
||||
]);
|
||||
});
|
||||
|
||||
it('an import outside the module, or the module root itself, resolves to nothing here', () => {
|
||||
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)).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', () => {
|
||||
// The two legs are independent implementations of one rule. Before #2881
|
||||
// they agreed on the wrong answer; they must agree on the right one, or
|
||||
// Go's LanguageProvider and ScopeResolver hooks disagree about which files
|
||||
// a package holds.
|
||||
for (const files of [
|
||||
['pkg/src/go/pkg/repo.go'],
|
||||
['a/pkg/b/pkg/x.go'],
|
||||
['a/pkg/b/x.go'],
|
||||
['internal/auth/service.go', 'internal/auth/token.go'],
|
||||
['a/internal/models/b/internal/models/user.go'],
|
||||
]) {
|
||||
for (const target of [
|
||||
'example.com/mod/pkg',
|
||||
'example.com/mod/internal/auth',
|
||||
'example.com/mod/internal/models',
|
||||
]) {
|
||||
expect(resolve(files, target)).toEqual([...indexed(files, target)]);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -10,19 +10,26 @@
|
|||
* through anything the type system or the existing tests can see:
|
||||
*
|
||||
* - Go sorts the root-package leg and does NOT sort the package-dir leg;
|
||||
* - Go and C# both take the FIRST occurrence of `/<segment>/` in the path, so
|
||||
* a directory nested inside a same-named directory does not match;
|
||||
* - Go and C# answer when the file's PARENT directory ends with the queried
|
||||
* segment. Both took the FIRST occurrence of `/<segment>/` until #2881, so a
|
||||
* directory nested inside a same-named one did not match;
|
||||
* - C#'s `resolveDirectMatch` lets a whole-path match win over a suffix match
|
||||
* found EARLIER in iteration order, while `resolveByProgressiveStripping`
|
||||
* takes whichever comes first;
|
||||
* - Dart tries `lib/<rel>` fully before bare `<rel>`, and compares raw paths
|
||||
* (no backslash normalization) on both legs.
|
||||
*
|
||||
* So this file keeps verbatim copies of the pre-change implementations and
|
||||
* asserts the new ones agree with them on a deterministic corpus built to force
|
||||
* exactly those cases. The copies are the specification; if a future change
|
||||
* makes one of these fail, the resolver's OUTPUT moved and the graph's edges
|
||||
* move with it.
|
||||
* So this file keeps copies of the pre-change implementations and asserts the
|
||||
* new ones agree with them on a deterministic corpus built to force exactly
|
||||
* those cases. The copies are the specification; if a future change makes one of
|
||||
* these fail, the resolver's OUTPUT moved and the graph's edges move with it.
|
||||
*
|
||||
* They were verbatim until #2881, which deliberately changed one rule and so had
|
||||
* to edit them too. Read them now as an independent re-derivation of the CURRENT
|
||||
* spec, not as a frozen record of what shipped before the hoist — a weaker claim,
|
||||
* and the reason the hand-built arm below pins ABSOLUTE expectations as well as
|
||||
* differential ones: a differential where both sides were edited together proves
|
||||
* only self-consistency.
|
||||
*
|
||||
* The second half asserts the index is built once per file set rather than once
|
||||
* per import, by counting how often the Set is iterated. It is the DETERMINISTIC
|
||||
|
|
@ -57,7 +64,7 @@ import { csharpSuffixFallbackAllowed } from '../../../src/core/ingestion/csharp-
|
|||
import { DART_HERITAGE_PREFIX } from '../../../src/core/ingestion/languages/dart/interpret.js';
|
||||
import { CountingSet } from '../../helpers/counting-file-set.js';
|
||||
|
||||
// ─── verbatim pre-change implementations ─────────────────────────────────────
|
||||
// ─── pre-change implementations, minus the rule #2881 removed ────────────────
|
||||
|
||||
function legacyFindRootPackageFiles(allFilePaths: ReadonlySet<string>): string[] {
|
||||
const result: string[] = [];
|
||||
|
|
@ -77,7 +84,10 @@ function legacyFindAllFilesInPkgDir(allFilePaths: ReadonlySet<string>, pkgPath:
|
|||
const normalized = '/' + raw.replace(/\\/g, '/');
|
||||
if (!normalized.includes(pkgDir)) continue;
|
||||
if (!normalized.endsWith('.go') || normalized.endsWith('_test.go')) continue;
|
||||
const afterPkg = normalized.substring(normalized.indexOf(pkgDir) + pkgDir.length);
|
||||
// `lastIndexOf` since #2881: `pkgDir` is '/'-anchored on both sides, so the
|
||||
// LAST occurrence is the file's own parent. `indexOf` asked for the first,
|
||||
// which made `a/pkg/b/pkg/x.go` not a member of `pkg`.
|
||||
const afterPkg = normalized.substring(normalized.lastIndexOf(pkgDir) + pkgDir.length);
|
||||
if (!afterPkg.includes('/')) result.push(raw);
|
||||
}
|
||||
return result;
|
||||
|
|
@ -203,17 +213,19 @@ function legacyFindDirectChild(
|
|||
allFilePaths: ReadonlySet<string>,
|
||||
dirSegment: string,
|
||||
): string | null {
|
||||
const dirPrefix = `${dirSegment}/`;
|
||||
const nestedDirPrefix = `/${dirPrefix}`;
|
||||
// Since #2881 this is plain "the file's parent directory ends with
|
||||
// `dirSegment`". The `atRoot`-then-`indexOf` pair it replaces expressed the
|
||||
// same thing PLUS "…and that occurrence is the first", which is the half that
|
||||
// was removed; the segment anchoring the leading '/' provided is kept by
|
||||
// testing `'/' + dir + '/'` against `'/' + dirSegment + '/'`.
|
||||
const needle = `/${dirSegment}/`;
|
||||
for (const raw of allFilePaths) {
|
||||
const f = raw.replace(/\\/g, '/');
|
||||
if (!f.endsWith('.cs')) continue;
|
||||
const atRoot = f.startsWith(dirPrefix);
|
||||
const atNested = f.includes(nestedDirPrefix);
|
||||
if (!atRoot && !atNested) continue;
|
||||
const idx = atRoot ? 0 : f.indexOf(nestedDirPrefix) + 1;
|
||||
const after = f.slice(idx + dirPrefix.length);
|
||||
if (after.length > 0 && !after.includes('/')) return raw;
|
||||
const lastSlash = f.lastIndexOf('/');
|
||||
if (lastSlash < 0) continue;
|
||||
if (!`/${f.slice(0, lastSlash)}/`.endsWith(needle)) continue;
|
||||
return raw;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
|
@ -298,11 +310,17 @@ function mix(n: number): number {
|
|||
}
|
||||
|
||||
/**
|
||||
* Directory shapes, chosen so the corpus contains every case where the naive
|
||||
* "does the dir end with the segment" rewrite diverges from the original
|
||||
* first-`indexOf` predicate: a directory name nested inside itself
|
||||
* Directory shapes, chosen so the corpus contains every case where the two
|
||||
* candidate predicates disagree: a directory name nested inside itself
|
||||
* (`pkg/pkg`, `a/pkg/b/pkg`), the same leaf under several parents (collision
|
||||
* tie-breaks), an absolute-rooted layout, and the repo root.
|
||||
*
|
||||
* The nested shapes were originally here to prove the "does the dir end with
|
||||
* the segment" rewrite was NOT safe, because the shipped predicate additionally
|
||||
* required the first `indexOf` occurrence. #2881 removed that requirement and
|
||||
* made the ends-with form the shipped one, so these shapes now pin the removal
|
||||
* instead — same shapes, opposite verdict, and still the only ones that can
|
||||
* tell the two apart.
|
||||
*/
|
||||
const DIRS = [
|
||||
'',
|
||||
|
|
@ -526,7 +544,7 @@ describe('import-target index hoist — output parity with the pre-change scans'
|
|||
},
|
||||
{
|
||||
lang: 'csharp',
|
||||
why: 'a namespace dir nested inside itself does not answer the query',
|
||||
why: 'a namespace dir nested inside itself DOES answer the query (#2881)',
|
||||
files: ['Models/Models/User.cs'],
|
||||
target: 'Models',
|
||||
},
|
||||
|
|
@ -570,15 +588,17 @@ describe('import-target index hoist — output parity with the pre-change scans'
|
|||
},
|
||||
{
|
||||
lang: 'go',
|
||||
why: 'a package dir nested inside itself does not answer the query',
|
||||
why: 'a package dir nested inside itself DOES answer the query (#2881)',
|
||||
files: ['a/pkg/b/pkg/x.go'],
|
||||
// Addressed through the MODULE leg as the single segment `pkg`, not as
|
||||
// `a/pkg`. `a/pkg` never reached the first-occurrence branch this case is
|
||||
// named for: `'/a/pkg/b/pkg/'.endsWith('/a/pkg/')` is already false, so
|
||||
// the naive `endsWith` rewrite agreed with the real predicate and the
|
||||
// case passed either way. With `pkg`, `endsWith('/pkg/')` is TRUE and only
|
||||
// the "…and that occurrence is the FIRST" half rejects it. The module leg
|
||||
// is required because the GOPATH cascade skips single-segment targets.
|
||||
// `a/pkg`. `a/pkg` never reached the first-occurrence branch this case
|
||||
// was named for: `'/a/pkg/b/pkg/'.endsWith('/a/pkg/')` is already false,
|
||||
// so the `endsWith` form agreed with the old predicate and the case
|
||||
// passed either way. With `pkg`, `endsWith('/pkg/')` is TRUE and ONLY the
|
||||
// "…and that occurrence is the FIRST" half rejected it — which is exactly
|
||||
// why this case is the one that flips, and why it is still the case that
|
||||
// tells the two predicates apart. The module leg is required because the
|
||||
// GOPATH cascade skips single-segment targets.
|
||||
target: 'example.com/mod/pkg',
|
||||
modulePath: 'example.com/mod',
|
||||
},
|
||||
|
|
@ -667,10 +687,11 @@ describe('import-target index hoist — output parity with the pre-change scans'
|
|||
|
||||
it('every hand-built layout resolves to something (they pin a winner, not a null)', () => {
|
||||
// `toEqual(null) === toEqual(null)` would make the arm above pass for the
|
||||
// wrong reason. Only the three "must NOT match" layouts may be null.
|
||||
// wrong reason. Only the "must NOT match" layouts may be null. The two
|
||||
// nested-inside-itself layouts left this set in #2881: they now resolve, so
|
||||
// they are held to the same "pin a winner" bar as everything else, which is
|
||||
// a stronger assertion than the null they used to carry.
|
||||
const mustBeNull = new Set([
|
||||
'a namespace dir nested inside itself does not answer the query',
|
||||
'a package dir nested inside itself does not answer the query',
|
||||
'_test.go files are a different package and never match',
|
||||
'paths are matched RAW — a backslash path is not normalized into a hit',
|
||||
]);
|
||||
|
|
@ -713,9 +734,14 @@ describe('import-target index hoist — output parity with the pre-change scans'
|
|||
if (csharp(t, cs) !== null) hits.csharp++;
|
||||
}
|
||||
}
|
||||
// Measured on this corpus: go 364, dart 75, ruby 259, csharp 196. Ruby and
|
||||
// Measured on this corpus: go 366, dart 75, ruby 259, csharp 220. Ruby and
|
||||
// C# gained 40 each from the `win\dir\thing.<ext>` targets — one per repo,
|
||||
// which is also the floor those two arms now defend.
|
||||
// which is also the floor those two arms now defend. #2881 moved go 364 ->
|
||||
// 366 and csharp 196 -> 220, from the corpus's `pkg/pkg`, `a/pkg/b/pkg` and
|
||||
// `Models/Models` directories: those now answer their own name. The floors
|
||||
// are deliberately NOT raised to lock that in — they exist to catch an arm
|
||||
// that stopped resolving at all, and a revert of #2881 is caught precisely
|
||||
// by the differential arms above, which compare against the real resolver.
|
||||
expect(hits.go).toBeGreaterThan(300);
|
||||
expect(hits.dart).toBeGreaterThan(60);
|
||||
expect(hits.ruby).toBeGreaterThan(220);
|
||||
|
|
|
|||
|
|
@ -17,13 +17,17 @@
|
|||
* — while its directory child is collected and returned only after the scan
|
||||
* completes, so file/suffix beats directory child within one `skip` level
|
||||
* regardless of order;
|
||||
* - the directory-child leg takes the FIRST `'/' + pathLike + '/'` occurrence,
|
||||
* so `com/example/com/example/Deep.java` does NOT answer `com.example`;
|
||||
* - the directory-child leg answers when the file's PARENT directory ends
|
||||
* with `pathLike`, so `com/example/com/example/Deep.java` DOES answer
|
||||
* `com.example`. It took the FIRST `'/' + pathLike + '/'` occurrence until
|
||||
* #2881, which made a package whose name repeats higher in its own path
|
||||
* unresolvable;
|
||||
* - a wildcard import drops its trailing `.*` before any of that runs;
|
||||
* - paths are compared normalized (`\` → `/`) but returned RAW.
|
||||
*
|
||||
* So this file keeps a VERBATIM copy of the pre-change implementation — the
|
||||
* `resolveJavaImportTarget` that shipped before #2908, scans and all — and
|
||||
* So this file keeps a copy of the pre-change implementation — the
|
||||
* `resolveJavaImportTarget` that shipped before #2908, scans and all, minus the
|
||||
* one rule #2881 deliberately removed (see tie-break 3) — and
|
||||
* asserts the new one agrees with it, both on hand-built corpora built to force
|
||||
* exactly those cases and on a generated corpus replayed under three insertion
|
||||
* orders — order being the only channel most of these tie-breaks travel on.
|
||||
|
|
@ -46,7 +50,7 @@ import type { ParsedImport, WorkspaceIndex } from 'gitnexus-shared';
|
|||
import { resolveJavaImportTarget } from '../../../src/core/ingestion/languages/java/import-target.js';
|
||||
import { CountingSet } from '../../helpers/counting-file-set.js';
|
||||
|
||||
// ─── verbatim pre-change implementation ──────────────────────────────────────
|
||||
// ─── pre-change implementation, minus the rule #2881 removed ─────────────────
|
||||
|
||||
interface LegacyJavaResolveContext {
|
||||
readonly fromFile: string;
|
||||
|
|
@ -81,8 +85,7 @@ function legacyResolveJavaImportTarget(
|
|||
let exactFile: string | null = null;
|
||||
let suffixFile: string | null = null;
|
||||
let directoryChild: string | null = null;
|
||||
const dirPrefix = `${pathLike}/`;
|
||||
const suffixDirPrefix = `/${dirPrefix}`;
|
||||
const suffixDirPrefix = `/${pathLike}/`;
|
||||
|
||||
for (const raw of ctx.allFilePaths) {
|
||||
const f = raw.replace(/\\/g, '/');
|
||||
|
|
@ -95,14 +98,13 @@ function legacyResolveJavaImportTarget(
|
|||
suffixFile = raw;
|
||||
}
|
||||
if (directoryChild === null) {
|
||||
const atRoot = f.startsWith(dirPrefix);
|
||||
const atNested = f.includes(suffixDirPrefix);
|
||||
if (atRoot || atNested) {
|
||||
const idx = atRoot ? 0 : f.indexOf(suffixDirPrefix) + 1;
|
||||
const after = f.slice(idx + dirPrefix.length);
|
||||
if (after.length > 0 && !after.includes('/')) {
|
||||
directoryChild = raw;
|
||||
}
|
||||
// Since #2881: "the file's parent directory ends with `pathLike`". The
|
||||
// `atRoot`/`indexOf` pair this replaces said that AND "…and that is the
|
||||
// first occurrence". `>= 0`, not `> 0` — a repo-root file has `lastSlash`
|
||||
// 0 for `/Top.java` shapes and the bare-wildcard case depends on it.
|
||||
const lastSlash = f.lastIndexOf('/');
|
||||
if (lastSlash >= 0 && `/${f.slice(0, lastSlash)}/`.endsWith(suffixDirPrefix)) {
|
||||
directoryChild = raw;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -119,8 +121,7 @@ function legacyResolveJavaImportTarget(
|
|||
if (tail === '') continue;
|
||||
const tailFile = `${tail}.java`;
|
||||
const tailSuffix = `/${tailFile}`;
|
||||
const tailDir = `${tail}/`;
|
||||
const tailSuffixDir = `/${tailDir}`;
|
||||
const tailSuffixDir = `/${tail}/`;
|
||||
let tailDirectChild: string | null = null;
|
||||
for (const raw of ctx.allFilePaths) {
|
||||
const f = raw.replace(/\\/g, '/');
|
||||
|
|
@ -128,12 +129,9 @@ function legacyResolveJavaImportTarget(
|
|||
if (f === tailFile) return raw;
|
||||
if (f.endsWith(tailSuffix)) return raw;
|
||||
if (tailDirectChild === null) {
|
||||
const atRoot = f.startsWith(tailDir);
|
||||
const atNested = f.includes(tailSuffixDir);
|
||||
if (atRoot || atNested) {
|
||||
const idx = atRoot ? 0 : f.indexOf(tailSuffixDir) + 1;
|
||||
const after = f.slice(idx + tailDir.length);
|
||||
if (after.length > 0 && !after.includes('/')) tailDirectChild = raw;
|
||||
const lastSlash = f.lastIndexOf('/');
|
||||
if (lastSlash >= 0 && `/${f.slice(0, lastSlash)}/`.endsWith(tailSuffixDir)) {
|
||||
tailDirectChild = raw;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -222,13 +220,20 @@ const HAND_CASES: readonly Case[] = [
|
|||
target: 'com.example.service.*',
|
||||
},
|
||||
{
|
||||
// Tie-break 5: the FIRST `/com/example/` occurrence leaves `com/example/
|
||||
// Deep.java` after it, which still contains a slash — so no match.
|
||||
label: 'self-nested-directory-does-not-match-outer',
|
||||
// Tie-break 5: the parent directory `com/example/com/example` ends with
|
||||
// `com/example`, so it answers. Until #2881 the leg took the FIRST
|
||||
// `/com/example/` occurrence, which leaves `com/example/Deep.java` after it
|
||||
// — still containing a slash — and the import resolved to null.
|
||||
label: 'self-nested-directory-answers-the-outer-package',
|
||||
files: ['com/example/com/example/Deep.java'],
|
||||
target: 'com.example',
|
||||
},
|
||||
{
|
||||
// Kept beside `self-nested-directory-answers-the-outer-package` even though
|
||||
// both now resolve to the same file: this one addresses the FULL path and
|
||||
// hits the exact-file/suffix tier, that one addresses the outer package and
|
||||
// hits the directory-child tier. Before #2881 the pair separated a hit from
|
||||
// a null; it now separates two tiers, which is why it is still two cases.
|
||||
label: 'self-nested-directory-matches-full-path',
|
||||
files: ['com/example/com/example/Deep.java'],
|
||||
target: 'com.example.com.example',
|
||||
|
|
@ -347,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. */
|
||||
|
|
@ -358,7 +387,7 @@ const HAND_EXPECTED: readonly string[] = [
|
|||
'directory-child-order-follows-insertion => com/example/service/Alpha.java',
|
||||
'wildcard-resolves-as-package-directory => com/example/service/Beta.java',
|
||||
'wildcard-exact-file-beats-directory => com/example/service.java',
|
||||
'self-nested-directory-does-not-match-outer => null',
|
||||
'self-nested-directory-answers-the-outer-package => com/example/com/example/Deep.java',
|
||||
'self-nested-directory-matches-full-path => com/example/com/example/Deep.java',
|
||||
'stripping-suffix-beats-earlier-directory-child => y/models/Order.java',
|
||||
'stripping-reaches-root-file => Order.java',
|
||||
|
|
@ -384,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 ────────────────────────────────────────────────────────
|
||||
|
|
@ -612,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 };
|
||||
|
|
|
|||
|
|
@ -8,6 +8,21 @@
|
|||
* the tie-breaks the scans implemented implicitly through iteration order.
|
||||
* These cases pin those semantics, so an index regression fails CI instead of
|
||||
* silently moving resolved edges in every Kotlin repository.
|
||||
*
|
||||
* ONE rule is deliberately no longer parity: #2881 removed the scan's
|
||||
* 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';
|
||||
|
|
@ -107,28 +122,37 @@ describe('resolveKotlinImportTarget — index parity', () => {
|
|||
expect(resolve(['pkg/A.java', 'pkg/A.md'], 'pkg.A')).toBeNull();
|
||||
});
|
||||
|
||||
it('only the FIRST occurrence of a repeated directory name counts', () => {
|
||||
// Deliberate parity with the scan: it tested `startsWith` first and then
|
||||
// used `indexOf` — the first `/data/` here is not the parent directory, and
|
||||
// it never looked for a second one. The file is therefore NOT a child of
|
||||
// `data`, even though it sits directly inside one.
|
||||
//
|
||||
// NOTE this case exercises the `startsWith` half only: `data` is the
|
||||
// LEADING segment, so the guard fires and the `indexOf` equality is never
|
||||
// reached. The mid-path case below is what pins that half — without it, a
|
||||
// resolver whose position check is relaxed to `indexOf(...) >= 0` passes
|
||||
// this whole file.
|
||||
expect(resolve(['data/src/main/kotlin/com/example/data/Repo.kt'], 'data.something')).toBeNull();
|
||||
it('a package name repeated as the LEADING segment still fans out (#2881)', () => {
|
||||
// The scan tested `startsWith` before `indexOf`, so a path whose leading
|
||||
// segment repeats the parent directory name was dropped from the `data`
|
||||
// bucket entirely and `import data.something` resolved to null. The file is
|
||||
// a direct child of a `data` directory, so it belongs in the bucket.
|
||||
expect(resolve(['data/src/main/kotlin/com/example/data/Repo.kt'], 'data.something')).toEqual([
|
||||
'data/src/main/kotlin/com/example/data/Repo.kt',
|
||||
]);
|
||||
// The single-file tiers were never affected — `suffixByStem` carries no
|
||||
// such guard — so this one resolved before the fix and still does.
|
||||
expect(resolve(['data/src/main/kotlin/com/example/data/Repo.kt'], 'data.Repo')).toBe(
|
||||
'data/src/main/kotlin/com/example/data/Repo.kt',
|
||||
);
|
||||
});
|
||||
|
||||
it('a repeated directory name below the root still only counts its first occurrence', () => {
|
||||
// Neither `data` is leading, so `startsWith` does not fire and the result
|
||||
// is decided by the `indexOf` position check alone. The first `/data/` is
|
||||
// not the parent, so this is not a child of `data`.
|
||||
expect(resolve(['top/data/mid/data/Repo.kt'], 'data.something')).toBeNull();
|
||||
expect(resolve(['a/c/b/c/File.kt'], 'c.X')).toBeNull();
|
||||
// Same shape, but the first occurrence IS the parent — this one resolves,
|
||||
// so the case above cannot pass by simply never matching anything.
|
||||
it('a package name repeated MID-PATH also fans out (#2881)', () => {
|
||||
// Neither `data` is leading, so `startsWith` never fired here and the null
|
||||
// came from the `indexOf` position check alone — a second, independent
|
||||
// guard. Both are gone; without this case a fix that only drops
|
||||
// `startsWith` passes the file above and still leaves this shape broken.
|
||||
expect(resolve(['top/data/mid/data/Repo.kt'], 'data.something')).toEqual([
|
||||
'top/data/mid/data/Repo.kt',
|
||||
]);
|
||||
// `['a/c/b/c/File.kt'], 'c.X'` used to sit here too. It is the same shape
|
||||
// with the segments renamed — four components, second and fourth equal,
|
||||
// query the repeated name — so it could not fail while the case above
|
||||
// passed. The bench corpus still carries it, where a second spelling of a
|
||||
// shape costs nothing; a unit case that cannot distinguish two
|
||||
// implementations is just a slower way to assert the first one.
|
||||
//
|
||||
// Unrepeated control: the parent is the only occurrence.
|
||||
expect(resolve(['top/data/Repo.kt'], 'data.something')).toEqual(['top/data/Repo.kt']);
|
||||
});
|
||||
|
||||
|
|
@ -143,8 +167,15 @@ describe('resolveKotlinImportTarget — index parity', () => {
|
|||
expect(resolve(['win\\pkg\\A.kt'], 'pkg.someFunction')).toEqual(['win\\pkg\\A.kt']);
|
||||
});
|
||||
|
||||
it('a path starting with the directory name is not a child of it unless direct', () => {
|
||||
it('a name that is not the PARENT directory stays out of the bucket', () => {
|
||||
// The rule is "the parent directory is named `s`", not "`s` appears
|
||||
// anywhere in the path" — dropping the two guards must not widen it that
|
||||
// far. Leading and mid-path, since those were the two positions the guards
|
||||
// distinguished; the new implementation has no positional logic at all, so
|
||||
// one case would do, and the second is kept only because it is the exact
|
||||
// shape the widened cases above use with the last segment changed.
|
||||
expect(resolve(['data/sub/Repo.kt'], 'data.something')).toBeNull();
|
||||
expect(resolve(['top/data/mid/Repo.kt'], 'data.something')).toBeNull();
|
||||
expect(resolve(['data/Repo.kt'], 'data.something')).toEqual(['data/Repo.kt']);
|
||||
});
|
||||
|
||||
|
|
@ -163,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');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,210 @@
|
|||
/**
|
||||
* Structural guard for the two `getKotlinFileIndex` optimizations added in
|
||||
* #2881 — the per-directory key memo and the bucket compaction.
|
||||
*
|
||||
* ## 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;
|
||||
* - 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
|
||||
* an implementation detail nothing depends on.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { ParsedImport } from 'gitnexus-shared';
|
||||
import { resolveKotlinImportTarget } from '../../../../src/core/ingestion/languages/kotlin/import-target.js';
|
||||
|
||||
/**
|
||||
* 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: files } as never);
|
||||
}
|
||||
|
||||
describe('getKotlinFileIndex internals (#2881)', () => {
|
||||
it('the memo hit path produces the same bucket as the miss path', () => {
|
||||
// Every file after the first in `pkg/` takes the memo, so a divergence
|
||||
// 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'];
|
||||
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', () => {
|
||||
// The memo is keyed on the full `dir`. Keying it on the last segment — the
|
||||
// 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'];
|
||||
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(set, 'pkg.fn')).toEqual(files);
|
||||
});
|
||||
|
||||
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(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 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 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(new Set(files), 'big.fn')).toEqual(files);
|
||||
});
|
||||
|
||||
it('the memo keys on the NORMALIZED directory while storing raw paths', () => {
|
||||
// `dir` is now sliced from `stem` rather than from `norm`. Both are the
|
||||
// backslash-normalized form and an extension holds no '/', so the last
|
||||
// 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.
|
||||
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(set, 'pkg.fn')).toEqual(['win\\pkg\\A.kt', 'win\\pkg\\B.kt']);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue