Skip to content

Commit 00a53b5

Browse files
committed
Skip void p.catch(() => {}) and try blocks that declare locals first
hono writes void reader.cancel().catch(() => {}) and try { const stats = await stat(p); isDir = stats.isDirectory(); } catch {}. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TrbmayRraf25aTwCCj1iv2
1 parent ca17c1a commit 00a53b5

4 files changed

Lines changed: 10 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ All notable changes will be documented in this file.
99
- `swallowed_error` (medium): a `catch` block whose only statements are `console` calls. The error is logged and the code carries on as if the operation succeeded. A comment in the block explaining why that is safe, a rethrow, or any other statement (such as `setError(e)`) means it is not reported, and so does a fallback after the `try`, as for `empty_catch`.
1010

1111
### Changed
12+
- `empty_catch` skips `void p.catch(() => {})`, where `void` says the result is dropped on purpose, and a `try` that declares locals before assigning the outer variable (`const stats = await stat(p); isDir = stats.isDirectory();`). hono uses both.
1213
- The package ships compiled JavaScript (`dist/`) and runs it directly with Node. It used to run the TypeScript sources through `tsx` on every call: `karpeslop --version` took 1.5 s and an install was 43 MB. It now takes 0.6 s and 31 MB, most of it the TypeScript compiler the rules use to parse code. `tsx` is no longer a runtime dependency.
1314
- `fresh_package_version` skips packages the scanned workspace publishes itself (any `name` in a scanned package.json) and lockfile entries that link a workspace folder. TanStack Query got 184 warnings for `@tanstack/*` versions it had just released.
1415
- Folders named `build`, `out` or `temp` are skipped only at the scan root or next to a package.json, where they hold build output. Deeper ones are often source: Next.js keeps its build pipeline in `packages/next/src/build/`, which was never scanned.

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ npx karpeslop@latest
2828
| `placeholder_implementation` | high | A body that returns a constant (or nothing) under a TODO or placeholder comment; a short body under a comment that says the real code is elsewhere ("Simulate API call", "In a real app you would..."); a body that only logs and returns success; or `throw new Error("Not implemented")` |
2929
| `elided_code_comment` | high | `// ... rest of the code`, `// ...existing code...`, `{/* ... other fields */}`, `// Add more cases as needed` and similar, in line, block and JSX comments |
3030
| `placeholder_value` | high | `"YOUR_API_KEY"`, `"<your-token>"`, `"Bearer <API_KEY>"`, `"sk-xxxxxx"`, `"sk_test_your_key_here"`, `"postgres://postgres:password@..."`, `"https://your-project.supabase.co"`, `"xxxx"` stored as a key or token, an `example.com` URL stored in config, and a hard-coded fallback for a secret (`process.env.JWT_SECRET \|\| "secret"`) |
31-
| `empty_catch` | high | `catch {}` or `.catch(() => {})` with no comment explaining why and no `_`-named binding. Not reported when the `try` returns and code after it is the fallback (`try { return parse(x) } catch {}` then a default), or when the `try` only assigns variables declared before it that are read after it (`let found = fallback; try { found = probe() } catch {}`) |
31+
| `empty_catch` | high | `catch {}` or `.catch(() => {})` with no comment explaining why, no `_`-named binding and no `void` in front. Not reported when the `try` returns and code after it is the fallback (`try { return parse(x) } catch {}` then a default), or when the `try` only assigns variables declared before it that are read after it (`let found = fallback; try { found = probe() } catch {}`) |
3232
| `swallowed_error` | medium | A `catch` block that only logs (`console.error('Failed', err)`) and lets execution continue as if nothing failed, or logs and then returns `[]`, `{}` or `''`, which the caller cannot tell apart from an empty success |
3333
| `unsafe_type_assertion` | high | `as any` |
3434
| `unsafe_double_type_assertion` | high | `as unknown as T` and other chained casts |

‎rules.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,8 @@ function isFallbackCatch(clause: ts.CatchClause): boolean {
10461046
* a return after the try.
10471047
*/
10481048
function assignsOnlyOuterVariables(tryStatement: ts.TryStatement): boolean {
1049-
const statements = tryStatement.tryBlock.statements;
1049+
// Local declarations feed the assignment: `const stats = await stat(p); isDir = stats.isDirectory();`
1050+
const statements = tryStatement.tryBlock.statements.filter(statement => !ts.isVariableStatement(statement));
10501051
if (statements.length === 0) return false;
10511052
const scope = enclosingFunctionOrFile(tryStatement);
10521053
return statements.every(statement => {
@@ -1245,8 +1246,9 @@ export function analyzeSource(sourceText: string, fileName: string): Finding[] {
12451246
if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression) &&
12461247
node.expression.name.text === 'catch' && node.arguments.length === 1) {
12471248
const handler = node.arguments[0];
1249+
// `void p.catch(() => {})` says the result is dropped on purpose.
12481250
if ((ts.isArrowFunction(handler) || ts.isFunctionExpression(handler)) && ts.isBlock(handler.body) &&
1249-
handler.body.statements.length === 0 &&
1251+
handler.body.statements.length === 0 && !ts.isVoidExpression(node.parent) &&
12501252
commentsWithin(handler.body).length === 0 && !isIgnoredBinding(handler.parameters[0]?.name)) {
12511253
report('empty_catch', node.expression.name);
12521254
}

‎tests/rules.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ test('empty catch with an underscore binding is deliberate', () => {
195195
assert.equal(countOf('try { a(); } catch (_err: any) {}', 'empty_catch'), 0);
196196
assert.equal(countOf('p.catch((_err) => {});', 'empty_catch'), 0);
197197
assert.equal(countOf('p.catch((err) => {});', 'empty_catch'), 1);
198+
assert.equal(countOf('void reader.cancel().catch(() => {});', 'empty_catch'), 0);
198199
assert.equal(countOf('try { a(); } catch (err) {}', 'empty_catch'), 1);
199200
});
200201

@@ -217,6 +218,7 @@ test('empty catch: a variable assigned in the try keeps its earlier value as the
217218
'function f(s: S) {\n let recursive = true;\n try {\n recursive = isRecursive(s);\n } catch {}\n return recursive;\n}',
218219
'async function isDir(p: string) {\n let dir;\n try {\n dir = (await stat(p)).isDirectory();\n } catch {}\n return dir;\n}',
219220
'let als: unknown;\nfor (const probe of probes) {\n try {\n als = probe();\n } catch {}\n if (als) break;\n}',
221+
'async function isDir(p: string) {\n let dir;\n try {\n const stats = await stat(p);\n dir = stats.isDirectory();\n } catch {}\n return dir;\n}',
220222
'function f(src: S) {\n let x = 1;\n try {\n x = load();\n } catch {}\n ({ a = x } = src);\n return a;\n}'
221223
]) {
222224
assert.equal(countOf(code, 'empty_catch'), 0, code);
@@ -230,7 +232,8 @@ test('empty catch: a variable assigned in the try keeps its earlier value as the
230232
// Written again, or read only by a callback's own parameter of the same name.
231233
'function f(src: S) {\n let x = 1;\n try {\n x = load();\n } catch {}\n ({ x } = src);\n [x] = [2];\n}',
232234
'function f(xs: number[]) {\n let x = 1;\n try {\n x = load();\n } catch {}\n return xs.map((x) => x * 2);\n}',
233-
'function f(xs: number[]) {\n let x = 1;\n try {\n x = load();\n } catch {}\n for (const x of xs) use(x);\n if (xs.length) { const x = 2; use(x); }\n xs.forEach(() => { var x = 3; use(x); });\n}'
235+
'function f(xs: number[]) {\n let x = 1;\n try {\n x = load();\n } catch {}\n for (const x of xs) use(x);\n if (xs.length) { const x = 2; use(x); }\n xs.forEach(() => { var x = 3; use(x); });\n}',
236+
'function f() {\n try {\n const x = load();\n } catch {}\n}'
234237
]) {
235238
assert.equal(countOf(code, 'empty_catch'), 1, code);
236239
}

0 commit comments

Comments
 (0)