Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -68,26 +68,26 @@ GITTENSORY_REVIEW_ENRICHMENT=false
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,commitLint
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,commitLint
#
# Profile defaults:
# fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
# hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild
# testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker
# debugLeftover,sizeSmell,floatingPromise,deepNesting
# debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow
# balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript
# heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight
# typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication
# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch
# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology
# todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting
# commitLint
# errorSwallow,commitLint
# deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency
# hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat
# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot
# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene
# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,commitLint
# conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,commitLint
# END GENERATED REES ANALYZERS

# Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep
Expand Down
22 changes: 22 additions & 0 deletions apps/gittensory-ui/src/lib/rees-analyzers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,28 @@ export const REES_ANALYZERS = [
"Counts braces opened by if/for/while/switch/try/catch/else/do/with, arrow bodies, and function bodies — not object-literal braces. Resets across context lines.",
},
},
{
name: "errorSwallow",
title: "Swallowed errors",
category: "quality",
cost: "local",
defaultEnabled: true,
profiles: ["fast", "balanced", "deep"],
requires: ["files"],
limits: {
maxFindings: 25,
maxLineChars: 2000,
},
docs: {
summary:
"Flags newly-added catch/except blocks that swallow the error — empty body, unused binding, or a bare `return null`.",
looksAt: "Added lines in changed non-test JS/TS/Python source files.",
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
network: "Pure local analyzer. No external network call.",
notes:
"Multiline catch bodies are collected with brace balance. Catches that log, rethrow, or reference the binding are not flagged. Brace counting is character-level (string literals are not stripped).",
},
},
{
name: "commitLint",
title: "Conventional-commit subjects",
Expand Down
26 changes: 26 additions & 0 deletions review-enrichment/analyzer-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,32 @@
"notes": "Counts braces opened by if/for/while/switch/try/catch/else/do/with, arrow bodies, and function bodies — not object-literal braces. Resets across context lines."
}
},
{
"name": "errorSwallow",
"title": "Swallowed errors",
"category": "quality",
"cost": "local",
"defaultEnabled": true,
"profiles": [
"fast",
"balanced",
"deep"
],
"requires": [
"files"
],
"limits": {
"maxFindings": 25,
"maxLineChars": 2000
},
"docs": {
"summary": "Flags newly-added catch/except blocks that swallow the error — empty body, unused binding, or a bare `return null`.",
"looksAt": "Added lines in changed non-test JS/TS/Python source files.",
"reports": "File, line, and kind: empty-catch, unused-binding, or return-null.",
"network": "Pure local analyzer. No external network call.",
"notes": "Multiline catch bodies are collected with brace balance. Catches that log, rethrow, or reference the binding are not flagged. Brace counting is character-level (string literals are not stripped)."
}
},
{
"name": "commitLint",
"title": "Conventional-commit subjects",
Expand Down
214 changes: 214 additions & 0 deletions review-enrichment/src/analyzers/error-swallow.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
// Empty-catch / error-swallow analyzer (#2014). Flags newly-added catch/except blocks that swallow the error
// (empty body, unused binding, or a bare `return null`) — a top source of silent failures. Pure compute over
// added diff lines, no network. Scoped to JS/TS/Python source files; Python `except: pass` is included.
import type { EnrichRequest, ErrorSwallowFinding } from "../types.js";
import { isTestPath } from "./test-ratio.js";

const MAX_FINDINGS = 25;
const MAX_LINE_CHARS = 2000;

const SOURCE_EXTS = new Set(["ts", "tsx", "js", "jsx", "mjs", "cjs", "mts", "cts", "py"]);

const CATCH_OPEN_RE = /catch\s*(?:\(\s*([\w$]+)?\s*\))?\s*\{/;
const PY_EXCEPT_PASS_RE = /^\s*except(?:\s+[\w.]+\s*(?:as\s+(\w+))?)?\s*:\s*pass\s*(?:#.*)?$/;

function isScannablePath(path: string): boolean {
const ext = /\.([^.]+)$/.exec(path)?.[1]?.toLowerCase();
return Boolean(ext && SOURCE_EXTS.has(ext) && !isTestPath(path));
}

function escapeRegExp(value: string): string {
return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&");
}

function referencesBinding(body: string, binding: string): boolean {
const escaped = escapeRegExp(binding);
const bindingRe = new RegExp(`(?<![A-Za-z0-9_$])${escaped}(?![A-Za-z0-9_$])`);
return bindingRe.test(body);
}

function bodySwallowsError(body: string, binding: string | null): ErrorSwallowFinding["kind"] | null {
const trimmed = body.trim();
if (!trimmed) return "empty-catch";
if (/^return\s+null\s*;?$/.test(trimmed)) return "return-null";
if (!binding) return null;
if (/\bthrow\b/.test(trimmed)) return null;
if (/\b(?:console|logger|log|winston|pino|bunyan)\s*[.(]/.test(trimmed)) return null;
if (/\bprint\s*\(/.test(trimmed)) return null;
if (!referencesBinding(trimmed, binding)) return "unused-binding";
return null;
}

function braceBalanceFrom(line: string, openBrace: number): number {
let depth = 0;
for (let i = openBrace; i < line.length; i++) {
const ch = line[i]!;
if (ch === "{") depth++;
else if (ch === "}") depth--;
}
return depth;
}

/** Extract a complete JS/TS catch block from one line using brace balance, or null if incomplete. Pure. */
export function parseCompleteCatchLine(
line: string,
): { binding: string | null; body: string } | null {
const open = CATCH_OPEN_RE.exec(line);
if (!open) return null;
const braceStart = line.indexOf("{", open.index ?? 0);
if (braceStart < 0) return null;

let depth = 0;
for (let i = braceStart; i < line.length; i++) {
const ch = line[i]!;
if (ch === "{") depth++;
else if (ch === "}") depth--;
if (depth === 0) {
return {
binding: open[1] ?? null,
body: line.slice(braceStart + 1, i),
};
}
}
return null;
}

/** Classify one source line for an error-swallow pattern, or null. Pure. */
export function detectErrorSwallow(line: string): ErrorSwallowFinding["kind"] | null {
const pyMatch = PY_EXCEPT_PASS_RE.exec(line);
if (pyMatch) {
return pyMatch[1] ? "unused-binding" : "empty-catch";
}

const complete = parseCompleteCatchLine(line);
if (complete) {
return bodySwallowsError(complete.body, complete.binding);
}

return null;
}

type PendingCatch = {
startLine: number;
binding: string | null;
body: string;
depth: number;
};

function updatePending(pending: PendingCatch, line: string): PendingCatch {
let depth = pending.depth;
for (const ch of line) {
if (ch === "{") depth++;
else if (ch === "}") depth--;
}
return { ...pending, body: `${pending.body}\n${line}`, depth };
}

function flushPending(pending: PendingCatch): ErrorSwallowFinding["kind"] | null {
const body = pending.body.replace(/^\s*\{/, "").replace(/\}\s*$/, "");
return bodySwallowsError(body, pending.binding);
}

type ScanLimits = {
maxFindings?: number;
signal?: AbortSignal;
};

/** Scan one file patch's added lines for swallowed errors, line-cited via hunk headers. Pure. */
export function scanPatchForErrorSwallow(
path: string,
patch: string,
limits: ScanLimits = {},
): ErrorSwallowFinding[] {
const maxFindings = limits.maxFindings ?? MAX_FINDINGS;
if (maxFindings <= 0 || !isScannablePath(path)) return [];
const findings: ErrorSwallowFinding[] = [];
let newLine = 0;
let inHunk = false;
let pending: PendingCatch | null = null;

const pushFinding = (line: number, kind: ErrorSwallowFinding["kind"]) => {
findings.push({ file: path, line, kind });
};

for (const line of patch.split("\n")) {
if (limits.signal?.aborted) throw new Error("analyzer_aborted");
const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
if (hunk) {
newLine = Number(hunk[1]);
inHunk = true;
pending = null;
continue;
}
if (!inHunk) continue;

if (line.startsWith("+")) {
const body = line.slice(1);
if (body.length <= MAX_LINE_CHARS) {
if (pending) {
pending = updatePending(pending, body);
if (pending.depth <= 0) {
const kind = flushPending(pending);
if (kind) {
pushFinding(pending.startLine, kind);
if (findings.length >= maxFindings) return findings;
}
pending = null;
}
} else {
const kind = detectErrorSwallow(body);
if (kind) {
pushFinding(newLine, kind);
if (findings.length >= maxFindings) return findings;
} else {
const open = CATCH_OPEN_RE.exec(body);
if (open) {
const braceIndex = body.indexOf("{", open.index ?? 0);
if (braceIndex >= 0) {
const depth = braceBalanceFrom(body, braceIndex);
if (depth > 0) {
pending = {
startLine: newLine,
binding: open[1] ?? null,
body: body.slice(braceIndex),
depth,
};
}
}
}
}
}
}
newLine++;
} else if (!line.startsWith("-") && !line.startsWith("\\")) {
pending = null;
newLine++;
} else {
pending = null;
}

if (findings.length >= maxFindings) return findings;
}

return findings;
}

/** Analyzer entrypoint: scan every changed scannable file's added lines for swallowed errors. */
export async function scanErrorSwallow(
req: EnrichRequest,
signal?: AbortSignal,
): Promise<ErrorSwallowFinding[]> {
const findings: ErrorSwallowFinding[] = [];
for (const file of req.files ?? []) {
if (signal?.aborted) throw new Error("analyzer_aborted");
if (!file.patch) continue;
for (const finding of scanPatchForErrorSwallow(file.path, file.patch, {
maxFindings: MAX_FINDINGS - findings.length,
signal,
})) {
findings.push(finding);
if (findings.length >= MAX_FINDINGS) return findings;
}
}
return findings;
}
30 changes: 30 additions & 0 deletions review-enrichment/src/analyzers/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { scanMagicNumbers } from "./magic-number.js";
import { scanConflictMarkers } from "./conflict-marker.js";
import { scanDebugLeftover } from "./debug-leftover.js";
import { scanDeepNesting } from "./deep-nesting.js";
import { scanErrorSwallow } from "./error-swallow.js";
import { scanFloatingPromise } from "./floating-promise.js";
import { scanSizeSmell } from "./size-smell.js";
import { scanCommitLint } from "./commit-lint.js";
Expand Down Expand Up @@ -1106,6 +1107,35 @@ export const ANALYZER_DESCRIPTORS = [
},
run: (req, { signal }) => scanDeepNesting(req, signal),
}),
descriptor({
name: "errorSwallow",
title: "Swallowed errors",
category: "quality",
cost: "local",
defaultEnabled: true,
requires: ["files"],
limits: { maxFindings: 25, maxLineChars: 2000 },
docs: {
summary:
"Flags newly-added catch/except blocks that swallow the error — empty body, unused binding, or a bare `return null`.",
looksAt: "Added lines in changed non-test JS/TS/Python source files.",
reports: "File, line, and kind: empty-catch, unused-binding, or return-null.",
network: "Pure local analyzer. No external network call.",
notes:
"Multiline catch bodies are collected with brace balance. Catches that log, rethrow, or reference the binding are not flagged. Brace counting is character-level (string literals are not stripped).",
},
render: (findings, helpers) => {
if (!findings.length) return [];
const lines = ["### Swallowed errors (empty catch / unused binding / return null)"];
for (const item of findings) {
lines.push(
`- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.kind)}`,
);
}
return lines;
},
run: (req, { signal }) => scanErrorSwallow(req, signal),
}),
descriptor({
name: "commitLint",
title: "Conventional-commit subjects",
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ export function renderBrief(
lines.push(...renderDescriptorSection("sizeSmell", findings.sizeSmell));
lines.push(...renderDescriptorSection("floatingPromise", findings.floatingPromise));
lines.push(...renderDescriptorSection("deepNesting", findings.deepNesting));
lines.push(...renderDescriptorSection("errorSwallow", findings.errorSwallow));
lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl));
lines.push(...renderDescriptorSection("commitLint", findings.commitLint));

Expand Down
9 changes: 9 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,14 @@ export interface SizeSmellFinding {
name?: string;
}

/** A swallowed-error catch/except block newly added in the diff (#2014, part of #1499).
* Reports file, line, and kind only — never catch body content. */
export interface ErrorSwallowFinding {
file: string;
line: number;
kind: "empty-catch" | "unused-binding" | "return-null";
}

/** Deep control-flow nesting newly added in the diff (#2030, part of #1499).
* Reports file, line, measured depth, and threshold — never source content. */
export interface DeepNestingFinding {
Expand Down Expand Up @@ -570,6 +578,7 @@ export interface BriefFindings {
sizeSmell?: SizeSmellFinding[];
floatingPromise?: FloatingPromiseFinding[];
deepNesting?: DeepNestingFinding[];
errorSwallow?: ErrorSwallowFinding[];
hardcodedUrl?: HardcodedUrlFinding[];
commitLint?: CommitLintFinding[];
}
Expand Down
1 change: 1 addition & 0 deletions review-enrichment/test/analyzer-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const EXPECTED_ANALYZERS = [
"sizeSmell",
"floatingPromise",
"deepNesting",
"errorSwallow",
"commitLint",
];

Expand Down
Loading
Loading