Skip to content

Commit 37d5f1a

Browse files
committed
chore(observability-map): remove dead scanner surface
EntryPoint.calleeTexts, catchRethrows, catchBranches, catchesNarrowly and LogCall.hasObjectArgument were written by the scanner and read by nothing: errorClassification reads the per-clause CatchEvidence fields, sensitivity and triviality read calleeNames, not calleeTexts. Removed the fields, their derivations, and the tests that asserted them directly; reworded doc comments that referenced them.
1 parent 98932b8 commit 37d5f1a

4 files changed

Lines changed: 56 additions & 191 deletions

File tree

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

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,10 @@ function calleeName(expr: ts.Expression): string | null {
6969
}
7070

7171
/**
72-
* Callee as recorded in `calleeTexts`: the whole path, `prisma.organization.findFirst` rather than
73-
* `findFirst`. Null when the path runs through something with no name of its own, e.g.
74-
* `new PromptService().createOverride`, where the caller falls back to the bare name.
72+
* The whole callee path of a call, `prisma.organization.findFirst` rather than `findFirst`. Used to
73+
* match a call against `LOGGER_CALLEE` and `PARSE_CALLEE`. Null when the path runs through something
74+
* with no name of its own, e.g. `new PromptService().createOverride`, where the caller falls back to
75+
* the bare name.
7576
*/
7677
function calleeText(expr: ts.Expression): string | null {
7778
const target = unwrap(expr);
@@ -92,7 +93,7 @@ function calleeText(expr: ts.Expression): string | null {
9293
const LOGGER_CALLEE = /(^|\.)(logger|log)\.[A-Za-z_$][\w$]*$/;
9394

9495
/** Property names on the first object-literal argument, e.g. `{ environmentId, error }`. */
95-
function objectArgumentFields(call: ts.CallExpression): { found: boolean; fields: string[] } {
96+
function objectArgumentFields(call: ts.CallExpression): string[] {
9697
for (const arg of call.arguments) {
9798
const target = unwrap(arg);
9899
if (!ts.isObjectLiteralExpression(target)) continue;
@@ -101,9 +102,9 @@ function objectArgumentFields(call: ts.CallExpression): { found: boolean; fields
101102
const name = propertyName(property);
102103
if (name) fields.push(name);
103104
}
104-
return { found: true, fields };
105+
return fields;
105106
}
106-
return { found: false, fields: [] };
107+
return [];
107108
}
108109

109110
/**
@@ -128,8 +129,8 @@ const PARSE_CALLEE = /(^|\.)(parse|safeParse|parseAsync|safeParseAsync|decode)$|
128129
const PARSE_CONSTRUCTORS = new Set(["URL", "URLSearchParams", "RegExp"]);
129130

130131
/**
131-
* Whether the guarded region parses something. A `new URL(x)` counts, and has to be read here
132-
* because constructors are absent from `calleeTexts`.
132+
* Whether the guarded region parses something. A `new URL(x)` counts, and has to be read here as a
133+
* `ts.isNewExpression`, because the call-callee scan that builds `calleeNames` never sees it.
133134
*/
134135
function guardsParse(tryBlock: ts.Block): boolean {
135136
let found = false;
@@ -518,7 +519,6 @@ export function scanFile(fileName: string, source: string): EntryPoint | null {
518519
let hasTryCatch = false;
519520
const catches: CatchEvidence[] = [];
520521
const calleeNames: string[] = [];
521-
const calleeTexts: string[] = [];
522522
const logCalls: LogCall[] = [];
523523

524524
const localFunctions = collectLocalFunctions(sf);
@@ -558,14 +558,11 @@ export function scanFile(fileName: string, source: string): EntryPoint | null {
558558
if (cn) {
559559
const text = calleeText(node.expression) ?? cn;
560560
calleeNames.push(cn);
561-
calleeTexts.push(text);
562561

563562
if (LOGGER_CALLEE.test(text)) {
564-
const argument = objectArgumentFields(node);
565563
logCalls.push({
566564
callee: text,
567-
hasObjectArgument: argument.found,
568-
fields: argument.fields,
565+
fields: objectArgumentFields(node),
569566
inCatch,
570567
});
571568
}
@@ -599,13 +596,8 @@ export function scanFile(fileName: string, source: string): EntryPoint | null {
599596
actionInitializerCallee: target.actionInitializerCallee,
600597
importedNames,
601598
calleeNames,
602-
calleeTexts,
603599
hasTryCatch,
604600
catches,
605-
// Kept as aggregates of `catches` so the checks can migrate one at a time.
606-
catchRethrows: catches.some((c) => c.rethrows),
607-
catchBranches: catches.some((c) => c.branches),
608-
catchesNarrowly: catches.length > 0 && catches.every((c) => c.narrow),
609601
logCalls,
610602
statementCount,
611603
};

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

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,19 @@ export type CatchEvidence = {
1717
/** The clause contains a `throw`. */
1818
rethrows: boolean;
1919
/**
20-
* The clause picks what to do from what it caught: an `if`, a `switch`, or a conditional that is
21-
* the whole `return`/`throw`. An `instanceof` used only to word a message,
22-
* `json({ error: e instanceof Error ? e.message : String(e) })`, does not count: every error
23-
* still leaves by the same path.
20+
* The clause picks what to do from what it caught: an `if` or `switch` whose condition references
21+
* the caught error binding, or a conditional that is the whole `return`/`throw`. `if (retries > 0)`
22+
* does not count, and a bindingless `catch { ... }` cannot count at all. An `instanceof` used only
23+
* to word a message, `json({ error: e instanceof Error ? e.message : String(e) })`, does not
24+
* count either: every error still leaves by the same path.
2425
*/
2526
branches: boolean;
2627
/**
2728
* The guarded region parses something: `JSON.parse`, `request.json()`, a zod `parse`/`safeParse`,
2829
* a `decode`, or a `new URL`/`URLSearchParams`/`RegExp`. Those three constructors are read here
29-
* because constructors never appear in `calleeTexts`; other constructors do not count, or every
30-
* `new SomePresenter()` in a try would excuse its catch.
30+
* because a `new` expression is not a call, so the call-callee scan that feeds this check never
31+
* sees them; other constructors do not count, or every `new SomePresenter()` in a try would excuse
32+
* its catch.
3133
*/
3234
guardsParse: boolean;
3335
/** Statements in the guarded try block, counted as `statementCount` counts them. */
@@ -38,9 +40,7 @@ export type CatchEvidence = {
3840
export type LogCall = {
3941
/** Full callee path, e.g. `logger.error`. */
4042
callee: string;
41-
/** Whether an object literal was passed as an argument. */
42-
hasObjectArgument: boolean;
43-
/** Property names on that object literal, e.g. `["environmentId", "error"]`. */
43+
/** Property names on the first object-literal argument, e.g. `["environmentId", "error"]`. */
4444
fields: string[];
4545
/** Whether the call sits inside a catch clause, i.e. on the failure path. */
4646
inCatch: boolean;
@@ -58,12 +58,6 @@ export type EntryPoint = {
5858
importedNames: string[];
5959
/** Names of functions called inside the loader/action bodies, or in a same-file helper they call. */
6060
calleeNames: string[];
61-
/**
62-
* The same calls as `calleeNames`, same order and same length, but as the whole callee path:
63-
* `prisma.organization.findFirst` where `calleeNames` has `findFirst`. A path that runs through
64-
* something unnameable (`new PromptService().createOverride`) falls back to the bare name.
65-
*/
66-
calleeTexts: string[];
6761
/**
6862
* Whether a `try` appears in the loader/action bodies, or in a same-file helper they call. Note
6963
* that this says a `try`, not a catch: a `try`/`finally` sets it while `catches` stays empty and
@@ -72,26 +66,6 @@ export type EntryPoint = {
7266
hasTryCatch: boolean;
7367
/** One entry per catch clause in those bodies, in source order. */
7468
catches: CatchEvidence[];
75-
/**
76-
* Whether any catch clause in those bodies contains a `throw`. A catch that rethrows has decided
77-
* the error is not its to answer, which is a different act from swallowing it. Aggregate of
78-
* `catches`, kept so existing consumers keep working.
79-
*/
80-
catchRethrows: boolean;
81-
/**
82-
* Whether any catch clause in those bodies branches on the error: an `if`, a `switch`, or an
83-
* `instanceof`. With `catchRethrows` both false while `hasTryCatch` is true, every catch in the
84-
* entry point takes one path out regardless of what was thrown.
85-
*/
86-
catchBranches: boolean;
87-
/**
88-
* Whether every catch in those bodies guards a specific operation rather than the handler: the
89-
* entry point has at least one catch clause, and no try block with a catch holds more than two
90-
* statements. The `try { body = await request.json() } catch { 400 }` idiom, which takes one path
91-
* out and is still deliberate. False when any catch wraps the bulk of a body, and false when
92-
* there is no catch clause at all.
93-
*/
94-
catchesNarrowly: boolean;
9569
/** Calls to a `logger.*` or `log.*` callee in those bodies, in source order. */
9670
logCalls: LogCall[];
9771
/**

internal-packages/observability-map/test/checks.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -565,9 +565,10 @@ describe("request-context", () => {
565565
expect(r.status).toBe("fail");
566566
});
567567

568-
// Was a known false positive: `new URL()` is a constructor, so the parse was invisible while the
569-
// evidence came from `calleeTexts`. `CatchEvidence.guardsParse` covers constructors, so the guard
570-
// is legible now and the route is no longer judged as though it kept its failures.
568+
// Was a known false positive: `new URL()` is a constructor, so the parse was invisible to the
569+
// call-callee scan the evidence used to come from. `CatchEvidence.guardsParse` covers
570+
// constructors, so the guard is legible now and the route is no longer judged as though it kept
571+
// its failures.
571572
it("fails a route whose only catch guards a constructor parse", () => {
572573
const r = run(
573574
"request-context",

0 commit comments

Comments
 (0)