Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions eslint-factory/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions eslint-factory/eslint.config.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
{
Expand Down
2 changes: 2 additions & 0 deletions eslint-factory/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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,
},
};

Expand Down
63 changes: 63 additions & 0 deletions eslint-factory/src/rules/no-empty-catch-block.test.ts
Original file line number Diff line number Diff line change
@@ -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" }],
},
],
});
});
});
40 changes: 40 additions & 0 deletions eslint-factory/src/rules/no-empty-catch-block.ts
Original file line number Diff line number Diff line change
@@ -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: {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] The docs.description says in

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",
});
},
};
},
});
Loading