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
5 changes: 5 additions & 0 deletions .changeset/zod-errors-to-issues.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 10 additions & 1 deletion packages/core/src/packs/react-router/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
74 changes: 72 additions & 2 deletions packages/core/src/packs/zod/error-members.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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',
]) {
Expand All @@ -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);
Expand All @@ -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 }));
Expand Down Expand Up @@ -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',
Expand Down
83 changes: 79 additions & 4 deletions packages/core/src/packs/zod/error-members.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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/;
Expand Down Expand Up @@ -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]) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -509,3 +531,56 @@ export function rewriteErrorMembers(text: string, finding: Finding): TransformRe
reason: `formErrors was flatten(); z.flattenError keeps .${parent.getName()}`,
};
}

/**
* `<err>.errors` becomes `<err>.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',
};
}
29 changes: 29 additions & 0 deletions packages/core/src/packs/zod/fixtures/error-issues/after.ts
Original file line number Diff line number Diff line change
@@ -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;
29 changes: 29 additions & 0 deletions packages/core/src/packs/zod/fixtures/error-issues/before.ts
Original file line number Diff line number Diff line change
@@ -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;
14 changes: 13 additions & 1 deletion packages/core/src/packs/zod/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Expand Down
Loading