Duplicated types error handling #91
Conversation
openapi/generator_test.go
Outdated
| @@ -3,6 +3,8 @@ package openapi | |||
| import ( | |||
| "encoding/json" | |||
| "fmt" | |||
| "github.com/wI2L/fizz/testdata/test_types" | |||
There was a problem hiding this comment.
Please use import aliases in camelCase for the two type packages.
testdata/test_types/types.go
Outdated
| @@ -0,0 +1,53 @@ | |||
| package test_types | |||
There was a problem hiding this comment.
I'd rather see these packages outside of testdata/, which is supposed to contain static data/fixtures for the tests, not type definitions for the test code.
This could live in the openapi/ folder, within subfolders.
testdata/test_types_extra/types.go
Outdated
| } | ||
|
|
||
| type D struct { | ||
| W W |
There was a problem hiding this comment.
Just a nitpick, but to convey the intent, I'd rename the fields Winternal, and Wexternal, or something like to indicate that.
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
==========================================
+ Coverage 94.37% 94.40% +0.02%
==========================================
Files 7 7
Lines 978 983 +5
==========================================
+ Hits 923 928 +5
Misses 39 39
Partials 16 16
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@lukap3 Need a rebase. |
|
@wI2L done! thanks |
| package extra_types | ||
|
|
||
| import ( | ||
| "github.com/wI2L/fizz/openapi/test_types/base_types" |
There was a problem hiding this comment.
camelCase import alias here aswell please.
| schema := g.newSchemaFromType(rt(new(extraTypes.D))) | ||
| assert.NotNil(t, schema) | ||
| assert.Len(t, g.Errors(), 1) | ||
| assert.Implements(t, (*error)(nil), g.Errors()[0]) |
There was a problem hiding this comment.
Could you also test that the error is a TypeError containing the message encountered duplicated type, where the error.T is effectively extratypes.D ?
Defining N>1 different types with the same name and generating a schema using them results in the last type "overriding" the rest without any error/warning.
There is a simple solution: using
UseFullSchemaNames(true)adds the package name as a prefix which should resolve the conflict (except in cases where the package name is also the same)However, the issue remains: there is no error for these conflicts and it can result in incorrect schemas being generated without the user's knowledge.
This PR adds an error to the generator for cases when multiple types defined with the same name (even when using full schema names)