From 3b1d19a432c69a83dfa17d69b1648097d131e716 Mon Sep 17 00:00:00 2001 From: Anton Kucherov Date: Wed, 10 Jun 2020 09:53:20 +0300 Subject: [PATCH 1/4] Add operationId uniqueness check. --- parser.go | 69 ++++++++++++++++++++++++++++++++++ parser_test.go | 9 +++++ testdata/duplicated/api/api.go | 31 +++++++++++++++ testdata/duplicated/main.go | 22 +++++++++++ 4 files changed, 131 insertions(+) create mode 100644 testdata/duplicated/api/api.go create mode 100644 testdata/duplicated/main.go diff --git a/parser.go b/parser.go index 4f972b78e..3d709e09a 100644 --- a/parser.go +++ b/parser.go @@ -75,6 +75,9 @@ type Parser struct { // excludes excludes dirs and files in SearchDir excludes map[string]bool + + // operationsIds contains all operationId annotations to check it's unique + operationsIds map[string]string } // New creates a new Parser with default properties. @@ -100,6 +103,7 @@ func New(options ...func(*Parser)) *Parser { CustomPrimitiveTypes: make(map[string]string), registerTypes: make(map[string]*ast.TypeSpec), excludes: make(map[string]bool), + operationsIds: make(map[string]string), } for _, option := range options { @@ -174,6 +178,10 @@ func (parser *Parser) ParseAPI(searchDir string, mainAPIFile string) error { } } + if err := parser.checkOperationIdUniqueness(); err != nil { + return err + } + return parser.parseDefinitions() } @@ -1500,6 +1508,67 @@ func (parser *Parser) parseFile(path string) error { return nil } +func (parser *Parser) checkOperationIdUniqueness() error { + for path, itm := range parser.swagger.Paths.Paths { + if itm.Get != nil { + currentPath := fmt.Sprintf("%s %s", "GET", path) + if err := parser.saveOperationId(itm.Get.ID, currentPath); err != nil { + return err + } + } + if itm.Put != nil { + currentPath := fmt.Sprintf("%s %s", "PUT", path) + if err := parser.saveOperationId(itm.Put.ID, currentPath); err != nil { + return err + } + } + if itm.Post != nil { + currentPath := fmt.Sprintf("%s %s", "POST", path) + if err := parser.saveOperationId(itm.Post.ID, currentPath); err != nil { + return err + } + } + if itm.Delete != nil { + currentPath := fmt.Sprintf("%s %s", "DELETE", path) + if err := parser.saveOperationId(itm.Delete.ID, currentPath); err != nil { + return err + } + } + if itm.Options != nil { + currentPath := fmt.Sprintf("%s %s", "OPTIONS", path) + if err := parser.saveOperationId(itm.Options.ID, currentPath); err != nil { + return err + } + } + if itm.Head != nil { + currentPath := fmt.Sprintf("%s %s", "HEAD", path) + if err := parser.saveOperationId(itm.Head.ID, currentPath); err != nil { + return err + } + } + if itm.Patch != nil { + currentPath := fmt.Sprintf("%s %s", "PATCH", path) + if err := parser.saveOperationId(itm.Patch.ID, currentPath); err != nil { + return err + } + } + } + return nil +} + +func (parser *Parser) saveOperationId(operationId, currentPath string) error { + if operationId == "" { + return nil + } + if previousPath, ok := parser.operationsIds[operationId]; ok { + return fmt.Errorf( + "duplicated @id annotation '%s' found in '%s', previously declared in: '%s'", + operationId, currentPath, previousPath) + } + parser.operationsIds[operationId] = currentPath + return nil +} + // Skip returns filepath.SkipDir error if match vendor and hidden folder func (parser *Parser) Skip(path string, f os.FileInfo) error { if f.IsDir() { diff --git a/parser_test.go b/parser_test.go index c172dab6d..f61dc5f18 100644 --- a/parser_test.go +++ b/parser_test.go @@ -2294,6 +2294,15 @@ func TestParseNested(t *testing.T) { assert.Equal(t, string(expected), string(b)) } +func TestParseDuplicated(t *testing.T) { + searchDir := "testdata/duplicated" + mainAPIFile := "main.go" + p := New() + p.ParseDependency = true + err := p.ParseAPI(searchDir, mainAPIFile) + assert.Errorf(t, err, "duplicated @id declarations successfully found") +} + func TestParser_ParseStructArrayObject(t *testing.T) { src := ` package api diff --git a/testdata/duplicated/api/api.go b/testdata/duplicated/api/api.go new file mode 100644 index 000000000..1a94506d4 --- /dev/null +++ b/testdata/duplicated/api/api.go @@ -0,0 +1,31 @@ +package api + +import ( + "github.com/gin-gonic/gin" +) + +type Foo struct { + Field1 string `validate:"required"` +} + +// @Description get Foo +// @ID get-foo +// @Accept json +// @Produce json +// @Success 200 {object} api.Foo +// @Router /testapi/get-foo [get] +func GetFoo(c *gin.Context) { + //write your code + var _ = Foo{} +} + +// @Description get Foo +// @ID get-foo +// @Accept json +// @Produce json +// @Success 200 {object} api.Foo +// @Router /testapi/get-bar [post] +func GetBar(c *gin.Context) { + //write your code + var _ = Foo{} +} diff --git a/testdata/duplicated/main.go b/testdata/duplicated/main.go new file mode 100644 index 000000000..582548cab --- /dev/null +++ b/testdata/duplicated/main.go @@ -0,0 +1,22 @@ +package composition + +import ( + "github.com/gin-gonic/gin" + + "github.com/swaggo/swag/testdata/duplicated/api" +) + +// @title Swagger Example API +// @version 1.0 +// @description This is a sample server +// @termsOfService http://swagger.io/terms/ + +// @host petstore.swagger.io +// @BasePath /v2 + +func main() { + r := gin.New() + r.GET("/testapi/get-foo", api.GetFoo) + r.GET("/testapi/get-bar", api.GetBar) + r.Run() +} From 6133cec5e18bee28df73794897b466fbc84061aa Mon Sep 17 00:00:00 2001 From: Anton Kucherov Date: Wed, 10 Jun 2020 10:07:15 +0300 Subject: [PATCH 2/4] Fix test --- testdata/duplicated/api/api.go | 26 ++++++-------------------- testdata/duplicated/main.go | 2 +- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/testdata/duplicated/api/api.go b/testdata/duplicated/api/api.go index 1a94506d4..a840fd806 100644 --- a/testdata/duplicated/api/api.go +++ b/testdata/duplicated/api/api.go @@ -4,28 +4,14 @@ import ( "github.com/gin-gonic/gin" ) -type Foo struct { - Field1 string `validate:"required"` -} - // @Description get Foo // @ID get-foo -// @Accept json -// @Produce json -// @Success 200 {object} api.Foo +// @Success 200 {string} string // @Router /testapi/get-foo [get] -func GetFoo(c *gin.Context) { - //write your code - var _ = Foo{} -} +func GetFoo(c *gin.Context) {} -// @Description get Foo +// @Description post Bar // @ID get-foo -// @Accept json -// @Produce json -// @Success 200 {object} api.Foo -// @Router /testapi/get-bar [post] -func GetBar(c *gin.Context) { - //write your code - var _ = Foo{} -} +// @Success 200 {string} string +// @Router /testapi/post-bar [post] +func PostBar(c *gin.Context) {} diff --git a/testdata/duplicated/main.go b/testdata/duplicated/main.go index 582548cab..75bada3ea 100644 --- a/testdata/duplicated/main.go +++ b/testdata/duplicated/main.go @@ -17,6 +17,6 @@ import ( func main() { r := gin.New() r.GET("/testapi/get-foo", api.GetFoo) - r.GET("/testapi/get-bar", api.GetBar) + r.POST("/testapi/post-bar", api.PostBar) r.Run() } From 014aac747f195e697bd171e0fa475903579d96d7 Mon Sep 17 00:00:00 2001 From: Anton Kucherov Date: Wed, 10 Jun 2020 10:12:29 +0300 Subject: [PATCH 3/4] Fix linter rules --- parser.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/parser.go b/parser.go index 3d709e09a..01629a2b1 100644 --- a/parser.go +++ b/parser.go @@ -178,7 +178,7 @@ func (parser *Parser) ParseAPI(searchDir string, mainAPIFile string) error { } } - if err := parser.checkOperationIdUniqueness(); err != nil { + if err := parser.checkOperationIDUniqueness(); err != nil { return err } @@ -1508,47 +1508,47 @@ func (parser *Parser) parseFile(path string) error { return nil } -func (parser *Parser) checkOperationIdUniqueness() error { +func (parser *Parser) checkOperationIDUniqueness() error { for path, itm := range parser.swagger.Paths.Paths { if itm.Get != nil { currentPath := fmt.Sprintf("%s %s", "GET", path) - if err := parser.saveOperationId(itm.Get.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Get.ID, currentPath); err != nil { return err } } if itm.Put != nil { currentPath := fmt.Sprintf("%s %s", "PUT", path) - if err := parser.saveOperationId(itm.Put.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Put.ID, currentPath); err != nil { return err } } if itm.Post != nil { currentPath := fmt.Sprintf("%s %s", "POST", path) - if err := parser.saveOperationId(itm.Post.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Post.ID, currentPath); err != nil { return err } } if itm.Delete != nil { currentPath := fmt.Sprintf("%s %s", "DELETE", path) - if err := parser.saveOperationId(itm.Delete.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Delete.ID, currentPath); err != nil { return err } } if itm.Options != nil { currentPath := fmt.Sprintf("%s %s", "OPTIONS", path) - if err := parser.saveOperationId(itm.Options.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Options.ID, currentPath); err != nil { return err } } if itm.Head != nil { currentPath := fmt.Sprintf("%s %s", "HEAD", path) - if err := parser.saveOperationId(itm.Head.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Head.ID, currentPath); err != nil { return err } } if itm.Patch != nil { currentPath := fmt.Sprintf("%s %s", "PATCH", path) - if err := parser.saveOperationId(itm.Patch.ID, currentPath); err != nil { + if err := parser.saveOperationID(itm.Patch.ID, currentPath); err != nil { return err } } @@ -1556,16 +1556,16 @@ func (parser *Parser) checkOperationIdUniqueness() error { return nil } -func (parser *Parser) saveOperationId(operationId, currentPath string) error { - if operationId == "" { +func (parser *Parser) saveOperationID(operationID, currentPath string) error { + if operationID == "" { return nil } - if previousPath, ok := parser.operationsIds[operationId]; ok { + if previousPath, ok := parser.operationsIds[operationID]; ok { return fmt.Errorf( "duplicated @id annotation '%s' found in '%s', previously declared in: '%s'", - operationId, currentPath, previousPath) + operationID, currentPath, previousPath) } - parser.operationsIds[operationId] = currentPath + parser.operationsIds[operationID] = currentPath return nil } From ca81920f3b38b5b035146a299cf5ba020d69a554 Mon Sep 17 00:00:00 2001 From: Anton Kucherov Date: Fri, 12 Jun 2020 11:32:26 +0300 Subject: [PATCH 4/4] Move operationsIds map and function to local scope --- parser.go | 46 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/parser.go b/parser.go index 01629a2b1..22c498b25 100644 --- a/parser.go +++ b/parser.go @@ -75,9 +75,6 @@ type Parser struct { // excludes excludes dirs and files in SearchDir excludes map[string]bool - - // operationsIds contains all operationId annotations to check it's unique - operationsIds map[string]string } // New creates a new Parser with default properties. @@ -103,7 +100,6 @@ func New(options ...func(*Parser)) *Parser { CustomPrimitiveTypes: make(map[string]string), registerTypes: make(map[string]*ast.TypeSpec), excludes: make(map[string]bool), - operationsIds: make(map[string]string), } for _, option := range options { @@ -1509,46 +1505,61 @@ func (parser *Parser) parseFile(path string) error { } func (parser *Parser) checkOperationIDUniqueness() error { + // operationsIds contains all operationId annotations to check it's unique + operationsIds := make(map[string]string) + saveOperationID := func(operationID, currentPath string) error { + if operationID == "" { + return nil + } + if previousPath, ok := operationsIds[operationID]; ok { + return fmt.Errorf( + "duplicated @id annotation '%s' found in '%s', previously declared in: '%s'", + operationID, currentPath, previousPath) + } + operationsIds[operationID] = currentPath + return nil + } + for path, itm := range parser.swagger.Paths.Paths { if itm.Get != nil { currentPath := fmt.Sprintf("%s %s", "GET", path) - if err := parser.saveOperationID(itm.Get.ID, currentPath); err != nil { + if err := saveOperationID(itm.Get.ID, currentPath); err != nil { return err } } if itm.Put != nil { currentPath := fmt.Sprintf("%s %s", "PUT", path) - if err := parser.saveOperationID(itm.Put.ID, currentPath); err != nil { + if err := saveOperationID(itm.Put.ID, currentPath); err != nil { return err } } if itm.Post != nil { currentPath := fmt.Sprintf("%s %s", "POST", path) - if err := parser.saveOperationID(itm.Post.ID, currentPath); err != nil { + if err := saveOperationID(itm.Post.ID, currentPath); err != nil { return err } } if itm.Delete != nil { currentPath := fmt.Sprintf("%s %s", "DELETE", path) - if err := parser.saveOperationID(itm.Delete.ID, currentPath); err != nil { + if err := saveOperationID(itm.Delete.ID, currentPath); err != nil { return err } } if itm.Options != nil { currentPath := fmt.Sprintf("%s %s", "OPTIONS", path) - if err := parser.saveOperationID(itm.Options.ID, currentPath); err != nil { + if err := saveOperationID(itm.Options.ID, currentPath); err != nil { return err } } if itm.Head != nil { currentPath := fmt.Sprintf("%s %s", "HEAD", path) - if err := parser.saveOperationID(itm.Head.ID, currentPath); err != nil { + if err := saveOperationID(itm.Head.ID, currentPath); err != nil { return err } } if itm.Patch != nil { currentPath := fmt.Sprintf("%s %s", "PATCH", path) - if err := parser.saveOperationID(itm.Patch.ID, currentPath); err != nil { + if err := saveOperationID(itm.Patch.ID, currentPath); err != nil { return err } } @@ -1556,19 +1567,6 @@ func (parser *Parser) checkOperationIDUniqueness() error { return nil } -func (parser *Parser) saveOperationID(operationID, currentPath string) error { - if operationID == "" { - return nil - } - if previousPath, ok := parser.operationsIds[operationID]; ok { - return fmt.Errorf( - "duplicated @id annotation '%s' found in '%s', previously declared in: '%s'", - operationID, currentPath, previousPath) - } - parser.operationsIds[operationID] = currentPath - return nil -} - // Skip returns filepath.SkipDir error if match vendor and hidden folder func (parser *Parser) Skip(path string, f os.FileInfo) error { if f.IsDir() {