diff --git a/eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts b/eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts index df67fbe7a30..6f599c5dca1 100644 --- a/eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts +++ b/eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts @@ -342,6 +342,96 @@ try { }); }); + it("valid: typeof err === 'object' guard suppresses all warnings in the catch block", () => { + cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { + valid: [ + `try { f(); } catch (err) { if (typeof err === 'object') { console.log(err.status); } }`, + `try { f(); } catch (err) { if (typeof err === 'object' && err !== null) { console.log(err.status); } }`, + `try { f(); } catch (err) { if ('object' === typeof err) { console.log(err.status); } }`, + ], + invalid: [], + }); + }); + + it("invalid: err.status without guard is flagged", () => { + cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { if (err.status === 404) { } }`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "status", errorVar: "err" }, + suggestions: [ + { + messageId: "wrapWithInstanceof", + data: { errorVar: "err", prop: "status" }, + output: `try { f(); } catch (err) { if ((err instanceof Error ? err.status : undefined) === 404) { } }`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: err.cause without guard is flagged", () => { + cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { console.log(err.cause); }`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "cause", errorVar: "err" }, + suggestions: [ + { + messageId: "wrapWithInstanceof", + data: { errorVar: "err", prop: "cause" }, + output: `try { f(); } catch (err) { console.log((err instanceof Error ? err.cause : undefined)); }`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: err.name without guard is flagged", () => { + cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { console.log(err.name); }`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "name", errorVar: "err" }, + suggestions: [ + { + messageId: "wrapWithInstanceof", + data: { errorVar: "err", prop: "name" }, + output: `try { f(); } catch (err) { console.log((err instanceof Error ? err.name : undefined)); }`, + }, + ], + }, + ], + }, + ], + }); + }); + + it("valid: typeof err === 'object' guard suppresses .status access (mirrors real call sites)", () => { + cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { + valid: [`try { f(); } catch (err) { if (typeof err === 'object' && err.status === 404) { } }`], + invalid: [], + }); + }); + it("valid: nested try/catch — inner guard does not affect the outer catch block", () => { cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { valid: [], diff --git a/eslint-factory/src/rules/no-unsafe-catch-error-property.ts b/eslint-factory/src/rules/no-unsafe-catch-error-property.ts index d23e955df99..ef389728788 100644 --- a/eslint-factory/src/rules/no-unsafe-catch-error-property.ts +++ b/eslint-factory/src/rules/no-unsafe-catch-error-property.ts @@ -2,7 +2,7 @@ import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils" const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); -const UNSAFE_PROPERTIES = new Set(["message", "stack", "code"]); +const UNSAFE_PROPERTIES = new Set(["message", "stack", "code", "status", "cause", "name"]); interface CatchFrame { varName: string; @@ -16,7 +16,7 @@ export const noUnsafeCatchErrorPropertyRule = createRule({ type: "problem", hasSuggestions: true, docs: { - description: "Disallow direct access to .message, .stack, or .code on a caught error variable without a getErrorMessage guard", + description: "Disallow direct access to .message, .stack, .code, .status, .cause, or .name on a caught error variable without a getErrorMessage guard", }, schema: [], messages: { @@ -90,6 +90,7 @@ export const noUnsafeCatchErrorPropertyRule = createRule({ }, // Detect catchVar instanceof Error — also accepted as a safe guard + // Detect typeof catchVar === 'object' — also accepted as a safe guard BinaryExpression(node) { if (catchStack.length === 0) return; const top = catchStack[catchStack.length - 1]; @@ -97,11 +98,33 @@ export const noUnsafeCatchErrorPropertyRule = createRule({ if (node.operator === "instanceof" && node.left.type === AST_NODE_TYPES.Identifier && node.left.name === top.varName) { top.hasGuard = true; + return; + } + + // typeof varName === 'object' or 'object' === typeof varName + if (node.operator === "===") { + const { left, right } = node; + const isTypeofObject = + (left.type === AST_NODE_TYPES.UnaryExpression && + left.operator === "typeof" && + left.argument.type === AST_NODE_TYPES.Identifier && + left.argument.name === top.varName && + right.type === AST_NODE_TYPES.Literal && + right.value === "object") || + (right.type === AST_NODE_TYPES.UnaryExpression && + right.operator === "typeof" && + right.argument.type === AST_NODE_TYPES.Identifier && + right.argument.name === top.varName && + left.type === AST_NODE_TYPES.Literal && + left.value === "object"); + if (isTypeofObject) { + top.hasGuard = true; + } } }, - // Collect catchVar.message / catchVar.stack / catchVar.code accesses - // Also detects computed string-literal access: catchVar["message"], catchVar["stack"], catchVar["code"] + // Collect catchVar.message / catchVar.stack / catchVar.code / catchVar.status / catchVar.cause / catchVar.name accesses + // Also detects computed string-literal access: catchVar["message"], catchVar["status"], etc. MemberExpression(node) { if (catchStack.length === 0) return; const top = catchStack[catchStack.length - 1]; @@ -112,13 +135,13 @@ export const noUnsafeCatchErrorPropertyRule = createRule({ if (obj.type !== AST_NODE_TYPES.Identifier || obj.name !== top.varName) return; - // Non-computed dot access: err.message / err.stack / err.code + // Non-computed dot access: err.message / err.stack / err.code / err.status / err.cause / err.name if (!node.computed && prop.type === AST_NODE_TYPES.Identifier && UNSAFE_PROPERTIES.has(prop.name)) { top.unsafeNodes.push({ node, prop: prop.name }); return; } - // Computed string-literal access: err["message"] / err["stack"] / err["code"] + // Computed string-literal access: err["message"] / err["stack"] / err["status"] / etc. // Dynamic access (err[prop]) is kept out of scope intentionally. if (node.computed && prop.type === AST_NODE_TYPES.Literal && typeof prop.value === "string" && UNSAFE_PROPERTIES.has(prop.value)) { top.unsafeNodes.push({ node, prop: prop.value }); diff --git a/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.test.ts b/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.test.ts index 0d994873b3c..e990948a3f3 100644 --- a/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.test.ts +++ b/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.test.ts @@ -240,6 +240,64 @@ describe("no-unsafe-promise-catch-error-property", () => { }); }); + it("valid: typeof err === 'object' guard suppresses warnings in .catch() callback", () => { + cjsRuleTester.run("no-unsafe-promise-catch-error-property", noUnsafePromiseCatchErrorPropertyRule, { + valid: [`promise.catch(err => { if (typeof err === 'object') { console.log(err.status); } });`, `promise.catch(err => { if ('object' === typeof err) { console.log(err.status); } });`], + invalid: [], + }); + }); + + it("invalid: err.status without guard is flagged in .catch() callback", () => { + cjsRuleTester.run("no-unsafe-promise-catch-error-property", noUnsafePromiseCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `promise.catch(err => { if (err.status === 404) { } });`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "status", errorVar: "err" }, + }, + ], + }, + ], + }); + }); + + it("invalid: err.cause without guard is flagged in .catch() callback", () => { + cjsRuleTester.run("no-unsafe-promise-catch-error-property", noUnsafePromiseCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `promise.catch(err => { console.log(err.cause); });`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "cause", errorVar: "err" }, + }, + ], + }, + ], + }); + }); + + it("invalid: err.name without guard is flagged in .catch() callback", () => { + cjsRuleTester.run("no-unsafe-promise-catch-error-property", noUnsafePromiseCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `promise.catch(err => { console.log(err.name); });`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "name", errorVar: "err" }, + }, + ], + }, + ], + }); + }); + it("invalid: nested .catch() callbacks are checked independently", () => { cjsRuleTester.run("no-unsafe-promise-catch-error-property", noUnsafePromiseCatchErrorPropertyRule, { valid: [], diff --git a/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.ts b/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.ts index f7735cbfdf2..2114e612052 100644 --- a/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.ts +++ b/eslint-factory/src/rules/no-unsafe-promise-catch-error-property.ts @@ -2,7 +2,7 @@ import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils" const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); -const UNSAFE_PROPERTIES = new Set(["message", "stack", "code"]); +const UNSAFE_PROPERTIES = new Set(["message", "stack", "code", "status", "cause", "name"]); interface CatchFrame { varName: string; @@ -25,7 +25,7 @@ export const noUnsafePromiseCatchErrorPropertyRule = createRule({ type: "problem", hasSuggestions: true, docs: { - description: "Disallow direct access to .message, .stack, or .code on a promise .catch() callback parameter without a getErrorMessage guard", + description: "Disallow direct access to .message, .stack, .code, .status, .cause, or .name on a promise .catch() callback parameter without a getErrorMessage guard", }, schema: [], messages: { @@ -101,6 +101,7 @@ export const noUnsafePromiseCatchErrorPropertyRule = createRule({ }, // Detect catchVar instanceof Error — also accepted as a safe guard + // Detect typeof catchVar === 'object' — also accepted as a safe guard BinaryExpression(node) { if (stack.length === 0) return; const top = stack[stack.length - 1]; @@ -108,10 +109,32 @@ export const noUnsafePromiseCatchErrorPropertyRule = createRule({ if (node.operator === "instanceof" && node.left.type === AST_NODE_TYPES.Identifier && node.left.name === top.varName) { top.hasGuard = true; + return; + } + + // typeof varName === 'object' or 'object' === typeof varName + if (node.operator === "===") { + const { left, right } = node; + const isTypeofObject = + (left.type === AST_NODE_TYPES.UnaryExpression && + left.operator === "typeof" && + left.argument.type === AST_NODE_TYPES.Identifier && + left.argument.name === top.varName && + right.type === AST_NODE_TYPES.Literal && + right.value === "object") || + (right.type === AST_NODE_TYPES.UnaryExpression && + right.operator === "typeof" && + right.argument.type === AST_NODE_TYPES.Identifier && + right.argument.name === top.varName && + left.type === AST_NODE_TYPES.Literal && + left.value === "object"); + if (isTypeofObject) { + top.hasGuard = true; + } } }, - // Collect catchVar.message / catchVar.stack / catchVar.code accesses + // Collect catchVar.message / catchVar.stack / catchVar.code / catchVar.status / catchVar.cause / catchVar.name accesses MemberExpression(node) { if (stack.length === 0) return; const top = stack[stack.length - 1];