diff --git a/eslint-factory/README.md b/eslint-factory/README.md index a0d170f6376..9a83eec7690 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -16,6 +16,25 @@ This project hosts custom ESLint linters for `/actions/setup/js`. ## Rules +### `no-json-stringify-error` + +Disallow `JSON.stringify()` on caught error variables. `Error` properties (`message`, `stack`, etc.) are non-enumerable, so `JSON.stringify(err)` silently produces `{}`. + +**Detected scopes:** +- `try { } catch (err) { }` — catch-clause bindings. +- `p.catch(err => ...)` — inline arrow or function callbacks passed as the first argument to `.catch()`. +- `p.then(onFulfilled, err => ...)` — inline rejection handlers passed as the **second** argument to `.then()`, which are semantically equivalent to `.catch()`. + +**Out of scope:** named-reference handlers such as `p.catch(handler)` or `p.then(ok, handler)` — the rule does not follow references across files or scopes. + +Flagged forms: +- `JSON.stringify(err)` where `err` is a catch-clause or inline rejection-handler parameter. +- `JSON.stringify(err, null, 2)` (with replacer/space arguments). + +Safe alternatives: +- `getErrorMessage(err)` from `error_helpers.cjs` (auto-suggested fix). +- `JSON.stringify({ message: err.message, stack: err.stack })` — explicitly serializing safe string properties. + ### `prefer-number-isnan` Prefer `Number.isNaN()` over global `isNaN()` to avoid silent coercion of non-numeric inputs. diff --git a/eslint-factory/src/rules/no-json-stringify-error.test.ts b/eslint-factory/src/rules/no-json-stringify-error.test.ts index 0b58fd18d2f..76100f6b42e 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.test.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.test.ts @@ -183,6 +183,91 @@ describe("no-json-stringify-error", () => { }); }); + it("valid: p.catch(handler) named-reference is not flagged", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [`const handler = err => JSON.stringify(err); p.catch(handler);`], + invalid: [], + }); + }); + + it("valid: p.then(ok, handler) named-reference rejection handler is not flagged", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [`function onRejected(err) { JSON.stringify(err); } p.then(ok, onRejected);`], + invalid: [], + }); + }); + + it("invalid: JSON.stringify(err) in .then() second-argument rejection handler (arrow) is flagged", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `p.then(result => result, err => core.error(JSON.stringify(err)));`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `p.then(result => result, err => core.error(getErrorMessage(err)));`, + }, + ], + }, + ], + }, + { + code: `p.then(null, err => core.error(JSON.stringify(err, null, 2)));`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `p.then(null, err => core.error(getErrorMessage(err)));`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: JSON.stringify(err) in .then() second-argument rejection handler (function) is flagged", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `p.then(null, function(err) { core.error(JSON.stringify(err)); });`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `p.then(null, function(err) { core.error(getErrorMessage(err)); });`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("valid: first argument of .then() (onFulfilled) is not treated as a rejection handler", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [`p.then(result => JSON.stringify(result));`, `p.then(function(result) { JSON.stringify(result); });`, `p.then(err => core.info(JSON.stringify(err)));`], + invalid: [], + }); + }); + it("invalid: works with ES module syntax", () => { esmRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { valid: [], @@ -203,6 +288,22 @@ describe("no-json-stringify-error", () => { }, ], }, + { + code: `p.then(ok, err => console.error(JSON.stringify(err)));`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `p.then(ok, err => console.error(getErrorMessage(err)));`, + }, + ], + }, + ], + }, ], }); }); diff --git a/eslint-factory/src/rules/no-json-stringify-error.ts b/eslint-factory/src/rules/no-json-stringify-error.ts index 6da57870020..5ba4e56773e 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.ts @@ -8,17 +8,28 @@ interface ErrorScope { } /** - * Returns true when the function is passed directly as the first argument to a - * `.catch()` call. Named-reference handlers (for example `p.catch(handler)`) - * are intentionally out of scope. + * Returns true when the function node is an inline rejection handler passed to + * a promise method: + * - First argument of `.catch(fn)` — the canonical rejection handler. + * - Second argument of `.then(onFulfilled, onRejected)` — the rejection-handler + * position, analogous to `.catch(fn)`. + * + * Named-reference handlers (for example `p.catch(handler)`) are intentionally + * out of scope because the rule cannot statically follow the reference to + * inspect its parameter list or body. */ -function isCatchCallback(node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression): boolean { +function isInlineRejectionHandler(node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression): boolean { const parent = node.parent; if (!parent || parent.type !== AST_NODE_TYPES.CallExpression) return false; const callee = parent.callee; if (callee.type !== AST_NODE_TYPES.MemberExpression || callee.computed) return false; const prop = callee.property; - return prop.type === AST_NODE_TYPES.Identifier && prop.name === "catch" && parent.arguments[0] === node; + if (prop.type !== AST_NODE_TYPES.Identifier) return false; + // .catch(fn) — first argument + if (prop.name === "catch" && parent.arguments[0] === node) return true; + // .then(onFulfilled, onRejected) — second argument is the rejection handler + if (prop.name === "then" && parent.arguments[1] === node) return true; + return false; } export const noJsonStringifyErrorRule = createRule({ @@ -27,7 +38,10 @@ export const noJsonStringifyErrorRule = createRule({ type: "problem", hasSuggestions: true, docs: { - description: "Disallow JSON.stringify() on caught error variables — Error properties (message, stack, etc.) are non-enumerable and produce {} silently", + description: + "Disallow JSON.stringify() on caught error variables — Error properties (message, stack, etc.) are non-enumerable and produce {} silently. " + + "Detected scopes: try/catch bindings, .catch(fn) inline callbacks, and .then(onFulfilled, onRejected) inline rejection handlers. " + + "Named-reference handlers (e.g. p.catch(handler)) are out of scope because the rule cannot statically follow the reference.", }, schema: [], messages: { @@ -55,7 +69,7 @@ export const noJsonStringifyErrorRule = createRule({ } function enterFunction(node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression): void { - if (isCatchCallback(node)) { + if (isInlineRejectionHandler(node)) { const params = node.params; if (params.length === 1 && params[0].type === AST_NODE_TYPES.Identifier) { scopeStack.push({ varName: params[0].name, isSentinel: false }); @@ -64,7 +78,7 @@ export const noJsonStringifyErrorRule = createRule({ scopeStack.push({ varName: "", isSentinel: true }); } } else { - // Non-.catch() function: sentinel to avoid false positives from shadowed names + // Non-rejection-handler function: sentinel to avoid false positives from shadowed names scopeStack.push({ varName: "", isSentinel: true }); } } @@ -87,7 +101,7 @@ export const noJsonStringifyErrorRule = createRule({ scopeStack.pop(); }, - // Track .catch() callback parameters + // Track inline rejection-handler parameters from .catch(fn) and .then(_, fn) ArrowFunctionExpression: enterFunction, "ArrowFunctionExpression:exit": exitFunction, FunctionExpression: enterFunction,