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
90 changes: 90 additions & 0 deletions eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) { } }`,
Comment on lines +368 to +370
},
],
},
],
},
],
});
});

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: [],
Expand Down
35 changes: 29 additions & 6 deletions eslint-factory/src/rules/no-unsafe-catch-error-property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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: {
Expand Down Expand Up @@ -90,18 +90,41 @@ 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];
if (!top || top.hasGuard || !top.varName) return;

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 &&
Comment on lines +104 to +108
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];
Expand All @@ -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 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down
29 changes: 26 additions & 3 deletions eslint-factory/src/rules/no-unsafe-promise-catch-error-property.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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: {
Expand Down Expand Up @@ -101,17 +101,40 @@ 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];
if (!top || top.hasGuard || !top.varName) return;

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 &&
Comment on lines +115 to +119
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];
Expand Down
Loading