mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-11 08:37:35 +00:00
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
This commit is contained in:
parent
5f9648744c
commit
b53a47cfc6
7 changed files with 133 additions and 92 deletions
File diff suppressed because one or more lines are too long
|
|
@ -727,8 +727,10 @@ function unwiredLanguage(where, lang) {
|
|||
* 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.
|
||||
* package-dir-index.ts). Kotlin no longer applies that tie-break — #2881
|
||||
* removed it there and only there — so this one corpus shape is deliberately
|
||||
* scored two different ways: the kotlin arm resolves its `d % 7` slice and the
|
||||
* go, csharp and java arms still miss on theirs.
|
||||
*/
|
||||
function uniqueDir(lang, d, i) {
|
||||
if (lang === 'go') return d % 7 === 0 ? `src/pkg${d}/internal/pkg${d}` : `src/pkg${d}`;
|
||||
|
|
@ -1246,13 +1248,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'][
|
||||
|
|
|
|||
|
|
@ -1,14 +1,14 @@
|
|||
{
|
||||
"_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. Of 19968 distinct records, 235 moved: 54 NULL -> resolved (the fix itself, and exactly the +54 in non_null, 13256 -> 13310) and 181 answers that changed within a now-larger `dirChildren` bucket — 149 where the first-child tier picks a different member and 32 where a fan-out array gained entries. ZERO buckets lost a member, zero results were dropped, and in all 149 reselections the new answer's parent directory is the queried package segment. 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. Caught against the CURRENT baseline (#2881): skipping the dirChildren suffix loop 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, the mutation #2881 makes newly plausible, a HALF fix that drops only the `startsWith` guard and keeps the `indexOf` first-occurrence check (836977b83bf0…, non_null 13310 -> 13282), which leaves every mid-path repeat such as `top/data/mid/data/Repo.kt` still broken. Caught against the previous baseline and unaffected by the re-baseline: capping suffixByStem key depth at 7 (a0e6eb98f9…). 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.",
|
||||
"fingerprint": "d91110bee389891c313811c5b4bae61d909561156e1458d38d487be969f0059c",
|
||||
"cases": 20106,
|
||||
"non_null": 13256,
|
||||
"non_null": 13310,
|
||||
"scaling_budget": 1.6,
|
||||
"depth_budget": 2.4,
|
||||
"depth_budget": 2.2,
|
||||
"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.",
|
||||
"_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 before #2881; 1.27-1.40 over five after. That change deleted two string comparisons per component of every dir, which is per-depth work, so the depth arm got cheaper while the file-count arm did not. The BUDGET moved with the band — 2.4 -> 2.2, holding the ~1.6x headroom over the band's top that 2.4 expressed against the old one; left at 2.4 it would have quietly become 1.8x, 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 either number, 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 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."
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -22,6 +22,18 @@
|
|||
* a cleanup and moves edges in every repository that nests a directory name
|
||||
* inside itself (`internal/…/internal`, `Models/…/Models`).
|
||||
*
|
||||
* Condition 2 is no longer universal across the codebase. Kotlin does not use
|
||||
* this index — it builds its own `dirChildren` in
|
||||
* `languages/kotlin/import-target.ts` — and #2881 removed the equivalent rule
|
||||
* there, because a root-level package whose name repeats higher in the tree
|
||||
* (`data/src/main/kotlin/com/example/data/Repo.kt` for `import data.helper`)
|
||||
* resolved to null. The languages served here (Go, Java, C#) still carry it and
|
||||
* still have that shape; fixing them means re-baselining three languages and
|
||||
* editing the verbatim pre-change scans that
|
||||
* `test/unit/scope-resolution/import-target-index-parity.test.ts` keeps as the
|
||||
* specification, which is why #2881 did not reach them. Read this comment as
|
||||
* "what these three languages do", not "what package resolution means".
|
||||
*
|
||||
* Candidates are narrowed by the directory's LAST segment rather than by
|
||||
* indexing every directory suffix: a suffix map costs O(files × depth) entries,
|
||||
* which is exactly the memory this codebase runs out of at kernel scale
|
||||
|
|
|
|||
|
|
@ -156,21 +156,21 @@ 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.
|
||||
*/
|
||||
interface KotlinFileIndex {
|
||||
|
|
@ -214,29 +214,37 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
|
|||
// 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`":
|
||||
// Every component-suffix of the directory also qualifies: `s` is a suffix
|
||||
// of `dir` starting after a '/', so `dir` ends with `/s` by construction
|
||||
// and the file IS a direct child of a directory 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.
|
||||
// #2881: this loop used to carry two guards, both inherited from the
|
||||
// pre-index per-import scan rather than from anything Kotlin requires —
|
||||
// `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 was affected —
|
||||
// `data.Repo` answered from `suffixByStem`, which never had such a guard,
|
||||
// which is why the shape looked narrow enough to preserve.
|
||||
//
|
||||
// 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.
|
||||
// Widening is filtered downstream for tier 3, which hands the finalize pass
|
||||
// a candidate list (`findKotlinPackageFiles`), but NOT for the tier-1
|
||||
// fallback, which commits to `children[0]` unfiltered
|
||||
// (`findKotlinDirectoryChild`) — and that unfiltered tier is where most of
|
||||
// the change lands: of the 235 corpus records that moved, 149 are a
|
||||
// different first child and only 32 are a wider fan-out array. Both are
|
||||
// deliberate. A narrower bucket for the first-child tier alone would keep
|
||||
// its answers byte-identical, but it 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. The bucket is either
|
||||
// right or it is not; there is no version of it that is right for one
|
||||
// consumer and wrong for the other.
|
||||
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);
|
||||
}
|
||||
addChild(dirChildren, dir.slice(i + 1), raw);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,11 @@
|
|||
* 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.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { resolveKotlinImportTarget } from '../../../../src/core/ingestion/languages/kotlin/import-target.js';
|
||||
|
|
@ -107,31 +112,41 @@ 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',
|
||||
]);
|
||||
expect(resolve(['a/c/b/c/File.kt'], 'c.X')).toEqual(['a/c/b/c/File.kt']);
|
||||
// Unrepeated control: the parent is the only occurrence.
|
||||
expect(resolve(['top/data/Repo.kt'], 'data.something')).toEqual(['top/data/Repo.kt']);
|
||||
});
|
||||
|
||||
it('a repeated 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. `data` is a component of the path but the parent is `mid`.
|
||||
expect(resolve(['top/data/mid/Repo.kt'], 'data.something')).toBeNull();
|
||||
});
|
||||
|
||||
it('the package directory is derived from the normalized path, not the raw one', () => {
|
||||
// A backslash path whose `dirChildren` key must come from the normalized
|
||||
// form: reading the raw path's last separator instead would make the whole
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue