From f6e97b3e5d5ca93fa10daf33edf56e425235404f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:14:43 +0000 Subject: [PATCH 1/2] eslint-factory: add no-empty-catch-block rule Adds a new custom ESLint rule that flags empty catch blocks (`catch {}` / `catch (err) {}`) in actions/setup/js scripts. An empty catch silently swallows the original error with no logging, fallback assignment, or explanatory comment, making failures (corrupted state files, cleanup errors) hard to diagnose from CI logs. The rule allows catch blocks that contain a comment documenting the intentional no-op, keeping false positives at zero. Scanning actions/setup/js confirmed 3 real occurrences with this exact pattern (fuzz_template_substitution_harness.cjs, load_experiment_state_from_repo.cjs, pick_experiment.cjs), and lint:setup-js now reports exactly those 3 with no other matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eslint-factory/eslint.config.cjs | 1 + eslint-factory/src/index.ts | 2 + .../src/rules/no-empty-catch-block.test.ts | 49 +++++++++++++++++++ .../src/rules/no-empty-catch-block.ts | 38 ++++++++++++++ 4 files changed, 90 insertions(+) create mode 100644 eslint-factory/src/rules/no-empty-catch-block.test.ts create mode 100644 eslint-factory/src/rules/no-empty-catch-block.ts diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index fc4e3597cf2..bc692cef70a 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -59,6 +59,7 @@ module.exports = [ "gh-aw-custom/require-error-code-in-thrown-error": "warn", "gh-aw-custom/require-invalid-date-check-before-compare": "warn", "gh-aw-custom/require-sync-exec-timeout": "warn", + "gh-aw-custom/no-empty-catch-block": "warn", }, }, { diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index d1b1e4ab30f..f4a60e69fdd 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -45,6 +45,7 @@ import { requireFetchResponseBodyTryCatchRule } from "./rules/require-fetch-resp import { requireErrorCodeInThrownErrorRule } from "./rules/require-error-code-in-thrown-error"; import { requireInvalidDateCheckBeforeCompareRule } from "./rules/require-invalid-date-check-before-compare"; import { requireSyncExecTimeoutRule } from "./rules/require-sync-exec-timeout"; +import { noEmptyCatchBlockRule } from "./rules/no-empty-catch-block"; const plugin = { meta: { @@ -99,6 +100,7 @@ const plugin = { "require-error-code-in-thrown-error": requireErrorCodeInThrownErrorRule, "require-invalid-date-check-before-compare": requireInvalidDateCheckBeforeCompareRule, "require-sync-exec-timeout": requireSyncExecTimeoutRule, + "no-empty-catch-block": noEmptyCatchBlockRule, }, }; diff --git a/eslint-factory/src/rules/no-empty-catch-block.test.ts b/eslint-factory/src/rules/no-empty-catch-block.test.ts new file mode 100644 index 00000000000..f12e92068dc --- /dev/null +++ b/eslint-factory/src/rules/no-empty-catch-block.test.ts @@ -0,0 +1,49 @@ +import { RuleTester } from "eslint"; +import { describe, expect, it } from "vitest"; +import { noEmptyCatchBlockRule } from "./no-empty-catch-block"; + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("no-empty-catch-block", () => { + it("uses the correct docs URL", () => { + expect(noEmptyCatchBlockRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#no-empty-catch-block"); + }); + + it("accepts catch blocks that log, assign a fallback, or document intent", () => { + ruleTester.run("no-empty-catch-block", noEmptyCatchBlockRule, { + valid: [ + `try { risky(); } catch (err) { core.debug(getErrorMessage(err)); }`, + `try { value = JSON.parse(raw); } catch { value = {}; }`, + `try { risky(); } catch { /* best-effort cleanup, ignore */ }`, + `try { risky(); } catch (err) { throw err; }`, + `try { risky(); } catch (err) {\n // intentional no-op: file may not exist on first run\n}`, + ], + invalid: [], + }); + }); + + it("reports catch blocks with no statements and no explanatory comment", () => { + ruleTester.run("no-empty-catch-block", noEmptyCatchBlockRule, { + valid: [], + invalid: [ + { + code: `try { risky(); } catch {}`, + errors: [{ messageId: "noEmptyCatch" }], + }, + { + code: `try { risky(); } catch (err) {}`, + errors: [{ messageId: "noEmptyCatch" }], + }, + { + code: `try { risky(); } catch (err) {\n\n}`, + errors: [{ messageId: "noEmptyCatch" }], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/no-empty-catch-block.ts b/eslint-factory/src/rules/no-empty-catch-block.ts new file mode 100644 index 00000000000..680591d6035 --- /dev/null +++ b/eslint-factory/src/rules/no-empty-catch-block.ts @@ -0,0 +1,38 @@ +import { ESLintUtils, TSESTree } from "@typescript-eslint/utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +export const noEmptyCatchBlockRule = createRule({ + name: "no-empty-catch-block", + meta: { + type: "problem", + docs: { + description: "Disallow empty catch blocks in actions/setup/js scripts. Swallowing an error with no logging, fallback assignment, or comment hides real failures (corrupted state files, cleanup errors) that are hard to diagnose from CI logs.", + }, + schema: [], + messages: { + noEmptyCatch: "Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add a comment explaining why the error is intentionally ignored.", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + + return { + CatchClause(node: TSESTree.CatchClause) { + const body = node.body.body; + if (body.length !== 0) return; + + // A comment inside the (otherwise empty) braces documents intent, e.g.: + // } catch { /* best-effort cleanup, ignore */ } + const commentsInside = sourceCode.getCommentsInside(node.body); + if (commentsInside.length > 0) return; + + context.report({ + node: node.body, + messageId: "noEmptyCatch", + }); + }, + }; + }, +}); From d4d709666e086e78b113893469007bbdaa1a01a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:28:18 +0000 Subject: [PATCH 2/2] Tighten empty catch comment handling Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- eslint-factory/README.md | 7 +++++++ .../src/rules/no-empty-catch-block.test.ts | 16 +++++++++++++++- eslint-factory/src/rules/no-empty-catch-block.ts | 12 +++++++----- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/eslint-factory/README.md b/eslint-factory/README.md index 26a2c5f9a04..4d331957a5a 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -20,6 +20,7 @@ This project hosts custom ESLint linters for `/actions/setup/js`. |---|---| | [`no-core-exportvariable-non-string`](#no-core-exportvariable-non-string) | Require explicit string values for `core.exportVariable` calls | | [`no-core-setoutput-non-string`](#no-core-setoutput-non-string) | Require explicit string values for `core.setOutput` calls | +| [`no-empty-catch-block`](#no-empty-catch-block) | Disallow undocumented empty `catch` blocks | | [`no-duplicate-constant-values`](#no-duplicate-constant-values) | Report constants with duplicate static primitive values in the same file | | [`no-child-process-interpolated-command`](#no-child-process-interpolated-command) | Disallow interpolated command strings in shell-evaluated `child_process` calls | | [`no-github-request-interpolated-route`](#no-github-request-interpolated-route) | Disallow interpolated route arguments in Octokit `.request()` calls | @@ -61,6 +62,12 @@ This project hosts custom ESLint linters for `/actions/setup/js`. | [`no-core-error-then-setfailed`](#no-core-error-then-setfailed) | Disallow a redundant `core.error()` call immediately before `core.setFailed()` with the same message | | [`require-escaped-regexp-interpolation`](#require-escaped-regexp-interpolation) | Require regex-escaping of interpolated values in `new RegExp()` template literals | +### `no-empty-catch-block` + +Disallow empty `catch` blocks, which silently swallow errors that otherwise remain invisible in CI logs. + +Empty catch blocks are allowed only when their comment explicitly documents an intentional no-op with `intentional`, `best-effort`, or `best effort` (case-insensitive). Otherwise, log the error, assign a fallback value, or rethrow it. + ### `no-duplicate-constant-values` Inventory module-level `const` declarations with static primitive initializers and report each declaration after the first one that uses the same value in a file. The diagnostic names both constants and shows the duplicated value. diff --git a/eslint-factory/src/rules/no-empty-catch-block.test.ts b/eslint-factory/src/rules/no-empty-catch-block.test.ts index f12e92068dc..1d250ac418b 100644 --- a/eslint-factory/src/rules/no-empty-catch-block.test.ts +++ b/eslint-factory/src/rules/no-empty-catch-block.test.ts @@ -19,9 +19,11 @@ describe("no-empty-catch-block", () => { valid: [ `try { risky(); } catch (err) { core.debug(getErrorMessage(err)); }`, `try { value = JSON.parse(raw); } catch { value = {}; }`, - `try { risky(); } catch { /* best-effort cleanup, ignore */ }`, + `try { risky(); } catch { /* best-effort cleanup */ }`, + `try { risky(); } catch { /* best effort cleanup */ }`, `try { risky(); } catch (err) { throw err; }`, `try { risky(); } catch (err) {\n // intentional no-op: file may not exist on first run\n}`, + `try { risky(); } catch { /* intentional ignore: optional file is absent */ }`, ], invalid: [], }); @@ -43,6 +45,18 @@ describe("no-empty-catch-block", () => { code: `try { risky(); } catch (err) {\n\n}`, errors: [{ messageId: "noEmptyCatch" }], }, + { + code: `try { risky(); } catch { /* TODO */ }`, + errors: [{ messageId: "noEmptyCatch" }], + }, + { + code: `try { risky(); } catch { /* file processing failed */ }`, + errors: [{ messageId: "noEmptyCatch" }], + }, + { + code: `try { risky(); } catch { /* eslint-ignore */ }`, + errors: [{ messageId: "noEmptyCatch" }], + }, ], }); }); diff --git a/eslint-factory/src/rules/no-empty-catch-block.ts b/eslint-factory/src/rules/no-empty-catch-block.ts index 680591d6035..5b3d7418ae0 100644 --- a/eslint-factory/src/rules/no-empty-catch-block.ts +++ b/eslint-factory/src/rules/no-empty-catch-block.ts @@ -7,11 +7,12 @@ export const noEmptyCatchBlockRule = createRule({ meta: { type: "problem", docs: { - description: "Disallow empty catch blocks in actions/setup/js scripts. Swallowing an error with no logging, fallback assignment, or comment hides real failures (corrupted state files, cleanup errors) that are hard to diagnose from CI logs.", + description: + "Disallow empty catch blocks in actions/setup/js scripts. Swallowing an error with no logging, fallback assignment, or explicit intentional-ignore comment hides real failures (corrupted state files, cleanup errors) that are hard to diagnose from CI logs.", }, schema: [], messages: { - noEmptyCatch: "Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add a comment explaining why the error is intentionally ignored.", + noEmptyCatch: "Empty catch block silently swallows the error. Log it (e.g. core.debug/core.warning), assign a fallback value, or add an explicit comment explaining why the error is intentionally ignored.", }, }, defaultOptions: [], @@ -23,10 +24,11 @@ export const noEmptyCatchBlockRule = createRule({ const body = node.body.body; if (body.length !== 0) return; - // A comment inside the (otherwise empty) braces documents intent, e.g.: - // } catch { /* best-effort cleanup, ignore */ } + // An explicit intentional-ignore comment inside the otherwise empty + // braces documents intent, e.g.: + // } catch { /* best-effort cleanup */ } const commentsInside = sourceCode.getCommentsInside(node.body); - if (commentsInside.length > 0) return; + if (commentsInside.some(comment => /\bintentional\b|\bbest[- ]effort\b/i.test(comment.value))) return; context.report({ node: node.body,