mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(scope-resolution): allow same-range Module-as-parent for top-level scopes (closes #1086) (#1087)
* fix(scope-resolution): allow same-range Module-as-parent for top-level scopes (closes #1086) When a C# file consists of a single top-level `namespace_declaration` that ends exactly at EOF (no trailing newline, no leading content outside the namespace's `{}` body), tree-sitter-c-sharp 0.23.1 reports identical byte ranges for `compilation_unit` and `namespace_declaration`. Pre-fix the scope-extractor parent-finder relied on strict containment, so the Module was popped off the stack and the Namespace ended up with `parent === null` → `ScopeTreeInvariantError: non-module-requires-parent` → `extractParsedFile` swallowed the throw and the whole file was dropped from the registry-primary path. Cross-file IMPORTS / CALLS edges originating in or terminating at that file vanished. Hit on three real-world `*.Designer.cs` files in PersistentWindows (`HotKeyWindow.Designer.cs`, `LaunchProcess.Designer.cs`, `DbKeySelect.Designer.cs`) — all have the byte signature `<BOM><CRLF>namespace ... { ... }<EOF>` (last hex = `... 7D 0D 0A 7D`). The fix is a single carve-out in the parent-validity contract: a `Module` may parent a same-range non-`Module` child. The relationship stays acyclic because the carve-out is direction-asymmetric — only Module-as- outer parents a same-range non-Module, never the reverse. Two coordinated changes: * `gitnexus/src/core/ingestion/scope-extractor.ts` — `pass1BuildScopes` now consults a new `canParentScope` helper instead of `rangeStrictlyContains` directly. Sort tie-breaker added so a same- range Module always sorts before a non-Module candidate, ensuring the Module lands on the parent-stack first regardless of tree-sitter capture iteration order. * `gitnexus-shared/src/scope-resolution/scope-tree.ts` — `buildScopeTree`'s `parent-must-contain-child` check now uses the same `canParentScope` carve-out so the validator agrees with the extractor on what a well-formed parent edge looks like. Error message updated to spell out the new contract. `rangeStrictlyContains` keeps its strict semantics in both files — position-index lookups, hook-side range comparisons, and other call sites are unchanged. * `gitnexus/test/fixtures/lang-resolution/csharp-namespace-as-root-no-trailing-newline/` — minimal regression fixture mirroring the PersistentWindows shape: both `Models/User.cs` and `App/Program.cs` end exactly on the closing `}` of their namespace with no trailing newline. The trigger is shape- driven, not size-driven, so the fixture stays small (~250 bytes total). * New `csharp.test.ts` describe block: scope extraction completes for both files, and the cross-file `IMPORTS` edge resolves through the scope-resolution path with `reason: 'csharp-scope: using'`. * `scope-tree.test.ts`: replaced the prior "rejects child ranges identical to the parent" case with three new ones — non-Module parent still rejected at equal range; Module-as-parent of a same-range non- Module accepted (the #1086 carve-out); Module-as-parent of another Module still rejected (the asymmetry guard). * `npx vitest run test/unit/scope-resolution test/integration/resolvers` → 2514 passed / 77 skipped / 0 failed (52 test files). * `npx tsc --noEmit` clean in both `gitnexus/` and `gitnexus-shared/`. * End-to-end on PersistentWindows (after rebuilding the Docker image with this branch): 3 prior `scope extraction failed for *.Designer.cs` warnings → 0. Pre-fix index numbers will be re-checked here once the branch is built and indexed; the existing post-#1082 baseline is 1113 nodes / 2987 edges / 39 clusters / 97 flows. `canParentScope` is language-agnostic. Other languages whose query emits `(compilation_unit) @scope.module` plus a single same-range top-level scope can naturally hit the same byte shape on minimal files; this fix applies to all of them uniformly. Refs: #1086 (issue with full root-cause analysis + 4-case empirical repro through `extractParsedFile`). * refactor(scope-resolution): export canParentScope from gitnexus-shared Addresses #1087 review (medium): the helper was previously duplicated byte-for-byte in `scope-extractor.ts` and `scope-tree.ts`. Per DoD "single source of truth in shared", the contract piece belongs in gitnexus-shared (Ring 2 SHARED #912) and the consuming layer should import it. Eliminates the silent-drift surface where a future edit to one copy would produce extractor/validator disagreement on what a well-formed parent edge looks like. Changes: - gitnexus-shared/src/scope-resolution/scope-tree.ts: add `export` to `canParentScope`. - gitnexus-shared/src/index.ts: re-export `canParentScope`. - gitnexus/src/core/ingestion/scope-extractor.ts: remove the local `canParentScope` definition (and its now-unused local copy of `rangeStrictlyContains`), import from `gitnexus-shared`. The local `rangesEqual` stays — it's still used in capture-anchor logic at two unrelated sites. Validation (per DoD §4.4 — both CLI and web consumers verified): - npx tsc --noEmit clean in gitnexus/ and gitnexus-shared/ - cd gitnexus-web && npx tsc -b --noEmit clean - gitnexus-shared `npm run build` clean - Targeted: vitest run test/unit/scope-resolution test/integration/resolvers → 2522 passed / 0 failed / 77 skipped (54 files) - Full suite: vitest run → 7238 passed / 1 failed / 97 skipped. The single failure is `test/unit/ignore-service.test.ts > warns on EACCES but does not throw`, which cannot run when uid=0 (root bypasses POSIX permission checks). Pre-existing on this branch before the refactor; unrelated to scope-resolution.
This commit is contained in:
parent
8fbbb35718
commit
1e80285c47
8 changed files with 197 additions and 34 deletions
|
|
@ -134,7 +134,11 @@ export type {
|
|||
// Scope tree spine + position lookup (RFC §2.2 + §3.1; Ring 2 SHARED #912)
|
||||
export { makeScopeId, clearScopeIdInternPool } from './scope-resolution/scope-id.js';
|
||||
export type { ScopeIdInput } from './scope-resolution/scope-id.js';
|
||||
export { buildScopeTree, ScopeTreeInvariantError } from './scope-resolution/scope-tree.js';
|
||||
export {
|
||||
buildScopeTree,
|
||||
canParentScope,
|
||||
ScopeTreeInvariantError,
|
||||
} from './scope-resolution/scope-tree.js';
|
||||
export type { ScopeTree } from './scope-resolution/scope-tree.js';
|
||||
export { buildPositionIndex } from './scope-resolution/position-index.js';
|
||||
export type { PositionIndex } from './scope-resolution/position-index.js';
|
||||
|
|
|
|||
|
|
@ -119,10 +119,10 @@ export function buildScopeTree(scopes: readonly Scope[]): ScopeTree {
|
|||
`Scope '${scope.id}' (${scope.filePath}) has parent '${parent.id}' in a different file (${parent.filePath}). Parent/child scopes must share filePath.`,
|
||||
);
|
||||
}
|
||||
if (!rangeStrictlyContains(parent.range, scope.range)) {
|
||||
if (!canParentScope(parent.range, scope.range, parent.kind, scope.kind)) {
|
||||
throw new ScopeTreeInvariantError(
|
||||
'parent-must-contain-child',
|
||||
`Parent scope '${parent.id}' at ${formatRange(parent.range)} does not strictly contain child '${scope.id}' at ${formatRange(scope.range)}.`,
|
||||
`Parent scope '${parent.id}' at ${formatRange(parent.range)} does not contain child '${scope.id}' at ${formatRange(scope.range)} (allowed: strict containment, or equal-range Module-as-parent).`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -230,6 +230,47 @@ function rangeStrictlyContains(outer: Range, inner: Range): boolean {
|
|||
return outerStartsAtOrBefore && outerEndsAtOrAfter;
|
||||
}
|
||||
|
||||
function rangesEqual(a: Range, b: Range): boolean {
|
||||
return (
|
||||
a.startLine === b.startLine &&
|
||||
a.startCol === b.startCol &&
|
||||
a.endLine === b.endLine &&
|
||||
a.endCol === b.endCol
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `outer` (kind `outerKind`) is a valid parent for `inner` (kind
|
||||
* `innerKind`).
|
||||
*
|
||||
* Strict containment is the general rule. The single carve-out is the
|
||||
* `Module`/non-`Module` pair whose ranges are exactly equal — this happens
|
||||
* naturally when tree-sitter reports identical byte spans for the
|
||||
* `compilation_unit` (or equivalent file-root construct) and the file's
|
||||
* single top-level scope. Common shape: a C# file consisting of nothing
|
||||
* but `namespace X { ... }` with no leading or trailing trivia outside the
|
||||
* namespace's `{}` body — `compilation_unit` and `namespace_declaration`
|
||||
* both span exactly the same byte range. The `Module` is the universal
|
||||
* outer of any file-level scope by language semantics, so coincident
|
||||
* ranges should not break the parent chain.
|
||||
*
|
||||
* The carve-out is direction-asymmetric: only `Module`-as-outer parents a
|
||||
* same-range non-`Module`, never the reverse. This preserves the
|
||||
* acyclicity buildScopeTree relies on, and matches the corresponding
|
||||
* helper in `scope-extractor.ts` so `pass1BuildScopes` and the validator
|
||||
* agree on what a well-formed parent edge looks like.
|
||||
*/
|
||||
export function canParentScope(
|
||||
outer: Range,
|
||||
inner: Range,
|
||||
outerKind: Scope['kind'],
|
||||
innerKind: Scope['kind'],
|
||||
): boolean {
|
||||
if (rangeStrictlyContains(outer, inner)) return true;
|
||||
if (outerKind === 'Module' && innerKind !== 'Module' && rangesEqual(outer, inner)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Two ranges overlap when neither finishes before the other begins. Ranges
|
||||
* that merely touch at a single boundary point (`a.end === b.start`) do
|
||||
|
|
|
|||
|
|
@ -74,7 +74,7 @@ import type {
|
|||
SymbolDefinition,
|
||||
TypeRef,
|
||||
} from 'gitnexus-shared';
|
||||
import { buildPositionIndex, buildScopeTree, makeScopeId } from 'gitnexus-shared';
|
||||
import { buildPositionIndex, buildScopeTree, canParentScope, makeScopeId } from 'gitnexus-shared';
|
||||
import type { LanguageProvider } from './language-provider.js';
|
||||
|
||||
// ─── Narrow hook surface the extractor actually uses ───────────────────────
|
||||
|
|
@ -331,20 +331,37 @@ function pass1BuildScopes(
|
|||
}
|
||||
|
||||
// Sort by (startLine, startCol) ASC, (endLine, endCol) DESC so outer
|
||||
// scopes appear before their children for parent-resolution.
|
||||
// scopes appear before their children for parent-resolution. When two
|
||||
// candidates have exactly equal ranges (e.g. a `compilation_unit` and
|
||||
// the only top-level scope in the file — see `canParentScope`), Module
|
||||
// sorts first so it lands on the stack ahead of the candidate that will
|
||||
// claim it as parent.
|
||||
candidates.sort((a, b) => {
|
||||
if (a.range.startLine !== b.range.startLine) return a.range.startLine - b.range.startLine;
|
||||
if (a.range.startCol !== b.range.startCol) return a.range.startCol - b.range.startCol;
|
||||
if (a.range.endLine !== b.range.endLine) return b.range.endLine - a.range.endLine;
|
||||
return b.range.endCol - a.range.endCol;
|
||||
if (a.range.endCol !== b.range.endCol) return b.range.endCol - a.range.endCol;
|
||||
if (a.kind === b.kind) return 0;
|
||||
if (a.kind === 'Module') return -1;
|
||||
if (b.kind === 'Module') return 1;
|
||||
return 0;
|
||||
});
|
||||
|
||||
const drafts: ScopeDraft[] = [];
|
||||
const stack: Candidate[] = []; // enclosing real scopes, outermost at [0]
|
||||
|
||||
for (const cand of candidates) {
|
||||
// Pop the stack until the top strictly contains this candidate.
|
||||
while (stack.length > 0 && !rangeStrictlyContains(stack[stack.length - 1]!.range, cand.range)) {
|
||||
// Pop the stack until the top can parent this candidate (strict
|
||||
// containment, plus the equal-range Module carve-out).
|
||||
while (
|
||||
stack.length > 0 &&
|
||||
!canParentScope(
|
||||
stack[stack.length - 1]!.range,
|
||||
cand.range,
|
||||
stack[stack.length - 1]!.kind,
|
||||
cand.kind,
|
||||
)
|
||||
) {
|
||||
stack.pop();
|
||||
}
|
||||
|
||||
|
|
@ -907,24 +924,6 @@ function rangesEqual(a: Range, b: Range): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
function rangeStrictlyContains(outer: Range, inner: Range): boolean {
|
||||
if (
|
||||
outer.startLine === inner.startLine &&
|
||||
outer.startCol === inner.startCol &&
|
||||
outer.endLine === inner.endLine &&
|
||||
outer.endCol === inner.endCol
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
const startsBefore =
|
||||
outer.startLine < inner.startLine ||
|
||||
(outer.startLine === inner.startLine && outer.startCol <= inner.startCol);
|
||||
const endsAfter =
|
||||
outer.endLine > inner.endLine ||
|
||||
(outer.endLine === inner.endLine && outer.endCol >= inner.endCol);
|
||||
return startsBefore && endsAfter;
|
||||
}
|
||||
|
||||
/**
|
||||
* Capture names that are never anchors — they are sub-tags nested inside a
|
||||
* larger anchor (e.g., the receiver expression inside a `@reference.call`
|
||||
|
|
|
|||
|
|
@ -0,0 +1,13 @@
|
|||
using NoTrailingNewline.Models;
|
||||
|
||||
namespace NoTrailingNewline.App
|
||||
{
|
||||
public class Program
|
||||
{
|
||||
public void Run()
|
||||
{
|
||||
var u = new User();
|
||||
u.GetName();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
namespace NoTrailingNewline.Models
|
||||
{
|
||||
public class User
|
||||
{
|
||||
public string GetName() { return "u"; }
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
<PropertyGroup>
|
||||
<TargetFramework>net8.0</TargetFramework>
|
||||
<RootNamespace>NoTrailingNewline</RootNamespace>
|
||||
</PropertyGroup>
|
||||
</Project>
|
||||
|
|
@ -2535,3 +2535,50 @@ describe('C# frozen-binding collision via using-import (issue #1066 companion)',
|
|||
expect(ctor!.targetFilePath).toBe('App/Program.cs');
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Issue #1086 regression: when a C# file consists of a single top-level
|
||||
// namespace_declaration that ends exactly at EOF (no trailing newline,
|
||||
// no leading content outside the namespace block), tree-sitter-c-sharp
|
||||
// reports identical ranges for `compilation_unit` and `namespace_declaration`.
|
||||
// Pre-fix, scope-extractor's parent-finder relied on strict containment, so
|
||||
// the Module was popped off the stack and the Namespace ended up with
|
||||
// parent=null → ScopeTreeInvariantError → scopeResolution silently aborted
|
||||
// for the file (extractParsedFile swallows). Post-fix, `canParentScope`
|
||||
// allows a same-range Module to keep parenthood, so extraction completes
|
||||
// and the file's symbols stay reachable to the cross-file resolver.
|
||||
//
|
||||
// Hit on real PersistentWindows .Designer.cs files. The fixture mirrors
|
||||
// that shape minimally — both files end on the closing `}` with no
|
||||
// trailing newline.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('C# namespace-as-root with no trailing newline (issue #1086)', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(
|
||||
path.join(FIXTURES, 'csharp-namespace-as-root-no-trailing-newline'),
|
||||
() => {},
|
||||
{ workerThresholdsForTest: { minFiles: 1, minBytes: 0 } },
|
||||
);
|
||||
}, 60000);
|
||||
|
||||
it('completes scope extraction for both files (no Namespace-as-root abort)', () => {
|
||||
expect(getNodesByLabel(result, 'Class')).toEqual(expect.arrayContaining(['User', 'Program']));
|
||||
});
|
||||
|
||||
it('emits the using-import edge App/Program.cs -> Models/User.cs through the scope-resolution path', () => {
|
||||
// The `csharp-scope: using` reason on the IMPORTS edge is the signal
|
||||
// that scope-resolution drove the resolution (not the legacy DAG
|
||||
// fallback). Pre-fix, Models/User.cs aborted in scope-extractor and
|
||||
// the only IMPORTS edge available — if any — would have come from a
|
||||
// path with a different reason tag, or be missing entirely.
|
||||
const imports = getRelationships(result, 'IMPORTS');
|
||||
const edge = imports.find(
|
||||
(e) => e.sourceFilePath === 'App/Program.cs' && e.targetFilePath === 'Models/User.cs',
|
||||
);
|
||||
expect(edge).toBeDefined();
|
||||
expect(edge!.rel.reason).toBe('csharp-scope: using');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -213,7 +213,7 @@ describe('buildScopeTree', () => {
|
|||
expect(() => buildScopeTree([fn])).toThrowError(ScopeTreeInvariantError);
|
||||
});
|
||||
|
||||
it('throws when a parent range does not strictly contain a child range', () => {
|
||||
it('throws when a parent range does not contain a child range', () => {
|
||||
const mod = mkScope({ id: 'scope:m', parent: null, kind: 'Module', range: r(1, 0, 10, 0) });
|
||||
const fn = mkScope({
|
||||
id: 'scope:f',
|
||||
|
|
@ -222,18 +222,64 @@ describe('buildScopeTree', () => {
|
|||
range: r(5, 0, 50, 0), // extends beyond the module
|
||||
});
|
||||
expect(() => buildScopeTree([mod, fn])).toThrowError(ScopeTreeInvariantError);
|
||||
expect(() => buildScopeTree([mod, fn])).toThrowError(/strictly contain/i);
|
||||
expect(() => buildScopeTree([mod, fn])).toThrowError(/contain child/i);
|
||||
});
|
||||
|
||||
it('rejects child ranges identical to the parent (not strictly contained)', () => {
|
||||
it('rejects child ranges identical to a non-Module parent', () => {
|
||||
// Same-range parent-child is only legal when the parent is the
|
||||
// file's Module (the universal-outer carve-out — see the
|
||||
// namespace-as-root case below). For non-Module parents (Namespace,
|
||||
// Class, Function, Block, …) the strict-containment rule still holds.
|
||||
const mod = mkScope({ id: 'scope:m', parent: null, kind: 'Module', range: r(1, 0, 10, 0) });
|
||||
const fn = mkScope({
|
||||
id: 'scope:f',
|
||||
const ns = mkScope({
|
||||
id: 'scope:ns',
|
||||
parent: 'scope:m',
|
||||
kind: 'Function',
|
||||
range: r(1, 0, 10, 0),
|
||||
kind: 'Namespace',
|
||||
range: r(2, 0, 9, 0),
|
||||
});
|
||||
expect(() => buildScopeTree([mod, fn])).toThrowError(ScopeTreeInvariantError);
|
||||
const cls = mkScope({
|
||||
id: 'scope:c',
|
||||
parent: 'scope:ns',
|
||||
kind: 'Class',
|
||||
range: r(2, 0, 9, 0), // same as ns
|
||||
});
|
||||
expect(() => buildScopeTree([mod, ns, cls])).toThrowError(ScopeTreeInvariantError);
|
||||
expect(() => buildScopeTree([mod, ns, cls])).toThrowError(/contain child/i);
|
||||
});
|
||||
|
||||
it('accepts a same-range non-Module child whose parent is the Module (issue #1086)', () => {
|
||||
// Triggered by C# files consisting of a single top-level
|
||||
// `namespace_declaration` that ends exactly at EOF (no trailing
|
||||
// newline, no leading content): tree-sitter reports identical byte
|
||||
// ranges for `compilation_unit` and `namespace_declaration`. The
|
||||
// Module is the universal outer of any file-level scope by language
|
||||
// semantics, so equal ranges should not break the parent chain when
|
||||
// the parent is the Module.
|
||||
const mod = mkScope({ id: 'scope:m', parent: null, kind: 'Module', range: r(1, 0, 10, 0) });
|
||||
const ns = mkScope({
|
||||
id: 'scope:ns',
|
||||
parent: 'scope:m',
|
||||
kind: 'Namespace',
|
||||
range: r(1, 0, 10, 0), // exactly equal to the Module
|
||||
});
|
||||
expect(() => buildScopeTree([mod, ns])).not.toThrow();
|
||||
const tree = buildScopeTree([mod, ns]);
|
||||
expect(tree.getParent('scope:ns' as ScopeId)?.id).toBe('scope:m');
|
||||
expect(tree.getChildren('scope:m' as ScopeId)).toEqual(['scope:ns']);
|
||||
});
|
||||
|
||||
it('still rejects same-range Module-as-parent of another Module', () => {
|
||||
// The carve-out is asymmetric: only Module-as-outer parents a
|
||||
// same-range non-Module. Module-Module at equal ranges is rejected
|
||||
// because two Modules would imply two roots / cyclic structure.
|
||||
const m1 = mkScope({ id: 'scope:m1', parent: null, kind: 'Module', range: r(0, 0, 10, 0) });
|
||||
const m2 = mkScope({
|
||||
id: 'scope:m2',
|
||||
parent: 'scope:m1',
|
||||
kind: 'Module',
|
||||
range: r(0, 0, 10, 0),
|
||||
});
|
||||
expect(() => buildScopeTree([m1, m2])).toThrowError(ScopeTreeInvariantError);
|
||||
});
|
||||
|
||||
it('throws when sibling ranges overlap', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue