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/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..1d250ac418b --- /dev/null +++ b/eslint-factory/src/rules/no-empty-catch-block.test.ts @@ -0,0 +1,63 @@ +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 */ }`, + `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: [], + }); + }); + + 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" }], + }, + { + 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 new file mode 100644 index 00000000000..5b3d7418ae0 --- /dev/null +++ b/eslint-factory/src/rules/no-empty-catch-block.ts @@ -0,0 +1,40 @@ +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 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 an explicit 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; + + // 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.some(comment => /\bintentional\b|\bbest[- ]effort\b/i.test(comment.value))) return; + + context.report({ + node: node.body, + messageId: "noEmptyCatch", + }); + }, + }; + }, +});