diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index d996283bf73..d93f602eaa4 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -50,6 +50,7 @@ module.exports = [ "gh-aw-custom/require-escaped-regexp-interpolation": "warn", "gh-aw-custom/require-fetch-timeout": "warn", "gh-aw-custom/require-nan-check-after-env-numeric-parse": "warn", + "gh-aw-custom/prefer-structured-clone": "warn", }, }, { diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index a65eb3d6c51..be18821cd12 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -36,6 +36,7 @@ import { noDuplicateConstantValuesRule } from "./rules/no-duplicate-constant-val import { requireEscapedRegexpInterpolationRule } from "./rules/require-escaped-regexp-interpolation"; import { requireFetchTimeoutRule } from "./rules/require-fetch-timeout"; import { requireNanCheckAfterEnvNumericParseRule } from "./rules/require-nan-check-after-env-numeric-parse"; +import { preferStructuredCloneRule } from "./rules/prefer-structured-clone"; const plugin = { meta: { @@ -81,6 +82,7 @@ const plugin = { "require-escaped-regexp-interpolation": requireEscapedRegexpInterpolationRule, "require-fetch-timeout": requireFetchTimeoutRule, "require-nan-check-after-env-numeric-parse": requireNanCheckAfterEnvNumericParseRule, + "prefer-structured-clone": preferStructuredCloneRule, }, }; diff --git a/eslint-factory/src/rules/prefer-structured-clone.test.ts b/eslint-factory/src/rules/prefer-structured-clone.test.ts new file mode 100644 index 00000000000..1fadf3c3a90 --- /dev/null +++ b/eslint-factory/src/rules/prefer-structured-clone.test.ts @@ -0,0 +1,83 @@ +import { RuleTester } from "eslint"; +import { describe, expect, it } from "vitest"; +import { preferStructuredCloneRule } from "./prefer-structured-clone"; + +const cjsRuleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("prefer-structured-clone", () => { + it("uses the correct docs URL", () => { + expect(preferStructuredCloneRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#prefer-structured-clone"); + }); + + it("valid: unrelated JSON.parse / JSON.stringify usages are accepted", () => { + cjsRuleTester.run("prefer-structured-clone", preferStructuredCloneRule, { + valid: [ + `const data = JSON.parse(rawText);`, + `const text = JSON.stringify(obj);`, + `structuredClone(obj);`, + // Replacer/indent argument changes stringify semantics; excluded to avoid false positives. + `const clone = JSON.parse(JSON.stringify(obj, null, 2));`, + // Custom reviver on parse changes semantics too; still matched today since only the + // parse call itself is checked for a single stringify argument — but the inner + // stringify call must have exactly one argument, so this is out of scope here. + `const clone = JSON.parse(JSON.stringify(obj), reviver);`, + ], + invalid: [], + }); + }); + + it("invalid: JSON.parse(JSON.stringify(x)) is flagged and suggests structuredClone(x)", () => { + cjsRuleTester.run("prefer-structured-clone", preferStructuredCloneRule, { + valid: [], + invalid: [ + { + code: `const clone = JSON.parse(JSON.stringify(tool));`, + errors: [ + { + messageId: "preferStructuredClone", + suggestions: [ + { + messageId: "replaceWithStructuredClone", + output: `const clone = structuredClone(tool);`, + }, + ], + }, + ], + }, + { + code: `const runs = state.runs.map(run => JSON.parse(JSON.stringify(run)));`, + errors: [ + { + messageId: "preferStructuredClone", + suggestions: [ + { + messageId: "replaceWithStructuredClone", + output: `const runs = state.runs.map(run => structuredClone(run));`, + }, + ], + }, + ], + }, + { + code: `const clone = JSON["parse"](JSON["stringify"](tool));`, + errors: [ + { + messageId: "preferStructuredClone", + suggestions: [ + { + messageId: "replaceWithStructuredClone", + output: `const clone = structuredClone(tool);`, + }, + ], + }, + ], + }, + ], + }); + }); +}); diff --git a/eslint-factory/src/rules/prefer-structured-clone.ts b/eslint-factory/src/rules/prefer-structured-clone.ts new file mode 100644 index 00000000000..f3c9eb0f29b --- /dev/null +++ b/eslint-factory/src/rules/prefer-structured-clone.ts @@ -0,0 +1,79 @@ +import { AST_NODE_TYPES, ESLintUtils, TSESLint, TSESTree } from "@typescript-eslint/utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +/** + * Returns true when the call expression is `JSON.parse(...)` (direct or computed access). + */ +function isJsonParseCall(node: TSESTree.CallExpression): boolean { + const callee = node.callee; + if (callee.type !== AST_NODE_TYPES.MemberExpression) return false; + if (callee.object.type !== AST_NODE_TYPES.Identifier || callee.object.name !== "JSON") return false; + const property = callee.property; + const isDirectAccess = !callee.computed && property.type === AST_NODE_TYPES.Identifier && property.name === "parse"; + const isComputedAccess = callee.computed && property.type === AST_NODE_TYPES.Literal && property.value === "parse"; + return isDirectAccess || isComputedAccess; +} + +/** + * Returns true when the call expression is `JSON.stringify(...)` (direct or computed access) + * with exactly one argument (a replacer/indent argument changes the round-trip semantics and + * is intentionally excluded from this check to keep false positives low). + */ +function isPlainJsonStringifyCall(node: TSESTree.CallExpression): boolean { + const callee = node.callee; + if (callee.type !== AST_NODE_TYPES.MemberExpression) return false; + if (callee.object.type !== AST_NODE_TYPES.Identifier || callee.object.name !== "JSON") return false; + const property = callee.property; + const isDirectAccess = !callee.computed && property.type === AST_NODE_TYPES.Identifier && property.name === "stringify"; + const isComputedAccess = callee.computed && property.type === AST_NODE_TYPES.Literal && property.value === "stringify"; + if (!isDirectAccess && !isComputedAccess) return false; + return node.arguments.length === 1; +} + +export const preferStructuredCloneRule = createRule({ + name: "prefer-structured-clone", + meta: { + type: "suggestion", + hasSuggestions: true, + docs: { + description: "Prefer structuredClone(...) over JSON.parse(JSON.stringify(...)) for deep-cloning plain data in actions/setup/js scripts. The JSON round-trip is slower, silently drops values it cannot represent (undefined, functions, Date becomes a string), and throws on circular references, whereas structuredClone (Node >=17, available globally in the Node 24 runtime this action targets) clones plain objects and JSON-safe data directly.", + }, + schema: [], + messages: { + preferStructuredClone: "Replace JSON.parse(JSON.stringify({{arg}})) with structuredClone({{arg}}) — the JSON round-trip silently drops undefined/function values, converts Dates to strings, and throws on circular references.", + replaceWithStructuredClone: "Replace with structuredClone(...).", + }, + }, + defaultOptions: [], + create(context) { + const sourceCode = context.sourceCode; + + return { + CallExpression(node) { + if (!isJsonParseCall(node)) return; + if (node.arguments.length !== 1) return; + + const innerArg = node.arguments[0]; + if (innerArg.type !== AST_NODE_TYPES.CallExpression) return; + if (!isPlainJsonStringifyCall(innerArg)) return; + + const clonedExpressionText = sourceCode.getText(innerArg.arguments[0]); + + context.report({ + node, + messageId: "preferStructuredClone", + data: { arg: clonedExpressionText }, + suggest: [ + { + messageId: "replaceWithStructuredClone", + fix(fixer: TSESLint.RuleFixer) { + return fixer.replaceText(node, `structuredClone(${clonedExpressionText})`); + }, + }, + ], + }); + }, + }; + }, +});