Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces Zod v4 schema generation as the default output format, featuring object shape spreads for embedded structs and specialized builders for string formats, while providing a legacy v3 compatibility mode and an updated golden-file-based testing suite. Feedback from the review highlights critical runtime and compatibility issues: the implementation of self-referential types using get accessors will likely trigger ReferenceError or TypeError during schema construction, and several emitted helpers—such as z.email(), z.uuid(), and z.partialRecord—are not standard Zod APIs, which would cause failures for users of the official library.
String schema generation: - Replace chunk-based approach with semantic validators (stringValidator) that separate parsing from rendering - Remove stringSchemaParts struct — renderStringSchema returns string directly - Add renderV3Chain, renderV4Chain, renderV4FormatBase for version-specific output - Skip redundant .min(1) from required when format validators are present (they already reject empty strings), except base64/hex in v4 which accept empty - Panic on impossible combinations: format+union (email,ip) and multiple formats (email,url) - Add AddTypeWithName for registering anonymous structs with custom names Tests: - Add buildValidatorConverter and assertValidators shared helpers for table-driven golden file tests with dynamic structs - Consolidate TestStringValidations from 30+ subtests into table-driven test (2 golden files) - Consolidate TestNumberValidations, TestMapWithValidations, TestConvertSliceWithValidations into table-driven tests - Consolidate TestFormatValidators: all 25 format + 2 union tags tested bare and with required modifier (8 golden files) - Add test for dive,oneof on slices - Add tests for format+union panic and multiple formats panic - Remove 24 redundant individual format tests from TestStringValidations - Remove 4 redundant tests from TestZodV4Defaults - Golden files reduced from 171 to 75 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Security:
- Add escapeJSString() to escape backslashes and double quotes in
generated JS string literals (contains, startswith, endswith, eq, ne,
oneof). Prevents broken output from struct tag values with special chars.
Refactors:
- Replace lastFieldSelfRef side-channel flag with convertResult struct
that explicitly returns {text, selfRef} from convertType(). Public
ConvertType API unchanged via thin wrapper.
- Extract renderChain() with shared cases from renderV3Chain/renderV4Chain,
eliminating ~80 lines of duplication.
- Simplify parseStringValidators: replace ~90 lines of switch cases with
knownStringTags map lookup.
- Extract regex rendering into maps (regexChainMap, unicodeRegexChainMap,
v3FormatRegexMap) with renderRegex()/renderUnicodeRegex() helpers.
Bug fixes:
- Fix v4 embedded field ordering: spreads now come before named fields
so named fields override embedded ones (last key wins in JS), matching
Go's shadowing semantics. Previously spreads came after, causing
embedded fields to override named fields incorrectly.
- Panic on non-integer gt=/lt= arguments instead of silently using 0.
Tests:
- Add field shadowing test (named field overrides embedded field, v3+v4)
- Add escapeJSString unit tests
- Add gt/lt non-integer panic tests
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use version-aware dispatch in enum branch (renderV3Chain for v3,
renderChain for v4) instead of always calling renderV3Chain.
Defensive change — no current behavioral difference.
- Replace magic number rawPart[6:] with rawPart[len("oneof="):]
- Use strings.ReplaceAll instead of strings.Replace with count -1
- Panic in renderV4FormatBase default case instead of returning ""
to catch future omissions when adding format tags
- Validate numeric arguments for len/min/max/gte/lte string validators
using requireIntArg helper (same treatment as gt/lt)
- Remove renderV4Chain wrapper — inline renderChain at all call sites
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove unreachable len(vals) == 0 panic in oneof handler — the case guard already requires valValue != "" and FindAllString always matches - Separate escapeJSString and requireIntArg doc comments with blank line Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use json.Marshal in escapeJSString for complete JS string escaping
(newlines, control characters, not just quotes and backslashes)
- Pass reflect.TypeOf("") instead of reflect.TypeOf(0) to custom tag
handlers in string validation context
- Replace string concatenation with strings.Builder in renderStringSchema
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Format validators (email, url, uuid, etc.) now use z.string().check(z.email()) instead of z.email() as the schema base. This fixes ordering bugs where z.email().trim() would validate before trimming, rejecting valid spaced input. With .check(), the z.string() base is preserved so transforms like .trim() chain correctly in any position. The required tag now always emits .min(1) in both v3 and v4, including when combined with format or union validators. Previously it was skipped because format validators reject empty strings, but explicit .min(1) is more correct and consistent. IP unions updated to z.union([z.string().check(z.ipv4()), ...]) for consistency with the .check() approach. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Review Summary
The latest changes (1816be7 — .check() pattern + always emit .min(1) for required) are a major improvement that significantly simplifies the codebase:
What improved:
.check()pattern — Format validators now usez.string().check(z.email())instead of top-levelz.email()builders. This addresses the earlier Gemini review concern about non-standard Zod APIs and eliminates the complexhasTransformBefore/formatIdx/v4AcceptsEmptyordering logic entirely.requiredalways emits.min(1)— Removes allskipRequired/keepRequiredbranching for format/union validators, making behavior consistent and predictable across all string schemas.- IP unions use
z.string().check()—z.union([z.string().check(z.ipv4()), z.string().check(z.ipv6())])properly keeps thez.string()base, ensuring transforms chain correctly. - Simplified
goldenAssert— Removed thegoldenMeta/goldenOptindirection in favor of a direct(t, data, version)signature. resolveGoldenin test infra — Tests now reference directories (e.g.,"TestNestedStruct") instead of version-specific files, with automatic version resolution. Cleaner test case definitions.
Architecture highlights:
- Clean v3/v4 split with
WithZodV3()opt-in — minimal branching in core logic parseStringValidators/renderStringSchemaseparation is solid- Golden file + docker type-check + vitest runtime tests provide strong end-to-end confidence
- Security: All string interpolation paths properly escaped via
escapeJSString+json.Marshal, numeric args validated viarequireIntArg/requireNumericArg
No new issues found. Previous review feedback has been well addressed across all rounds. The code is clean, well-tested, and ready to merge.
Summary
Migrate zen to support Zod v4 as the default output, with v3 available via
WithZodV3(). Includes a refactor of string schema generation and a new test infrastructure.Zod v4 output changes
Format validators use
.check()patternFormat validators use
z.string().check(z.email())instead ofz.email()as the schema base. This preserves thez.string()base so that transforms like.trim(),.min(), etc. chain correctly in any position. For example,validate:"trim,email"producesz.string().trim().check(z.email())which trims before validating. See colinhacks/zod#4642IP union
ip/ip_addrtags producez.union([z.string().check(z.ipv4()), z.string().check(z.ipv6())])with constraints distributed to each arm.Self-referential getter pattern
get field() { return Schema; }instead ofz.lazy(() => Schema).Embedded struct spreads
...Schema.shapeinstead of.merge(Schema). Spreads are placed before named fields so that Go's field shadowing semantics are preserved (last key wins in JS).Partial records
z.partialRecord()for enum-keyed maps.Invalid combinations panic
email,ip(format + union) andemail,url(multiple formats) panic instead of producing broken outputoneof/boolean) combined with non-enum validators (e.g.oneof=a b,contains=x) panics —oneoffully constrains the valuedive,keyswithout matchingendkeyspanics with a clear messagegt=abc) panic viarequireIntArg/requireNumericArgString schema generation refactor
The original
validateStringbuilt Zod output directly using astrings.Builder— mixing parsing and rendering in one pass with no intermediate representation. This made v3/v4 branching difficult and led to edge cases with tag ordering.parseStringValidators— pure parser producing[]stringValidatorviaknownStringTagsmap lookuprenderStringSchema— version-aware renderer that classifies validators, validates combinations, and renders the complete schema in one passrenderChain— shared rendering for version-independent validatorsrenderV3Chain— v3-specific format chains (.email(),.ip(),.regex())renderV4FormatCheck— v4 format check chains (.check(z.email()),.check(z.uuid()), etc.)renderRegex()helpersBug fixes (pre-existing)
preprocessValidationTagPartnow passes actualreflect.Typeto custom tag handlers instead of alwaysreflect.TypeOf("")strings.ToTitle(deprecated) replaced withstrings.ToUppergetValidateValuesguards against missingendkeysand barediveSecurity
escapeJSString()escapes\and"in generated JS string literalslen/min/max/gt/gte/lt/lteNew public API
WithZodV3() Opt— opt into Zod v3 outputAddTypeWithName(input, name)— register anonymous structs with custom namesTest infrastructure
github.com/xorcare/golden—make test-updateto regeneratemake docker-test) — typechecks all golden files with tsc against zod@3 and zod@4, then runs vitest runtime tests against both versionsassertValidators+buildValidatorConverterusing dynamic structsTest plan
🤖 Generated with Claude Code