mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-04 02:31:36 +00:00
Code-review follow-up. The scan read each .cs fully into a string behind a 512KB size cap (the tree-sitter parse budget); a single larger generated file (*.g.cs, EF/gRPC output) tripped `truncated`, making the #1881 suffix-fallback gate fail open repo-wide and silently undoing the fix on real repos. Stream each .cs line-by-line via createReadStream + readline into a new incremental scanner (createCsharpStructureScanner) instead of buffering the whole file. Memory is now constant regardless of file size, so the per-file size cap is dropped for the namespace line-scan and large generated files are fully collected. extractCsharpStructureViaScanner is reimplemented on the same incremental scanner (byte-identical; C# parity 2/2). collectDeclaredNamespaces returns 'ok' | 'truncated' (truncation now only from an unreadable file) and the truncation warn lists its real causes. csproj reads keep their size guard. Prior art: ripgrep/ctags/Node readline stream rather than cap for line scans; GitHub (384KB) and Sourcegraph (1MB) cap only their full-content indexes.
This commit is contained in:
parent
f42dc41a8e
commit
33fc24de91
3 changed files with 101 additions and 64 deletions
|
|
@ -1,7 +1,9 @@
|
|||
import fs from 'fs/promises';
|
||||
import { createReadStream } from 'fs';
|
||||
import { createInterface } from 'readline';
|
||||
import path from 'path';
|
||||
import type { ImportConfigs } from './import-resolvers/types.js';
|
||||
import type { CsharpFileStructure } from './languages/csharp/namespace-siblings.js';
|
||||
import type { CsharpStructureLineScanner } from './languages/csharp/namespace-siblings.js';
|
||||
|
||||
import { isDev } from './utils/env.js';
|
||||
import { getMaxFileSizeBytes } from './utils/max-file-size.js';
|
||||
|
|
@ -203,14 +205,14 @@ const CSHARP_ROOT_NAMESPACE_RE = /<RootNamespace>\s*([^<]+)\s*<\/RootNamespace>/
|
|||
// with phantom namespaces. Imported lazily (and memoized) so the always-on
|
||||
// `loadImportConfigs` path — every repo, every language — doesn't eagerly
|
||||
// pull tree-sitter-c-sharp in via `namespace-siblings.ts` → `query.ts`.
|
||||
let csharpScannerPromise: Promise<(content: string) => CsharpFileStructure> | undefined;
|
||||
function getCsharpStructureScanner(): Promise<(content: string) => CsharpFileStructure> {
|
||||
if (csharpScannerPromise === undefined) {
|
||||
csharpScannerPromise = import('./languages/csharp/namespace-siblings.js').then(
|
||||
(mod) => mod.extractCsharpStructureViaScanner,
|
||||
let csharpScannerFactoryPromise: Promise<() => CsharpStructureLineScanner> | undefined;
|
||||
function getCsharpStructureScannerFactory(): Promise<() => CsharpStructureLineScanner> {
|
||||
if (csharpScannerFactoryPromise === undefined) {
|
||||
csharpScannerFactoryPromise = import('./languages/csharp/namespace-siblings.js').then(
|
||||
(mod) => mod.createCsharpStructureScanner,
|
||||
);
|
||||
}
|
||||
return csharpScannerPromise;
|
||||
return csharpScannerFactoryPromise;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -298,29 +300,25 @@ export async function scanCSharpProject(repoRoot: string): Promise<CSharpProject
|
|||
const batch = csNames.slice(i, i + CSHARP_SCAN_READ_CONCURRENCY);
|
||||
const settled = await Promise.allSettled(
|
||||
batch.map((name) =>
|
||||
collectDeclaredNamespaces(
|
||||
path.join(dir, name),
|
||||
declaredNamespaces,
|
||||
rootNamespaces,
|
||||
maxFileSizeBytes,
|
||||
),
|
||||
collectDeclaredNamespaces(path.join(dir, name), declaredNamespaces, rootNamespaces),
|
||||
),
|
||||
);
|
||||
// A `.cs` that was skipped (oversized), unreadable, or whose read/scan
|
||||
// unexpectedly rejected leaves its namespaces uncollected → mark truncated
|
||||
// to fail the #1881 gate OPEN rather than wrongly suppress an import.
|
||||
// A `.cs` that was unreadable (or whose read/scan unexpectedly rejected)
|
||||
// leaves its namespaces uncollected → mark truncated to fail the #1881
|
||||
// gate OPEN rather than wrongly suppress an import. The scan streams each
|
||||
// file, so file size no longer trips truncation.
|
||||
for (const r of settled) {
|
||||
if (r.status !== 'fulfilled' || r.value) truncated = true;
|
||||
if (r.status !== 'fulfilled' || r.value === 'truncated') truncated = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (truncated) {
|
||||
// Surface the fail-open so a too-small cap (or an unreadable subtree)
|
||||
// silently disabling the #1881 gate repo-wide is observable (#4) rather
|
||||
// than a mystery edge regression.
|
||||
// Surface the fail-open so an incomplete scan (dir/depth cap, or an
|
||||
// unreadable directory or `.cs` file) silently disabling the #1881 gate
|
||||
// repo-wide is observable (#4) rather than a mystery edge regression.
|
||||
logger.warn(
|
||||
`[csharp] namespace scan of ${repoRoot} truncated (dir cap ${CSHARP_SCAN_MAX_DIRS}, depth cap ${CSHARP_SCAN_MAX_DEPTH}, or an unreadable directory); the #1881 suffix-fallback gate fails open for unmatched usings`,
|
||||
`[csharp] namespace scan of ${repoRoot} truncated (dir cap ${CSHARP_SCAN_MAX_DIRS}, depth cap ${CSHARP_SCAN_MAX_DEPTH}, an unreadable directory, or an unreadable .cs file); the #1881 suffix-fallback gate fails open for unmatched usings`,
|
||||
);
|
||||
}
|
||||
return { configs, declaredNamespaces, rootNamespaces, truncated };
|
||||
|
|
@ -352,36 +350,47 @@ async function readCsprojConfig(
|
|||
}
|
||||
|
||||
/**
|
||||
* Collect declared `namespace` names from one `.cs` file into the shared Sets.
|
||||
* Stream one `.cs` file line-by-line and collect its declared `namespace` names
|
||||
* into the shared Sets.
|
||||
*
|
||||
* Returns `true` when the file was skipped (oversized) or could not be read, so
|
||||
* the caller can mark the scan truncated — its namespaces are missing, and the
|
||||
* #1881 gate must fail OPEN rather than wrongly suppress an import declared in
|
||||
* the unread file. Returns `false` on a successful read.
|
||||
* Streaming (rather than reading the whole file into a string) keeps memory
|
||||
* constant regardless of file size, so a large generated `.cs` (`*.g.cs`, EF /
|
||||
* gRPC output) is fully scanned instead of skipped by a per-file size cap —
|
||||
* which would otherwise trip `truncated` and disable the #1881 gate repo-wide.
|
||||
* Only the cheap line scan streams here; the tree-sitter PARSE path keeps its
|
||||
* own size cap.
|
||||
*
|
||||
* Returns `'truncated'` when the file could not be read, so the caller marks the
|
||||
* scan truncated and the #1881 gate fails OPEN rather than wrongly suppress an
|
||||
* import declared in the unread file. Returns `'ok'` on a complete read.
|
||||
*/
|
||||
async function collectDeclaredNamespaces(
|
||||
filePath: string,
|
||||
declaredNamespaces: Set<string>,
|
||||
rootNamespaces: Set<string>,
|
||||
maxFileSizeBytes: number,
|
||||
): Promise<boolean> {
|
||||
let content: string;
|
||||
): Promise<'ok' | 'truncated'> {
|
||||
const createScanner = await getCsharpStructureScannerFactory();
|
||||
const scanner = createScanner();
|
||||
try {
|
||||
const stat = await fs.stat(filePath);
|
||||
if (stat.size > maxFileSizeBytes) {
|
||||
return true; // oversized source → skip unread, signal truncation
|
||||
// `crlfDelay: Infinity` treats every `\r\n` as a single break; the line
|
||||
// scanner is terminator-agnostic, so a streamed scan yields the same
|
||||
// namespaces as scanning the whole file content at once.
|
||||
const lines = createInterface({
|
||||
input: createReadStream(filePath, { encoding: 'utf-8' }),
|
||||
crlfDelay: Infinity,
|
||||
});
|
||||
for await (const line of lines) {
|
||||
scanner.pushLine(line);
|
||||
}
|
||||
content = await fs.readFile(filePath, 'utf-8');
|
||||
} catch {
|
||||
return true; // unreadable source → signal truncation (was a silent skip)
|
||||
return 'truncated'; // unreadable source → signal truncation (fail open)
|
||||
}
|
||||
const scan = await getCsharpStructureScanner();
|
||||
for (const ns of scan(content).namespaces) {
|
||||
for (const ns of scanner.result().namespaces) {
|
||||
declaredNamespaces.add(ns);
|
||||
const dot = ns.indexOf('.');
|
||||
rootNamespaces.add(dot === -1 ? ns : ns.slice(0, dot));
|
||||
}
|
||||
return false;
|
||||
return 'ok';
|
||||
}
|
||||
|
||||
export async function loadSwiftPackageConfig(repoRoot: string): Promise<SwiftPackageConfig | null> {
|
||||
|
|
|
|||
|
|
@ -182,26 +182,51 @@ function advanceCsScanState(
|
|||
* AST is a declaration whose keyword is not at the start of a code line
|
||||
* (split across lines, or sharing a line with a comment/string closer).
|
||||
* Mirrors PHP's `extractNamespaceViaScanner` (issue #1741). */
|
||||
export function extractCsharpStructureViaScanner(content: string): CsharpFileStructure {
|
||||
/** Incremental form of {@link extractCsharpStructureViaScanner}: feed lines one
|
||||
* at a time via `pushLine` (in source order), then read the accumulated
|
||||
* structure with `result()`. Lets a caller stream a file off disk
|
||||
* (`createReadStream` + `readline`) and scan it for `namespace` / `using
|
||||
* static` declarations in CONSTANT memory rather than buffering the whole file
|
||||
* into a string — the line splitting and per-line matching are identical, so a
|
||||
* streamed scan yields the same result as scanning the full content. The line
|
||||
* terminator must be stripped (as `readline` does, or `String.split('\n')`); a
|
||||
* trailing `\r` on a CRLF line is inert to both the matchers and the lexer. */
|
||||
export interface CsharpStructureLineScanner {
|
||||
pushLine(line: string): void;
|
||||
result(): CsharpFileStructure;
|
||||
}
|
||||
|
||||
/** Create a fresh stateful line scanner — see {@link CsharpStructureLineScanner}. */
|
||||
export function createCsharpStructureScanner(): CsharpStructureLineScanner {
|
||||
const namespaces: string[] = [];
|
||||
const usingStaticPaths: string[] = [];
|
||||
let state: CsScanState = 'code';
|
||||
let rawFence = 0;
|
||||
for (const line of content.split('\n')) {
|
||||
// Only match when the line START is real code — keywords reached while
|
||||
// inside a block comment / multi-line string are skipped.
|
||||
if (state === 'code') {
|
||||
const ns = CS_NAMESPACE_RE.exec(line);
|
||||
if (ns !== null) {
|
||||
namespaces.push(ns[1]!);
|
||||
} else {
|
||||
const us = CS_USING_STATIC_RE.exec(line);
|
||||
if (us !== null) usingStaticPaths.push(us[1]!);
|
||||
return {
|
||||
pushLine(line: string): void {
|
||||
// Only match when the line START is real code — keywords reached while
|
||||
// inside a block comment / multi-line string are skipped.
|
||||
if (state === 'code') {
|
||||
const ns = CS_NAMESPACE_RE.exec(line);
|
||||
if (ns !== null) {
|
||||
namespaces.push(ns[1]!);
|
||||
} else {
|
||||
const us = CS_USING_STATIC_RE.exec(line);
|
||||
if (us !== null) usingStaticPaths.push(us[1]!);
|
||||
}
|
||||
}
|
||||
}
|
||||
[state, rawFence] = advanceCsScanState(line, state, rawFence);
|
||||
}
|
||||
return { namespaces, usingStaticPaths };
|
||||
[state, rawFence] = advanceCsScanState(line, state, rawFence);
|
||||
},
|
||||
result(): CsharpFileStructure {
|
||||
return { namespaces, usingStaticPaths };
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export function extractCsharpStructureViaScanner(content: string): CsharpFileStructure {
|
||||
const scanner = createCsharpStructureScanner();
|
||||
for (const line of content.split('\n')) scanner.pushLine(line);
|
||||
return scanner.result();
|
||||
}
|
||||
|
||||
/** Build a structural view of a C# file. Prefers `cachedTree` (handed in
|
||||
|
|
|
|||
|
|
@ -609,24 +609,27 @@ describe('loadCsharpResolutionConfig — one-pass namespace scan (#1881)', () =>
|
|||
}
|
||||
});
|
||||
|
||||
it('skips an oversized .cs file and fails open via truncation (#1881)', async () => {
|
||||
// A .cs file larger than the per-file size cap is skipped unread so the
|
||||
// always-on scan can't pull an unbounded buffer into memory. Its namespace
|
||||
// is then missing, so `truncated` trips and the gate fails OPEN rather than
|
||||
// wrongly suppress an import declared there. Sized to the real cap — do NOT
|
||||
// lower the production cap for the test.
|
||||
it('streams a large .cs file end-to-end, collecting namespaces past the old size cap (#1881)', async () => {
|
||||
// The namespace scan streams each file, so a `.cs` far larger than the old
|
||||
// per-file size cap is read end-to-end in constant memory instead of being
|
||||
// skipped. A namespace at the START and one at the very END (well past the
|
||||
// old cap boundary) must BOTH be collected, and `truncated` must stay false
|
||||
// — a big generated file no longer disables the #1881 gate repo-wide.
|
||||
const cap = getMaxFileSizeBytes();
|
||||
const oversized = `namespace Deep.Ns;\n${'// pad\n'.repeat(Math.ceil(cap / 7) + 1)}`;
|
||||
const padLine = '// pad pad pad pad pad pad\n';
|
||||
const padding = padLine.repeat(Math.ceil((cap * 3) / padLine.length));
|
||||
const huge = `namespace Generated.Head;\n${padding}namespace Generated.Tail { }\n`;
|
||||
const root = await makeTempRepo({
|
||||
'Shallow.cs': 'namespace Shallow.Ns;',
|
||||
'Huge.cs': oversized,
|
||||
'Hand.cs': 'namespace Hand.Written;',
|
||||
'Generated.cs': huge,
|
||||
});
|
||||
try {
|
||||
const config = await loadCsharpResolutionConfig(root);
|
||||
const ns = config.namespaces!;
|
||||
expect(ns.truncated).toBe(true);
|
||||
expect(ns.declaredNamespaces!.has('Shallow.Ns')).toBe(true);
|
||||
expect(ns.declaredNamespaces!.has('Deep.Ns')).toBe(false);
|
||||
expect(ns.truncated).toBe(false);
|
||||
expect(ns.declaredNamespaces!.has('Hand.Written')).toBe(true);
|
||||
expect(ns.declaredNamespaces!.has('Generated.Head')).toBe(true);
|
||||
expect(ns.declaredNamespaces!.has('Generated.Tail')).toBe(true);
|
||||
} finally {
|
||||
await fsp.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue