diff --git a/.changeset/zod-errors-to-issues.md b/.changeset/zod-errors-to-issues.md new file mode 100644 index 0000000..4bd424d --- /dev/null +++ b/.changeset/zod-errors-to-issues.md @@ -0,0 +1,5 @@ +--- +'uptide': patch +--- + +The zod pack migrates `ZodError#errors` to `.issues` by rule: reads (`error.errors[0].message`, `error?.errors`) and destructuring (`const { errors } = error` becomes `const { issues: errors } = error`) where the type checker resolves the receiver to `ZodError`, and reads off an untyped catch binding of a zod parse. Writes are left to the agent. diff --git a/packages/core/src/packs/react-router/verification.json b/packages/core/src/packs/react-router/verification.json index 3991ed7..f10f776 100644 --- a/packages/core/src/packs/react-router/verification.json +++ b/packages/core/src/packs/react-router/verification.json @@ -5,6 +5,15 @@ "breaking": { "precision": 1, "recall": 0.875, - "falsePositives": 0 + "falsePositives": 0, + "truePositives": 14, + "falseNegatives": 2 + }, + "behavior": { + "precision": 1, + "recall": 1, + "falsePositives": 0, + "truePositives": 0, + "falseNegatives": 0 } } diff --git a/packages/core/src/packs/zod/error-members.test.ts b/packages/core/src/packs/zod/error-members.test.ts index cf985e3..81acf61 100644 --- a/packages/core/src/packs/zod/error-members.test.ts +++ b/packages/core/src/packs/zod/error-members.test.ts @@ -4,6 +4,7 @@ import { dirname, join } from 'node:path'; import { afterAll, describe, expect, it } from 'vitest'; import type { Finding } from '../../domain/report.js'; import type { Usage } from '../../domain/usage.js'; +import { ruleFor } from '../contract.js'; import { errorMemberFindings, errorMemberSites, rewriteErrorMembers } from './error-members.js'; import { zodPack } from './index.js'; @@ -133,7 +134,6 @@ describe('error-members rewrite', () => { it('leaves every other shape to the agent', () => { for (const read of [ 'error.formErrors', - 'error.errors', 'error?.formErrors?.fieldErrors', 'error.formErrors.fieldErrors.name', ]) { @@ -151,6 +151,20 @@ describe('error-members rewrite', () => { reason: expect.stringContaining('`z` names something else'), }); }); + it('renames errors to issues off a caught zod error, never off another try', () => { + for (const read of ['error.errors[0].message', '(error as any).errors?.length']) { + const text = form("import { z } from 'zod';", read); + const result = zodPack.transform(text, finding(text), context); + expect(result).toMatchObject({ applied: true, rule: 'error-members' }); + expect(result.text).toBe(text.replace('.errors', '.issues')); + } + const text = form("import { z } from 'zod';", 'error.errors'); + const site = finding(text); + const other = text.replace('schema.parse(values);', 'JSON.parse(values as string);'); + expect(zodPack.transform(other, site, context).applied).toBe(false); + const write = form("import { z } from 'zod';", '(error.errors = [])'); + expect(zodPack.transform(write, finding(write), context).applied).toBe(false); + }); it('never edits a read detection would not report, and is idempotent', () => { const text = form("import { z } from 'zod';", 'error.formErrors.fieldErrors'); const site = finding(text); @@ -161,6 +175,62 @@ describe('error-members rewrite', () => { }); }); +describe('error-issues', () => { + /** What the type diff reports at a typed read: the checker resolved the member's declaration. */ + const typed = (text: string, at: string, path = 'ZodError#errors'): Finding => { + const line = text.split('\n').findIndex((l) => l.includes(at)) + 1; + const column = (text.split('\n')[line - 1] as string).indexOf(at) + 1; + return { + change: { + package: 'zod', + ...context, + path, + kind: 'removed', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file: 'a.ts', + line, + column, + endLine: line, + endColumn: column, + symbolPath: path, + access: 'read', + snippet: '', + via: 'direct', + }, + severity: 'breaking', + confidence: 1, + fixability: 'manual', + reason: 'removed', + }; + }; + it('claims only the removed ZodError#errors', () => { + const text = 'r.error.errors'; + expect(ruleFor(zodPack.rules, typed(text, 'errors'))?.id).toBe('error-issues'); + for (const path of ['Form#errors', 'ZodError#formErrors', 'ZodIssue#errors']) + expect(ruleFor(zodPack.rules, typed(text, 'errors', path))?.id).not.toBe('error-issues'); + }); + it('renames the typed read and keeps a destructured local name', () => { + const text = 'const m = r.error.errors[0]?.message;\nconst { errors } = r.error;\n'; + const read = zodPack.transform(text, typed(text, 'errors['), context); + expect(read).toMatchObject({ applied: true, rule: 'error-issues' }); + expect(read.text).toBe(text.replace('errors[0]', 'issues[0]')); + const bound = zodPack.transform(text, typed(text, 'errors }'), context); + expect(bound.text).toBe(text.replace('{ errors }', '{ issues: errors }')); + }); + it('declines writes, rest elements and a site with no errors read at it', () => { + for (const [text, at] of [ + ['e.errors = [];', 'errors'], + ['delete e.errors;', 'errors'], + ['const { ...errors } = e;', '...errors'], + ] as const) + expect(zodPack.transform(text, typed(text, at), context).applied).toBe(false); + }); +}); + describe('error-members in check', () => { const root = mkdtempSync(join(tmpdir(), 'uptide-zod-error-members-')); afterAll(() => rmSync(root, { recursive: true, force: true })); @@ -237,7 +307,7 @@ describe('error-members in check', () => { expect( run().map((f) => `${f.usage.file}:${f.usage.line} ${f.change.path} ${f.fixability}`), ).toEqual([ - 'components/account/UpdateEmail.tsx:6 ZodError#errors assisted', + 'components/account/UpdateEmail.tsx:6 ZodError#errors mechanical', 'components/account/UpdateName.tsx:6 ZodError#formErrors mechanical', 'components/apiKey/NewAPIKey.tsx:7 ZodError#formErrors mechanical', 'components/team/TeamSettings.tsx:7 ZodError#formErrors mechanical', diff --git a/packages/core/src/packs/zod/error-members.ts b/packages/core/src/packs/zod/error-members.ts index cd8f911..6d4d9c0 100644 --- a/packages/core/src/packs/zod/error-members.ts +++ b/packages/core/src/packs/zod/error-members.ts @@ -1,6 +1,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join, relative, resolve } from 'node:path'; import { + type BindingElement, type Expression, type Identifier, Node, @@ -28,6 +29,8 @@ import type { TransformResult } from '../types.js'; * reads one of the removed members off its own binding. */ export const RULE = 'error-members'; +/** `ZodError#errors` where the compiler sees it: the type diff's finding, renamed to `issues`. */ +export const ISSUES_RULE = 'error-issues'; const REMOVED = new Set(['formErrors', 'errors']); const FLAT = new Set(['fieldErrors', 'formErrors']); const MENTIONS = /\bformErrors\b|\.errors\b/; @@ -377,12 +380,17 @@ export function errorMemberFindings(input: { const IMPORT_LINE = "import { z } from 'zod';"; -/** The read at the reported site, allowing for the one import line this rule may have added above it. */ +/** + * The read of `name` at the reported site, allowing for the one import line this rule may have + * added above it. Detection reports a read where it starts (`e.formErrors`); the type diff + * where the member is named (`errors` in `e.errors`, or in a destructured `{ errors }`). + */ function readAt( source: SourceFile, text: string, finding: Finding, -): PropertyAccessExpression | undefined { + name = 'formErrors', +): PropertyAccessExpression | BindingElement | undefined { const lines = text.split('\n'); const added = lines.indexOf(IMPORT_LINE); for (const shift of added >= 0 && added < finding.usage.line - 1 ? [0, 1] : [0]) { @@ -396,9 +404,21 @@ function readAt( // The innermost: `e.formErrors.formErrors` starts both reads at one place. const access = source .getDescendantsOfKind(SyntaxKind.PropertyAccessExpression) - .filter((a) => a.getStart() === position && a.getName() === 'formErrors') + .filter( + (a) => + a.getName() === name && + (a.getStart() === position || a.getNameNode().getStart() === position), + ) .sort((x, y) => x.getWidth() - y.getWidth())[0]; if (access) return access; + const element = source + .getDescendantsOfKind(SyntaxKind.BindingElement) + .find( + (e) => + e.getStart() === position && + (e.getPropertyNameNode() ?? e.getNameNode()).getText() === name, + ); + if (element) return element; } return undefined; } @@ -460,11 +480,13 @@ function zodName( */ export function rewriteErrorMembers(text: string, finding: Finding): TransformResult { const decline = (reason: string): TransformResult => ({ text, applied: false, reason }); + if (/ZodError#errors$/.test(finding.change.path)) return rewriteErrors(text, finding, true); if (!/ZodError#formErrors$/.test(finding.change.path)) return decline('only formErrors.fieldErrors and formErrors.formErrors are rewritten by rule'); const source = parse(text, finding.usage.file); const access = readAt(source, text, finding); - if (!access) return decline('the reported formErrors read is not at the site'); + if (!access || Node.isBindingElement(access)) + return decline('the reported formErrors read is not at the site'); // Only a read detection reports: caught from a zod parse. Any imported name may be the schema // here, since `check` proved which ones are before it reported the site. if (!removedReads(source, localImports(source, /^(?!zod$)/)).includes(access)) @@ -509,3 +531,56 @@ export function rewriteErrorMembers(text: string, finding: Finding): TransformRe reason: `formErrors was flatten(); z.flattenError keeps .${parent.getName()}`, }; } + +/** + * `.errors` becomes `.issues`, and a destructured `{ errors }` keeps its local name as + * `{ issues: errors }`: zod 3's `errors` was an alias of `issues` + * (https://zod.dev/v4/changelog#drops-errors). `caught` is a site this rule detected off an + * untyped catch binding, rewritten only where detection still reports it; otherwise the site is + * the type diff's, whose receiver the checker resolved to `ZodError` (never the name alone). + */ +export function rewriteErrors(text: string, finding: Finding, caught = false): TransformResult { + const decline = (reason: string): TransformResult => ({ text, applied: false, reason }); + if (!/ZodError#errors$/.test(finding.change.path)) + return decline('only ZodError#errors is renamed to issues'); + const source = parse(text, finding.usage.file); + const at = readAt(source, text, finding, 'errors'); + if (!at) return decline('the reported errors read is not at the site'); + let edit: { start: number; end: number; text: string }; + if (Node.isBindingElement(at)) { + if (caught) return decline('not an errors read off an error caught from a zod parse'); + if (at.getDotDotDotToken()) return decline('a rest element needs manual review'); + const property = at.getPropertyNameNode(); + edit = property + ? { start: property.getStart(), end: property.getEnd(), text: 'issues' } + : { start: at.getNameNode().getStart(), end: at.getNameNode().getStart(), text: 'issues: ' }; + } else { + if (caught && !removedReads(source, localImports(source, /^(?!zod$)/)).includes(at)) + return decline('not an errors read off an error caught from a zod parse'); + let target: Node = at; + while ( + Node.isParenthesizedExpression(target.getParent()) || + Node.isNonNullExpression(target.getParent()) + ) + target = target.getParent() as Node; + const parent = target.getParent(); + if ( + (Node.isBinaryExpression(parent) && + parent.getLeft() === target && + parent.getOperatorToken().getKind() >= SyntaxKind.FirstAssignment && + parent.getOperatorToken().getKind() <= SyntaxKind.LastAssignment) || + Node.isDeleteExpression(parent) || + ((Node.isPrefixUnaryExpression(parent) || Node.isPostfixUnaryExpression(parent)) && + [SyntaxKind.PlusPlusToken, SyntaxKind.MinusMinusToken].includes(parent.getOperatorToken())) + ) + return decline('a write to errors needs manual review'); + const name = at.getNameNode(); + edit = { start: name.getStart(), end: name.getEnd(), text: 'issues' }; + } + return { + text: text.slice(0, edit.start) + edit.text + text.slice(edit.end), + applied: true, + rule: caught ? RULE : ISSUES_RULE, + reason: 'errors was an alias of issues', + }; +} diff --git a/packages/core/src/packs/zod/fixtures/error-issues/after.ts b/packages/core/src/packs/zod/fixtures/error-issues/after.ts new file mode 100644 index 0000000..44114f0 --- /dev/null +++ b/packages/core/src/packs/zod/fixtures/error-issues/after.ts @@ -0,0 +1,29 @@ +import { z, type ZodError } from 'zod'; + +const schema = z.object({ email: z.string() }); + +// Shape of components/teams/add-team-member-modal.tsx in papermark: an inferred result.error. +export function firstMessage(input: unknown) { + const validation = schema.safeParse(input); + if (!validation.success) return validation.error.issues[0].message; // @uptide error-issues kind:removed path:ZodError#errors at:errors[0] + return undefined; +} + +export function optional(error?: ZodError) { + return error?.issues?.length ?? 0; // @uptide error-issues kind:removed path:ZodError#errors at:errors?. +} + +export function destructured(error: ZodError) { + const { issues: errors } = error; // @uptide error-issues kind:removed path:ZodError#errors at:errors + const { issues: list = [] } = error; // @uptide error-issues kind:removed path:ZodError#errors at:errors: + return [...errors, ...list]; +} + +// Left alone: a write to the removed member is the agent's. +export function reset(error: ZodError) { + error.errors = []; // @uptide error-issues keep kind:removed path:ZodError#errors at:errors +} + +// Never reported: an object of the repository's own with an errors field. +export const form = { errors: ['required'] }; +export const formErrors = form.errors; diff --git a/packages/core/src/packs/zod/fixtures/error-issues/before.ts b/packages/core/src/packs/zod/fixtures/error-issues/before.ts new file mode 100644 index 0000000..fa233f8 --- /dev/null +++ b/packages/core/src/packs/zod/fixtures/error-issues/before.ts @@ -0,0 +1,29 @@ +import { z, type ZodError } from 'zod'; + +const schema = z.object({ email: z.string() }); + +// Shape of components/teams/add-team-member-modal.tsx in papermark: an inferred result.error. +export function firstMessage(input: unknown) { + const validation = schema.safeParse(input); + if (!validation.success) return validation.error.errors[0].message; // @uptide error-issues kind:removed path:ZodError#errors at:errors[0] + return undefined; +} + +export function optional(error?: ZodError) { + return error?.errors?.length ?? 0; // @uptide error-issues kind:removed path:ZodError#errors at:errors?. +} + +export function destructured(error: ZodError) { + const { errors } = error; // @uptide error-issues kind:removed path:ZodError#errors at:errors + const { errors: list = [] } = error; // @uptide error-issues kind:removed path:ZodError#errors at:errors: + return [...errors, ...list]; +} + +// Left alone: a write to the removed member is the agent's. +export function reset(error: ZodError) { + error.errors = []; // @uptide error-issues keep kind:removed path:ZodError#errors at:errors +} + +// Never reported: an object of the repository's own with an errors field. +export const form = { errors: ['required'] }; +export const formErrors = form.errors; diff --git a/packages/core/src/packs/zod/index.ts b/packages/core/src/packs/zod/index.ts index 5a067d5..6bf0f36 100644 --- a/packages/core/src/packs/zod/index.ts +++ b/packages/core/src/packs/zod/index.ts @@ -17,8 +17,10 @@ import type { PackContext, TransformResult } from '../types.js'; import { errorMemberFindings, errorMemberSites, + ISSUES_RULE, RULE, rewriteErrorMembers, + rewriteErrors, } from './error-members.js'; import { zodGuide } from './guide.js'; import { defaultMessageSites } from './messages.js'; @@ -361,13 +363,23 @@ export const zodPack: Pack = { summary: 'ZodError#formErrors and #errors were removed; read off an untyped catch binding, the compiler cannot see it', severity: 'breaking', - // Only its own sites: a typed read is the compiler's finding, under the rules above. + // Only its own sites: a typed read is the compiler's finding (`error-issues` for errors). kinds: [], symbols: /$^/, guide: zodGuide.errorMembers, detect: (text, file) => errorMemberSites(text, file), rewrite: (text, finding) => rewriteErrorMembers(text, finding), }, + { + id: ISSUES_RULE, + summary: 'ZodError#errors was an alias of issues and was removed: errors becomes issues', + severity: 'breaking', + // The type diff's finding: the checker resolved the receiver to ZodError. + kinds: ['removed'], + symbols: /^ZodError#errors$/, + guide: zodGuide.errorMembers, + rewrite: (text, finding) => rewriteErrors(text, finding), + }, { id: 'types', summary: 'ZodType generics and ZodTypeDef changed; type-level code is migrated by the agent',