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 6f599c5dca1..9c416c8f65d 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,17 +342,76 @@ try { }); }); - it("valid: typeof err === 'object' guard suppresses all warnings in the catch block", () => { + it("valid: typeof err === 'object' with non-null guard suppresses 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); } }`, + `try { f(); } catch (err) { if ('object' === typeof err && null !== err) { console.log(err.status); } }`, + `try { f(); } catch (err) { if (typeof err === 'object' && err != null) { console.log(err.status); } }`, + `try { f(); } catch (err) { if (err && typeof err === 'object') { console.log(err.status); } }`, + `try { f(); } catch (err) { if (!err) return; if (typeof err === 'object') { console.log(err.status); } }`, ], invalid: [], }); }); + it("invalid: bare typeof err === 'object' guard is insufficient", () => { + cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { if (typeof err === 'object') { console.log(err.status); } }`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "status", errorVar: "err" }, + suggestions: [ + { + messageId: "wrapWithInstanceof", + data: { errorVar: "err", prop: "status" }, + output: `try { f(); } catch (err) { if (typeof err === 'object') { console.log((err instanceof Error ? err.status : undefined)); } }`, + }, + ], + }, + ], + }, + { + code: `try { f(); } catch (err) { if ('object' === typeof err) { console.log(err.status); } }`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "status", errorVar: "err" }, + suggestions: [ + { + messageId: "wrapWithInstanceof", + data: { errorVar: "err", prop: "status" }, + output: `try { f(); } catch (err) { if ('object' === typeof err) { console.log((err instanceof Error ? err.status : undefined)); } }`, + }, + ], + }, + ], + }, + { + // Standalone err !== null in a separate if (without return) does not count as companion guard + code: `try { f(); } catch (err) { if (err !== null) { } if (typeof err === 'object') { console.log(err.status); } }`, + errors: [ + { + messageId: "unsafeProperty", + data: { prop: "status", errorVar: "err" }, + suggestions: [ + { + messageId: "wrapWithInstanceof", + data: { errorVar: "err", prop: "status" }, + output: `try { f(); } catch (err) { if (err !== null) { } if (typeof err === 'object') { console.log((err instanceof Error ? err.status : undefined)); } }`, + }, + ], + }, + ], + }, + ], + }); + }); + it("invalid: err.status without guard is flagged", () => { cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { valid: [], @@ -425,9 +484,9 @@ try { }); }); - it("valid: typeof err === 'object' guard suppresses .status access (mirrors real call sites)", () => { + it("valid: typeof err === 'object' with truthy err guard suppresses .status access", () => { cjsRuleTester.run("no-unsafe-catch-error-property", noUnsafeCatchErrorPropertyRule, { - valid: [`try { f(); } catch (err) { if (typeof err === 'object' && err.status === 404) { } }`], + valid: [`try { f(); } catch (err) { if (err && typeof err === 'object' && err.status === 404) { } }`], invalid: [], }); }); 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 ef389728788..94865b531b0 100644 --- a/eslint-factory/src/rules/no-unsafe-catch-error-property.ts +++ b/eslint-factory/src/rules/no-unsafe-catch-error-property.ts @@ -7,9 +7,34 @@ const UNSAFE_PROPERTIES = new Set(["message", "stack", "code", "status", "cause" interface CatchFrame { varName: string; hasGuard: boolean; + hasNonNullGuard: boolean; unsafeNodes: Array<{ node: TSESTree.MemberExpression; prop: string }>; } +function isTypeofObjectCheck(node: TSESTree.Expression, varName: string): boolean { + if (node.type !== AST_NODE_TYPES.BinaryExpression || node.operator !== "===") return false; + const { left, right } = node; + return ( + (left.type === AST_NODE_TYPES.UnaryExpression && left.operator === "typeof" && left.argument.type === AST_NODE_TYPES.Identifier && left.argument.name === 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 === varName && left.type === AST_NODE_TYPES.Literal && left.value === "object") + ); +} + +function isNonNullGuardCheck(node: TSESTree.Expression, varName: string): boolean { + if (node.type === AST_NODE_TYPES.Identifier) { + return node.name === varName; + } + + if (node.type !== AST_NODE_TYPES.BinaryExpression || (node.operator !== "!==" && node.operator !== "!=")) { + return false; + } + + const isVarRef = (value: TSESTree.Expression): value is TSESTree.Identifier => value.type === AST_NODE_TYPES.Identifier && value.name === varName; + const isNullLiteral = (value: TSESTree.Expression): value is TSESTree.Literal => value.type === AST_NODE_TYPES.Literal && value.value === null; + + return (isVarRef(node.left) && isNullLiteral(node.right)) || (isVarRef(node.right) && isNullLiteral(node.left)); +} + export const noUnsafeCatchErrorPropertyRule = createRule({ name: "no-unsafe-catch-error-property", meta: { @@ -36,11 +61,11 @@ export const noUnsafeCatchErrorPropertyRule = createRule({ // Only handle simple identifier bindings; skip bare catch {} and destructuring patterns. // Push a sentinel frame so CatchClause:exit always has a matching pop. if (!param || param.type !== AST_NODE_TYPES.Identifier) { - catchStack.push({ varName: "", hasGuard: true, unsafeNodes: [] }); + catchStack.push({ varName: "", hasGuard: true, hasNonNullGuard: true, unsafeNodes: [] }); return; } - catchStack.push({ varName: param.name, hasGuard: false, unsafeNodes: [] }); + catchStack.push({ varName: param.name, hasGuard: false, hasNonNullGuard: false, unsafeNodes: [] }); }, "CatchClause:exit"() { @@ -90,7 +115,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 + // Detect typeof catchVar === 'object' with a non-null companion guard BinaryExpression(node) { if (catchStack.length === 0) return; const top = catchStack[catchStack.length - 1]; @@ -101,25 +126,48 @@ export const noUnsafeCatchErrorPropertyRule = createRule({ 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; + if (isTypeofObjectCheck(node, top.varName) && top.hasNonNullGuard) { + top.hasGuard = true; + } + }, + + LogicalExpression(node) { + if (catchStack.length === 0) return; + const top = catchStack[catchStack.length - 1]; + if (!top || top.hasGuard || !top.varName || node.operator !== "&&") return; + + const conjuncts: TSESTree.Expression[] = []; + const collectConjuncts = (expr: TSESTree.Expression): void => { + if (expr.type === AST_NODE_TYPES.LogicalExpression && expr.operator === "&&") { + collectConjuncts(expr.left); + collectConjuncts(expr.right); + return; } + conjuncts.push(expr); + }; + collectConjuncts(node); + + const hasTypeofObject = conjuncts.some(expr => isTypeofObjectCheck(expr, top.varName)); + const hasNonNullGuard = conjuncts.some(expr => isNonNullGuardCheck(expr, top.varName)); + if (hasTypeofObject && hasNonNullGuard) { + top.hasGuard = true; + top.hasNonNullGuard = true; + } + }, + + IfStatement(node) { + if (catchStack.length === 0) return; + const top = catchStack[catchStack.length - 1]; + if (!top || top.hasGuard || !top.varName) return; + + if ( + node.test.type === AST_NODE_TYPES.UnaryExpression && + node.test.operator === "!" && + node.test.argument.type === AST_NODE_TYPES.Identifier && + node.test.argument.name === top.varName && + node.consequent.type === AST_NODE_TYPES.ReturnStatement + ) { + top.hasNonNullGuard = true; } }, 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 e990948a3f3..13bb44f0a10 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,13 +240,40 @@ describe("no-unsafe-promise-catch-error-property", () => { }); }); - it("valid: typeof err === 'object' guard suppresses warnings in .catch() callback", () => { + it("valid: typeof err === 'object' with non-null 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); } });`], + valid: [ + `promise.catch(err => { if (typeof err === 'object' && err !== null) { console.log(err.status); } });`, + `promise.catch(err => { if ('object' === typeof err && null !== err) { console.log(err.status); } });`, + `promise.catch(err => { if (typeof err === 'object' && err != null) { console.log(err.status); } });`, + `promise.catch(err => { if (err && typeof err === 'object') { console.log(err.status); } });`, + `promise.catch(err => { if (!err) return; if (typeof err === 'object') { console.log(err.status); } });`, + ], invalid: [], }); }); + it("invalid: bare typeof err === 'object' guard is insufficient in .catch() callback", () => { + cjsRuleTester.run("no-unsafe-promise-catch-error-property", noUnsafePromiseCatchErrorPropertyRule, { + valid: [], + invalid: [ + { + code: `promise.catch(err => { if (typeof err === 'object') { console.log(err.status); } });`, + errors: [{ messageId: "unsafeProperty", data: { prop: "status", errorVar: "err" } }], + }, + { + code: `promise.catch(err => { if ('object' === typeof err) { console.log(err.status); } });`, + errors: [{ messageId: "unsafeProperty", data: { prop: "status", errorVar: "err" } }], + }, + { + // Standalone err !== null in a separate if (without return) does not count as companion guard + code: `promise.catch(err => { if (err !== null) { } if (typeof err === 'object') { console.log(err.status); } });`, + errors: [{ messageId: "unsafeProperty", data: { prop: "status", errorVar: "err" } }], + }, + ], + }); + }); + it("invalid: err.status without guard is flagged in .catch() callback", () => { 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 2114e612052..8a733333fe7 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 @@ -7,9 +7,34 @@ const UNSAFE_PROPERTIES = new Set(["message", "stack", "code", "status", "cause" interface CatchFrame { varName: string; hasGuard: boolean; + hasNonNullGuard: boolean; unsafeNodes: Array<{ node: TSESTree.MemberExpression; prop: string }>; } +function isTypeofObjectCheck(node: TSESTree.Expression, varName: string): boolean { + if (node.type !== AST_NODE_TYPES.BinaryExpression || node.operator !== "===") return false; + const { left, right } = node; + return ( + (left.type === AST_NODE_TYPES.UnaryExpression && left.operator === "typeof" && left.argument.type === AST_NODE_TYPES.Identifier && left.argument.name === 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 === varName && left.type === AST_NODE_TYPES.Literal && left.value === "object") + ); +} + +function isNonNullGuardCheck(node: TSESTree.Expression, varName: string): boolean { + if (node.type === AST_NODE_TYPES.Identifier) { + return node.name === varName; + } + + if (node.type !== AST_NODE_TYPES.BinaryExpression || (node.operator !== "!==" && node.operator !== "!=")) { + return false; + } + + const isVarRef = (value: TSESTree.Expression): value is TSESTree.Identifier => value.type === AST_NODE_TYPES.Identifier && value.name === varName; + const isNullLiteral = (value: TSESTree.Expression): value is TSESTree.Literal => value.type === AST_NODE_TYPES.Literal && value.value === null; + + return (isVarRef(node.left) && isNullLiteral(node.right)) || (isVarRef(node.right) && isNullLiteral(node.left)); +} + function isCatchCallback(node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression): boolean { const parent = node.parent; if (!parent || parent.type !== AST_NODE_TYPES.CallExpression) return false; @@ -45,14 +70,14 @@ export const noUnsafePromiseCatchErrorPropertyRule = createRule({ const params = node.params; // Only handle simple identifier bindings; skip no-param and destructuring callbacks. if (params.length === 1 && params[0].type === AST_NODE_TYPES.Identifier) { - stack.push({ varName: params[0].name, hasGuard: false, unsafeNodes: [] }); + stack.push({ varName: params[0].name, hasGuard: false, hasNonNullGuard: false, unsafeNodes: [] }); } else { - stack.push({ varName: "", hasGuard: true, unsafeNodes: [] }); + stack.push({ varName: "", hasGuard: true, hasNonNullGuard: true, unsafeNodes: [] }); } } else { // Sentinel: prevents the outer .catch() frame from collecting accesses // to a shadowed parameter name inside this nested function. - stack.push({ varName: "", hasGuard: true, unsafeNodes: [] }); + stack.push({ varName: "", hasGuard: true, hasNonNullGuard: true, unsafeNodes: [] }); } } @@ -101,7 +126,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 + // Detect typeof catchVar === 'object' with a non-null companion guard BinaryExpression(node) { if (stack.length === 0) return; const top = stack[stack.length - 1]; @@ -112,25 +137,48 @@ export const noUnsafePromiseCatchErrorPropertyRule = createRule({ 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; + if (isTypeofObjectCheck(node, top.varName) && top.hasNonNullGuard) { + top.hasGuard = true; + } + }, + + LogicalExpression(node) { + if (stack.length === 0) return; + const top = stack[stack.length - 1]; + if (!top || top.hasGuard || !top.varName || node.operator !== "&&") return; + + const conjuncts: TSESTree.Expression[] = []; + const collectConjuncts = (expr: TSESTree.Expression): void => { + if (expr.type === AST_NODE_TYPES.LogicalExpression && expr.operator === "&&") { + collectConjuncts(expr.left); + collectConjuncts(expr.right); + return; } + conjuncts.push(expr); + }; + collectConjuncts(node); + + const hasTypeofObject = conjuncts.some(expr => isTypeofObjectCheck(expr, top.varName)); + const hasNonNullGuard = conjuncts.some(expr => isNonNullGuardCheck(expr, top.varName)); + if (hasTypeofObject && hasNonNullGuard) { + top.hasGuard = true; + top.hasNonNullGuard = true; + } + }, + + IfStatement(node) { + if (stack.length === 0) return; + const top = stack[stack.length - 1]; + if (!top || top.hasGuard || !top.varName) return; + + if ( + node.test.type === AST_NODE_TYPES.UnaryExpression && + node.test.operator === "!" && + node.test.argument.type === AST_NODE_TYPES.Identifier && + node.test.argument.name === top.varName && + node.consequent.type === AST_NODE_TYPES.ReturnStatement + ) { + top.hasNonNullGuard = true; } },