diff --git a/README.md b/README.md index cccc008..864458c 100644 --- a/README.md +++ b/README.md @@ -369,6 +369,31 @@ func(c *Converter, t reflect.Type, validate string, indent int) string We can use `c` to process nested types. Indent level is for passing to other converter APIs. +## Name Clashes + +Schema names are derived from the Go type name alone, so two types with the same name from different packages (e.g. `api.User` +and `db.User`, or `Page[api.User]` and `Page[db.User]`) would generate the same schema. `zen` panics when this happens, instead of +silently reusing one type's schema for the other. + +To resolve it, give the types of a package a prefix. It applies wherever those types appear, including as generic type arguments: + +```go +type Account struct { + Owner api.User + Record db.User + History Page[db.User] +} + +opt := zen.WithPackagePrefixes(map[string]string{ + "github.com/acme/app/db": "Db", +}) +c := zen.NewConverterWithOpts(opt) + +c.Convert(Account{}) +``` + +Outputs `UserSchema`, `DbUserSchema` and `PageDbUserSchema`. + ## Caveats - Does not support cyclic types - it's a limitation of zod, but self-referential types are supported. diff --git a/internal/testmodels/testmodels.go b/internal/testmodels/testmodels.go new file mode 100644 index 0000000..5c8b04d --- /dev/null +++ b/internal/testmodels/testmodels.go @@ -0,0 +1,51 @@ +// Package testmodels holds the zen tests' types that cannot be declared inside +// a test function: generic types, mutually recursive types, and the types they +// use. Being in another package, they also serve to test name clashes with the +// tests' local types. +package testmodels + +type GenericModel struct { + ID string +} + +type GenericPair[T any, U any] struct { + First T + Second U +} + +type StringIntPair GenericPair[string, int] + +type PairMap[K comparable, T any, U any] struct { + Items map[K]GenericPair[T, U] `json:"items"` +} + +type EmbeddedIntPair struct { + GenericPair[int, int] +} + +type EmbeddedIntTriplet struct { + GenericPair[int, GenericPair[int, int]] +} + +type EmbeddedIntModelPair struct { + GenericPair[int, GenericModel] +} + +// Wrapper mimics a generic optional type like 4d63.com/optional.Optional[T]. +type Wrapper[T any] struct{ Value T } + +type CyclicA struct { + B *CyclicB +} + +type CyclicB struct { + A *CyclicA +} + +type User struct { + Email string +} + +type Node struct { + Children []Node +} diff --git a/testdata/TestTypeNameClashes/package_prefixes_disambiguate/v3.golden b/testdata/TestTypeNameClashes/package_prefixes_disambiguate/v3.golden new file mode 100644 index 0000000..0601e1f --- /dev/null +++ b/testdata/TestTypeNameClashes/package_prefixes_disambiguate/v3.golden @@ -0,0 +1,35 @@ +// @zod-version: v3 +// @typecheck +export const GenericModelSchema = z.object({ + Name: z.string(), +}) +export type GenericModel = z.infer + +export const OtherGenericModelSchema = z.object({ + ID: z.string(), +}) +export type OtherGenericModel = z.infer + +export const OtherGenericPairIntOtherGenericModelSchema = z.object({ + First: z.number(), + Second: OtherGenericModelSchema, +}) +export type OtherGenericPairIntOtherGenericModel = z.infer + +export type OtherNode = { + Children: OtherNode[] | null, +} +const OtherNodeSchemaShape = { + Children: z.lazy(() => OtherNodeSchema).array().nullable(), +} +export const OtherNodeSchema: z.ZodType = z.object(OtherNodeSchemaShape) + +export const HolderSchema = z.object({ + Model: GenericModelSchema, + OtherModel: OtherGenericModelSchema, + Pair: OtherGenericPairIntOtherGenericModelSchema, + Tree: OtherNodeSchema, + At: z.coerce.date(), +}) +export type Holder = z.infer + diff --git a/testdata/TestTypeNameClashes/package_prefixes_disambiguate/v4.golden b/testdata/TestTypeNameClashes/package_prefixes_disambiguate/v4.golden new file mode 100644 index 0000000..181b593 --- /dev/null +++ b/testdata/TestTypeNameClashes/package_prefixes_disambiguate/v4.golden @@ -0,0 +1,35 @@ +// @zod-version: v4 +// @typecheck +export const GenericModelSchema = z.object({ + Name: z.string(), +}) +export type GenericModel = z.infer + +export const OtherGenericModelSchema = z.object({ + ID: z.string(), +}) +export type OtherGenericModel = z.infer + +export const OtherGenericPairIntOtherGenericModelSchema = z.object({ + First: z.number(), + Second: OtherGenericModelSchema, +}) +export type OtherGenericPairIntOtherGenericModel = z.infer + +export type OtherNode = { + Children: OtherNode[] | null, +} +const OtherNodeSchemaShape = { + get Children() { return OtherNodeSchema.array().nullable(); }, +} +export const OtherNodeSchema: z.ZodType = z.object(OtherNodeSchemaShape) + +export const HolderSchema = z.object({ + Model: GenericModelSchema, + OtherModel: OtherGenericModelSchema, + Pair: OtherGenericPairIntOtherGenericModelSchema, + Tree: OtherNodeSchema, + At: z.coerce.date(), +}) +export type Holder = z.infer + diff --git a/zod.go b/zod.go index 2c9de9f..031d275 100644 --- a/zod.go +++ b/zod.go @@ -24,6 +24,16 @@ func WithPrefix(prefix string) Opt { } } +// WithPackagePrefixes prefixes the generated schema and type names of all +// types from a package, including when they appear as generic type arguments. +// The map is keyed on the full package path. Use it to disambiguate types with +// the same name from different packages. +func WithPackagePrefixes(prefixes map[string]string) Opt { + return func(c *Converter) { + maps.Copy(c.packagePrefixes, prefixes) + } +} + // WithCustomTypes adds custom handler/converters for types. The map should be // keyed on the fully qualified type name (excluding generic type arguments), // ie. package.typename. @@ -61,11 +71,13 @@ func WithZodV3() Opt { // NewConverterWithOpts initializes and returns a new converter instance. func NewConverterWithOpts(opts ...Opt) *Converter { c := &Converter{ - prefix: "", - customTypes: make(map[string]CustomFn), - customTags: make(map[string]CustomFn), - ignoreTags: []string{}, - outputs: make(map[string]entry), + prefix: "", + packagePrefixes: make(map[string]string), + customTypes: make(map[string]CustomFn), + customTags: make(map[string]CustomFn), + ignoreTags: []string{}, + outputs: make(map[string]entry), + names: make(map[string]reflect.Type), } for _, opt := range opts { @@ -93,7 +105,7 @@ func (c *Converter) AddTypeWithName(input any, name string) { // multiple times, followed by Export to get the corresponding zod schemas. func (c *Converter) AddType(input any) { t := reflect.TypeOf(input) - name := typeName(t) + name := c.typeName(t) if name == "" { panic("input must be a named struct; use AddTypeWithName for anonymous structs") } @@ -108,6 +120,7 @@ func (c *Converter) addType(t reflect.Type, name string) { panic("name must not be empty") } + c.claimName(name, t) if _, ok := c.outputs[name]; ok { return } @@ -189,14 +202,40 @@ type stringValidator struct { } type Converter struct { - prefix string - customTypes map[string]CustomFn - customTags map[string]CustomFn - ignoreTags []string - zodV3 bool - structs int - outputs map[string]entry - stack []meta + prefix string + packagePrefixes map[string]string + customTypes map[string]CustomFn + customTags map[string]CustomFn + ignoreTags []string + zodV3 bool + structs int + outputs map[string]entry + stack []meta + // names maps each schema name to the type it was generated from, so that + // outputs and stack can be matched by name alone. + names map[string]reflect.Type +} + +// claimName panics if name is already taken by a different type, as its +// schema would otherwise silently stand in for the other type. +func (c *Converter) claimName(name string, t reflect.Type) { + existing, ok := c.names[name] + if !ok { + c.names[name] = t + return + } + if existing != t { + panic(fmt.Sprintf( + "schema name %q is used by both %s and %s; use WithPackagePrefixes to disambiguate", + name, qualifiedTypeName(existing), qualifiedTypeName(t))) + } +} + +func qualifiedTypeName(t reflect.Type) string { + if t.Name() == "" { + return t.String() + } + return t.PkgPath() + "." + t.Name() } func (c *Converter) addSchema(name string, data string, selfRef bool) { @@ -253,18 +292,21 @@ func fieldName(input reflect.StructField) string { return input.Name } -func typeName(t reflect.Type) string { +func (c *Converter) typeName(t reflect.Type) string { if t.Kind() == reflect.Struct { - return getTypeNameWithGenerics(t.Name()) + if t.Name() == "" { + return "" + } + return c.packagePrefixes[t.PkgPath()] + getTypeNameWithGenerics(t.Name(), c.packagePrefixes) } if t.Kind() == reflect.Pointer { - return typeName(t.Elem()) + return c.typeName(t.Elem()) } if t.Kind() == reflect.Slice { - return typeName(t.Elem()) + return c.typeName(t.Elem()) } if t.Kind() == reflect.Map { - return typeName(t.Elem()) + return c.typeName(t.Elem()) } return "UNKNOWN" @@ -526,17 +568,20 @@ func (c *Converter) convertType(t reflect.Type, validate string, indent int) con var validateStr strings.Builder var refines []string var selfRef bool - name := typeName(t) + name := c.typeName(t) + // Checked on the Go name, since a package prefix may apply to the time package. + isTime := t.Name() == "Time" parts := strings.Split(validate, ",") - switch name { - case "": + switch { + case name == "": // Handle fields with non-defined types - these are inline. validateStr.WriteString(c.convertStruct(t, indent)) - case "Time": + case isTime: // timestamps are to be coerced to date by zod. JSON.parse only serializes to string validateStr.WriteString("z.coerce.date()") default: + c.claimName(name, t) if c.stack[len(c.stack)-1].name == name { c.stack[len(c.stack)-1].selfRef = true if c.zodV3 { @@ -562,7 +607,7 @@ func (c *Converter) convertType(t reflect.Type, validate string, indent int) con switch valName { case "required": - if name == "Time" { + if isTime { // We compare with both the zero value from go and the zero value that zod coerces to refines = append( refines, @@ -618,7 +663,7 @@ func (c *Converter) getType(t reflect.Type, indent int) string { } if t.Kind() == reflect.Struct { - name := typeName(t) + name := c.typeName(t) switch t.Name() { case "": @@ -683,7 +728,7 @@ func (c *Converter) convertNamedField(f reflect.StructField, indent int, optiona func (c *Converter) convertEmbeddedFieldMerge(f reflect.StructField, indent int) (string, bool) { t := c.convertType(f.Type, f.Tag.Get("validate"), indent).text - name := typeName(f.Type) + name := c.typeName(f.Type) ent, ok := c.outputs[name] if ok && ent.selfRef { // Since we are spreading shape, we won't be able to support any validation tags on the embedded field @@ -695,7 +740,7 @@ func (c *Converter) convertEmbeddedFieldMerge(f reflect.StructField, indent int) func (c *Converter) convertEmbeddedFieldSpread(f reflect.StructField, indent int) string { t := c.convertType(f.Type, f.Tag.Get("validate"), indent).text - typeName := typeName(f.Type) + typeName := c.typeName(f.Type) ent, ok := c.outputs[typeName] if ok && ent.selfRef { // Since we are spreading shape, we won't be able to support any validation tags on the embedded field @@ -740,7 +785,7 @@ func (c *Converter) getTypeField(f reflect.StructField, indent int, optional, nu optionalCallUndef), false } - return typeName(f.Type), true + return c.typeName(f.Type), true } func (c *Converter) convertSliceAndArray(t reflect.Type, validate string, indent int) convertResult { @@ -1634,7 +1679,9 @@ func detectCycle(name string, stack []meta) { } } -func getTypeNameWithGenerics(name string) string { +// getTypeNameWithGenerics flattens generic type arguments into the type name, +// prefixing each argument from a package in packagePrefixes. +func getTypeNameWithGenerics(name string, packagePrefixes map[string]string) string { typeArgsIdx := strings.Index(name, "[") if typeArgsIdx == -1 { return name @@ -1675,6 +1722,7 @@ func getTypeNameWithGenerics(name string) string { partName := typeArgs[:partEnd] typeArgs = typeArgs[partEnd:] if packageEnd := strings.LastIndex(partName, "."); packageEnd != -1 { + sb.WriteString(packagePrefixes[partName[:packageEnd]]) partName = partName[packageEnd+1:] } if partName == "" { diff --git a/zod_test.go b/zod_test.go index 9122c8a..789d4f1 100644 --- a/zod_test.go +++ b/zod_test.go @@ -3,12 +3,15 @@ package zen import ( "fmt" "reflect" + "regexp" "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/xorcare/golden" + + "github.com/hypersequent/zen/internal/testmodels" ) // goldenAssert wraps golden.Assert, prepending metadata comments to the file. @@ -894,10 +897,6 @@ func TestDuration(t *testing.T) { assertSchema(t, User{}) } -// Wrapper mimics a generic optional type like 4d63.com/optional.Optional[T]. -// The custom handler resolves the inner type via ConvertType(t.Elem(), ...). -type Wrapper[T any] struct{ Value T } - func TestCustomTypes(t *testing.T) { t.Run("custom type mapped to string", func(t *testing.T) { type Decimal struct { @@ -928,14 +927,14 @@ func TestCustomTypes(t *testing.T) { Bio string } type User struct { - MaybeName Wrapper[string] - MaybeAge Wrapper[int] - MaybeHeight Wrapper[float64] - MaybeProfile Wrapper[Profile] + MaybeName testmodels.Wrapper[string] + MaybeAge testmodels.Wrapper[int] + MaybeHeight testmodels.Wrapper[float64] + MaybeProfile testmodels.Wrapper[Profile] } customTypes := map[string]CustomFn{ - "github.com/hypersequent/zen.Wrapper": func(c *Converter, t reflect.Type, validate string, i int) string { + "github.com/hypersequent/zen/internal/testmodels.Wrapper": func(c *Converter, t reflect.Type, validate string, i int) string { return fmt.Sprintf("%s.optional().nullish()", c.ConvertType(t.Field(0).Type, validate, i)) }, } @@ -951,11 +950,11 @@ func TestCustomTypes(t *testing.T) { t.Run("custom type with nullable control", func(t *testing.T) { type User struct { Name string - Email *Wrapper[string] + Email *testmodels.Wrapper[string] } customTypes := map[string]CustomFn{ - "github.com/hypersequent/zen.Wrapper": func(c *Converter, t reflect.Type, validate string, i int) string { + "github.com/hypersequent/zen/internal/testmodels.Wrapper": func(c *Converter, t reflect.Type, validate string, i int) string { return fmt.Sprintf("%s.optional()", c.ConvertType(t.Field(0).Type, validate, i)) }, } @@ -1193,47 +1192,12 @@ func TestRecursive2(t *testing.T) { assertSchema(t, Parent{}, "v3", "v4") } -type TestCyclicA struct { - B *TestCyclicB -} - -type TestCyclicB struct { - A *TestCyclicA -} - func TestCyclic(t *testing.T) { assert.Panics(t, func() { - StructToZodSchema(TestCyclicA{}) + StructToZodSchema(testmodels.CyclicA{}) }) } -type GenericModel struct { - ID string -} - -type GenericPair[T any, U any] struct { - First T - Second U -} - -type StringIntPair GenericPair[string, int] - -type PairMap[K comparable, T any, U any] struct { - Items map[K]GenericPair[T, U] `json:"items"` -} - -type EmbeddedIntPair struct { - GenericPair[int, int] -} - -type EmbeddedIntTriplet struct { - GenericPair[int, GenericPair[int, int]] -} - -type EmbeddedIntModelPair struct { - GenericPair[int, GenericModel] -} - func TestGenerics(t *testing.T) { for _, version := range []string{"v3", "v4"} { t.Run(version, func(t *testing.T) { @@ -1243,12 +1207,12 @@ func TestGenerics(t *testing.T) { } c := NewConverterWithOpts(opts...) - c.AddType(StringIntPair{}) - c.AddType(GenericPair[int, bool]{}) - c.AddType(PairMap[string, int, bool]{}) - c.AddType(EmbeddedIntPair{}) - c.AddType(EmbeddedIntTriplet{}) - c.AddType(EmbeddedIntModelPair{}) + c.AddType(testmodels.StringIntPair{}) + c.AddType(testmodels.GenericPair[int, bool]{}) + c.AddType(testmodels.PairMap[string, int, bool]{}) + c.AddType(testmodels.EmbeddedIntPair{}) + c.AddType(testmodels.EmbeddedIntTriplet{}) + c.AddType(testmodels.EmbeddedIntModelPair{}) goldenAssert(t, c.Export(), version) }) } @@ -1276,7 +1240,23 @@ func TestGetTypeNameWithGenerics(t *testing.T) { for input, expected := range tests { t.Run(input, func(t *testing.T) { - assert.Equal(t, expected, getTypeNameWithGenerics(input)) + assert.Equal(t, expected, getTypeNameWithGenerics(input, nil)) + }) + } + + prefixes := map[string]string{"github.com/hypersequent/zen": "Zen"} + prefixedTests := map[string]string{ + "GenericPair[int,bool]": "GenericPairIntBool", + "GenericPair[int,github.com/hypersequent/zen.GenericModel]": "GenericPairIntZenGenericModel", + "GenericPair[int,map[string][]*github.com/hypersequent/zen.GenericModel]": "GenericPairIntMapStringSlicePointerZenGenericModel", + "GenericPair[int,github.com/hypersequent/zen.GenericPair[string,github.com/hypersequent/zen.GenericModel]]": "GenericPairIntZenGenericPairStringZenGenericModel", + // Package paths must match exactly. + "GenericPair[int,github.com/hypersequent/zen/other.GenericModel]": "GenericPairIntGenericModel", + } + + for input, expected := range prefixedTests { + t.Run("prefixed "+input, func(t *testing.T) { + assert.Equal(t, expected, getTypeNameWithGenerics(input, prefixes)) }) } } @@ -1301,6 +1281,96 @@ func TestAddTypeRejectsEmptyNames(t *testing.T) { }) } +func TestTypeNameClashes(t *testing.T) { + const zenPkg = "github.com/hypersequent/zen" + const otherPkg = "github.com/hypersequent/zen/internal/testmodels" + + type GenericModel struct { + Name string + } + + t.Run("panics on same-named types from different packages", func(t *testing.T) { + c := NewConverterWithOpts() + c.AddType(GenericModel{}) + assert.PanicsWithValue( + t, + `schema name "GenericModel" is used by both `+zenPkg+`.GenericModel and `+otherPkg+`.GenericModel; use WithPackagePrefixes to disambiguate`, + func() { c.AddType(testmodels.GenericModel{}) }, + ) + }) + + t.Run("panics on same-named nested types", func(t *testing.T) { + type Holder struct { + Model GenericModel + OtherModel testmodels.GenericModel + } + assert.PanicsWithValue( + t, + `schema name "GenericModel" is used by both `+zenPkg+`.GenericModel and `+otherPkg+`.GenericModel; use WithPackagePrefixes to disambiguate`, + func() { StructToZodSchema(Holder{}) }, + ) + }) + + // Without the check, the nested type is mistaken for a self-reference. + t.Run("panics on nested type named like its parent", func(t *testing.T) { + type User struct { + Profile testmodels.User + } + assert.PanicsWithValue( + t, + `schema name "User" is used by both `+zenPkg+`.User and `+otherPkg+`.User; use WithPackagePrefixes to disambiguate`, + func() { StructToZodSchema(User{}) }, + ) + }) + + t.Run("panics on generic types whose flattened names clash", func(t *testing.T) { + c := NewConverterWithOpts() + c.AddType(testmodels.GenericPair[int, GenericModel]{}) + var msg any + func() { + defer func() { msg = recover() }() + c.AddType(testmodels.GenericPair[int, testmodels.GenericModel]{}) + }() + // Reflection suffixes local type arguments with an unstable "·N". + assert.Regexp( + t, + `^schema name "GenericPairIntGenericModel" is used by both `+ + regexp.QuoteMeta(otherPkg+`.GenericPair[int,`+zenPkg+`.GenericModel`)+`·\d+\] and `+ + regexp.QuoteMeta(otherPkg+`.GenericPair[int,`+otherPkg+`.GenericModel]`)+`; use WithPackagePrefixes to disambiguate$`, + msg, + ) + }) + + t.Run("allows a type under multiple names", func(t *testing.T) { + c := NewConverterWithOpts() + assert.NotPanics(t, func() { + c.AddType(GenericModel{}) + c.AddType(GenericModel{}) + c.AddTypeWithName(GenericModel{}, "Model") + }) + }) + + t.Run("package prefixes disambiguate", func(t *testing.T) { + type Holder struct { + Model GenericModel + OtherModel testmodels.GenericModel + Pair testmodels.GenericPair[int, testmodels.GenericModel] + Tree testmodels.Node + At time.Time // stays a date even when the time package is prefixed + } + + for _, version := range []string{"v3", "v4"} { + t.Run(version, func(t *testing.T) { + opts := []Opt{WithPackagePrefixes(map[string]string{otherPkg: "Other", "time": "Go"})} + if version == "v3" { + opts = append(opts, WithZodV3()) + } + goldenAssert(t, StructToZodSchema(Holder{}, opts...), version) + }) + } + }) +} + func TestSliceFields(t *testing.T) { type TestSliceFieldsStruct struct { NoValidate []int