mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-11 08:37:35 +00:00
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
This commit is contained in:
parent
b53a47cfc6
commit
233f7118d3
10 changed files with 243 additions and 171 deletions
File diff suppressed because one or more lines are too long
|
|
@ -726,14 +726,21 @@ 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). 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.
|
||||
* 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. Each of
|
||||
* those four languages repeats the shape at whatever granularity ITS query uses
|
||||
* — the last segment for csharp/java/kotlin, the whole package path for go —
|
||||
* because a repeat the query cannot even ask about leaves the arm blind.
|
||||
*/
|
||||
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}`;
|
||||
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') {
|
||||
|
|
@ -784,7 +791,10 @@ 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`;
|
||||
}
|
||||
if (lang === 'csharp') return d % 7 === 0 ? `Src${d}/Models/Inner/Models` : `Src${d}/Models`;
|
||||
|
|
@ -1205,11 +1215,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`;
|
||||
|
|
@ -1289,13 +1301,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'][
|
||||
|
|
|
|||
|
|
@ -25,16 +25,18 @@ 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.
|
||||
|
|
@ -179,11 +181,12 @@ function* matchingDirPositions(
|
|||
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;
|
||||
// `endsWith` subsumes the length guard the `indexOf` form needed: a shorter
|
||||
// haystack is simply false, where `indexOf` returned -1 and
|
||||
// `haystack.length - needle.length` could also be -1 and report a bogus
|
||||
// match. Still deliberately UNANCHORED (no leading '/'), so `src/SubModels`
|
||||
// keeps answering `Models` — see the derivation above.
|
||||
if (!haystack.endsWith(needle)) continue;
|
||||
const positions = index.positionsByDir.get(dir);
|
||||
if (positions !== undefined) yield positions;
|
||||
}
|
||||
|
|
@ -286,8 +289,16 @@ export function resolveCSharpImportInternal(
|
|||
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 + '/');
|
||||
// Direct child of a directory ENDING with `dirPrefix` — the same
|
||||
// predicate as before minus "…and that occurrence is the FIRST one".
|
||||
// An empty `dirPrefix` keeps `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, an empty prefix would accept every `.cs` that
|
||||
// sits in any directory and step 2 would diverge from step 3.
|
||||
const needle = dirPrefix + '/';
|
||||
const prefixIdx =
|
||||
dirPrefix === '' ? normalized.indexOf(needle) : normalized.lastIndexOf(needle);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -301,7 +312,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
|
||||
|
|
|
|||
|
|
@ -46,7 +46,10 @@ export function resolveGoPackage(
|
|||
normalized.endsWith('.go') &&
|
||||
!normalized.endsWith('_test.go')
|
||||
) {
|
||||
const afterPkg = normalized.substring(normalized.indexOf(pkgSuffix) + pkgSuffix.length);
|
||||
// `lastIndexOf`, not `indexOf`: `pkgSuffix` 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` (#2881).
|
||||
const afterPkg = normalized.substring(normalized.lastIndexOf(pkgSuffix) + pkgSuffix.length);
|
||||
if (!afterPkg.includes('/')) {
|
||||
matches.push(allFileList[i]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,27 +12,26 @@
|
|||
*
|
||||
* 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 and the C# csproj index
|
||||
* followed, so the four resolvers agree again.
|
||||
*
|
||||
* 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".
|
||||
* 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,
|
||||
|
|
@ -135,11 +134,11 @@ function* matchingDirs(index: PackageDirIndex, pkgPath: string): Generator<reado
|
|||
const needle = `/${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;
|
||||
// `endsWith` subsumes the length guard the `indexOf` form needed: a shorter
|
||||
// haystack is simply false, where `indexOf` returned -1 and
|
||||
// `haystack.length - needle.length` could also be -1 and report a bogus
|
||||
// match.
|
||||
if (!haystack.endsWith(needle)) continue;
|
||||
const files = index.filesByDir.get(dir);
|
||||
if (files !== undefined) yield files;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,10 +42,15 @@
|
|||
* 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.
|
||||
*/
|
||||
|
||||
import type { ParsedImport, WorkspaceIndex } from 'gitnexus-shared';
|
||||
|
|
|
|||
|
|
@ -219,7 +219,10 @@ const getKotlinFileIndex = perFileSet((allFilePaths: ReadonlySet<string>): Kotli
|
|||
// and the file IS a direct child of a directory named `s`.
|
||||
//
|
||||
// #2881: this loop used to carry two guards, both inherited from the
|
||||
// pre-index per-import scan rather than from anything Kotlin requires —
|
||||
// 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
|
||||
|
|
|
|||
|
|
@ -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 the guard in both copies). Step 3 therefore answers
|
||||
* strictly more:
|
||||
*
|
||||
* - `dirPrefix = 'ubModels'` matches `src/SubModels/` (character suffix of a
|
||||
* segment, not a segment);
|
||||
|
|
@ -104,8 +107,14 @@ function legacyResolveCSharpImportInternal(
|
|||
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 + '/');
|
||||
// Direct child of a directory ENDING with `dirPrefix` — since #2881,
|
||||
// minus "…and that occurrence is the FIRST". Empty `dirPrefix` keeps
|
||||
// `indexOf`: its needle is a bare '/', and step 3 answers that query
|
||||
// from the one-directory-deep set, which only the first occurrence
|
||||
// expresses.
|
||||
const needle2 = dirPrefix + '/';
|
||||
const prefixIdx =
|
||||
dirPrefix === '' ? normalized.indexOf(needle2) : normalized.lastIndexOf(needle2);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -121,7 +130,8 @@ function legacyResolveCSharpImportInternal(
|
|||
for (let i = 0; i < normalizedFileList.length; i++) {
|
||||
const normalized = normalizedFileList[i];
|
||||
if (!normalized.endsWith('.cs')) continue;
|
||||
const prefixIdx = normalized.indexOf(dirTrail);
|
||||
const prefixIdx =
|
||||
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirTrail.length);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -189,7 +199,9 @@ function skipStep3WhenIndexed(
|
|||
const dirFiles = index.getFilesInDir(dirPrefix, '.cs');
|
||||
for (const f of dirFiles) {
|
||||
const normalized = f.replace(/\\/g, '/');
|
||||
const prefixIdx = normalized.indexOf(dirPrefix + '/');
|
||||
const needle2 = dirPrefix + '/';
|
||||
const prefixIdx =
|
||||
dirPrefix === '' ? normalized.indexOf(needle2) : normalized.lastIndexOf(needle2);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirPrefix.length + 1);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -204,7 +216,8 @@ function skipStep3WhenIndexed(
|
|||
for (let i = 0; i < normalizedFileList.length; i++) {
|
||||
const normalized = normalizedFileList[i];
|
||||
if (!normalized.endsWith('.cs')) continue;
|
||||
const prefixIdx = normalized.indexOf(dirTrail);
|
||||
const prefixIdx =
|
||||
dirPrefix === '' ? normalized.indexOf(dirTrail) : normalized.lastIndexOf(dirTrail);
|
||||
if (prefixIdx < 0) continue;
|
||||
const afterDir = normalized.substring(prefixIdx + dirTrail.length);
|
||||
if (!afterDir.includes('/')) {
|
||||
|
|
@ -252,8 +265,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 +498,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 +513,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 +527,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',
|
||||
]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -77,7 +77,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 +206,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;
|
||||
}
|
||||
|
|
@ -526,7 +531,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 +575,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 +674,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 +721,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,8 +17,11 @@
|
|||
* — 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.
|
||||
*
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -128,12 +130,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,9 +221,11 @@ 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',
|
||||
},
|
||||
|
|
@ -358,7 +359,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',
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue