feat!: transforms and tests are now typesafe for the schema you are u… #149
Conversation
…sing. No more typecasting in Zog!
WalkthroughThis update refactors the schema validation and transformation framework to use Go generics with explicit pointer types throughout all schema types and their associated processing, transformation, and test methods. All relevant type aliases, interfaces, and method signatures are updated to accept generic pointer types, replacing previous usage of the empty interface ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Schema (generic)
participant Processor (generic)
participant Test (generic)
participant Transform (generic)
participant Ctx
User->>Schema: Define schema with Test[*T], Transform[*T]
User->>Schema: Call Validate/Parse(input)
Schema->>Processor: Process input using ZProcess(*T, Ctx)
Processor->>Transform: (if transform) Transform(*T, Ctx)
Processor->>Test: (if test) Test.Func(*T, Ctx)
Test->>Ctx: Report issue via TestInterface if validation fails
Schema->>User: Return result or issues
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (4)
zogSchema.go (2)
73-75:⚠️ Potential issuePass the pointer (
destPtr), not the dereferenced value, toIssueFromTest
requiredis declared as*p.Test[*T], i.e. the test expects a pointer toT.
By dereferencingdestPtryou are handing a value (T) to a function that is instantiated for*T, which results in a compile-time type mismatch:ctx.AddIssue(ctx.IssueFromTest(required, *destPtr)) // ❌ does not compileFix:
- ctx.AddIssue(ctx.IssueFromTest(required, *destPtr)) + ctx.AddIssue(ctx.IssueFromTest(required, destPtr))The same pattern occurs in
primitiveValidation(see comment below).
Without this change the project will not build.
125-126:⚠️ Potential issueSame pointer / value mix-up in
primitiveValidationAnalogous to the previous comment, use
valPtrinstead of*valPtr:- ctx.AddIssue(ctx.IssueFromTest(required, *valPtr)) + ctx.AddIssue(ctx.IssueFromTest(required, valPtr))This removes the generic type conflict and allows the file to compile.
internals/tests.go (1)
123-164:⚠️ Potential issueNil-pointer dereference in length helpers
All
LenMin/LenMax/Lenhelpers dereference*valwithout verifying thatvalis non-nil.
Anil *sliceornil *stringwill panic at runtime during validation.-fn := func(val *T, ctx Ctx) bool { - return len(*val) >= n -} +fn := func(val *T, ctx Ctx) bool { + if val == nil { + return false // treat nil as failing the test + } + return len(*val) >= n +}Apply the same guard to
LenMaxandLen.string.go (1)
345-360:⚠️ Potential issueOrder of option application
Options are applied after the test function is wrapped.
If an option overwritesIssueCode, it will not be reflected in the already-wrappedFunc.
Swap the order to avoid subtle bugs:-if v.isNot { +for _, opt := range options { + opt(&t) +} + +if v.isNot { … } -… -for _, opt := range options { - opt(&t) -}
🧹 Nitpick comments (29)
internals/types.go (1)
10-10: Consider updating PostTransform to be generic for complete consistency.While
Transformhas been made generic with type parameterT, thePostTransformtype on line 10 is still using the untypedanyparameter. For complete consistency in the type system, consider updatingPostTransformto also use a generic type parameter, or document why it remains non-generic if there's a specific reason.-type PostTransform = func(dataPtr any, ctx Ctx) error +type PostTransform[T any] = func(dataPtr T, ctx Ctx) errorstruct_test.go (1)
209-212: Consider updating this test function in a future PRWhile this PR focuses on making transforms and tests typesafe, there's still a type assertion in this test function (
val.(*CustomStruct)). Consider updating this in a future PR to also use a typed pointer parameter for complete type safety.struct_helper_test.go (1)
173-177: Consider updating transform functions in a future PRWhile this PR replaces deprecated methods, the transform functions still use
anytypes with type assertions. Consider updating these in a future PR to use typed pointer parameters for complete type safety, similar to how the test functions were updated.Also applies to: 203-207
time_validate_test.go (1)
53-58: Type-safe transform function with simplified codeThe transform function now uses a typed pointer parameter (
*time.Time) instead ofany, improving type safety. However, the code could be simplified by removing the temporary variable.validator := Time().Transform(func(dataPtr *time.Time, ctx Ctx) error { // Set the time to noon - t := dataPtr - *t = time.Date(t.Year(), t.Month(), t.Day(), 12, 0, 0, 0, t.Location()) + *dataPtr = time.Date(dataPtr.Year(), dataPtr.Month(), dataPtr.Day(), 12, 0, 0, 0, dataPtr.Location()) return nil })numbers.go (3)
168-170: Guard against a nil transform being registered
TransformProcessor.ZProcesswill panic if the suppliedTransformisnil; add an early check so misuse is caught at the call-site and the stack-trace is easier to read.func (v *NumberSchema[T]) Transform(transform p.Transform[*T]) *NumberSchema[T] { - v.processors = append(v.processors, &p.TransformProcessor[*T]{Transform: transform}) + if transform == nil { + panic("zog: NumberSchema.Transform received a nil function") + } + v.processors = append(v.processors, &p.TransformProcessor[*T]{Transform: transform}) return v }
176-183: Allocate the ‘required’ test directly to avoid pointer-to-stack subtletyEscaping works fine, but using
newmakes the intent explicit and avoids an extra copy when options mutate the struct.- r := p.Required[*T]() - for _, opt := range options { - opt(&r) - } - v.required = &r + req := p.Required[*T]() + for _, opt := range options { + opt(&req) + } + v.required = &req
204-207: *Prefer accepting a Test to avoid an extra struct copyThe current signature forces callers to pass a value, only for the method to take its address again. Accepting a pointer avoids the copy and lines up with how
NewTestFuncreturns its value.-func (v *NumberSchema[T]) Test(t Test[*T]) *NumberSchema[T] { - x := p.Test[*T](t) - v.processors = append(v.processors, &x) +func (v *NumberSchema[T]) Test(t *p.Test[*T]) *NumberSchema[T] { + v.processors = append(v.processors, t) return v }Updating this requires changing the two call-sites (
TestFunchere and the tests elsewhere) to pass the pointer directly.boolean.go (4)
91-94: Avoid copying the Test struct unnecessarilySame optimisation as suggested for
NumberSchema:-func (v *BoolSchema[T]) Test(t p.Test[*T]) *BoolSchema[T] { - v.processors = append(v.processors, &t) +func (v *BoolSchema[T]) Test(t *p.Test[*T]) *BoolSchema[T] { + v.processors = append(v.processors, t) return v }
97-101: Pass pointer returned byNewTestFuncstraight throughOnce the signature above is changed, this can avoid the dereference / copy.
- test := p.NewTestFunc("", testFunc, options...) - v.Test(*test) + v.Test(p.NewTestFunc("", testFunc, options...))
104-107: Nil-transform guardReplicate the safeguard proposed for
NumberSchema.Transformto catch accidentalnil:func (v *BoolSchema[T]) Transform(transform p.Transform[*T]) *BoolSchema[T] { - v.processors = append(v.processors, &p.TransformProcessor[*T]{Transform: transform}) + if transform == nil { + panic("zog: BoolSchema.Transform received a nil function") + } + v.processors = append(v.processors, &p.TransformProcessor[*T]{Transform: transform}) return v }
112-118: Same pointer-to-stack remark as forNumberSchema.RequiredConsider:
- r := p.Required[*T]() - for _, opt := range options { - opt(&r) - } - v.required = &r + req := p.Required[*T]() + for _, opt := range options { + opt(&req) + } + v.required = &reqstruct.go (3)
193-197: Ensure transform is non-nilSame small guard as in the primitive schemas:
func (v *StructSchema) Transform(transform p.Transform[any]) *StructSchema { - v.processors = append(v.processors, &p.TransformProcessor[any]{Transform: transform}) + if transform == nil { + panic("zog: StructSchema.Transform received a nil function") + } + v.processors = append(v.processors, &p.TransformProcessor[any]{Transform: transform}) return v }
228-231: *Accept Test instead of value for consistency & zero copies-func (v *StructSchema) Test(t Test[any]) *StructSchema { - x := p.Test[any](t) - v.processors = append(v.processors, &x) +func (v *StructSchema) Test(t *p.Test[any]) *StructSchema { + v.processors = append(v.processors, t) return v }This matches the pattern used inside primitives once adjusted.
235-238: Propagate pointer after the signature tweak- test := p.NewTestFunc("", p.BoolTFunc[any](testFunc), options...) - v.Test(Test[any](*test)) + v.Test(p.NewTestFunc("", p.BoolTFunc[any](testFunc), options...))zogSchema.go (1)
47-49: Consider returning a pointer fromTestFuncto avoid unnecessary copying
p.NewTestFuncalready returns*p.Test[T]. Dereferencing it to a value, only to
take its address again in every call site (&x,&r, …), causes an avoidable allocation / copy.-func TestFunc[T any](IssueCode zconst.ZogIssueCode, fn BoolTFunc[T], options ...p.TestOption) Test[T] { - return Test[T](*p.NewTestFunc(IssueCode, p.BoolTFunc[T](fn), options...)) +func TestFunc[T any](IssueCode zconst.ZogIssueCode, fn BoolTFunc[T], options ...p.TestOption) *Test[T] { + return (*Test[T])(p.NewTestFunc(IssueCode, p.BoolTFunc[T](fn), options...)) }This is an optional micro-optimisation; feel free to ignore if the value semantics are intentional.
time.go (2)
115-118: Ignore-on-error policy is unclear inTransformwrapper
Transformfunctions return anerror, but the enclosingTransformProcessor
silently discards it unless the inner implementation callsctx.AddIssue
itself.
If that’s intended, add a short comment; otherwise consider propagating the
error throughctx.AddIssue(ctx.IssueFromTransform(err))(or similar) so that
users don’t accidentally lose transform failures.
173-175: Nil-safety for custom time tests
fn := func(v *time.Time, ctx Ctx) bool { return (*v).After(t) }If the caller supplies
nil(which can happen when the field is optional and
unset), this will panic. A defensive check is cheap:- return (*v).After(t) + if v == nil { + return false + } + return v.After(t)The same applies to
BeforeandEQ.slices.go (1)
222-226: Pointer escapes on everyTestcallx := p.Test[any](t) v.processors = append(v.processors, &x)Creating a new
xvalue only to take its address causes an extra heap
allocation. If you apply the earlier suggestion to makeTestFunc
return*Test[T], you can store the pointer directly and avoid this
pattern entirely.internals/tests.go (4)
52-61: Interface getters / setters are verboseBecause every method is simple field access, an embedded struct or
struct{ Test[*T] }wrapper could expose the fields directly and drop eight boiler-plate methods.
Not critical, but worth considering for maintainability.
77-79: Receiver pointer vs value
ZProcesstakesvalPtr T, not*T. This matches the generic instantiation (Test[*T]) but is slightly surprising in isolation. A short comment explaining thatTmay itself be a pointer (e.g.*string) would help future readers.
114-121:Required()returns a test with a nilFuncDown-stream processors must special-case a nil function. If that behaviour is relied upon, add a short doc-comment to avoid accidental misuse.
166-182: Avoidreflect.DeepEqualinIn()
reflect.DeepEqualis slow and blocks comparability with non-comparable elements.
If you constrainTtocomparable, a direct==comparison is possible and faster:-func In[T any](values []T) +func In[T comparable](values []T)For non-comparable types you could fall back to hashing or keep the current reflect-based path behind a build tag.
string.go (7)
176-179: Heap escape for localx
xescapes to the heap when you store&xinprocessors, so there is no dangling-pointer bug.
Still, you could avoid the extra copy by appendingtPtr := &tdirectly.- x := p.Test[*T](t) - v.processors = append(v.processors, &x) + tPtr := p.Test[*T](t) // copy for safety + v.processors = append(v.processors, &tPtr)
183-187: Empty issue code inTestFunc
NewTestFunc("", …)produces a test with an emptyIssueCode, which may confuse issue formatters.Expose
IssueCodeas an optional parameter or provide a sensible default (e.g."custom").
214-218: Email regex correctnessThe current RFC-5322-inspired regex rejects valid internationalised email addresses (IDNA).
Consider switching tonet/mail’s parser or documenting the limitation.
224-230: URL validation accepts spaces
url.Parsehappily parses strings with spaces ("http://example.com /foo").
A stricter check likestrings.ContainsAny(raw, " \t\r\n")prevents false positives.
270-281: ASCII-only uppercase check
r >= 'A' && r <= 'Z'fails for Unicode uppercase letters such asÆorÑ.-import "strings" +import ( + "strings" + "unicode" +) … -for _, r := range string(*v) { - if r >= 'A' && r <= 'Z' { +for _, r := range string(*v) { + if unicode.IsUpper(r) { return true } }
300-314: Special-character range misses some symbolsCharacters outside
'!'–'~'(e.g., non-ASCII punctuation) are skipped.
If that is intentional, add a comment; otherwise considerunicode.IsPunctor a predefined class.
318-323: UUID regexThe current pattern permits upper-case A–F, 👍.
Consider anchoring with^/$or usingParseUUIDfromgithub.com/google/uuidfor guaranteed compliance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
boolean.go(2 hunks)boolean_test.go(2 hunks)boolean_validate_test.go(3 hunks)custom.go(2 hunks)internals/contexts.go(2 hunks)internals/processors.go(1 hunks)internals/tests.go(4 hunks)internals/types.go(1 hunks)numbers.go(3 hunks)numbers_custom_test.go(3 hunks)numbers_int64_test.go(3 hunks)numbers_test.go(3 hunks)numbers_validate_test.go(3 hunks)pointers.go(2 hunks)reusable_tests_test.go(1 hunks)slices.go(8 hunks)string.go(8 hunks)string_custom_test.go(3 hunks)string_test.go(2 hunks)string_validate_test.go(2 hunks)struct.go(3 hunks)struct_helper_test.go(3 hunks)struct_helpers.go(1 hunks)struct_test.go(2 hunks)struct_validate_test.go(2 hunks)time.go(5 hunks)time_test.go(2 hunks)time_validate_test.go(2 hunks)utils.go(1 hunks)utilsOptions.go(3 hunks)zhttp/zhttp_test.go(1 hunks)zogSchema.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (15)
struct_helpers.go (1)
internals/processors.go (1)
ZProcessor(4-6)
utils.go (1)
internals/tests.go (1)
Test(64-75)
reusable_tests_test.go (6)
internals/tests.go (2)
TestOption(17-17)Test(64-75)utilsOptions.go (2)
TestOption(10-10)Message(13-19)utils.go (2)
Test(42-42)Ctx(12-12)zogSchema.go (1)
TestFunc(47-49)zconst/consts.go (1)
ZogIssueCode(54-54)internals/contexts.go (1)
Ctx(8-42)
time_validate_test.go (5)
time.go (1)
Time(37-45)internals/types.go (1)
Transform(12-12)zogSchema.go (2)
Transform(34-34)TestFunc(47-49)internals/contexts.go (1)
Ctx(8-42)utils.go (1)
Ctx(12-12)
pointers.go (1)
internals/tests.go (1)
Test(64-75)
custom.go (4)
internals/tests.go (3)
Test(64-75)TestOption(17-17)TestFuncFromBool(19-28)utils.go (2)
Test(42-42)Ctx(12-12)internals/contexts.go (1)
Ctx(8-42)utilsOptions.go (1)
TestOption(10-10)
struct_validate_test.go (3)
internals/contexts.go (1)
Ctx(8-42)internals/types.go (1)
Transform(12-12)zogSchema.go (1)
Transform(34-34)
boolean_test.go (5)
internals/types.go (1)
Transform(12-12)zogSchema.go (2)
Transform(34-34)TestFunc(47-49)internals/contexts.go (1)
Ctx(8-42)utils.go (1)
Ctx(12-12)boolean.go (1)
Bool(34-42)
boolean.go (6)
internals/processors.go (2)
ZProcessor(4-6)TransformProcessor(8-10)internals/tests.go (5)
Test(64-75)BoolTFunc(11-11)TestOption(17-17)NewTestFunc(41-50)Required(114-121)utils.go (1)
Test(42-42)zogSchema.go (3)
TestFunc(47-49)BoolTFunc(44-44)Transform(34-34)utilsOptions.go (1)
TestOption(10-10)internals/types.go (1)
Transform(12-12)
struct.go (6)
internals/processors.go (2)
ZProcessor(4-6)TransformProcessor(8-10)internals/tests.go (4)
Test(64-75)BoolTFunc(11-11)TestOption(17-17)NewTestFunc(41-50)utils.go (1)
Test(42-42)internals/types.go (1)
Transform(12-12)zogSchema.go (3)
Transform(34-34)TestFunc(47-49)BoolTFunc(44-44)utilsOptions.go (1)
TestOption(10-10)
numbers.go (6)
internals/processors.go (2)
ZProcessor(4-6)TransformProcessor(8-10)internals/tests.go (5)
Test(64-75)Required(114-121)TestOption(17-17)BoolTFunc(11-11)NewTestFunc(41-50)utils.go (1)
Test(42-42)internals/types.go (1)
Transform(12-12)zogSchema.go (3)
Transform(34-34)TestFunc(47-49)BoolTFunc(44-44)utilsOptions.go (1)
TestOption(10-10)
time.go (8)
internals/processors.go (2)
ZProcessor(4-6)TransformProcessor(8-10)internals/tests.go (5)
Test(64-75)Required(114-121)TestOption(17-17)BoolTFunc(11-11)NewTestFunc(41-50)utils.go (2)
Test(42-42)Ctx(12-12)internals/types.go (1)
Transform(12-12)zogSchema.go (3)
Transform(34-34)TestFunc(47-49)BoolTFunc(44-44)utilsOptions.go (3)
TestOption(10-10)IssueCode(32-36)Params(57-61)internals/contexts.go (1)
Ctx(8-42)zconst/consts.go (1)
IssueCodeBefore(168-168)
zogSchema.go (9)
internals/types.go (2)
Transform(12-12)ZogPrimitive(15-17)internals/Issues.go (1)
IssueFmtFunc(10-10)internals/tests.go (5)
TFunc(14-14)BoolTFunc(11-11)TestOption(17-17)Test(64-75)NewTestFunc(41-50)utilsOptions.go (2)
IssueCode(32-36)TestOption(10-10)zconst/consts.go (1)
ZogIssueCode(54-54)utils.go (2)
Test(42-42)CoercerFunc(37-37)internals/contexts.go (1)
SchemaCtx(132-142)internals/processors.go (1)
ZProcessor(4-6)internals/zeroValues.go (1)
IsZeroValueFunc(7-7)
slices.go (8)
internals/processors.go (2)
ZProcessor(4-6)TransformProcessor(8-10)zogSchema.go (4)
ZogSchema(10-15)Transform(34-34)TestFunc(47-49)BoolTFunc(44-44)internals/tests.go (6)
Test(64-75)Required(114-121)BoolTFunc(11-11)TestOption(17-17)NewTestFunc(41-50)Len(153-164)utils.go (2)
Test(42-42)Ctx(12-12)internals/types.go (1)
Transform(12-12)utilsOptions.go (3)
TestOption(10-10)IssueCode(32-36)Params(57-61)zconst/consts.go (2)
IssueCodeMin(89-89)IssueCodeMax(93-93)internals/contexts.go (1)
Ctx(8-42)
string.go (8)
internals/processors.go (2)
ZProcessor(4-6)TransformProcessor(8-10)internals/tests.go (5)
Test(64-75)Required(114-121)TestOption(17-17)BoolTFunc(11-11)NewTestFunc(41-50)utils.go (2)
Test(42-42)Ctx(12-12)internals/types.go (1)
Transform(12-12)zogSchema.go (3)
Transform(34-34)TestFunc(47-49)BoolTFunc(44-44)internals/contexts.go (1)
Ctx(8-42)utilsOptions.go (3)
TestOption(10-10)IssueCode(32-36)Params(57-61)zconst/consts.go (10)
IssueCodeEmail(123-123)IssueCodeURL(135-135)IssueCodeHasPrefix(139-139)IssueCodeHasSuffix(143-143)IssueCodeContains(101-101)IssueCodeContainsUpper(147-147)IssueCodeContainsDigit(155-155)IssueCodeContainsSpecial(159-159)IssueCodeUUID(127-127)IssueCodeMatch(131-131)
🔇 Additional comments (78)
utils.go (1)
42-42: Well implemented type safety improvement.Excellent change replacing the non-generic
Testalias with a genericTest[T any]type. This change aligns perfectly with the PR objective of making transforms and tests typesafe for the schema being used, eliminating the need for typecasting. It ensures that test functions will receive properly typed arguments at compile time rather than requiring runtime type assertions.struct_helpers.go (1)
26-26: Good adoption of generics for type safety.The change from a non-generic
[]p.ZProcessorto a generic[]p.ZProcessor[any]correctly updates the type of the processors slice to match the generic interface definition. This is consistent with the overall refactoring to use generics throughout the codebase, providing better type safety while maintaining compatibility with existing code by usinganyas the type parameter.zhttp/zhttp_test.go (1)
39-41: Improved type safety in test function.Good refactoring of the test function to use a typed
*stringparameter instead ofany, allowing direct value access via dereferencing (*val) rather than using type assertions. This change enhances code readability and compile-time type safety.internals/types.go (1)
12-12: Excellent foundational change for type safety.Converting the
Transformtype from a non-generic function to a generic function with type parameterTis a key architectural improvement. This change allows transform functions to be specialized for different schema types, enabling compile-time type checking rather than relying on runtime type assertions. This is a core enhancement that supports the PR's goal of making transforms typesafe.reusable_tests_test.go (1)
10-17: Good refactoring to support type safety!The function
MyTesthas been updated to use generics with a typed pointer parameter (*string), eliminating the need for type assertions. This change aligns perfectly with the PR objective of making transforms and tests typesafe for the schema being used.pointers.go (2)
15-15: Good update to align with generic typing system!The
requiredfield has been correctly updated to use the genericp.Test[any]type, which aligns with the broader refactoring to use generics across the codebase.
126-132: Implementation correctly updated for type safetyThe
NotNilmethod has been properly updated to instantiate ap.Test[any]struct instead of the previous non-generic type. This maintains the same functionality while integrating with the new type system.struct_test.go (2)
174-176: Good refactoring for type safety!The custom test function now accepts a typed pointer parameter (
*int) instead ofany, and properly dereferences it instead of using type assertions. This aligns with the PR objective of ensuring type safety and eliminating typecasting.
307-307: Method correctly updated to use the new Transform APIThe deprecated
PostTransformmethod has been correctly replaced with the newTransformmethod, which is part of the consistent API update across the codebase.struct_helper_test.go (4)
56-60: Method correctly updated to use the new Transform APIThe deprecated
PostTransformmethod has been correctly replaced with the newTransformmethod, which is part of the consistent API update across the codebase.
63-67: Method correctly updated to use the new Transform APIThe deprecated
PostTransformmethod has been correctly replaced with the newTransformmethod, maintaining the same transformation functionality.
173-177: Method correctly updated to use the new Transform APIThe deprecated
PostTransformmethod has been correctly replaced with the newTransformmethod, which is part of the consistent API update across the codebase.
203-207: Method correctly updated to use the new Transform APIThe deprecated
PostTransformmethod has been correctly replaced with the newTransformmethod, maintaining the same transformation functionality.numbers_test.go (3)
118-123: Type-safe transformations - good job!The transformation function now uses a typed pointer parameter (
*int) instead ofany, which eliminates the need for type assertions and improves type safety. The method call has also been updated from the deprecated.PostTransform()to.Transform().
134-140: Type-safe transform chain implemented correctlyThe transformation function signature has been updated to use typed pointers, and the method calls have been updated to use the new
.Transform()method, maintaining the ability to chain multiple transformations with improved type safety.
258-262: Custom test function with proper type safetyGreat job updating the test function to use a typed pointer parameter (
*int) instead ofany. The code now directly dereferences the pointer with*valfor comparison, eliminating the need for type assertions and improving type safety.struct_validate_test.go (2)
111-113: Type-safe custom test functionThe custom test function now uses a strongly typed pointer parameter (
*int) and directly dereferences it, eliminating the need for type assertions and improving type safety.
180-180: Updated to use Transform instead of PostTransformThe method call has been correctly updated from the deprecated
.PostTransform()to.Transform(), aligning with the broader refactoring in the codebase to eliminate deprecated methods.time_validate_test.go (1)
105-107: Type-safe custom test functionThe test function now uses a typed pointer parameter (
*time.Time) instead ofany, with proper dereferencing when calling methods on the time object. This eliminates the need for type assertions and improves type safety.time_test.go (2)
51-55: Type-safe transform implementationThe transform function now uses a typed pointer parameter (
*time.Time) and correctly dereferences it to modify the time value. This eliminates the need for type assertions and improves type safety.
103-105: Type-safe custom test functionThe test function now uses a typed pointer parameter (
*time.Time) with proper dereferencing when calling methods on the time object. This eliminates type assertions and improves type safety.string_test.go (2)
59-63: Great type safety improvement!The transformation function now uses a strongly typed
*stringparameter instead of the previousanytype with internal type assertions. This provides better type safety at compile time and eliminates potential runtime errors from incorrect type assertions.
83-85: Enhanced type safety for test functionsThe test function now directly accepts a strongly typed
*stringparameter instead ofany, and uses proper dereferencing for the string comparison. This is consistent with the PR's goal of making transforms and tests typesafe for the schema being used.string_validate_test.go (2)
62-64: Strongly typed transform function improves safetyGood refactoring to use a typed
*stringparameter instead ofany, which provides compile-time type checking and eliminates the need for manual type assertions within the function body.
84-86: Improved type safety in test functionThe TestFunc callback now correctly uses a strongly typed
*stringparameter instead of a genericanytype, which improves code clarity and prevents potential runtime type errors.numbers_validate_test.go (4)
86-92: Strong typing for number transform functionsThe transformation function now uses a strongly typed
*intparameter instead ofany, which removes the need for type assertions and improves type safety.
91-91: Deprecated method replaced with type-safe alternativeGood change from the deprecated
.PostTransformmethod to the new.Transformmethod that accepts a strongly typed function parameter.
102-108: Consistent type-safe transform implementationThis transformation function follows the same pattern of using strongly typed
*intparameters rather thanany, providing consistent type safety throughout the codebase.
245-248: Type-safe test functions for numeric schemasThe test function now uses a strongly typed
*intparameter rather thanany, eliminating runtime type assertions and improving code clarity through direct dereferencing.numbers_custom_test.go (3)
130-136: Type safety for custom numeric typesThe transformation function now correctly uses
*MyIntinstead ofany, providing type safety for custom numeric types as well. The call to.Transform()properly replaces the deprecated.PostTransform()method.
146-152: Consistent type safety in multiple transformationsGood implementation of multiple transform functions with strongly typed
*MyIntparameters. This ensures type safety is maintained throughout transformation chains.
269-273: Type-safe custom test functionsThe test function now uses strongly typed
*MyIntparameters instead ofany, which ensures type safety when working with custom numeric types and eliminates the need for type assertions inside the function body.internals/contexts.go (2)
141-141: Type change aligns with generic architectureThe
Processorfield type has been changed from a specific interface toanyto accommodate the new generic processor types. This allows the schema context to work with different generic processor implementations.
166-180: Good abstraction through interface usageThe method has been updated to work with the new
TestInterfacetype instead of a concrete*Testpointer, which is consistent with the broader refactoring to use interfaces and generics. The code properly uses accessor methods instead of direct field access, improving encapsulation.internals/processors.go (3)
4-6: Excellent use of generics for type safetyConverting the
ZProcessorinterface to use a generic type parameterTeliminates the need for type assertions when processing values. This is a key improvement that supports the PR's goal of making the codebase more type-safe.
8-10: Type-safe transform processor implementationMaking
TransformProcessorgeneric with type parameterTensures type consistency between the processor and its transform function, reducing the risk of runtime type errors.
12-19: Type-safe process method implementationThe
ZProcessmethod now accepts a typed parameterTinstead ofany, which eliminates the need for type assertions within the method. The error handling logic remains unchanged, maintaining the same behavior while improving type safety.custom.go (3)
12-12: Generic test field improves type safetyUsing the generic pointer type
p.Test[*T]for the test field ensures that test functions receive correctly typed pointers, eliminating the need for type assertions in test implementations.
16-19: Type-safe custom function implementationThe function now creates and initializes the test with properly typed functions that directly accept
*Tpointers instead of untypedanyvalues, which eliminates the need for runtime type assertions within the test functions.
76-76: Explicit type casting maintains safetyThe validate method now explicitly casts
ctx.ValPtrto*Tbefore passing it to the test function, ensuring type consistency. This targeted type assertion is safer than implicit assertions inside the test functions.string_custom_test.go (3)
65-68: Type-safe transform implementationThe
Transformmethod now accepts a typed pointer*Envrather than an untypedany, which aligns with the PR's goal of making transforms type-safe. This eliminates the need for type assertions inside the transformation function.
97-97: Added explicit error code verificationThe additional assertion to verify that the error code matches the expected constant
zconst.IssueCodeContainsimproves test coverage by ensuring that the correct error type is being raised.
234-234: Consistent parameter type in error mapsThe error parameter type for prefix validation now uses a raw string
"test_"instead of wrapping it asEnv("test_"). This improves consistency with how parameters are handled in the test interface implementation.numbers_int64_test.go (5)
107-109: Type-safe transform function using pointer parameterThe function signature has been updated to accept a strongly typed pointer (
*int64) instead of an interface type (any). This improves type safety by ensuring the transform function operates directly on the typed value without requiring type assertions or conversions.
112-112: Replaced deprecated PostTransform with Transform methodThe code correctly uses the new
Transformmethod instead of the deprecatedPostTransformmethod, aligning with the API changes in this PR.
123-126: Type-safe transform function using pointer parameterAnother instance of updating the function signature to accept a strongly typed pointer (
*int64) instead of an interface type, improving type safety by eliminating the need for type assertions.
128-128: Replaced deprecated PostTransform with Transform methodConsistent with previous change, using the updated
Transformmethod instead of the deprecatedPostTransformmethod.
246-249: Type-safe test function using pointer parameterThe test function has been updated to use a strongly typed pointer (
*int64) instead of an interface type, improving type safety. The assertion is also updated to dereference the pointer correctly.boolean_validate_test.go (6)
203-205: Updated test function to use typed pointer parameterThe test function now accepts a strongly typed
*boolpointer instead of an untyped interface, improving type safety. The function body correctly dereferences the pointer using*valto access the boolean value.
287-287: Updated transform type to use generic pointer typeThe transform type declaration now uses the generic
p.Transform[*bool]type with explicit pointer type parameter, ensuring type-safe transformations for boolean values.
294-296: Type-safe transform function using pointer parameterThe transform function signature now uses a strongly typed
*boolpointer parameter and correctly modifies the value by dereferencing the pointer, improving type safety and eliminating the need for type assertions.
303-305: Type-safe transform function using pointer parameterConsistent with previous changes, this transform function now uses a strongly typed
*boolpointer parameter.
311-313: Type-safe transform function using pointer parameterAnother instance of updating a transform function to use strongly typed pointer parameters, maintaining consistency throughout the codebase.
339-342: Type-safe test function using pointer parameterThe custom test function now accepts a strongly typed
*boolpointer and correctly uses it through dereferencing (assert.Equal(t, true, *val)), eliminating the need for type assertions.boolean_test.go (5)
387-387: Updated transform type to use generic pointer typeThe transform type has been updated to use the generic
p.Transform[*bool]with an explicit pointer type parameter, ensuring type-safe transformations for boolean values.
394-396: Type-safe transform function using pointer parameterThe transform function now accepts a strongly typed
*boolpointer and correctly modifies the value by dereferencing the pointer with*val = !*val, improving type safety by eliminating type assertions.
403-405: Type-safe transform function using pointer parameterConsistent with other changes, this transform function now uses a strongly typed
*boolpointer parameter.
411-413: Type-safe transform function using pointer parameterAnother example of consistent updating to use strongly typed pointer parameters throughout the transform functions.
441-444: Type-safe test function using pointer parameterThe custom test function now accepts a strongly typed
*boolpointer and correctly accesses the value through dereferencing (*val == true), improving type safety by eliminating type assertions.utilsOptions.go (5)
14-18: Improved encapsulation using TestInterfaceThe implementation now uses the
p.TestInterfaceabstraction instead of directly manipulating concrete test struct fields. This improves encapsulation by using the interface methodSetIssueFmtFuncto set the message formatter.
22-25: Improved encapsulation using TestInterfaceUpdated the
MessageFuncoption to use thep.TestInterfaceabstraction, promoting better encapsulation by using interface methods instead of direct field access.
33-35: Improved encapsulation using TestInterfaceThe
IssueCodeoption now uses thep.TestInterfaceabstraction with theSetIssueCodemethod, maintaining consistency with the interface-based approach.
50-52: Improved encapsulation using TestInterfaceThe
IssuePathoption now uses thep.TestInterfaceabstraction with theSetIssuePathmethod, following the pattern of better encapsulation.
58-60: Improved encapsulation using TestInterfaceThe
Paramsoption now uses thep.TestInterfaceabstraction with theSetParamsmethod, completing the consistent pattern of using interface methods instead of direct field access.numbers.go (2)
15-18: Processors & required fields now use pointer-generic types – looks goodSwitching to
[]p.ZProcessor[*T]and*p.Test[*T]is consistent with the new pointer-generic APIs introduced ininternals/. ✅
211-214: Empty IssueCode string – verify this is intentional
NewTestFuncexpects azconst.ZogIssueCode; passing""compiles only because the underlying type isstring.
If the formatter relies on a non-empty code, this may surface as confusing messages.Please double-check whether a dedicated code constant (e.g.
zconst.IssueCodeCustom) should be used instead.boolean.go (1)
12-17: Pointer-generic fields mirrorNumberSchema– good consistencyNo functional concerns here. 👍
struct.go (1)
16-19: Generic processor/test fields cleanly reflect the new internalsNo issues spotted – the change removes several old type assertions.
internals/tests.go (2)
11-15: Generics migration looks solidThe switch from
interface{}to[T any]for bothBoolTFuncandTFuncimmediately removes the need for unsafe type-casting and gives callers compile-time guarantees. Nicely done.
17-18: Good abstraction for optionsAccepting a
TestInterfaceinstead of*Testkeeps the option helpers decoupled from the concrete implementation and prevents accidental reliance on private fields.string.go (8)
45-48: Processor slice generified successfullyStoring
[]p.ZProcessor[*T]and a typedrequiredpointer removes every run-time cast previously scattered through the string schema.
Looks clean. 👍
139-142: Transform helper LGTMAccepting
p.Transform[*T]keeps the public API aligned with the internal processor model.
148-153: Reuse ofRequiredhelper is neatThe pass-through of
TestOptions to the generated test preserves customisation. No issues spotted.
234-240: Prefix test LGTM
244-253: Suffix test LGTM
257-266: Contains test LGTM
285-296: Digit check LGTM – ASCII digits are usually sufficient for validation.
328-335: Match test LGTM
| func (v *SliceSchema) Transform(transform Transform[any]) *SliceSchema { | ||
| v.processors = append(v.processors, &p.TransformProcessor[any]{ | ||
| Transform: p.Transform[any](transform), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make Transform generic over the concrete slice type
Using Transform[any] forces every transform to perform type assertions. You
already know the element type from v.schema; you can expose that via a type
parameter on SliceSchema, e.g.:
type SliceSchema[T any] struct {
processors []p.ZProcessor[*[]T]
// ...
}This propagates type-safety to callers and removes a whole class of
runtime panics. Refactor is optional but strongly recommended if API
stability allows.
| func EQ[T comparable](n T) (Test[*T], BoolTFunc[*T]) { | ||
| fn := func(val *T, ctx Ctx) bool { | ||
| return *val == n | ||
| } | ||
|
|
||
| t := Test{ | ||
| t := Test[*T]{ | ||
| IssueCode: zconst.IssueCodeEQ, | ||
| Params: make(map[string]any, 1), | ||
| } | ||
| t.Params[zconst.IssueCodeEQ] = n | ||
| return t, fn | ||
| } | ||
|
|
||
| func LTE[T constraints.Ordered](n T) (Test, BoolTFunc) { | ||
| fn := func(val any, ctx Ctx) bool { | ||
| v, ok := val.(*T) | ||
| if !ok { | ||
| return false | ||
| } | ||
| return *v <= n | ||
| func LTE[T constraints.Ordered](n T) (Test[*T], BoolTFunc[*T]) { | ||
| fn := func(val *T, ctx Ctx) bool { | ||
| return *val <= n | ||
| } | ||
|
|
||
| t := Test{ | ||
| t := Test[*T]{ | ||
| IssueCode: zconst.IssueCodeLTE, | ||
| Params: make(map[string]any, 1), | ||
| } | ||
| t.Params[zconst.IssueCodeLTE] = n | ||
| return t, fn | ||
| } | ||
|
|
||
| func GTE[T constraints.Ordered](n T) (Test, BoolTFunc) { | ||
| fn := func(val any, ctx Ctx) bool { | ||
| v, ok := val.(*T) | ||
| if !ok { | ||
| return false | ||
| } | ||
| return *v >= n | ||
| func GTE[T constraints.Ordered](n T) (Test[*T], BoolTFunc[*T]) { | ||
| fn := func(val *T, ctx Ctx) bool { | ||
| return *val >= n | ||
| } | ||
|
|
||
| t := Test{ | ||
| t := Test[*T]{ | ||
| IssueCode: zconst.IssueCodeGTE, | ||
| Params: make(map[string]any, 1), | ||
| } | ||
| t.Params[zconst.IssueCodeGTE] = n | ||
| return t, fn | ||
| } |
There was a problem hiding this comment.
Same nil-pointer risk in comparison helpers
EQ/LT/LTE/GTE/GT functions also dereference a possibly nil pointer.
Fix pattern identical to the earlier length helpers.
| func TestFuncFromBool[T any](fn BoolTFunc[T], test *Test[T]) { | ||
| test.Func = func(val T, ctx Ctx) { | ||
| if fn(val, ctx) { | ||
| return | ||
| } | ||
|
|
||
| c := ctx.(*SchemaCtx) | ||
| ctx.AddIssue(c.IssueFromTest(c.Processor.(*Test), val)) | ||
| ctx.AddIssue(c.IssueFromTest(c.Processor.(TestInterface), val)) | ||
| } | ||
| } | ||
|
|
||
| func TestNotFuncFromBool(fn BoolTFunc, test *Test) { | ||
| test.Func = func(val any, ctx Ctx) { | ||
| func TestNotFuncFromBool[T any](fn BoolTFunc[T], test *Test[T]) { | ||
| test.Func = func(val T, ctx Ctx) { | ||
| if !fn(val, ctx) { | ||
| return | ||
| } | ||
|
|
||
| c := ctx.(*SchemaCtx) | ||
| ctx.AddIssue(c.IssueFromTest(c.Processor.(*Test), val)) | ||
| ctx.AddIssue(c.IssueFromTest(c.Processor.(TestInterface), val)) | ||
| } |
There was a problem hiding this comment.
Panic risk when Ctx is not a *SchemaCtx
ctx.(*SchemaCtx) will panic if a user passes a custom context that merely implements the public Ctx interface.
Consider a type-switch or an early guard to avoid a hard crash.
- c := ctx.(*SchemaCtx)
+ sc, ok := ctx.(*SchemaCtx)
+ if !ok {
+ // fall back or return; do not panic
+ return
+ }Committable suggestion skipped: line range outside the PR's diff.
| v.processors = append(v.processors, &p.TransformProcessor[*T]{ | ||
| Transform: func(val *T, ctx Ctx) error { | ||
| *val = T(strings.TrimSpace(string(*val))) | ||
| return nil | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Nil-pointer safety in Trim() transform
Trim() assumes the value pointer is never nil. That is true during parsing but not necessarily during standalone Validate.
-Transform: func(val *T, ctx Ctx) error {
- *val = T(strings.TrimSpace(string(*val)))
+Transform: func(val *T, ctx Ctx) error {
+ if val == nil {
+ return nil // nothing to trim
+ }
+ *val = T(strings.TrimSpace(string(*val)))
return nil
},📝 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.
| v.processors = append(v.processors, &p.TransformProcessor[*T]{ | |
| Transform: func(val *T, ctx Ctx) error { | |
| *val = T(strings.TrimSpace(string(*val))) | |
| return nil | |
| }, | |
| }) | |
| v.processors = append(v.processors, &p.TransformProcessor[*T]{ | |
| Transform: func(val *T, ctx Ctx) error { | |
| if val == nil { | |
| return nil // nothing to trim | |
| } | |
| *val = T(strings.TrimSpace(string(*val))) | |
| return nil | |
| }, | |
| }) |
…sing. No more typecasting in Zog!
Summary by CodeRabbit