Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 4 additions & 2 deletions any.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ func (v *AnySchema) process(ctx *p.SchemaCtx) {

destPtr, ok := ctx.ValPtr.(*any)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.AddIssue(ctx.IssueFromInvalidType("*any", ctx.ValPtr, "parsing an any schema"))
return
}

// Handle default/required for nil values
Expand Down Expand Up @@ -129,7 +130,8 @@ func (v *AnySchema) validate(ctx *p.SchemaCtx) {

valPtr, ok := ctx.ValPtr.(*any)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.AddIssue(ctx.IssueFromInvalidType("*any", ctx.ValPtr, "validating an any schema"))
return
}

// Handle default/required for zero values
Expand Down
27 changes: 27 additions & 0 deletions boolean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package zog
import (
"fmt"
"testing"
"time"

p "github.com/Oudwins/zog/pkgs/internals"
"github.com/Oudwins/zog/pkgs/internals/tutils"
Expand Down Expand Up @@ -505,3 +506,29 @@ func TestBoolGetType(t *testing.T) {
s := Bool()
assert.Equal(t, zconst.TypeBool, s.getType())
}

func TestBoolInvalidType(t *testing.T) {
var nilString *string
cases := []struct {
Value any
}{
{Value: "string"},
{Value: 12},
{Value: time.Now()},
{Value: 12.5},
{Value: true},
{Value: []string{"hello"}},
{Value: nil},
{Value: nilString},
}

schema := Bool().Required()

for _, c := range cases {
ctx, errs := tutils.FakeContextFromValue(c.Value, "boolean")
schema.process(ctx)
assert.NotNil(t, errs.List)
assert.Equal(t, errs.List[0].Code, zconst.IssueCodeInvalidType)

Comment on lines +530 to +532

@coderabbitai coderabbitai Bot Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against empty issue lists before indexing.

At Line 531, errs.List[0] is accessed without asserting non-empty; this can panic and obscure failures when no issues are recorded.

Proposed fix
 		schema.process(ctx)
-		assert.NotNil(t, errs.List)
-		assert.Equal(t, errs.List[0].Code, zconst.IssueCodeInvalidType)
+		assert.NotEmpty(t, errs.List)
+		assert.Equal(t, zconst.IssueCodeInvalidType, errs.List[0].Code)
📝 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.

Suggested change
assert.NotNil(t, errs.List)
assert.Equal(t, errs.List[0].Code, zconst.IssueCodeInvalidType)
assert.NotEmpty(t, errs.List)
assert.Equal(t, zconst.IssueCodeInvalidType, errs.List[0].Code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@boolean_test.go` around lines 530 - 532, Add a guard asserting the issue list
is non-empty before indexing errs.List[0]: ensure you first assert that
errs.List is not nil and has length > 0 (e.g., using assert.NotEmpty or
assert.Len on errs.List) and only then verify errs.List[0].Code equals
zconst.IssueCodeInvalidType so the test cannot panic when no issues are
recorded.

@Oudwins Oudwins Jun 9, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeRabbit I don't write defensive code inside tests. Please remember that. its not the first time you suggest this stupid changes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Oudwins, my apologies — I'll stop suggesting defensive guards in test code. Noted and remembered.

(ᵕ︵ᵕ)


✏️ Learnings added
Learnt from: Oudwins
Repo: Oudwins/zog PR: 229
File: boolean_test.go:530-532
Timestamp: 2026-06-09T06:24:52.565Z
Learning: In the `Oudwins/zog` repository, the author does not write defensive code inside tests. Do not suggest adding guards (e.g., asserting non-empty before indexing, nil checks, length checks) in test functions. Such suggestions are explicitly unwanted.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}
}
28 changes: 28 additions & 0 deletions boolean_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package zog
import (
"fmt"
"testing"
"time"

p "github.com/Oudwins/zog/pkgs/internals"
"github.com/Oudwins/zog/pkgs/internals/tutils"
"github.com/Oudwins/zog/zconst"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -375,3 +377,29 @@ func TestBoolValidateCustomTest(t *testing.T) {
}
assert.Equal(t, true, dest)
}

func TestBoolValidateInvalidType(t *testing.T) {
var nilString *string
cases := []struct {
Value any
}{
{Value: "string"},
{Value: 12},
{Value: time.Now()},
{Value: 12.5},
{Value: true},
{Value: []string{"hello"}},
{Value: nil},
{Value: nilString},
}

schema := Bool().Required()

for _, c := range cases {
ctx, errs := tutils.FakeContextFromValue(c.Value, "boolean")
schema.validate(ctx)
assert.NotNil(t, errs.List)
assert.Equal(t, errs.List[0].Code, zconst.IssueCodeInvalidType)

Comment thread
Oudwins marked this conversation as resolved.
}
}
6 changes: 4 additions & 2 deletions custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ func (c *Custom[T]) process(ctx *p.SchemaCtx) {
}
ptr, ok := ctx.ValPtr.(*T)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.AddIssue(ctx.IssueFromInvalidType("pointer matching custom schema type", ctx.ValPtr, "parsing a custom schema"))
return
}
*ptr = d

Expand Down Expand Up @@ -79,7 +80,8 @@ func (c *Custom[T]) validate(ctx *p.SchemaCtx) {
ctx.Processor = &c.test
ptr, ok := ctx.ValPtr.(*T)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.AddIssue(ctx.IssueFromInvalidType("pointer matching custom schema type", ctx.ValPtr, "validating a custom schema"))
return
}
c.test.Func(ptr, ctx)
}
Expand Down
10 changes: 8 additions & 2 deletions maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,16 @@ func (v *MapSchema[K, V]) process(ctx *p.SchemaCtx) {
}

// Create destination map
destVal := reflect.ValueOf(ctx.ValPtr).Elem()
destPtrVal := reflect.ValueOf(ctx.ValPtr)
if destPtrVal.Kind() != reflect.Pointer {
ctx.AddIssue(ctx.IssueFromInvalidType("pointer to map", ctx.ValPtr, "processing a map schema"))
return
}
destVal := destPtrVal.Elem()
destType := destVal.Type()
if destType.Kind() != reflect.Map {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.AddIssue(ctx.IssueFromInvalidType("map", ctx.ValPtr, "processing a map schema"))
return
}
destMap := reflect.MakeMap(destType)

Expand Down
7 changes: 0 additions & 7 deletions pkgs/internals/Issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,6 @@ type ZogIssueList = []*ZogIssue
// Users should migrate to using ZogIssueList and access paths via issue.Path
type ZogIssueMap = map[string]ZogIssueList

// INTERNAL ONLY: Interface used to add errors during parsing & validation. It represents a group of errors
type ZogIssues interface {
Add(err *ZogIssue)
IsEmpty() bool
Free()
}

// ErrsList - internal structure for collecting issues during schema execution
type ErrsList struct {
List ZogIssueList
Expand Down
12 changes: 10 additions & 2 deletions pkgs/internals/contexts.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type Ctx interface {
HasErrored() bool
}

func NewExecCtx(errs ZogIssues, fmter IssueFmtFunc) *ExecCtx {
func NewExecCtx(errs *ErrsList, fmter IssueFmtFunc) *ExecCtx {
c := ExecCtxPool.Get().(*ExecCtx)
c.Fmter = fmter
c.Errors = errs
Comment on lines +46 to 49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against nil errs in NewExecCtx.

Line 46 now accepts a nullable pointer, but Lines 63 and 86 dereference c.Errors unconditionally. A nil errs will panic on first HasErrored()/AddIssue() call.

Suggested fix
 func NewExecCtx(errs *ErrsList, fmter IssueFmtFunc) *ExecCtx {
+	if errs == nil {
+		errs = NewErrsList()
+	}
 	c := ExecCtxPool.Get().(*ExecCtx)
 	c.Fmter = fmter
 	c.Errors = errs
 	if c.m != nil {
 		clear(c.m)
 	}
 	return c
 }

Also applies to: 58-64, 82-87

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkgs/internals/contexts.go` around lines 46 - 49, NewExecCtx can accept a nil
errs and later dereferences c.Errors in methods like HasErrored and AddIssue;
fix by guarding inside NewExecCtx (and similarly in other constructors if
present) to ensure c.Errors is never nil: if errs == nil create a new ErrsList
instance and assign it to c.Errors before returning the pooled ExecCtx (refer to
NewExecCtx, ExecCtxPool, ExecCtx, ErrsList, and the methods HasErrored/AddIssue
to locate usage).

Expand All @@ -55,7 +55,7 @@ func NewExecCtx(errs ZogIssues, fmter IssueFmtFunc) *ExecCtx {

type ExecCtx struct {
Fmter IssueFmtFunc
Errors ZogIssues
Errors *ErrsList
m map[string]any
}

Expand Down Expand Up @@ -167,6 +167,14 @@ func (c *SchemaCtx) Issue() *ZogIssue {
return NewZogIssue().SetPath(c.Path.ToListClone()).SetDType(c.DType).SetValue(c.Data)
}

func (c *SchemaCtx) IssueFromInvalidType(expected string, received any, operation string) *ZogIssue {
return c.Issue().SetCode(zconst.IssueCodeInvalidType).SetError(zconst.ErrorInvalidTypeMessage(expected, received, c.Path.String(), c.DType, c.Data, operation))
}

func (c *SchemaCtx) IssueFromMissingStructField(fieldName string) *ZogIssue {
return c.Issue().SetCode(zconst.IssueCodeMissingField).SetError(zconst.ErrorMissingStructField(fieldName, c.Path.String(), c.DType, c.Data))
}

// Please don't depend on this method it may change
func (c *SchemaCtx) IssueFromTest(test TestInterface, val any) *ZogIssue {
e := ZogIssuePool.Get().(*ZogIssue)
Expand Down
5 changes: 1 addition & 4 deletions pkgs/internals/panics.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ package internals
import "fmt"

const (
PanicTypeCast = "Zog Panic: Type Cast Error\n Current context: %s\n Expected valPtr type to correspond with type defined in schema. But it does not. Expected type: *%T, got: %T\nFor more information see: https://zog.dev/panics#type-cast-errors"
PanicTypeCastCoercer = "Zog Panic: Type Cast Error\n Current context: %s\n Expected coercer return value to correspond with type defined in schema. But it does not. Expected type: *%T, got: %T\nFor more information see: https://zog.dev/panics#type-cast-errors"
PanicMissingStructField = "Zog Panic: Struct Schema Definition Error\n Current context: %s\n Provided struct is missing expected schema key: %s.\n This means you have made a mistake in your schema definition.\nFor more information see: https://zog.dev/panics#schema-definition-errors"
PanicInvalidArgumentsExpectedPointer = "Zog Panic: Expected destination value to be a pointer but it was not. This is generally caused by forgetting to pass a pointer to your Validate/Parse function. Do schema.Validate(&myStruct), not schema.Validate(myStruct) "
PanicTypeCastCoercer = "Zog Panic: Type Cast Error\n Current context: %s\n Expected coercer return value to correspond with type defined in schema. But it does not. Expected type: *%T, got: %T\nFor more information see: https://zog.dev/panics#type-cast-errors"
)

func Panicf(format string, args ...any) {
Expand Down
17 changes: 17 additions & 0 deletions pkgs/internals/tutils/fakeContext.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package tutils

import (
"github.com/Oudwins/zog/conf"
"github.com/Oudwins/zog/pkgs/internals"
"github.com/Oudwins/zog/zconst"
)

func FakeContextFromValue(val any, schemaType zconst.ZogType) (*internals.SchemaCtx, *internals.ErrsList) {
errs := internals.NewErrsList()
ctx := internals.NewExecCtx(errs, conf.IssueFormatter)

path := internals.NewPathBuilder()
sctx := ctx.NewSchemaCtx(val, val, path, schemaType)

return sctx, errs
}
3 changes: 2 additions & 1 deletion preprocess.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ func (s *PreprocessSchema[F, T]) process(ctx *p.SchemaCtx) {
func (s *PreprocessSchema[F, T]) validate(ctx *p.SchemaCtx) {
v, ok := ctx.ValPtr.(F)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), new(F), ctx.ValPtr)
ctx.AddIssue(ctx.IssueFromInvalidType("preprocess input type", ctx.ValPtr, "validating a preprocessed schema"))
return
}
out, err := s.fn(v, ctx)
if err != nil {
Expand Down
6 changes: 4 additions & 2 deletions struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ func (v *StructSchema) process(ctx *p.SchemaCtx) {
structRefVal := reflect.ValueOf(ctx.ValPtr)
kind := structRefVal.Kind()
if kind != reflect.Pointer && kind != reflect.Interface {
p.Panicf(p.PanicInvalidArgumentsExpectedPointer)
ctx.AddIssue(ctx.IssueFromInvalidType("pointer to struct", ctx.ValPtr, "processing a struct schema"))
return
}
structVal := structRefVal.Elem()
subCtx := ctx.NewSchemaCtx(ctx.Data, ctx.ValPtr, ctx.Path, v.getType())
Expand All @@ -105,7 +106,8 @@ func (v *StructSchema) process(ctx *p.SchemaCtx) {

fieldMeta, ok := structVal.Type().FieldByName(key)
if !ok {
p.Panicf(p.PanicMissingStructField, ctx.String(), key)
ctx.AddIssue(ctx.IssueFromMissingStructField(key))
continue
}
destPtr := structVal.FieldByName(key).Addr().Interface()

Expand Down
7 changes: 4 additions & 3 deletions struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,10 @@ func TestStructPanicsOnSchemaMismatch(t *testing.T) {
"bol": true,
"tim": "2024-08-06T00:00:00Z",
}
assert.Panics(t, func() {
objSchema.Parse(data, &o)
})

errs := objSchema.Parse(data, &o)
assert.NotNil(t, errs)
assert.Equal(t, zconst.IssueCodeMissingField, errs[0].Code)
Comment thread
Oudwins marked this conversation as resolved.
}

func TestStructPostTransforms(t *testing.T) {
Expand Down
15 changes: 0 additions & 15 deletions struct_validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,18 +218,3 @@ func TestValidateStructGetType(t *testing.T) {
})
assert.Equal(t, zconst.TypeStruct, s.getType())
}

func TestValidateStructInvalidSchema(t *testing.T) {
schema := Struct(Shape{
"field": String(),
})

type TestStruct struct {
Field int
}

var dest TestStruct
assert.Panics(t, func() {
schema.Validate(&dest)
})
}
16 changes: 16 additions & 0 deletions zconst/consts.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package zconst

import "fmt"

const (
// ISSUE_KEY_ROOT is the key for root-level issues on complex schemas on flattened maps
ISSUE_KEY_ROOT = "$root"
Expand Down Expand Up @@ -73,6 +75,12 @@ const (
ErrCodeCoerce ZogErrCode = "coerce" // all
IssueCodeCoerce ZogIssueCode = "coerce" // all

// Invalid type happens when you provide a boolean to a schema that expects something else.
IssueCodeInvalidType ZogIssueCode = "invalid_type"
Comment thread
greptile-apps[bot] marked this conversation as resolved.

// Missing field happens when you provide a schema shape with a field that is not present in the underlying data structure (generally a struct)
IssueCodeMissingField ZogIssueCode = "missing_field"

// Deprecated: Use IssueCodeFallback instead
// all. Applied when other errror code is not implemented. Required to be implemented for every zog type!
ErrCodeFallback ZogErrCode = "fallback"
Expand Down Expand Up @@ -219,3 +227,11 @@ const (
ZogProcessorTransform ZogProcessor = "transform"
ZogProcessorRequired ZogProcessor = "required"
)

func ErrorInvalidTypeMessage(expected string, received any, path string, dtype ZogType, value any, operation string) error {
return fmt.Errorf("[invalid type] zog expected a different type from what was provided while %s. This is an invariant. Unless you are using union schema it means you have made a mistake in your code.\nPath: %q\nSchema type: %s\nExpected: %s\nReceived: %T\nValue: %v", operation, path, dtype, expected, received, value)
}

func ErrorMissingStructField(fieldName string, path string, dtype ZogType, value any) error {
return fmt.Errorf("[missing structure field] zog expected struct to match schema but it did not. Provided struct is missing expected schema key. If you are not using union schema it means you have made a mistake in your schema definition.\nPath: %q\nSchema type: %s\nMissing field: %s\nValue: %v\nFor more information see: https://zog.dev/panics#schema-definition-errors", path, dtype, fieldName, value)
}
6 changes: 4 additions & 2 deletions zogSchema.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func primitiveParsing[T p.ZogPrimitive](ctx *p.SchemaCtx, processors []p.ZProces

destPtr, ok := ctx.ValPtr.(*T)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.Errors.Add(ctx.IssueFromInvalidType("pointer matching primitive schema type", ctx.ValPtr, "parsing a primitive schema"))
return
Comment thread
Oudwins marked this conversation as resolved.
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.

// 2. cast data to string & handle default/required
Expand Down Expand Up @@ -114,7 +115,8 @@ func primitiveValidation[T p.ZogPrimitive](ctx *p.SchemaCtx, processors []p.ZPro

valPtr, ok := ctx.ValPtr.(*T)
if !ok {
p.Panicf(p.PanicTypeCast, ctx.String(), ctx.DType, ctx.ValPtr)
ctx.Errors.Add(ctx.IssueFromInvalidType("pointer matching primitive schema type", ctx.ValPtr, "validating a primitive schema"))
return
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.

// 2. cast data to string & handle default/required
Expand Down
Loading