From 72df5b32f8ff9b947cd41657c6269031c4016167 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 08:44:40 +0000 Subject: [PATCH 1/4] Initial plan From a882f0e72c555bbef9b625a699dbc7a0f7495c71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:01:48 +0000 Subject: [PATCH 2/4] feat(no-json-stringify-error): add scope-aware suggestions and detail-preserving form - Add scope check: when `getErrorMessage` is not in scope, fall back to `String(errorVar)` (useStringFallback) instead of an unresolved identifier - Add second detail-preserving suggestion that emits `\`\${getErrorMessage(errorVar)}\\n\${errorVar.stack ?? ""}\`` (or `String()` variant when out of scope) so callers who wanted null,2-style detail retain the stack trace - Add new message IDs: useStringFallback, useDetailPreservingForm, useDetailPreservingFormFallback - Update all existing invalid test cases to reflect the new two-suggestion shape - Add three new test cases: in-scope getErrorMessage path (CJS require), in-scope path inside template literal, and explicit String() fallback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../src/rules/no-json-stringify-error.test.ts | 186 +++++++++++++++--- .../src/rules/no-json-stringify-error.ts | 53 ++++- 2 files changed, 210 insertions(+), 29 deletions(-) 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 76100f6b42e..04dd941cd1e 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.test.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.test.ts @@ -75,9 +75,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", + data: { errorVar: "err" }, + output: `try { f(); } catch (err) { core.error(String(err)); }`, + }, + { + messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: `try { f(); } catch (err) { core.error(getErrorMessage(err)); }`, + output: 'try { f(); } catch (err) { core.error(`${String(err)}\\n${err.stack ?? ""}`); }', }, ], }, @@ -99,9 +104,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "error" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", + data: { errorVar: "error" }, + output: `try { f(); } catch (error) { core.error(\`details: \${String(error)}\`); }`, + }, + { + messageId: "useDetailPreservingFormFallback", data: { errorVar: "error" }, - output: `try { f(); } catch (error) { core.error(\`details: \${getErrorMessage(error)}\`); }`, + output: 'try { f(); } catch (error) { core.error(`details: ${`${String(error)}\\n${error.stack ?? ""}`}`); }', }, ], }, @@ -123,9 +133,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", + data: { errorVar: "err" }, + output: `p.catch(err => core.error(String(err)));`, + }, + { + messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: `p.catch(err => core.error(getErrorMessage(err)));`, + output: 'p.catch(err => core.error(`${String(err)}\\n${err.stack ?? ""}`));', }, ], }, @@ -147,9 +162,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", data: { errorVar: "err" }, - output: `p.catch(function(err) { core.error(getErrorMessage(err)); });`, + output: `p.catch(function(err) { core.error(String(err)); });`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'p.catch(function(err) { core.error(`${String(err)}\\n${err.stack ?? ""}`); });', }, ], }, @@ -171,9 +191,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "outer" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", data: { errorVar: "outer" }, - output: `try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error(getErrorMessage(outer)); }`, + output: `try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error(String(outer)); }`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "outer" }, + output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error(`${String(outer)}\\n${outer.stack ?? ""}`); }', }, ], }, @@ -209,9 +234,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", + data: { errorVar: "err" }, + output: `p.then(result => result, err => core.error(String(err)));`, + }, + { + messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: `p.then(result => result, err => core.error(getErrorMessage(err)));`, + output: 'p.then(result => result, err => core.error(`${String(err)}\\n${err.stack ?? ""}`));', }, ], }, @@ -225,9 +255,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", data: { errorVar: "err" }, - output: `p.then(null, err => core.error(getErrorMessage(err)));`, + output: `p.then(null, err => core.error(String(err)));`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'p.then(null, err => core.error(`${String(err)}\\n${err.stack ?? ""}`));', }, ], }, @@ -249,9 +284,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", data: { errorVar: "err" }, - output: `p.then(null, function(err) { core.error(getErrorMessage(err)); });`, + output: `p.then(null, function(err) { core.error(String(err)); });`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'p.then(null, function(err) { core.error(`${String(err)}\\n${err.stack ?? ""}`); });', }, ], }, @@ -280,9 +320,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "e" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", data: { errorVar: "e" }, - output: `try { fetch(url); } catch (e) { console.error(getErrorMessage(e)); }`, + output: `try { fetch(url); } catch (e) { console.error(String(e)); }`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "e" }, + output: 'try { fetch(url); } catch (e) { console.error(`${String(e)}\\n${e.stack ?? ""}`); }', }, ], }, @@ -296,9 +341,14 @@ describe("no-json-stringify-error", () => { data: { errorVar: "err" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", data: { errorVar: "err" }, - output: `p.then(ok, err => console.error(getErrorMessage(err)));`, + output: `p.then(ok, err => console.error(String(err)));`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'p.then(ok, err => console.error(`${String(err)}\\n${err.stack ?? ""}`));', }, ], }, @@ -320,9 +370,101 @@ describe("no-json-stringify-error", () => { data: { errorVar: "inner" }, suggestions: [ { - messageId: "useGetErrorMessage", + messageId: "useStringFallback", + data: { errorVar: "inner" }, + output: `try { f(); } catch (outer) { try { g(); } catch (inner) { core.error(String(inner)); } }`, + }, + { + messageId: "useDetailPreservingFormFallback", data: { errorVar: "inner" }, - output: `try { f(); } catch (outer) { try { g(); } catch (inner) { core.error(getErrorMessage(inner)); } }`, + output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { core.error(`${String(inner)}\\n${inner.stack ?? ""}`); } }', + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is in scope, suggests getErrorMessage and detail-preserving form", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `const getErrorMessage = require('./error_helpers').getErrorMessage; try { f(); } catch (err) { core.error(JSON.stringify(err)); }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `const getErrorMessage = require('./error_helpers').getErrorMessage; try { f(); } catch (err) { core.error(getErrorMessage(err)); }`, + }, + { + messageId: "useDetailPreservingForm", + data: { errorVar: "err" }, + output: "const getErrorMessage = require('./error_helpers').getErrorMessage; try { f(); } catch (err) { core.error(`${getErrorMessage(err)}\\n${err.stack ?? \"\"}`); }", + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is in scope via destructured require, detail-preserving form uses getErrorMessage", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `const { getErrorMessage } = require('./error_helpers'); try { f(); } catch (error) { core.error(\`details: \${JSON.stringify(error, null, 2)}\`); }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "error" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "error" }, + output: `const { getErrorMessage } = require('./error_helpers'); try { f(); } catch (error) { core.error(\`details: \${getErrorMessage(error)}\`); }`, + }, + { + messageId: "useDetailPreservingForm", + data: { errorVar: "error" }, + output: "const { getErrorMessage } = require('./error_helpers'); try { f(); } catch (error) { core.error(`details: ${`${getErrorMessage(error)}\\n${error.stack ?? \"\"}`}`); }", + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is not in scope, first suggestion uses String() fallback", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { console.error(JSON.stringify(err)); }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useStringFallback", + data: { errorVar: "err" }, + output: `try { f(); } catch (err) { console.error(String(err)); }`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'try { f(); } catch (err) { console.error(`${String(err)}\\n${err.stack ?? ""}`); }', }, ], }, diff --git a/eslint-factory/src/rules/no-json-stringify-error.ts b/eslint-factory/src/rules/no-json-stringify-error.ts index 5ba4e56773e..0be3a87c4d7 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.ts @@ -48,6 +48,9 @@ export const noJsonStringifyErrorRule = createRule({ jsonStringifyError: "JSON.stringify({{errorVar}}) produces {} for Error objects — Error properties (message, stack, etc.) are non-enumerable. Prefer getErrorMessage({{errorVar}}) from error_helpers.cjs or explicitly serialize a guarded value after narrowing it.", useGetErrorMessage: "Replace with getErrorMessage({{errorVar}}) — ensure getErrorMessage is imported from error_helpers.cjs.", + useStringFallback: "Replace with String({{errorVar}}) — getErrorMessage is not in scope; String() is a safe import-free alternative.", + useDetailPreservingForm: "Replace with a template literal combining getErrorMessage({{errorVar}}) and {{errorVar}}.stack to preserve detail — ensure getErrorMessage is imported from error_helpers.cjs.", + useDetailPreservingFormFallback: "Replace with a template literal combining String({{errorVar}}) and {{errorVar}}.stack to preserve detail.", }, }, defaultOptions: [], @@ -125,18 +128,54 @@ export const noJsonStringifyErrorRule = createRule({ if (!caughtNames.has(firstArg.name)) return; const errorVar = firstArg.name; + + // Check whether getErrorMessage is resolvable in the current scope chain. + const getErrorMessageInScope = (() => { + let scope: ReturnType | null = context.sourceCode.getScope(node); + while (scope) { + if (scope.variables.some(v => v.name === "getErrorMessage")) return true; + scope = scope.upper; + } + return false; + })(); + context.report({ node, messageId: "jsonStringifyError", data: { errorVar }, suggest: [ - { - messageId: "useGetErrorMessage" as const, - data: { errorVar }, - fix(fixer) { - return fixer.replaceText(node, `getErrorMessage(${errorVar})`); - }, - }, + // Suggestion 1: concise message-only replacement + getErrorMessageInScope + ? ({ + messageId: "useGetErrorMessage" as const, + data: { errorVar }, + fix(fixer) { + return fixer.replaceText(node, `getErrorMessage(${errorVar})`); + }, + } as const) + : ({ + messageId: "useStringFallback" as const, + data: { errorVar }, + fix(fixer) { + return fixer.replaceText(node, `String(${errorVar})`); + }, + } as const), + // Suggestion 2: detail-preserving replacement (retains stack trace) + getErrorMessageInScope + ? ({ + messageId: "useDetailPreservingForm" as const, + data: { errorVar }, + fix(fixer) { + return fixer.replaceText(node, `\`\${getErrorMessage(${errorVar})}\\n\${${errorVar}.stack ?? ""}\``); + }, + } as const) + : ({ + messageId: "useDetailPreservingFormFallback" as const, + data: { errorVar }, + fix(fixer) { + return fixer.replaceText(node, `\`\${String(${errorVar})}\\n\${${errorVar}.stack ?? ""}\``); + }, + } as const), ], }); }, From c014ca1303e5f2b392b9f8e5a47745689d981f17 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 09:08:24 +0000 Subject: [PATCH 3/4] fix(no-json-stringify-error): use string concatenation for detail-preserving suggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace template-literal approach with string concatenation to avoid nested template literals when the flagged call is already inside a template expression. The detail-preserving suggestions now produce: getErrorMessage(err) + "\n" + (err.stack ?? "") (in-scope path) String(err) + "\n" + (err.stack ?? "") (fallback path) Both forms are valid in any syntactic context — standalone arguments, template expressions, and ternaries — without requiring template nesting. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../src/rules/no-json-stringify-error.test.ts | 28 +++++++++---------- .../src/rules/no-json-stringify-error.ts | 8 +++--- 2 files changed, 18 insertions(+), 18 deletions(-) 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 04dd941cd1e..2d1e12394f8 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.test.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.test.ts @@ -82,7 +82,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'try { f(); } catch (err) { core.error(`${String(err)}\\n${err.stack ?? ""}`); }', + output: 'try { f(); } catch (err) { core.error(String(err) + "\\n" + (err.stack ?? "")); }', }, ], }, @@ -111,7 +111,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "error" }, - output: 'try { f(); } catch (error) { core.error(`details: ${`${String(error)}\\n${error.stack ?? ""}`}`); }', + output: 'try { f(); } catch (error) { core.error(`details: ${String(error) + "\\n" + (error.stack ?? "")}`); }', }, ], }, @@ -140,7 +140,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.catch(err => core.error(`${String(err)}\\n${err.stack ?? ""}`));', + output: 'p.catch(err => core.error(String(err) + "\\n" + (err.stack ?? "")));', }, ], }, @@ -169,7 +169,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.catch(function(err) { core.error(`${String(err)}\\n${err.stack ?? ""}`); });', + output: 'p.catch(function(err) { core.error(String(err) + "\\n" + (err.stack ?? "")); });', }, ], }, @@ -198,7 +198,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "outer" }, - output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error(`${String(outer)}\\n${outer.stack ?? ""}`); }', + output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error(String(outer) + "\\n" + (outer.stack ?? "")); }', }, ], }, @@ -241,7 +241,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(result => result, err => core.error(`${String(err)}\\n${err.stack ?? ""}`));', + output: 'p.then(result => result, err => core.error(String(err) + "\\n" + (err.stack ?? "")));', }, ], }, @@ -262,7 +262,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(null, err => core.error(`${String(err)}\\n${err.stack ?? ""}`));', + output: 'p.then(null, err => core.error(String(err) + "\\n" + (err.stack ?? "")));', }, ], }, @@ -291,7 +291,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(null, function(err) { core.error(`${String(err)}\\n${err.stack ?? ""}`); });', + output: 'p.then(null, function(err) { core.error(String(err) + "\\n" + (err.stack ?? "")); });', }, ], }, @@ -327,7 +327,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "e" }, - output: 'try { fetch(url); } catch (e) { console.error(`${String(e)}\\n${e.stack ?? ""}`); }', + output: 'try { fetch(url); } catch (e) { console.error(String(e) + "\\n" + (e.stack ?? "")); }', }, ], }, @@ -348,7 +348,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(ok, err => console.error(`${String(err)}\\n${err.stack ?? ""}`));', + output: 'p.then(ok, err => console.error(String(err) + "\\n" + (err.stack ?? "")));', }, ], }, @@ -377,7 +377,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "inner" }, - output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { core.error(`${String(inner)}\\n${inner.stack ?? ""}`); } }', + output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { core.error(String(inner) + "\\n" + (inner.stack ?? "")); } }', }, ], }, @@ -406,7 +406,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingForm", data: { errorVar: "err" }, - output: "const getErrorMessage = require('./error_helpers').getErrorMessage; try { f(); } catch (err) { core.error(`${getErrorMessage(err)}\\n${err.stack ?? \"\"}`); }", + output: 'const getErrorMessage = require(\'./error_helpers\').getErrorMessage; try { f(); } catch (err) { core.error(getErrorMessage(err) + "\\n" + (err.stack ?? "")); }', }, ], }, @@ -435,7 +435,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingForm", data: { errorVar: "error" }, - output: "const { getErrorMessage } = require('./error_helpers'); try { f(); } catch (error) { core.error(`details: ${`${getErrorMessage(error)}\\n${error.stack ?? \"\"}`}`); }", + output: 'const { getErrorMessage } = require(\'./error_helpers\'); try { f(); } catch (error) { core.error(`details: ${getErrorMessage(error) + "\\n" + (error.stack ?? "")}`); }', }, ], }, @@ -464,7 +464,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'try { f(); } catch (err) { console.error(`${String(err)}\\n${err.stack ?? ""}`); }', + output: 'try { f(); } catch (err) { console.error(String(err) + "\\n" + (err.stack ?? "")); }', }, ], }, diff --git a/eslint-factory/src/rules/no-json-stringify-error.ts b/eslint-factory/src/rules/no-json-stringify-error.ts index 0be3a87c4d7..4936b21e4cd 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.ts @@ -49,8 +49,8 @@ export const noJsonStringifyErrorRule = createRule({ "JSON.stringify({{errorVar}}) produces {} for Error objects — Error properties (message, stack, etc.) are non-enumerable. Prefer getErrorMessage({{errorVar}}) from error_helpers.cjs or explicitly serialize a guarded value after narrowing it.", useGetErrorMessage: "Replace with getErrorMessage({{errorVar}}) — ensure getErrorMessage is imported from error_helpers.cjs.", useStringFallback: "Replace with String({{errorVar}}) — getErrorMessage is not in scope; String() is a safe import-free alternative.", - useDetailPreservingForm: "Replace with a template literal combining getErrorMessage({{errorVar}}) and {{errorVar}}.stack to preserve detail — ensure getErrorMessage is imported from error_helpers.cjs.", - useDetailPreservingFormFallback: "Replace with a template literal combining String({{errorVar}}) and {{errorVar}}.stack to preserve detail.", + useDetailPreservingForm: 'Replace with getErrorMessage({{errorVar}}) + "\\n" + ({{errorVar}}.stack ?? "") to preserve the stack trace — ensure getErrorMessage is imported from error_helpers.cjs.', + useDetailPreservingFormFallback: 'Replace with String({{errorVar}}) + "\\n" + ({{errorVar}}.stack ?? "") to preserve the stack trace.', }, }, defaultOptions: [], @@ -166,14 +166,14 @@ export const noJsonStringifyErrorRule = createRule({ messageId: "useDetailPreservingForm" as const, data: { errorVar }, fix(fixer) { - return fixer.replaceText(node, `\`\${getErrorMessage(${errorVar})}\\n\${${errorVar}.stack ?? ""}\``); + return fixer.replaceText(node, `getErrorMessage(${errorVar}) + "\\n" + (${errorVar}.stack ?? "")`); }, } as const) : ({ messageId: "useDetailPreservingFormFallback" as const, data: { errorVar }, fix(fixer) { - return fixer.replaceText(node, `\`\${String(${errorVar})}\\n\${${errorVar}.stack ?? ""}\``); + return fixer.replaceText(node, `String(${errorVar}) + "\\n" + (${errorVar}.stack ?? "")`); }, } as const), ], From 9636d69a622f51ed041bbbc7ebfc1fd8eb2fc2bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:22:48 +0000 Subject: [PATCH 4/4] fix no-json-stringify-error review feedback Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../src/rules/no-json-stringify-error.test.ts | 173 ++++++++++++++++-- .../src/rules/no-json-stringify-error.ts | 47 +++-- 2 files changed, 192 insertions(+), 28 deletions(-) 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 2d1e12394f8..c2f4c11347a 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.test.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.test.ts @@ -82,7 +82,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'try { f(); } catch (err) { core.error(String(err) + "\\n" + (err.stack ?? "")); }', + output: 'try { f(); } catch (err) { core.error((String(err) + "\\n" + (err?.stack ?? ""))); }', }, ], }, @@ -111,7 +111,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "error" }, - output: 'try { f(); } catch (error) { core.error(`details: ${String(error) + "\\n" + (error.stack ?? "")}`); }', + output: 'try { f(); } catch (error) { core.error(`details: ${(String(error) + "\\n" + (error?.stack ?? ""))}`); }', }, ], }, @@ -140,7 +140,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.catch(err => core.error(String(err) + "\\n" + (err.stack ?? "")));', + output: 'p.catch(err => core.error((String(err) + "\\n" + (err?.stack ?? ""))));', }, ], }, @@ -169,7 +169,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.catch(function(err) { core.error(String(err) + "\\n" + (err.stack ?? "")); });', + output: 'p.catch(function(err) { core.error((String(err) + "\\n" + (err?.stack ?? ""))); });', }, ], }, @@ -198,7 +198,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "outer" }, - output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error(String(outer) + "\\n" + (outer.stack ?? "")); }', + output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { } core.error((String(outer) + "\\n" + (outer?.stack ?? ""))); }', }, ], }, @@ -241,7 +241,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(result => result, err => core.error(String(err) + "\\n" + (err.stack ?? "")));', + output: 'p.then(result => result, err => core.error((String(err) + "\\n" + (err?.stack ?? ""))));', }, ], }, @@ -262,7 +262,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(null, err => core.error(String(err) + "\\n" + (err.stack ?? "")));', + output: 'p.then(null, err => core.error((String(err) + "\\n" + (err?.stack ?? ""))));', }, ], }, @@ -291,7 +291,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(null, function(err) { core.error(String(err) + "\\n" + (err.stack ?? "")); });', + output: 'p.then(null, function(err) { core.error((String(err) + "\\n" + (err?.stack ?? ""))); });', }, ], }, @@ -327,7 +327,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "e" }, - output: 'try { fetch(url); } catch (e) { console.error(String(e) + "\\n" + (e.stack ?? "")); }', + output: 'try { fetch(url); } catch (e) { console.error((String(e) + "\\n" + (e?.stack ?? ""))); }', }, ], }, @@ -348,7 +348,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'p.then(ok, err => console.error(String(err) + "\\n" + (err.stack ?? "")));', + output: 'p.then(ok, err => console.error((String(err) + "\\n" + (err?.stack ?? ""))));', }, ], }, @@ -377,7 +377,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "inner" }, - output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { core.error(String(inner) + "\\n" + (inner.stack ?? "")); } }', + output: 'try { f(); } catch (outer) { try { g(); } catch (inner) { core.error((String(inner) + "\\n" + (inner?.stack ?? ""))); } }', }, ], }, @@ -406,7 +406,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingForm", data: { errorVar: "err" }, - output: 'const getErrorMessage = require(\'./error_helpers\').getErrorMessage; try { f(); } catch (err) { core.error(getErrorMessage(err) + "\\n" + (err.stack ?? "")); }', + output: 'const getErrorMessage = require(\'./error_helpers\').getErrorMessage; try { f(); } catch (err) { core.error((getErrorMessage(err) + "\\n" + (err?.stack ?? ""))); }', }, ], }, @@ -435,7 +435,152 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingForm", data: { errorVar: "error" }, - output: 'const { getErrorMessage } = require(\'./error_helpers\'); try { f(); } catch (error) { core.error(`details: ${getErrorMessage(error) + "\\n" + (error.stack ?? "")}`); }', + output: 'const { getErrorMessage } = require(\'./error_helpers\'); try { f(); } catch (error) { core.error(`details: ${(getErrorMessage(error) + "\\n" + (error?.stack ?? ""))}`); }', + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is imported via ESM, suggestions use getErrorMessage", () => { + esmRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `import { getErrorMessage } from "./error_helpers"; try { f(); } catch (err) { console.error(JSON.stringify(err)); }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `import { getErrorMessage } from "./error_helpers"; try { f(); } catch (err) { console.error(getErrorMessage(err)); }`, + }, + { + messageId: "useDetailPreservingForm", + data: { errorVar: "err" }, + output: 'import { getErrorMessage } from "./error_helpers"; try { f(); } catch (err) { console.error((getErrorMessage(err) + "\\n" + (err?.stack ?? ""))); }', + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is declared earlier in the catch block, suggestions use getErrorMessage", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { const getErrorMessage = customHelper; console.error(JSON.stringify(err)); }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `try { f(); } catch (err) { const getErrorMessage = customHelper; console.error(getErrorMessage(err)); }`, + }, + { + messageId: "useDetailPreservingForm", + data: { errorVar: "err" }, + output: 'try { f(); } catch (err) { const getErrorMessage = customHelper; console.error((getErrorMessage(err) + "\\n" + (err?.stack ?? ""))); }', + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is declared after the call site, suggestions fall back to String()", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { console.error(JSON.stringify(err)); const getErrorMessage = customHelper; }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useStringFallback", + data: { errorVar: "err" }, + output: `try { f(); } catch (err) { console.error(String(err)); const getErrorMessage = customHelper; }`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'try { f(); } catch (err) { console.error((String(err) + "\\n" + (err?.stack ?? ""))); const getErrorMessage = customHelper; }', + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: detail-preserving suggestion is parenthesized when the call is chained", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `try { f(); } catch (err) { console.error(JSON.stringify(err).slice(0, 20)); }`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useStringFallback", + data: { errorVar: "err" }, + output: `try { f(); } catch (err) { console.error(String(err).slice(0, 20)); }`, + }, + { + messageId: "useDetailPreservingFormFallback", + data: { errorVar: "err" }, + output: 'try { f(); } catch (err) { console.error((String(err) + "\\n" + (err?.stack ?? "")).slice(0, 20)); }', + }, + ], + }, + ], + }, + ], + }); + }); + + it("invalid: when getErrorMessage is in scope inside a promise rejection handler, suggestions use getErrorMessage", () => { + cjsRuleTester.run("no-json-stringify-error", noJsonStringifyErrorRule, { + valid: [], + invalid: [ + { + code: `const { getErrorMessage } = require('./error_helpers'); p.catch(err => console.error(JSON.stringify(err)));`, + errors: [ + { + messageId: "jsonStringifyError", + data: { errorVar: "err" }, + suggestions: [ + { + messageId: "useGetErrorMessage", + data: { errorVar: "err" }, + output: `const { getErrorMessage } = require('./error_helpers'); p.catch(err => console.error(getErrorMessage(err)));`, + }, + { + messageId: "useDetailPreservingForm", + data: { errorVar: "err" }, + output: 'const { getErrorMessage } = require(\'./error_helpers\'); p.catch(err => console.error((getErrorMessage(err) + "\\n" + (err?.stack ?? ""))));', }, ], }, @@ -464,7 +609,7 @@ describe("no-json-stringify-error", () => { { messageId: "useDetailPreservingFormFallback", data: { errorVar: "err" }, - output: 'try { f(); } catch (err) { console.error(String(err) + "\\n" + (err.stack ?? "")); }', + output: 'try { f(); } catch (err) { console.error((String(err) + "\\n" + (err?.stack ?? ""))); }', }, ], }, diff --git a/eslint-factory/src/rules/no-json-stringify-error.ts b/eslint-factory/src/rules/no-json-stringify-error.ts index 4936b21e4cd..e1b6d8cfb08 100644 --- a/eslint-factory/src/rules/no-json-stringify-error.ts +++ b/eslint-factory/src/rules/no-json-stringify-error.ts @@ -46,15 +46,18 @@ export const noJsonStringifyErrorRule = createRule({ schema: [], messages: { jsonStringifyError: - "JSON.stringify({{errorVar}}) produces {} for Error objects — Error properties (message, stack, etc.) are non-enumerable. Prefer getErrorMessage({{errorVar}}) from error_helpers.cjs or explicitly serialize a guarded value after narrowing it.", + "JSON.stringify({{errorVar}}) produces {} for Error objects — Error properties (message, stack, etc.) are non-enumerable. Use getErrorMessage({{errorVar}}) if it is available, or String({{errorVar}}) as a safe import-free alternative.", useGetErrorMessage: "Replace with getErrorMessage({{errorVar}}) — ensure getErrorMessage is imported from error_helpers.cjs.", useStringFallback: "Replace with String({{errorVar}}) — getErrorMessage is not in scope; String() is a safe import-free alternative.", - useDetailPreservingForm: 'Replace with getErrorMessage({{errorVar}}) + "\\n" + ({{errorVar}}.stack ?? "") to preserve the stack trace — ensure getErrorMessage is imported from error_helpers.cjs.', - useDetailPreservingFormFallback: 'Replace with String({{errorVar}}) + "\\n" + ({{errorVar}}.stack ?? "") to preserve the stack trace.', + useDetailPreservingForm: 'Replace with (getErrorMessage({{errorVar}}) + "\\n" + ({{errorVar}}?.stack ?? "")) to preserve the stack trace — ensure getErrorMessage is imported from error_helpers.cjs.', + useDetailPreservingFormFallback: 'Replace with (String({{errorVar}}) + "\\n" + ({{errorVar}}?.stack ?? "")) to preserve the stack trace.', }, }, defaultOptions: [], create(context) { + const sourceCode = context.sourceCode; + type SourceCodeScope = ReturnType; + // Stack tracking caught error variable names. // Each scope entry holds varName (empty string for sentinel) and isSentinel flag. const scopeStack: ErrorScope[] = []; @@ -90,6 +93,30 @@ export const noJsonStringifyErrorRule = createRule({ scopeStack.pop(); } + function isDefinitionAvailableAtNode(definition: { type: string; name?: TSESTree.Node | null; node: TSESTree.Node }, node: TSESTree.Node): boolean { + if (definition.type === "ImportBinding" || definition.type === "FunctionName") { + return true; + } + + const definitionNode = definition.name ?? definition.node; + return definitionNode.range[0] < node.range[0]; + } + + function hasResolvableLocalBinding(node: TSESTree.Node, name: string): boolean { + let scope: SourceCodeScope | null = sourceCode.getScope(node); + + while (scope) { + const variable = scope.set.get(name); + if (variable && variable.defs.some(definition => isDefinitionAvailableAtNode(definition, node))) { + return true; + } + + scope = scope.upper; + } + + return false; + } + return { // Track catch clause parameters CatchClause(node) { @@ -129,15 +156,7 @@ export const noJsonStringifyErrorRule = createRule({ const errorVar = firstArg.name; - // Check whether getErrorMessage is resolvable in the current scope chain. - const getErrorMessageInScope = (() => { - let scope: ReturnType | null = context.sourceCode.getScope(node); - while (scope) { - if (scope.variables.some(v => v.name === "getErrorMessage")) return true; - scope = scope.upper; - } - return false; - })(); + const getErrorMessageInScope = hasResolvableLocalBinding(node, "getErrorMessage"); context.report({ node, @@ -166,14 +185,14 @@ export const noJsonStringifyErrorRule = createRule({ messageId: "useDetailPreservingForm" as const, data: { errorVar }, fix(fixer) { - return fixer.replaceText(node, `getErrorMessage(${errorVar}) + "\\n" + (${errorVar}.stack ?? "")`); + return fixer.replaceText(node, `(getErrorMessage(${errorVar}) + "\\n" + (${errorVar}?.stack ?? ""))`); }, } as const) : ({ messageId: "useDetailPreservingFormFallback" as const, data: { errorVar }, fix(fixer) { - return fixer.replaceText(node, `String(${errorVar}) + "\\n" + (${errorVar}.stack ?? "")`); + return fixer.replaceText(node, `(String(${errorVar}) + "\\n" + (${errorVar}?.stack ?? ""))`); }, } as const), ],