diff --git a/DOCS.md b/DOCS.md index 3bdd637..44790b7 100644 --- a/DOCS.md +++ b/DOCS.md @@ -5792,7 +5792,7 @@ A truncated or invalid lockfile stops the command. Warning codes cover condition | Code | Command | Meaning | | --- | --- | --- | -| `VG_WARN_PARSE_FAILED` | `vg build` | A source file failed to parse. The map continues without its symbols. | +| `VG_WARN_PARSE_FAILED` | `vg build`, `vg scan` | A supported-language source file failed to parse. The message names the file and the language. The map continues without that file's symbols. Correct the file or exclude the path with `--exclude`. | | `VG_WARN_BUILD_FILE_OVERSIZE` | `vg build` | A file exceeded the per-file size cap and was left out of the map. | | `VG_WARN_TSC_RESOLVER_SKIPPED` | `vg build` | The TypeScript resolver was skipped because the corpus exceeded its file cap. | | `VG_WARN_YAML_PARSE_FAILED` | `vg build` | YAML for an infrastructure file failed to parse. | diff --git a/src/engine/build.ts b/src/engine/build.ts index 3cf2b74..333d060 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -52,6 +52,7 @@ import type { ResolveResult } from './resolve.js'; import { fileRolesFromParses } from './ast-roles.js'; import type { AstRoleHit } from '../core-open/scanners/architecture/ast-roles.js'; import { stampWarning, WARNING_CODES, type CodedWarning } from '../core-open/warnings.js'; +import { isParseFailureWarning } from './parse-warning.js'; import { assembleEngineWarnings } from './warning-codes.js'; export interface BuildOptions { @@ -310,7 +311,7 @@ export async function buildGraph(options: BuildOptions): Promise { // corrupted wasm heap) would otherwise poison the cache for that content // hash and every later build would reuse the empty parse instead of // re-parsing the file. - if (p.defs.length === 0 && p.warnings?.some((w) => w.startsWith('parse failed:'))) continue; + if (p.defs.length === 0 && p.warnings?.some((w) => isParseFailureWarning(w))) continue; const st = fileStats.find((f) => f.rel === p.rel); cache.set(p.rel, p, st ? { mtimeMs: st.mtimeMs, size: st.size } : undefined); } diff --git a/src/engine/languages.ts b/src/engine/languages.ts index 81a52ee..20321b6 100644 --- a/src/engine/languages.ts +++ b/src/engine/languages.ts @@ -84,8 +84,8 @@ export const LANGUAGES: LanguageDef[] = [ { id: 'ex', label: 'Elixir', extensions: ['.ex', '.exs'], grammarFile: 'tree-sitter-elixir' }, // Known limitation: the bundled bash grammar's external scanner throws under // web-tree-sitter 0.25.10 on `case`/heredoc constructs. Such files degrade - // gracefully (per-file empty parse + a surfaced warning, never a build crash); - // functions in case/heredoc-free scripts extract normally. + // gracefully (per-file empty parse + a warning that names the file, never a + // build crash); functions in case/heredoc-free scripts extract normally. { id: 'sh', label: 'Shell', extensions: ['.sh', '.bash'], grammarFile: 'tree-sitter-bash' }, { id: 'zig', label: 'Zig', extensions: ['.zig'], grammarFile: 'tree-sitter-zig' }, { id: 'c', label: 'C', extensions: ['.c'], grammarFile: 'tree-sitter-c' }, diff --git a/src/engine/parse-warning.ts b/src/engine/parse-warning.ts new file mode 100644 index 0000000..837026c --- /dev/null +++ b/src/engine/parse-warning.ts @@ -0,0 +1,34 @@ +import { formatWarningLine, stampWarning, WARNING_CODES, warningPathLabel, type CodedWarning } from '../core-open/warnings.js'; +import { langById } from './languages.js'; + +/** + * Deterministic warning when a supported-language file cannot be parsed. + * + * The sentence is the whole operator-facing message: repo-relative path, + * language, and what to do next. It never includes a stack, a parser + * exception, or file contents — those can leak internals or source. + * Same path and language always produce the same string. + */ +export function parseFailureWarning(rel: string, langId: string): string { + const language = langById(langId)?.label ?? langId; + const file = warningPathLabel(rel); + return stampWarning( + WARNING_CODES.PARSE_FAILED, + `${file} (${language}): parse failed. The map continues without this file's symbols. Correct the file or exclude the path with --exclude.`, + ); +} + +/** True for a stamped parse-failure warning, including the older `parse failed:` prefix. */ +export function isParseFailureWarning(warning: string): boolean { + return warning.includes(`[${WARNING_CODES.PARSE_FAILED}]`) || warning.startsWith('parse failed:'); +} + +/** Stderr lines for parse failures, in the order `warnings` already has. */ +export function parseFailureWarningLines(warnings: readonly CodedWarning[]): string[] { + const lines: string[] = []; + for (const warning of warnings) { + if (warning.code !== WARNING_CODES.PARSE_FAILED) continue; + lines.push(formatWarningLine(warning)); + } + return lines; +} diff --git a/src/engine/parse-worker.ts b/src/engine/parse-worker.ts index 4790498..abd1a0f 100644 --- a/src/engine/parse-worker.ts +++ b/src/engine/parse-worker.ts @@ -2,7 +2,7 @@ import * as fs from 'node:fs'; import { parseSource } from './parse.js'; import { setGrammarsOverride, resetParser } from './grammars.js'; import type { FileParse } from './types.js'; -import { stampWarning, WARNING_CODES } from '../core-open/warnings.js'; +import { parseFailureWarning } from './parse-warning.js'; /** * tinypool worker entry. Receives a chunk of files, reads and parses each, and @@ -32,9 +32,11 @@ export default async function run(payload: ParsePayload): Promise { try { const source = fs.readFileSync(task.abs, 'utf8'); out.push(await parseSource(task.rel, task.lang, source)); - } catch (err) { + } catch { // A wasm-level parse crash can leave the language's reused parser // mid-state; drop it so the failure stays contained to this file. + // The exception text is intentionally dropped: it can be a wasm stack + // or a snippet of source. The warning names the file instead. resetParser(task.lang); out.push({ rel: task.rel, @@ -47,7 +49,7 @@ export default async function run(payload: ParsePayload): Promise { heritage: [], typeRefs: [], guards: [], - warnings: [stampWarning(WARNING_CODES.PARSE_FAILED, `parse failed: ${(err as Error).message}`)], + warnings: [parseFailureWarning(task.rel, task.lang)], }); } } diff --git a/src/engine/parse.ts b/src/engine/parse.ts index 88c1839..57d3214 100644 --- a/src/engine/parse.ts +++ b/src/engine/parse.ts @@ -14,6 +14,7 @@ function effectsRegexEnabled(): boolean { return !(v === '0' || v === 'false'); } import { extractDutiesWithCandidates, fileBindings, type Bindings } from './duties.js'; +import { parseFailureWarning } from './parse-warning.js'; import type { FileParse, RawCall, RawDef, RawGuard, RawHeritage, RawImport, RawTypeRef } from './types.js'; /** @@ -288,7 +289,10 @@ export async function parseSource( const language = await loadLanguage(effLangId); const parser = await parserFor(def); const tree = parser.parse(text); - if (!tree) return result; + if (!tree) { + result.warnings = [parseFailureWarning(rel, langId)]; + return result; + } const root = tree.rootNode; // --- definitions --- @@ -467,10 +471,44 @@ export async function parseSource( const roles = extractAstRolesFromTree(rel, effLangId, language, root, text); if (roles) result.roles = roles; + // Tree-sitter returns a tree for broken syntax instead of throwing. When + // that tree is only ERROR nodes and nothing was extracted, the file was + // skipped. `hasError` alone is not that signal: some grammars set it on + // valid nodes, and a reused parser can set it on a later file that parsed + // cleanly on its own (Lua `return 1`). + if (treeIsOnlyErrors(root) && parseYieldedNothing(result)) { + result.warnings = [...(result.warnings ?? []), parseFailureWarning(rel, langId)]; + } + tree.delete(); return result; } +function treeIsOnlyErrors(root: Node): boolean { + if (!root.hasError) return false; + if (root.type === 'ERROR') return true; + const count = root.namedChildCount; + if (count === 0) return false; + for (let i = 0; i < count; i++) { + const child = root.namedChild(i); + if (!child || child.type !== 'ERROR') return false; + } + return true; +} + +function parseYieldedNothing(result: FileParse): boolean { + return ( + result.defs.length === 0 && + result.calls.length === 0 && + result.imports.length === 0 && + result.heritage.length === 0 && + (result.typeRefs?.length ?? 0) === 0 && + (result.guards?.length ?? 0) === 0 && + (result.namespaces?.length ?? 0) === 0 && + (result.roles?.length ?? 0) === 0 + ); +} + /** * Dart splits a function into sibling signature + body nodes (and wraps class * methods in a method_signature). Return the trailing function_body so the def diff --git a/src/engine/pool.ts b/src/engine/pool.ts index 6759563..2498203 100644 --- a/src/engine/pool.ts +++ b/src/engine/pool.ts @@ -7,7 +7,7 @@ import { setGrammarsOverride, resetParser } from './grammars.js'; import { checkMemoryBudget, envJobs, envWorkerHeapMb, ResourceLimitError } from './limits.js'; import type { DiscoveredFile } from './discover.js'; import type { FileParse } from './types.js'; -import { stampWarning, WARNING_CODES } from '../core-open/warnings.js'; +import { parseFailureWarning } from './parse-warning.js'; import type { ParseTask } from './parse-worker.js'; import { parsePoolGuard, type ManagedPool } from './pool-guard.js'; @@ -81,11 +81,13 @@ async function parseInline(files: DiscoveredFile[], options: ParseOptions): Prom try { const source = fs.readFileSync(file.abs, 'utf8'); out.push(await parseSource(file.rel, file.lang.id, source)); - } catch (err) { + } catch { // A wasm-level parse crash can leave the language's reused parser // mid-state; drop it so the failure stays contained to this file. + // The exception text is intentionally dropped: it can be a wasm stack + // or a snippet of source. The warning names the file instead. resetParser(file.lang.id); - out.push(emptyParse(file, stampWarning(WARNING_CODES.PARSE_FAILED, `parse failed: ${(err as Error).message}`))); + out.push(emptyParse(file, parseFailureWarning(file.rel, file.lang.id))); } onProgress?.(out.length, files.length); if (out.length % MEM_CHECK_EVERY === 0) checkMemoryBudget('parse', memoryBudgetMb); diff --git a/src/reporting/commands/scan.ts b/src/reporting/commands/scan.ts index 555088c..ad467e5 100644 --- a/src/reporting/commands/scan.ts +++ b/src/reporting/commands/scan.ts @@ -52,6 +52,7 @@ import { emitIngestIdLine, emitDriftScoreLine } from '../utils/ingest-id-output. import { formatUploadHttpFailure, uploadScanArtifact } from '../utils/upload.js'; import { redactForDisplay } from '../../core-open/utils/redact.js'; import { buildGraph } from '../../engine/build.js'; +import { parseFailureWarningLines } from '../../engine/parse-warning.js'; import { writeArtifacts, resolveGraphPath } from '../../engine/artifacts.js'; import { readHaileSidecar } from '../../engine/haile/sidecar.js'; import { isUsableHaileSymbol } from '../../engine/haile/format.js'; @@ -709,6 +710,11 @@ export const scanCommand = new Command('scan') exclude: opts.exclude, onParseProgress: (done, total) => report(done, total, 'parsing'), }); + // Same coded warning `vg build` prints. Stderr only, so scan JSON + // on stdout stays the artifact. One line per file, already sorted. + for (const line of parseFailureWarningLines(result.codedWarnings)) { + console.error(chalk.yellow(line)); + } builtGraph = result.graph; const written = writeArtifacts(result.graph, { root: rootDir }); if (written.architecturePolicyError) console.error(chalk.red(`\narchitecture policy: ${written.architecturePolicyError}`)); diff --git a/test/adversarial-fixtures.test.ts b/test/adversarial-fixtures.test.ts index 73d039d..5e457dd 100644 --- a/test/adversarial-fixtures.test.ts +++ b/test/adversarial-fixtures.test.ts @@ -96,7 +96,7 @@ for (const lang of buildableLangs) { // (c) no parse failures. `BuildResult.warnings` is the engine's only // per-file failure signal (parse/query problems surface as - // "parse failed: ..." warnings from the worker); it must be empty. + // VG_WARN_PARSE_FAILED warnings); it must be empty. expect(first.warnings).toEqual([]); expect(first.warnings.filter((w) => /query|parse failed/i.test(w))).toEqual([]); diff --git a/test/parse-failure-warning.test.ts b/test/parse-failure-warning.test.ts new file mode 100644 index 0000000..03beb5c --- /dev/null +++ b/test/parse-failure-warning.test.ts @@ -0,0 +1,81 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { buildGraph } from '../src/engine/build.js'; +import { parseFailureWarning, parseFailureWarningLines } from '../src/engine/parse-warning.js'; +import { serializeGraph } from '../src/engine/serialize.js'; +import { cleanup, makeProject } from './helpers.js'; + +const PIN = '2020-01-01T00:00:00.000Z'; +const dirs: string[] = []; + +afterEach(() => { + while (dirs.length) cleanup(dirs.pop()!); +}); + +const SYNTAX_MSG = + "src/broken.ts (TypeScript): parse failed. The map continues without this file's symbols. Correct the file or exclude the path with --exclude."; +const SHELL_MSG = + "scripts/dispatch.sh (Shell): parse failed. The map continues without this file's symbols. Correct the file or exclude the path with --exclude."; + +function fixture(): string { + const root = makeProject({ + 'src/ok.ts': 'export function ok(): number { return 1; }\n', + 'src/value.ts': 'export const x = 1;\n', + 'src/partial.ts': 'export function kept(): number { return 1; }\n@@@\n', + 'src/broken.ts': 'export function broken(\n', + 'scripts/ok.sh': '#!/bin/sh\nhello() { echo hi; }\nhello\n', + // The bundled shell grammar throws on `case` (web-tree-sitter 0.25.x). + 'scripts/dispatch.sh': 'case "$1" in\n a) echo a ;;\nesac\n', + }); + dirs.push(root); + return root; +} + +function parseFailures(warnings: { code: string; message: string }[]): string[] { + return warnings.filter((warning) => warning.code === 'VG_WARN_PARSE_FAILED').map((warning) => warning.message); +} + +describe('parse failure warning', () => { + it('names the file and language, repeats exactly, and keeps recovered files', async () => { + const root = fixture(); + const opts = { root, generatedAt: PIN, inline: true, noCache: true }; + const first = await buildGraph(opts); + const second = await buildGraph(opts); + + expect(parseFailures(first.codedWarnings)).toEqual([SHELL_MSG, SYNTAX_MSG]); + expect(first.codedWarnings).toEqual(second.codedWarnings); + expect(first.warnings).toEqual(second.warnings); + expect(serializeGraph(first.graph)).toBe(serializeGraph(second.graph)); + + const text = parseFailures(first.codedWarnings).join('\n'); + expect(text).not.toMatch(/resolved is not a function/); + expect(text).not.toMatch(/\n\s+at /); + + const names = first.graph.nodes.map((node) => node.name); + expect(names).toContain('ok'); + expect(names).toContain('kept'); + expect(names).toContain('hello'); + expect(names).not.toContain('broken'); + }); + + it('still warns on a second build when the failed parse is not cached', async () => { + const root = fixture(); + const opts = { root, generatedAt: PIN, inline: true }; + const first = await buildGraph(opts); + const second = await buildGraph(opts); + expect(parseFailures(first.codedWarnings)).toEqual([SHELL_MSG, SYNTAX_MSG]); + expect(parseFailures(second.codedWarnings)).toEqual([SHELL_MSG, SYNTAX_MSG]); + }); + + it('keeps an escaping path to the file name and formats the scan line', () => { + const stored = parseFailureWarning('../secret.ts', 'ts'); + expect(stored).toContain('secret.ts (TypeScript)'); + expect(stored).not.toContain('..'); + expect(stored.endsWith(' [VG_WARN_PARSE_FAILED]')).toBe(true); + expect( + parseFailureWarningLines([ + { code: 'VG_WARN_PARSE_FAILED', message: SYNTAX_MSG }, + { code: 'VG_WARN_BUILD_FILE_OVERSIZE', message: 'src/big.ts: skipped' }, + ]), + ).toEqual([`warning [VG_WARN_PARSE_FAILED]: ${SYNTAX_MSG}`]); + }); +});