Skip to content

Commit a8bb298

Browse files
benvinegarbenvinegar
authored
Add stringified unknown errors rule (#20)
1 parent 7a04540 commit a8bb298

6 files changed

Lines changed: 343 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ Current checks focus on patterns that often show up in unreviewed generated code
135135
- [error-obscuring catch blocks](src/rules/error-obscuring/README.md) (default-return or generic replacement error)
136136
- [empty catch blocks](src/rules/empty-catch/README.md)
137137
- [promise `.catch()` default fallbacks](src/rules/promise-default-fallbacks/README.md)
138+
- [stringified unknown errors](src/rules/stringified-unknown-errors/README.md)
138139
- [async wrapper / `return await` noise](src/rules/async-noise/README.md)
139140
- [pass-through wrappers](src/rules/pass-through-wrappers/README.md)
140141
- [barrel density](src/rules/barrel-density/README.md)

‎src/default-registry.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { emptyCatchRule } from "./rules/empty-catch";
1818
import { errorObscuringRule } from "./rules/error-obscuring";
1919
import { errorSwallowingRule } from "./rules/error-swallowing";
2020
import { promiseDefaultFallbacksRule } from "./rules/promise-default-fallbacks";
21+
import { stringifiedUnknownErrorsRule } from "./rules/stringified-unknown-errors";
2122
import { barrelDensityRule } from "./rules/barrel-density";
2223
import { directoryFanoutHotspotRule } from "./rules/directory-fanout-hotspot";
2324
import { duplicateFunctionSignaturesRule } from "./rules/duplicate-function-signatures";
@@ -45,6 +46,7 @@ export function createDefaultRegistry(): Registry {
4546
registry.registerRule(errorObscuringRule);
4647
registry.registerRule(emptyCatchRule);
4748
registry.registerRule(promiseDefaultFallbacksRule);
49+
registry.registerRule(stringifiedUnknownErrorsRule);
4850
registry.registerRule(barrelDensityRule);
4951
registry.registerRule(passThroughWrappersRule);
5052
registry.registerRule(duplicateFunctionSignaturesRule);

‎src/rules/stringified-unknown-errors/README.md‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# defensive.stringified-unknown-errors
2+
3+
Flags code that flattens unknown caught values into generic strings like `error.message` or `String(error)`.
4+
5+
- **Family:** `defensive`
6+
- **Severity:** `strong`
7+
- **Scope:** `file`
8+
- **Requires:** `file.ast`
9+
10+
## How it works
11+
12+
The rule looks for conditional expressions like:
13+
14+
- `error instanceof Error ? error.message : String(error)`
15+
- `err instanceof Error ? err.message : String(err)`
16+
- equivalent forms assigned into `message` / `error` properties or returned directly
17+
18+
This pattern is common in generated wrapper code that wants a quick printable error value. It is sometimes reasonable, but repeated use often flattens richer failure objects into plain strings and makes downstream diagnostics more generic.
19+
20+
To avoid obvious vendored noise, the rule skips very large bundled/generated files over `5000` logical lines.
21+
22+
## Flagged examples
23+
24+
```ts
25+
catch (error) {
26+
return { success: false, error: error instanceof Error ? error.message : String(error) };
27+
}
28+
29+
setError(err instanceof Error ? err.message : String(err));
30+
31+
const message = error instanceof Error ? error.message : String(error);
32+
```
33+
34+
## Usually ignored
35+
36+
```ts
37+
catch (error) {
38+
throw error;
39+
}
40+
41+
catch (error) {
42+
logger.error({ error });
43+
return { success: false, error };
44+
}
45+
```
46+
47+
## Scoring
48+
49+
Each unknown-error stringification site adds `2` points.
50+
The file total is capped at `8`.
51+
52+
## Benchmark signal
53+
54+
Full pinned benchmark against the exact `known-ai-vs-solid-oss` cohort:
55+
56+
- Signal score: **0.88 / 1.00**
57+
- Best separating metric: **findings / file (0.88)**
58+
- Hit rate: **7/9 AI repos** vs **1/9 mature OSS repos**
59+
- Full results: [experimental rule report](../../../reports/autoresearch-candidate-rule.md#defensivestringified-unknown-errors)

‎src/rules/stringified-unknown-errors/index.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
import * as ts from "typescript";
2+
import type { RulePlugin } from "../../core/types";
3+
import { getLineNumber, unwrapExpression, walk } from "../../facts/ts-helpers";
4+
import { delta } from "../../rule-delta";
5+
6+
const MAX_LOGICAL_LINES = 5000;
7+
8+
type MatchKind =
9+
| "stringified-unknown-error"
10+
| "returned-stringified-unknown-error"
11+
| "property-stringified-unknown-error";
12+
13+
type StringifiedUnknownErrorMatch = {
14+
line: number;
15+
kind: MatchKind;
16+
};
17+
18+
function isErrorIdentifier(expression: ts.Expression, allowedNames: Set<string>): boolean {
19+
const unwrapped = unwrapExpression(expression);
20+
return ts.isIdentifier(unwrapped) && allowedNames.has(unwrapped.text);
21+
}
22+
23+
function isStringCallOfError(expression: ts.Expression, allowedNames: Set<string>): boolean {
24+
const unwrapped = unwrapExpression(expression);
25+
return (
26+
ts.isCallExpression(unwrapped) &&
27+
ts.isIdentifier(unwrapped.expression) &&
28+
unwrapped.expression.text === "String" &&
29+
unwrapped.arguments.length === 1 &&
30+
isErrorIdentifier(unwrapped.arguments[0]!, allowedNames)
31+
);
32+
}
33+
34+
function isErrorMessageAccess(expression: ts.Expression, allowedNames: Set<string>): boolean {
35+
const unwrapped = unwrapExpression(expression);
36+
return (
37+
ts.isPropertyAccessExpression(unwrapped) &&
38+
unwrapped.name.text === "message" &&
39+
isErrorIdentifier(unwrapped.expression, allowedNames)
40+
);
41+
}
42+
43+
function isErrorInstanceofCheck(expression: ts.Expression, allowedNames: Set<string>): boolean {
44+
const unwrapped = unwrapExpression(expression);
45+
return (
46+
ts.isBinaryExpression(unwrapped) &&
47+
unwrapped.operatorToken.kind === ts.SyntaxKind.InstanceOfKeyword &&
48+
isErrorIdentifier(unwrapped.left, allowedNames) &&
49+
ts.isIdentifier(unwrapExpression(unwrapped.right)) &&
50+
unwrapExpression(unwrapped.right).text === "Error"
51+
);
52+
}
53+
54+
function summarizeConditional(
55+
node: ts.ConditionalExpression,
56+
sourceFile: ts.SourceFile,
57+
): StringifiedUnknownErrorMatch | null {
58+
const allowedNames = new Set<string>();
59+
60+
const condition = unwrapExpression(node.condition);
61+
if (
62+
ts.isBinaryExpression(condition) &&
63+
condition.operatorToken.kind === ts.SyntaxKind.BarBarToken
64+
) {
65+
for (const side of [condition.left, condition.right]) {
66+
const unwrapped = unwrapExpression(side);
67+
if (
68+
ts.isBinaryExpression(unwrapped) &&
69+
unwrapped.operatorToken.kind === ts.SyntaxKind.InstanceOfKeyword
70+
) {
71+
const left = unwrapExpression(unwrapped.left);
72+
if (ts.isIdentifier(left)) {
73+
allowedNames.add(left.text);
74+
}
75+
}
76+
}
77+
} else if (isErrorInstanceofCheck(condition, new Set(["error", "err", "e", "cause"]))) {
78+
const left = unwrapExpression((condition as ts.BinaryExpression).left) as ts.Identifier;
79+
allowedNames.add(left.text);
80+
}
81+
82+
if (allowedNames.size === 0) {
83+
return null;
84+
}
85+
86+
const whenTrue = unwrapExpression(node.whenTrue);
87+
const whenFalse = unwrapExpression(node.whenFalse);
88+
const isEitherDirection =
89+
(isErrorMessageAccess(whenTrue, allowedNames) &&
90+
isStringCallOfError(whenFalse, allowedNames)) ||
91+
(isErrorMessageAccess(whenFalse, allowedNames) && isStringCallOfError(whenTrue, allowedNames));
92+
93+
if (!isEitherDirection) {
94+
return null;
95+
}
96+
97+
let kind: MatchKind = "stringified-unknown-error";
98+
if (ts.isReturnStatement(node.parent)) {
99+
kind = "returned-stringified-unknown-error";
100+
} else if (ts.isPropertyAssignment(node.parent)) {
101+
kind = "property-stringified-unknown-error";
102+
}
103+
104+
return {
105+
line: getLineNumber(sourceFile, node.getStart(sourceFile)),
106+
kind,
107+
};
108+
}
109+
110+
function findStringifiedUnknownErrors(sourceFile: ts.SourceFile): StringifiedUnknownErrorMatch[] {
111+
const matches: StringifiedUnknownErrorMatch[] = [];
112+
113+
walk(sourceFile, (node) => {
114+
if (!ts.isConditionalExpression(node)) {
115+
return;
116+
}
117+
118+
const match = summarizeConditional(node, sourceFile);
119+
if (match) {
120+
matches.push(match);
121+
}
122+
});
123+
124+
return matches;
125+
}
126+
127+
export const stringifiedUnknownErrorsRule: RulePlugin = {
128+
id: "defensive.stringified-unknown-errors",
129+
family: "defensive",
130+
severity: "strong",
131+
scope: "file",
132+
requires: ["file.ast"],
133+
delta: delta.byLocations(),
134+
supports(context) {
135+
return context.scope === "file" && Boolean(context.file);
136+
},
137+
evaluate(context) {
138+
if (context.file!.logicalLineCount > MAX_LOGICAL_LINES) {
139+
return [];
140+
}
141+
142+
const sourceFile = context.runtime.store.getFileFact<ts.SourceFile>(
143+
context.file!.path,
144+
"file.ast",
145+
);
146+
if (!sourceFile) {
147+
return [];
148+
}
149+
150+
const matches = findStringifiedUnknownErrors(sourceFile);
151+
if (matches.length === 0) {
152+
return [];
153+
}
154+
155+
return [
156+
{
157+
ruleId: "defensive.stringified-unknown-errors",
158+
family: "defensive",
159+
severity: "strong",
160+
scope: "file",
161+
path: context.file!.path,
162+
message: `Found ${matches.length} unknown-error normalization${matches.length === 1 ? "" : "s"} that collapse caught values into generic strings`,
163+
evidence: matches.map((match) => `line ${match.line}: ${match.kind}`),
164+
score: Math.min(8, matches.length * 2),
165+
locations: matches.map((match) => ({ path: context.file!.path, line: match.line })),
166+
},
167+
];
168+
},
169+
};

‎tests/heuristics.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ describe("heuristic rule pack", () => {
4444
" }",
4545
"}",
4646
"",
47+
"export function getErrorMessage(error: unknown) {",
48+
" const message = error instanceof Error ? error.message : String(error);",
49+
" return { message };",
50+
"}",
51+
"",
4752
].join("\n"),
4853
"src/service.ts": [
4954
"function getData(id: string) {",
@@ -80,6 +85,7 @@ describe("heuristic rule pack", () => {
8085
expect(ruleIds.has("comments.placeholder-comments")).toBe(true);
8186
expect(ruleIds.has("defensive.error-obscuring")).toBe(true);
8287
expect(ruleIds.has("defensive.promise-default-fallbacks")).toBe(true);
88+
expect(ruleIds.has("defensive.stringified-unknown-errors")).toBe(true);
8389
expect(ruleIds.has("defensive.async-noise")).toBe(true);
8490
expect(ruleIds.has("structure.pass-through-wrappers")).toBe(true);
8591
expect(ruleIds.has("structure.barrel-density")).toBe(true);

‎tests/stringified-unknown-errors.test.ts‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { afterEach, describe, expect, test } from "bun:test";
2+
import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
3+
import os from "node:os";
4+
import path from "node:path";
5+
import { DEFAULT_CONFIG } from "../src/config";
6+
import { analyzeRepository } from "../src/core/engine";
7+
import { Registry } from "../src/core/registry";
8+
import { createDefaultRegistry } from "../src/default-registry";
9+
import { stringifiedUnknownErrorsRule } from "../src/rules/stringified-unknown-errors";
10+
11+
const tempDirs: string[] = [];
12+
13+
afterEach(async () => {
14+
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
15+
});
16+
17+
async function writeRepoFiles(rootDir: string, files: Record<string, string>): Promise<void> {
18+
for (const [relativePath, content] of Object.entries(files)) {
19+
const absolutePath = path.join(rootDir, relativePath);
20+
await mkdir(path.dirname(absolutePath), { recursive: true });
21+
await writeFile(absolutePath, content);
22+
}
23+
}
24+
25+
async function createTempRepo(files: Record<string, string>): Promise<string> {
26+
const rootDir = await mkdtemp(path.join(os.tmpdir(), "slop-scan-stringified-errors-"));
27+
tempDirs.push(rootDir);
28+
await writeRepoFiles(rootDir, files);
29+
return rootDir;
30+
}
31+
32+
function createCandidateRegistry(): Registry {
33+
const baseRegistry = createDefaultRegistry();
34+
const registry = new Registry();
35+
36+
for (const language of baseRegistry.getLanguages()) {
37+
registry.registerLanguage(language);
38+
}
39+
40+
for (const provider of baseRegistry.getFactProviders()) {
41+
registry.registerFactProvider(provider);
42+
}
43+
44+
registry.registerRule(stringifiedUnknownErrorsRule);
45+
return registry;
46+
}
47+
48+
describe("stringified-unknown-errors rule", () => {
49+
test("flags unknown errors collapsed into generic strings", async () => {
50+
const rootDir = await createTempRepo({
51+
"src/slop.ts": [
52+
"export function getMessage(error: unknown) {",
53+
" return error instanceof Error ? error.message : String(error);",
54+
"}",
55+
"",
56+
"export function toResult(err: unknown) {",
57+
" return { ok: false, error: err instanceof Error ? err.message : String(err) };",
58+
"}",
59+
"",
60+
"export function updateState(error: unknown) {",
61+
" const next = error instanceof Error ? error.message : String(error);",
62+
" return { message: next };",
63+
"}",
64+
].join("\n"),
65+
"src/legit.ts": [
66+
"export function rethrow(error: unknown) {",
67+
" throw error;",
68+
"}",
69+
"",
70+
"export function preserve(error: unknown) {",
71+
" return { error };",
72+
"}",
73+
].join("\n"),
74+
});
75+
76+
const result = await analyzeRepository(rootDir, DEFAULT_CONFIG, createCandidateRegistry());
77+
const finding = result.findings.find(
78+
(nextFinding) => nextFinding.ruleId === "defensive.stringified-unknown-errors",
79+
);
80+
81+
expect(finding).toBeDefined();
82+
expect(finding?.path).toBe("src/slop.ts");
83+
expect(finding?.evidence).toEqual([
84+
"line 2: returned-stringified-unknown-error",
85+
"line 6: property-stringified-unknown-error",
86+
"line 10: stringified-unknown-error",
87+
]);
88+
expect(result.findings).toHaveLength(1);
89+
});
90+
91+
test("ignores giant bundled files that would otherwise create vendor noise", async () => {
92+
const hugeFile = [
93+
...Array.from({ length: 5001 }, (_, index) => `export const filler${index} = ${index};`),
94+
"export const message = error instanceof Error ? error.message : String(error);",
95+
"",
96+
].join("\n");
97+
98+
const rootDir = await createTempRepo({
99+
"src/bundle.ts": hugeFile,
100+
});
101+
102+
const result = await analyzeRepository(rootDir, DEFAULT_CONFIG, createCandidateRegistry());
103+
104+
expect(result.findings).toHaveLength(0);
105+
});
106+
});

0 commit comments

Comments
 (0)