Add support for custom validation tags #14
Conversation
hi-rai
requested review from
AndrianBdn,
ramilamparo,
satvik007,
gornostay25 and
ElenaGrasovskaya
WalkthroughThis pull request introduces significant enhancements to the Zen library, focusing on improving configurability and extensibility. The changes span multiple files, with key modifications to the Changes
Sequence DiagramsequenceDiagram
participant User
participant Converter
participant CustomTypeHandler
participant ValidationHandler
User->>Converter: Initialize with options
Converter->>CustomTypeHandler: Register custom types
Converter->>ValidationHandler: Register custom tags
User->>Converter: Convert struct to Zod schema
Converter-->>User: Generated Zod schema
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The changes are required for improving SortField validations as mentioned in https://github.com/Hypersequent/tms-issues/issues/1300 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
zod.go (7)
15-21: Consider validating or documentingprefixconstraints.If conflicting prefixes are passed in different calls, unexpected naming collisions might occur.
377-449: Large block inconvertStructand related validations.This block is quite extensive. Consider splitting it into smaller helper functions for maintainability.
582-619: Array and slice validation expanded.Logic is correct but quite lengthy; consider modularizing repeated validation patterns.
726-805:convertMaphandles map validations thoroughly.Suggestion: unify or standardize the refinement error messages so they follow a similar wording style.
Line range hint
890-966:validateNumbercovers many numeric validations.Consider extracting repeated logic (e.g., parse integer steps) into a helper for cleaner code.
Line range hint
972-1170:validateStringparallelsvalidateNumber.Similar refactoring could reduce duplication and improve readability.
Line range hint
2137-2184: AddedTestCustomTagtest coverage.Excellent demonstration of how custom tag handlers can extend the validation logic. Consider including a negative test case to ensure refinements catch errors properly.
custom/optional/optional_test.go (1)
13-16: LGTM! Consider adding a test case for invalid usage.The changes correctly implement the new options-based configuration system. The test demonstrates proper initialization of the converter with custom type handlers.
Consider adding a negative test case to verify that the converter properly handles invalid optional types or missing handlers.
README.md (1)
297-300: Enhance decimal type handler with validation.The current implementation could be improved by:
- Handling validation tags
- Adding refinements for valid decimal strings
Consider this enhanced implementation:
customTypeHandlers := map[string]zen.CustomFn{ "github.com/shopspring/decimal.Decimal": func (c *zen.Converter, t reflect.Type, v string, indent int) string { - // Shopspring's decimal type serialises to a string. - return "z.string()" + // Validate decimal string format and handle validation tags + base := "z.string().refine((val) => /^-?\\d*\\.?\\d+$/.test(val), 'Invalid decimal format')" + if v != "" { + return base + "." + v + } + return base }, }🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
297-297: Hard tabs
Column: 1(MD010, no-hard-tabs)
298-298: Hard tabs
Column: 1(MD010, no-hard-tabs)
299-299: Hard tabs
Column: 1(MD010, no-hard-tabs)
300-300: Hard tabs
Column: 1(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(3 hunks)custom/decimal/decimal_test.go(1 hunks)custom/optional/optional_test.go(1 hunks)zod.go(17 hunks)zod_test.go(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
221-221: Hard tabs
Column: 1
(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1
(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1
(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
228-228: Hard tabs
Column: 1
(MD010, no-hard-tabs)
229-229: Hard tabs
Column: 1
(MD010, no-hard-tabs)
230-230: Hard tabs
Column: 1
(MD010, no-hard-tabs)
231-231: Hard tabs
Column: 1
(MD010, no-hard-tabs)
235-235: Hard tabs
Column: 1
(MD010, no-hard-tabs)
236-236: Hard tabs
Column: 1
(MD010, no-hard-tabs)
237-237: Hard tabs
Column: 1
(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
239-239: Hard tabs
Column: 1
(MD010, no-hard-tabs)
240-240: Hard tabs
Column: 1
(MD010, no-hard-tabs)
241-241: Hard tabs
Column: 1
(MD010, no-hard-tabs)
242-242: Hard tabs
Column: 1
(MD010, no-hard-tabs)
243-243: Hard tabs
Column: 1
(MD010, no-hard-tabs)
244-244: Hard tabs
Column: 1
(MD010, no-hard-tabs)
245-245: Hard tabs
Column: 1
(MD010, no-hard-tabs)
246-246: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
297-297: Hard tabs
Column: 1
(MD010, no-hard-tabs)
298-298: Hard tabs
Column: 1
(MD010, no-hard-tabs)
299-299: Hard tabs
Column: 1
(MD010, no-hard-tabs)
300-300: Hard tabs
Column: 1
(MD010, no-hard-tabs)
306-306: Hard tabs
Column: 1
(MD010, no-hard-tabs)
314-314: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (19)
zod.go (15)
12-14: IntroduceOpttype for extensible converter configuration.This is a clean solution for passing behavior modifications into the converter.
22-31:WithCustomTypesapproach is straightforward.No issues found. The iteration over the provided map is an effective way to register custom types.
33-42:WithCustomTagsaligns well with the existing extension mechanism.No specific concerns here. The reusability of
CustomFnfor tag-based logic is a nice touch.
44-51: Confirm ignoring tags behaves as expected.Ensure that ignoring a tag doesn’t mask critical validations. Consider clarifying in docs that unknown, unignored tags panic.
52-63: Ensure thread safety inNewConverter.If multiple goroutines call methods on the same converter instance, we might need synchronization around shared maps (
customTypes,customTags, etc.).
70-70: No functional changes in doc comment.
112-113: Enhanced flexibility with variadic opts inStructToZodSchema.This is beneficial for passing multiple configuration options. Looks great.
156-162: Added new fields (prefix,customTypes,customTags,ignoreTags) toConverter.They make the internal state more explicit. No issues detected.
345-345: Refactor check for custom types is concise.Looks good. Just ensure
customTypesis not modified concurrently without a lock, if concurrency is a concern.
521-521: Detecting custom types inconvertField.No concerns. The logic for
isCustomis straightforward.
557-557: Reused pattern for detecting custom types ingetTypeField.No issues; consistent usage.
621-666: Robust validation switch for arrays.Double-check off-by-one logic for “gt/lt” transformations. Also watch out for panic messages being descriptive enough.
671-679: Completes array length checks for fixed-length arrays.Implementation looks consistent.
878-878: Ignore tag check is straightforward.No concerns here.
Line range hint
2136-2136: No functional changes on this blank line.custom/decimal/decimal_test.go (1)
14-17: Refactored to useWithCustomTypesfor converter initialization.This approach is consistent with the new options-based initialization in
zod.go.zod_test.go (2)
6-6: Importedstringspackage.No issues; presumably used for test logic or helper methods.
2137-2184: Comprehensive example inTestCustomTag.Demonstrates usage of multiple custom tag handlers. The
.extend()usage is especially helpful for advanced validations.README.md (1)
5-5: LGTM! Good addition of the reference link.Adding the link to go-validator repository improves documentation by providing direct access to the underlying validation library.
* Introduce converter options and use it in NewConverter * Add new option to specify handlers for custom validation tags * Add support for custom validation tags in all types * Improve ignore tags check for all types
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 3
🧹 Nitpick comments (5)
notes (5)
1-2: Consider merging top-level conversion logic for maintainability.
BothconvertStructTopLevelandgetTypehave similar responsibilities: handling the reflection logic required to process a type. If these grow in complexity, consider abstracting shared logic into a helper function to reduce duplication.
4-5: Extract common parts of struct conversion.
convertStructandgetTypeStructappear to serve complementary purposes (converting vs. retrieving type info). In the future, if they expand, watch for opportunities to factor out common reflection or error handling code for improved readability.
7-8: Ensure consistent field handling and reduce potential duplication.
convertFieldvs.getTypeFieldmight introduce duplicated logic when applying custom validation or optional checks. Consider consolidating repeated checks (e.g., optional, nullable, or anonymous logic) in a shared helper to avoid divergence.
27-30: Add clarifying comment about purpose ofentry.
Sinceentrytracks anorderanddata, consider adding a short doc comment indicating its role in conversion output ordering (e.g., “Represents a single unit of generated output with a defined insertion order”).
37-38: Enhance documentation forCustomFn.
CustomFnis a powerful extension point. A short comment specifying how each parameter is used (e.g., reflect.Type = the current type, string = the name, etc.) would clarify usage for future developers.
🛑 Comments failed to post (3)
.github/workflows/ci.yml (2)
24-24:
⚠️ Potential issueInvalid Go version specified.
The specified Go version
1.23.5does not exist yet. The latest stable version of Go is in the 1.21.x series.- go-version: '^1.23.5' + go-version: '^1.21.5'Also applies to: 26-26
41-41: 🛠️ Refactor suggestion
Ensure consistent golangci-lint versions.
The golangci-lint version
v1.63in the CI workflow should match exactly with the versionv1.63.4specified in the Makefile to ensure consistent behavior.- version: v1.63 + version: v1.63.4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.version: v1.63.4notes (1)
16-17: 🛠️ Refactor suggestion
Handle advanced string/number validation.
The currentvalidateStringandvalidateNumberplaceholders suggest potential for deeper rule sets (e.g., regex patterns for strings or range checks for numbers). Consider introducing an extensible mechanism for advanced validations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
zod_test.go (2)
2152-2166: Consider enhancing validation patterns.While the implementation is solid, consider these improvements:
- Make the identifier regex pattern more descriptive with a named constant
- Add length validation for the identifier
- Add range validation for pageParams (e.g., maximum allowed page size)
customTagHandlers := map[string]CustomFn{ "identifier": func(c *Converter, t reflect.Type, validate string, i int) string { - return ".refine((val) => !val || /^[a-z0-9_]*$/.test(val), 'Invalid search identifier')" + const identifierPattern = "^[a-z0-9_]{1,64}$" + return fmt.Sprintf(".refine((val) => !val || new RegExp('%s').test(val), 'Invalid search identifier: must be 1-64 characters of lowercase letters, numbers, or underscores')", identifierPattern) }, "pageParams": func(c *Converter, t reflect.Type, validate string, i int) string { - return ".refine((val) => !val.start || !val.end || val.start < val.end, 'Start should be less than end')" + return `.refine((val) => !val.start || !val.end || val.start < val.end, 'Start should be less than end')` + + `.refine((val) => !val.end || val.end <= 1000, 'Maximum page size exceeded')` },
2168-2183: Add test coverage for edge cases.Consider adding test cases for:
- Empty/invalid sort fields
- Negative page numbers
- Maximum page size limits
- Special characters in identifiers
func TestCustomTagEdgeCases(t *testing.T) { type RequestEdgeCases struct { SortParams `validate:"sortFields="` // Empty sort fields PaginationParams struct { Start *int `json:"start,omitempty" validate:"omitempty,gt=-1"` // Negative pages End *int `json:"end,omitempty" validate:"omitempty,gt=0,lte=1000"` // Max page size } `validate:"pageParams"` Search *string `json:"search,omitempty" validate:"identifier"` // Special chars } // Add assertions here }README.md (1)
215-328: Fix markdown formatting issues.The documentation content is clear and well-structured, but there are formatting inconsistencies with tabs vs spaces. Convert all tabs to spaces for consistent formatting.
- "identifier": func(c *zen.Converter, t reflect.Type, validate string, indent int) string { + "identifier": func(c *zen.Converter, t reflect.Type, validate string, indent int) string {🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
221-221: Hard tabs
Column: 1(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1(MD010, no-hard-tabs)
228-228: Hard tabs
Column: 1(MD010, no-hard-tabs)
229-229: Hard tabs
Column: 1(MD010, no-hard-tabs)
230-230: Hard tabs
Column: 1(MD010, no-hard-tabs)
231-231: Hard tabs
Column: 1(MD010, no-hard-tabs)
235-235: Hard tabs
Column: 1(MD010, no-hard-tabs)
236-236: Hard tabs
Column: 1(MD010, no-hard-tabs)
237-237: Hard tabs
Column: 1(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1(MD010, no-hard-tabs)
239-239: Hard tabs
Column: 1(MD010, no-hard-tabs)
240-240: Hard tabs
Column: 1(MD010, no-hard-tabs)
241-241: Hard tabs
Column: 1(MD010, no-hard-tabs)
242-242: Hard tabs
Column: 1(MD010, no-hard-tabs)
243-243: Hard tabs
Column: 1(MD010, no-hard-tabs)
244-244: Hard tabs
Column: 1(MD010, no-hard-tabs)
245-245: Hard tabs
Column: 1(MD010, no-hard-tabs)
246-246: Hard tabs
Column: 1(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1(MD010, no-hard-tabs)
297-297: Hard tabs
Column: 1(MD010, no-hard-tabs)
298-298: Hard tabs
Column: 1(MD010, no-hard-tabs)
299-299: Hard tabs
Column: 1(MD010, no-hard-tabs)
300-300: Hard tabs
Column: 1(MD010, no-hard-tabs)
306-306: Hard tabs
Column: 1(MD010, no-hard-tabs)
314-314: Hard tabs
Column: 1(MD010, no-hard-tabs)
zod.go (1)
Line range hint
878-885: Consider enhancing error handling for ignored tags.The current implementation silently ignores unknown tags. Consider adding logging or validation to help catch mistyped tag names.
func (c *Converter) checkIsIgnored(part string) bool { + // Create a map for O(1) lookup + ignoredTags := make(map[string]bool) + for _, tag := range c.ignoreTags { + ignoredTags[tag] = true + } + + isIgnored := ignoredTags[part] + if isIgnored { + // Optional: Log ignored tags to help catch mistakes + log.Printf("Ignoring validation tag: %s", part) + } + return isIgnored }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.workis excluded by!**/*.work
📒 Files selected for processing (11)
.github/workflows/ci.yml(2 hunks).golangci.yml(1 hunks)Makefile(1 hunks)README.md(3 hunks)custom/decimal/decimal_test.go(1 hunks)custom/decimal/go.mod(1 hunks)custom/optional/go.mod(1 hunks)custom/optional/optional_test.go(1 hunks)go.mod(1 hunks)zod.go(17 hunks)zod_test.go(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- go.mod
- custom/decimal/go.mod
- Makefile
- .golangci.yml
- custom/optional/optional_test.go
- custom/decimal/decimal_test.go
- .github/workflows/ci.yml
- custom/optional/go.mod
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
221-221: Hard tabs
Column: 1
(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1
(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1
(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
228-228: Hard tabs
Column: 1
(MD010, no-hard-tabs)
229-229: Hard tabs
Column: 1
(MD010, no-hard-tabs)
230-230: Hard tabs
Column: 1
(MD010, no-hard-tabs)
231-231: Hard tabs
Column: 1
(MD010, no-hard-tabs)
235-235: Hard tabs
Column: 1
(MD010, no-hard-tabs)
236-236: Hard tabs
Column: 1
(MD010, no-hard-tabs)
237-237: Hard tabs
Column: 1
(MD010, no-hard-tabs)
238-238: Hard tabs
Column: 1
(MD010, no-hard-tabs)
239-239: Hard tabs
Column: 1
(MD010, no-hard-tabs)
240-240: Hard tabs
Column: 1
(MD010, no-hard-tabs)
241-241: Hard tabs
Column: 1
(MD010, no-hard-tabs)
242-242: Hard tabs
Column: 1
(MD010, no-hard-tabs)
243-243: Hard tabs
Column: 1
(MD010, no-hard-tabs)
244-244: Hard tabs
Column: 1
(MD010, no-hard-tabs)
245-245: Hard tabs
Column: 1
(MD010, no-hard-tabs)
246-246: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
297-297: Hard tabs
Column: 1
(MD010, no-hard-tabs)
298-298: Hard tabs
Column: 1
(MD010, no-hard-tabs)
299-299: Hard tabs
Column: 1
(MD010, no-hard-tabs)
300-300: Hard tabs
Column: 1
(MD010, no-hard-tabs)
306-306: Hard tabs
Column: 1
(MD010, no-hard-tabs)
314-314: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (2)
zod_test.go (1)
2137-2184: Well-structured test implementation for custom validation tags!The test provides good coverage of the new custom tag functionality:
- Validates search identifiers with regex
- Ensures proper page parameter validation
- Implements sort field validation using enums
- Handles optional fields correctly
zod.go (1)
12-50: Well-structured option pattern implementation!The option pattern implementation is clean and extensible. Each option function is well-documented and has a single responsibility.
| "sortFields": func(c *zen.Converter, t reflect.Type, validate string, indent int) string { | ||
| sortFields := strings.Split(validate, " ") | ||
| for i := range sortFields { | ||
| sortFields[i] = fmt.Sprintf("'%s'", sortFields[i]) | ||
| } | ||
| return fmt.Sprintf(".extend({field: z.enum([%s])})", strings.Join(sortFields, ", ")) | ||
| }, |
There was a problem hiding this comment.
Seems quite useful for building advanced use cases.
Does go-validator just ignore these custom tags? If thats the case it may be better to use a new tag for it (zod:"identifier" or zen:"identifier"), to make it clear this validation won't run at go level.
| // Opt represents a converter option used to modify its behavior. | ||
| type Opt func(*Converter) | ||
|
|
||
| // Adds prefix to the generated schema and type names. | ||
| func WithPrefix(prefix string) Opt { | ||
| return func(c *Converter) { | ||
| c.prefix = prefix | ||
| } | ||
| } |
There was a problem hiding this comment.
I love this new API but we might be breaking existing code. How about keeping the original API and adding this new code with a different function name?
| case "omitempty": | ||
| case "required": | ||
| case "dive": | ||
| goto forEnd |
There was a problem hiding this comment.
How about breaking loop using go's breaks with label
https://www.ardanlabs.com/blog/2013/11/label-breaks-in-go.html
| idx := strings.Index(part, "=") | ||
| if idx == 0 || idx == len(part)-1 { | ||
| panic(fmt.Sprintf("invalid validation: %s", part)) | ||
| } | ||
|
|
||
| var valName string | ||
| var valValue string | ||
| if idx == -1 { | ||
| valName = part | ||
| } else { | ||
| valName = part[:idx] | ||
| valValue = part[idx+1:] | ||
| } | ||
|
|
||
| if c.checkIsIgnored(valName) { | ||
| continue | ||
| } | ||
|
|
||
| if h, ok := c.customTags[valName]; ok { | ||
| v := h(c, reflect.TypeOf(0), valValue, 0) | ||
| if strings.HasPrefix(v, ".refine") { | ||
| refines = append(refines, v) | ||
| } else { | ||
| validateStr.WriteString(v) | ||
| } | ||
| continue | ||
| } |
There was a problem hiding this comment.
This code seems to be duplicated thrice in this file.
Summary by CodeRabbit
Release Notes
Documentation
Improvements
Maintenance