Skip to content

Commit 467a7fd

Browse files
committed
fix(observability-map): stop a dead statement blinding the catch walk
catchClauseEvidence raised its exited flag off containsExit, which is true of a provably dead statement itself, so prepending if (false) { throw e; } to a deciding clause blinded the walk to the real classification below it: 78 real routes turned pass into a swallow verdict, the same set for all eleven dead-* corpus spellings. The returns veto had the same defect via containsReturn, regressing 11 rethrow-only routes from n/a to fail. The walk's four evidence reads now go through a liveness fold (literalTruth folding literal guards only) so a dead branch contributes nothing, while an undecidable guard keeps the containment answer and refusal stays intact. Real-tree report is byte-identical (global 19, measured 412); the twelve dead-prepend corpus entries measure falls 0 rises 0 in the mirror direction. New corpus entry dead-if-false-return covers the returns half at tree scale. containsReturn is deleted: its only read site is now the live fold.
1 parent c53cbfd commit 467a7fd

3 files changed

Lines changed: 274 additions & 15 deletions

File tree

internal-packages/observability-map/src/mutations.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,17 @@ export const MUTATIONS: Mutation[] = [
665665
"splice if (1 === 2) { throw e; } into every catch",
666666
(e) => `if (1 === 2) { throw ${e}; }`
667667
),
668+
// The returns half of the dead-prepend family. The eleven entries above put a dead THROW in;
669+
// this one puts a dead RETURN in, which used to veto `rethrows` through the containment read
670+
// and turn a rethrow-only clause into a swallow verdict on 11 real routes. Same blinding class
671+
// as `dead-if-false`, so like that entry it is not additive: it fakes no signal, it used to
672+
// destroy real signal.
673+
prependToEveryCatch(
674+
"dead-if-false-return",
675+
"preserving",
676+
"splice if (false) { return null; } into every catch",
677+
() => `if (false) { return null; }`
678+
),
668679
prependToEveryCatch(
669680
"registered-throw",
670681
"preserving",

internal-packages/observability-map/src/scan.test.ts

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,113 @@ describe("scanFile: catch clause evidence", () => {
758758
});
759759
});
760760

761+
// The mirror of the family above. Each dead spelling earns nothing, and it must also COST
762+
// nothing: `containsExit` was true of the dead statement itself, so prepending one raised the
763+
// `exited` flag and blinded the walk to the real classification below it, turning a pass into a
764+
// swallow verdict on 78 real routes. `containsLiveExit` folds the literal guard and sees no live
765+
// exit, so the deciding statements keep their credit. The spellings are the CORPUS spellings
766+
// from `dead-*` in `mutations.ts`, not the `DEAD_SHAPES` table's: that table's inner-try twin is
767+
// `try { doThing(); } catch { throw e; }`, which is NOT provably dead (`doThing` may throw and
768+
// the rethrow then runs), so conservatively raising the flag after it is correct and it gets no
769+
// twin here.
770+
describe("dead and deferred code prepended to a deciding catch does not blind it", () => {
771+
const deciding = (mutation: string) => `
772+
export async function loader() {
773+
try {
774+
return await prisma.thing.findMany();
775+
} catch (e) {
776+
${mutation}
777+
if (e instanceof Error) { return new Response(null, { status: 400 }); }
778+
return new Response(null, { status: 500 });
779+
}
780+
}
781+
`;
782+
783+
it("decides as a baseline with no mutation", () => {
784+
const ep = scanFile("x.ts", deciding(""));
785+
expect(ep!.catches[0]!.branches).toBe(true);
786+
});
787+
788+
const DEAD_PREPENDS: Array<[string, string]> = [
789+
["if (false)", "if (false) { throw e; }"],
790+
["while (false)", "while (false) { throw e; }"],
791+
["for (;false;)", "for (;false;) { throw e; }"],
792+
["if (true) else", "if (true) { 0; } else { throw e; }"],
793+
["switch with no matching case", "switch (1) { case 2: throw e; }"],
794+
["an inner try over a literal", "try { 0; } catch { throw e; }"],
795+
["for...of an empty array", "for (const obsMapItem of []) { throw e; }"],
796+
["for...in an empty object", "for (const obsMapKey in {}) { throw e; }"],
797+
["if on an empty string", 'if ("") { throw e; }'],
798+
["if on a negated literal", "if (!true) { throw e; }"],
799+
["if on a constant comparison", "if (1 === 2) { throw e; }"],
800+
];
801+
802+
for (const [label, shape] of DEAD_PREPENDS) {
803+
it(`keeps branches true past a dead throw inside ${label}`, () => {
804+
const ep = scanFile("x.ts", deciding(shape));
805+
expect(ep!.catches[0]!.branches).toBe(true);
806+
});
807+
}
808+
809+
// The composed shape: the dead if wrapped in a bare block, which the walk enters. The block's
810+
// own live-exit read has to fold too, or entering it re-raises the flag the fold lowered.
811+
it("keeps branches true past a dead throw in a block around an if (false)", () => {
812+
const ep = scanFile("x.ts", deciding("{ if (false) { throw e; } }"));
813+
expect(ep!.catches[0]!.branches).toBe(true);
814+
});
815+
816+
// The returns half. A dead `return null;` must not veto the rethrow: `containsReturn` saw the
817+
// return token inside `if (false)` and turned a rethrow-only clause into a swallow verdict,
818+
// which regressed 11 real routes from not-applicable to fail. `dead-if-false-return` in the
819+
// mutation corpus is the tree-scale version.
820+
it("still sets rethrows past a dead return in an if (false) arm", () => {
821+
const ep = scanFile(
822+
"x.ts",
823+
`export async function loader() {
824+
try { return await prisma.thing.findMany(); }
825+
catch (e) {
826+
if (false) { return null; }
827+
throw e;
828+
}
829+
}`
830+
);
831+
expect(ep!.catches[0]!.rethrows).toBe(true);
832+
});
833+
834+
// Negative controls: the fold only withholds blindness, it must never withhold refusal.
835+
// An always-true guard really can run its throw, so the error test after it stays dead.
836+
it("still refuses an error test after an always-true spelling that throws", () => {
837+
const ep = scanFile(
838+
"x.ts",
839+
`export async function loader() {
840+
try { return await prisma.thing.findMany(); }
841+
catch (e) {
842+
if (!false) { throw e; }
843+
if (e instanceof Error) { return new Response(null, { status: 400 }); }
844+
}
845+
}`
846+
);
847+
expect(ep!.catches[0]!.branches).toBe(false);
848+
});
849+
850+
// The fall-through slice: `case 1` matches and runs on into `case 2`, so the return is live
851+
// and vetoes the rethrow. Misreading the slice as dead would blind the returns veto, which is
852+
// the direction that hands out credit.
853+
it("reads a switch fall-through onto a live return as live", () => {
854+
const ep = scanFile(
855+
"x.ts",
856+
`export async function loader() {
857+
try { return await prisma.thing.findMany(); }
858+
catch (e) {
859+
switch (1) { case 1: 0; case 2: return null; }
860+
throw e;
861+
}
862+
}`
863+
);
864+
expect(ep!.catches[0]!.rethrows).toBe(false);
865+
});
866+
});
867+
761868
// S1. The other end of the same problem. `reachableStatements` used to cut the statement list
762869
// only on a BARE `return`/`throw`, while the walk descended into blocks and `do` bodies, so a
763870
// `throw e;` written after a nested construct that had already returned was still read as the
@@ -803,8 +910,12 @@ describe("scanFile: catch clause evidence", () => {
803910
//
804911
// This asks a weaker question than `definitelyExits` does, and on purpose: "could this have
805912
// exited", not "must it have". That is why `if (true)`, a labelled block, a `for...of` and a
806-
// `while` are all on the list even though none of them is guaranteed to run its body. Nothing
807-
// here evaluates a condition, and none of these needs one evaluated.
913+
// `while` are all on the list even though none of them is guaranteed to run its body. The flag
914+
// is read through `containsLiveExit`, which folds LITERAL guards only, so every wrapper here
915+
// still counts: `if (true)` descends its then-arm and finds the return, and a loop guarded by
916+
// an identifier keeps the plain containment answer. What the fold withholds is the provably
917+
// dead statement raising the flag itself, which is the twin family in `dead and deferred code
918+
// prepended to a deciding catch does not blind it`.
808919
//
809920
// The ordering is the whole trick and it is easy to get backwards. The flag is raised at the
810921
// END of each statement, after that statement's own branch check. Raising it first makes every

internal-packages/observability-map/src/scan.ts

Lines changed: 150 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -561,15 +561,6 @@ function reachableStatements(statements: readonly ts.Statement[]): readonly ts.S
561561
return index === -1 ? statements : statements.slice(0, index + 1);
562562
}
563563

564-
/** Whether the clause returns anywhere at all, nested functions excluded. A clause with a `return`
565-
* on any path has an exit that is not the throw, so the error does not leave it the way it arrived.
566-
*/
567-
function containsReturn(node: ts.Node): boolean {
568-
if (ts.isFunctionLike(node)) return false;
569-
if (ts.isReturnStatement(node)) return true;
570-
return ts.forEachChild(node, containsReturn) === true;
571-
}
572-
573564
/** Whether the tree rooted at `node` contains a `return` or a `throw` of its own, not counting one
574565
* inside a nested function. What separates an arm that takes the error somewhere from an arm that
575566
* runs and falls back into the clause's single common exit. */
@@ -579,6 +570,138 @@ function containsExit(node: ts.Node): boolean {
579570
return ts.forEachChild(node, containsExit) === true;
580571
}
581572

573+
/**
574+
* Literal truthiness of a guard expression: true, false, or null when not decidable from the
575+
* token alone. Only literal tokens fold; an identifier, call, bigint, `&&`, `||` or a template
576+
* literal with substitutions is always null, so a live guard can never be read as dead. The
577+
* always-true side is pinned by `still refuses an error test after an always-true spelling that
578+
* throws` and the fall-through slice by `reads a switch fall-through onto a live return as live`.
579+
*/
580+
function literalTruth(expr: ts.Expression): boolean | null {
581+
const target = unwrap(expr);
582+
if (target.kind === ts.SyntaxKind.TrueKeyword) return true;
583+
if (target.kind === ts.SyntaxKind.FalseKeyword) return false;
584+
if (target.kind === ts.SyntaxKind.NullKeyword) return false;
585+
if (ts.isStringLiteral(target) || ts.isNoSubstitutionTemplateLiteral(target)) {
586+
return target.text !== "";
587+
}
588+
if (ts.isNumericLiteral(target)) return Number(target.text) !== 0;
589+
if (ts.isPrefixUnaryExpression(target) && target.operator === ts.SyntaxKind.ExclamationToken) {
590+
const inner = literalTruth(target.operand);
591+
return inner === null ? null : !inner;
592+
}
593+
if (ts.isBinaryExpression(target)) {
594+
const op = target.operatorToken.kind;
595+
if (
596+
op === ts.SyntaxKind.EqualsEqualsEqualsToken ||
597+
op === ts.SyntaxKind.ExclamationEqualsEqualsToken
598+
) {
599+
const left = literalValue(target.left);
600+
const right = literalValue(target.right);
601+
if (left === undefined || right === undefined) return null;
602+
const equal = left === right;
603+
return op === ts.SyntaxKind.EqualsEqualsEqualsToken ? equal : !equal;
604+
}
605+
}
606+
return null;
607+
}
608+
609+
/** The value of a literal token, or undefined when the expression is not a bare literal. */
610+
function literalValue(expr: ts.Expression): string | number | boolean | null | undefined {
611+
const target = unwrap(expr);
612+
if (target.kind === ts.SyntaxKind.TrueKeyword) return true;
613+
if (target.kind === ts.SyntaxKind.FalseKeyword) return false;
614+
if (target.kind === ts.SyntaxKind.NullKeyword) return null;
615+
if (ts.isStringLiteral(target) || ts.isNoSubstitutionTemplateLiteral(target)) return target.text;
616+
if (ts.isNumericLiteral(target)) return Number(target.text);
617+
return undefined;
618+
}
619+
620+
/** Whether a try block could throw at all: false only when every statement is an expression
621+
* statement over a bare literal, the one shape that provably cannot raise. */
622+
function tryBlockMayThrow(block: ts.Block): boolean {
623+
return !block.statements.every(
624+
(s) => ts.isExpressionStatement(s) && literalValue(s.expression) !== undefined
625+
);
626+
}
627+
628+
/**
629+
* `containsExit`, minus exits that sit in a provably-untaken branch. `if (false) { throw e; }`
630+
* contains an exit and can never run one; treating it as an exit is what let a dead statement
631+
* blind the walk to the real classification below it, prepending one to a deciding clause turned
632+
* its pass into a swallow verdict on 78 real routes. Folds literal guards only, so an unknown
633+
* condition keeps the containsExit answer, which is the direction that refuses credit rather than
634+
* inventing it. The mirror twins under `dead and deferred code prepended to a deciding catch does
635+
* not blind it` hold the recovered half; the `BRANCH_EXITED` family holds the refusing half.
636+
*/
637+
function containsLiveWhere(root: ts.Node, hit: (n: ts.Node) => boolean): boolean {
638+
const walk = (node: ts.Node): boolean => {
639+
if (ts.isFunctionLike(node)) return false;
640+
if (hit(node)) return true;
641+
if (ts.isIfStatement(node)) {
642+
const truth = literalTruth(node.expression);
643+
if (truth === true) return walk(node.thenStatement);
644+
if (truth === false) {
645+
return node.elseStatement !== undefined && walk(node.elseStatement);
646+
}
647+
return (
648+
walk(node.thenStatement) || (node.elseStatement !== undefined && walk(node.elseStatement))
649+
);
650+
}
651+
if (ts.isWhileStatement(node)) {
652+
if (literalTruth(node.expression) === false) return false;
653+
return walk(node.statement);
654+
}
655+
if (ts.isForStatement(node)) {
656+
if (node.condition !== undefined && literalTruth(node.condition) === false) return false;
657+
return ts.forEachChild(node, walk) === true;
658+
}
659+
if (ts.isForOfStatement(node) || ts.isForInStatement(node)) {
660+
const iterable = unwrap(node.expression);
661+
const emptyArray = ts.isArrayLiteralExpression(iterable) && iterable.elements.length === 0;
662+
const emptyObject =
663+
ts.isForInStatement(node) &&
664+
ts.isObjectLiteralExpression(iterable) &&
665+
iterable.properties.length === 0;
666+
if (emptyArray || emptyObject) return false;
667+
return ts.forEachChild(node, walk) === true;
668+
}
669+
if (ts.isSwitchStatement(node)) {
670+
const disc = literalValue(node.expression);
671+
const clauses = node.caseBlock.clauses;
672+
const allLiteral =
673+
disc !== undefined &&
674+
clauses.every((c) => ts.isDefaultClause(c) || literalValue(c.expression) !== undefined);
675+
if (!allLiteral) return clauses.some((c) => c.statements.some(walk));
676+
// Fall-through: from the first matching (or default) clause, every later clause is reachable.
677+
let matched = clauses.findIndex(
678+
(c) => !ts.isDefaultClause(c) && literalValue(c.expression) === disc
679+
);
680+
if (matched === -1) matched = clauses.findIndex(ts.isDefaultClause);
681+
if (matched === -1) return false;
682+
return clauses.slice(matched).some((c) => c.statements.some(walk));
683+
}
684+
if (ts.isTryStatement(node)) {
685+
if (walk(node.tryBlock)) return true;
686+
if (node.finallyBlock !== undefined && walk(node.finallyBlock)) return true;
687+
if (node.catchClause !== undefined && tryBlockMayThrow(node.tryBlock)) {
688+
return walk(node.catchClause.block);
689+
}
690+
return false;
691+
}
692+
return ts.forEachChild(node, walk) === true;
693+
};
694+
return walk(root);
695+
}
696+
697+
function containsLiveExit(node: ts.Node): boolean {
698+
return containsLiveWhere(node, (n) => ts.isReturnStatement(n) || ts.isThrowStatement(n));
699+
}
700+
701+
function containsLiveReturn(node: ts.Node): boolean {
702+
return containsLiveWhere(node, ts.isReturnStatement);
703+
}
704+
582705
/**
583706
* Whether an `if`/`switch` sends at least one arm somewhere the others do not go, by returning or
584707
* throwing from inside it. `if (e instanceof Error) { }` and `if (e instanceof Error) { log(e); }`
@@ -653,6 +776,16 @@ function catchClauseEvidence(clause: ts.CatchClause): {
653776
// to 19. This ordering leaves the real-tree report
654777
// and all 240 clauses' evidence byte-identical. The tests are the cases in `dead throw written
655778
// after something that already exited`.
779+
//
780+
// Raised off `containsLiveExit`, never `containsExit`. The containment read is true of
781+
// `if (false) { throw e; }` itself, so a provably dead statement raised the flag and blinded the
782+
// walk to the real classification below it: prepending one to a deciding clause turned its pass
783+
// into a swallow verdict on 78 real routes, the same false accusation for all eleven dead
784+
// spellings. The liveness fold only ever withholds this blindness; where `literalTruth` cannot
785+
// decide, the containment answer stands and refusal is intact. The recovered half is `dead and
786+
// deferred code prepended to a deciding catch does not blind it`; the refusing half is the
787+
// `BRANCH_EXITED` list plus `still refuses an error test after an always-true spelling that
788+
// throws`.
656789
let exited = false;
657790
const bindingName = catchBindingName(clause);
658791

@@ -682,21 +815,25 @@ function catchClauseEvidence(clause: ts.CatchClause): {
682815
}
683816
if (ts.isBlock(statement)) {
684817
walk(statement.statements);
685-
if (containsExit(statement)) exited = true;
818+
if (containsLiveExit(statement)) exited = true;
686819
continue;
687820
}
688821
// A `do` body runs before its condition is ever read, so it is on the straight-line path
689822
// whatever the condition says. The only loop form that is; `definitelyExits` agrees.
690823
if (ts.isDoStatement(statement)) {
691824
const body = statement.statement;
692825
walk(ts.isBlock(body) ? body.statements : [body]);
693-
if (containsExit(statement)) exited = true;
826+
if (containsLiveExit(statement)) exited = true;
694827
continue;
695828
}
696829
// Any other reachable statement that could return means throwing is not the only way out.
697830
// Read here rather than over the whole clause so a `return` the walk has already cut as dead
698831
// does not count, which is what a `do { throw e; } while (false); return null;` produces.
699-
if (containsReturn(statement)) returns = true;
832+
// The LIVE read, not the containment one: `if (false) { return null; }` holds a return that
833+
// can never run, and vetoing the rethrow on it regressed a rethrow-only clause from
834+
// not-applicable to fail on 11 real routes. `still sets rethrows past a dead return in an
835+
// if (false) arm` is the pin.
836+
if (containsLiveReturn(statement)) returns = true;
700837

701838
if (bindingName !== null && !shadowed && !exited) {
702839
if (
@@ -713,7 +850,7 @@ function catchClauseEvidence(clause: ts.CatchClause): {
713850
}
714851
}
715852

716-
if (containsExit(statement)) exited = true;
853+
if (containsLiveExit(statement)) exited = true;
717854
}
718855
};
719856
walk(clause.block.statements);

0 commit comments

Comments
 (0)