From c9c97e2041e77eb011a634a65c521dae762de49b Mon Sep 17 00:00:00 2001 From: Danilo Pantani Date: Tue, 7 Nov 2023 14:15:15 +0100 Subject: [PATCH] fix(protoanalysis): fix wrong parser for proto package names (#3728) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix wrong parser for proto package names * add changelog * fix slice sort * rollback some changes * improve regex Co-authored-by: Jerónimo Albi --------- Co-authored-by: Pantani Co-authored-by: Jerónimo Albi --- changelog.md | 1 + go.mod | 2 +- go.sum | 4 +- ignite/pkg/cosmosanalysis/module/module.go | 3 +- ignite/pkg/protoanalysis/package.go | 181 ++++++++++++--------- ignite/pkg/protoanalysis/package_test.go | 82 ++++++++++ ignite/services/chain/generate.go | 10 +- 7 files changed, 195 insertions(+), 88 deletions(-) create mode 100644 ignite/pkg/protoanalysis/package_test.go diff --git a/changelog.md b/changelog.md index bde13d11d5..27ac053e75 100644 --- a/changelog.md +++ b/changelog.md @@ -38,6 +38,7 @@ - [#3661](https://github.com/ignite/cli/pull/3661) Change `pkg/cosmosanalysis` to find Cosmos SDK runtime app registered modules - [#3716](https://github.com/ignite/cli/pull/3716) Fix invalid plugin hook check - [#3725](https://github.com/ignite/cli/pull/3725) Fix flaky TS client generation issues on linux +- [#3728](https://github.com/ignite/cli/pull/3728) Fix wrong parser for proto package names - [#3729](https://github.com/ignite/cli/pull/3729) Fix broken generator due to caching /tmp include folders ## [`v0.27.0`](https://github.com/ignite/cli/releases/tag/v0.27.0) diff --git a/go.mod b/go.mod index 7481359fae..14bd50e4d4 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/cosmos/go-bip39 v1.0.0 github.com/cosmos/gogoproto v1.4.10 github.com/cosmos/ibc-go/v7 v7.1.0 - github.com/emicklei/proto v1.11.2 + github.com/emicklei/proto v1.12.1 github.com/emicklei/proto-contrib v0.14.0 github.com/go-delve/delve v1.20.2 github.com/go-git/go-git/v5 v5.6.1 diff --git a/go.sum b/go.sum index 624e6325ba..ba6420ea47 100644 --- a/go.sum +++ b/go.sum @@ -388,8 +388,8 @@ github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+m github.com/dvsekhvalnov/jose2go v1.5.0 h1:3j8ya4Z4kMCwT5nXIKFSV84YS+HdqSSO0VsTQxaLAeM= github.com/dvsekhvalnov/jose2go v1.5.0/go.mod h1:QsHjhyTlD/lAVqn/NSbVZmSCGeDehTB/mPZadG+mhXU= github.com/emicklei/proto v1.11.1/go.mod h1:rn1FgRS/FANiZdD2djyH7TMA9jdRDcYQ9IEN9yvjX0A= -github.com/emicklei/proto v1.11.2 h1:DiIeyTJ+gPSyJI+RIAqvuTeKb0tLUmaGXbYg6aFKsnE= -github.com/emicklei/proto v1.11.2/go.mod h1:rn1FgRS/FANiZdD2djyH7TMA9jdRDcYQ9IEN9yvjX0A= +github.com/emicklei/proto v1.12.1 h1:6n/Z2pZAnBwuhU66Gs8160B8rrrYKo7h2F2sCOnNceE= +github.com/emicklei/proto v1.12.1/go.mod h1:rn1FgRS/FANiZdD2djyH7TMA9jdRDcYQ9IEN9yvjX0A= github.com/emicklei/proto-contrib v0.14.0 h1:wFdptRZZ+JpkJtKTqOt2sXNPgwRGUkPzUL44cK9cMGE= github.com/emicklei/proto-contrib v0.14.0/go.mod h1:fpwMx68czS8x9ithaRGdLioTAadH01LLmND1Mr+FvP0= github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc= diff --git a/ignite/pkg/cosmosanalysis/module/module.go b/ignite/pkg/cosmosanalysis/module/module.go index 16d538e847..1f73179ecb 100644 --- a/ignite/pkg/cosmosanalysis/module/module.go +++ b/ignite/pkg/cosmosanalysis/module/module.go @@ -229,9 +229,8 @@ func (d *moduleDiscoverer) discover(pkg protoanalysis.Package) (Module, error) { return Module{}, nil } - namesplit := strings.Split(pkg.Name, ".") m := Module{ - Name: namesplit[len(namesplit)-1], + Name: pkg.ModuleName(), GoModulePath: d.basegopath, Pkg: pkg, } diff --git a/ignite/pkg/protoanalysis/package.go b/ignite/pkg/protoanalysis/package.go index 2d53d0e5a3..3e139c164d 100644 --- a/ignite/pkg/protoanalysis/package.go +++ b/ignite/pkg/protoanalysis/package.go @@ -1,58 +1,61 @@ package protoanalysis import ( + "errors" + "regexp" "strings" - "github.com/pkg/errors" + "golang.org/x/mod/semver" ) -type Packages []Package +type ( + // Packages represents slice of Package. + Packages []Package -func (p Packages) Files() Files { - var files []File - for _, pkg := range p { - files = append(files, pkg.Files...) - } - return files -} - -// Package represents a proto pkg. -type Package struct { - // Name of the proto pkg. - Name string + PkgName string - // Path of the package in the fs. - Path string + // Package represents a proto pkg. + Package struct { + // Name of the proto pkg. + Name string - // Files is a list of .proto files in the package. - Files Files + // Path of the package in the fs. + Path string - // GoImportName is the go package name of proto package. - GoImportName string + // Files is a list of .proto files in the package. + Files Files - // Messages is a list of proto messages defined in the package. - Messages []Message + // GoImportName is the go package name of proto package. + GoImportName string - // Services is a list of RPC services. - Services []Service -} + // Messages is a list of proto messages defined in the package. + Messages []Message -type Files []File + // Services is a list of RPC services. + Services []Service + } +) -type File struct { - // Path of the file. - Path string +var regexBetaVersion = regexp.MustCompile("^v[0-9]+(beta|alpha)[0-9]+") - // Dependencies is a list of imported .proto files in this package. - Dependencies []string +func (p Packages) Files() Files { + var files []File + for _, pkg := range p { + files = append(files, pkg.Files...) + } + return files } -func (f Files) Paths() []string { - var paths []string - for _, ff := range f { - paths = append(paths, ff.Path) +// ModuleName retrieves the single module name of the package. +func (p Package) ModuleName() (name string) { + names := strings.Split(p.Name, ".") + for i := len(names) - 1; i >= 0; i-- { + name = names[i] + if !semver.IsValid(name) && !regexBetaVersion.MatchString(name) { + break + } } - return paths + return } // MessageByName finds a message by its name inside Package. @@ -70,59 +73,81 @@ func (p Package) GoImportPath() string { return strings.Split(p.GoImportName, ";")[0] } -// Message represents a proto message. -type Message struct { - // Name of the message. - Name string +type ( + Files []File - // Path of the file where message is defined at. - Path string + File struct { + // Path of the file. + Path string - // HighestFieldNumber is the highest field number among fields of the message - // This allows to determine new field number when writing to proto message - HighestFieldNumber int + // Dependencies is a list of imported .proto files in this package. + Dependencies []string + } +) - // Fields contains message's field names and types - Fields map[string]string +func (f Files) Paths() []string { + var paths []string + for _, ff := range f { + paths = append(paths, ff.Path) + } + return paths } -// Service is an RPC service. -type Service struct { - // Name of the services. - Name string +type ( + // Message represents a proto message. + Message struct { + // Name of the message. + Name string - // RPC is a list of RPC funcs of the service. - RPCFuncs []RPCFunc -} + // Path of the file where message is defined at. + Path string -// RPCFunc is an RPC func. -type RPCFunc struct { - // Name of the RPC func. - Name string + // HighestFieldNumber is the highest field number among fields of the message + // This allows to determine new field number when writing to proto message + HighestFieldNumber int - // RequestType is the request type of RPC func. - RequestType string + // Fields contains message's field names and types + Fields map[string]string + } - // ReturnsType is the response type of RPC func. - ReturnsType string + // Service is an RPC service. + Service struct { + // Name of the services. + Name string - // HTTPRules keeps info about http rules of an RPC func. - // spec: - // https://github.com/googleapis/googleapis/blob/master/google/api/http.proto. - HTTPRules []HTTPRule + // RPC is a list of RPC funcs of the service. + RPCFuncs []RPCFunc + } - // Paginated indicates that the RPC function is using pagination. - Paginated bool -} + // RPCFunc is an RPC func. + RPCFunc struct { + // Name of the RPC func. + Name string -// HTTPRule keeps info about a configured http rule of an RPC func. -type HTTPRule struct { - // Params is a list of parameters defined in the http endpoint itself. - Params []string + // RequestType is the request type of RPC func. + RequestType string - // HasQuery indicates if there is a request query. - HasQuery bool + // ReturnsType is the response type of RPC func. + ReturnsType string - // HasBody indicates if there is a request payload. - HasBody bool -} + // HTTPRules keeps info about http rules of an RPC func. + // spec: + // https://github.com/googleapis/googleapis/blob/master/google/api/http.proto. + HTTPRules []HTTPRule + + // Paginated indicates that the RPC function is using pagination. + Paginated bool + } + + // HTTPRule keeps info about a configured http rule of an RPC func. + HTTPRule struct { + // Params is a list of parameters defined in the http endpoint itself. + Params []string + + // HasQuery indicates if there is a request query. + HasQuery bool + + // HasBody indicates if there is a request payload. + HasBody bool + } +) diff --git a/ignite/pkg/protoanalysis/package_test.go b/ignite/pkg/protoanalysis/package_test.go new file mode 100644 index 0000000000..e21c597f65 --- /dev/null +++ b/ignite/pkg/protoanalysis/package_test.go @@ -0,0 +1,82 @@ +package protoanalysis + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPackage_ModuleName(t *testing.T) { + tests := []struct { + name string + p Package + want string + }{ + { + name: "test single name", + p: Package{Name: "staking"}, + want: "staking", + }, + { + name: "test two names", + p: Package{Name: "cosmos.staking"}, + want: "staking", + }, + { + name: "test three name", + p: Package{Name: "cosmos.ignite.staking"}, + want: "staking", + }, + { + name: "test with the version 1", + p: Package{Name: "cosmos.staking.v1"}, + want: "staking", + }, + { + name: "test with the version 2", + p: Package{Name: "cosmos.staking.v2"}, + want: "staking", + }, + { + name: "test with the version 10", + p: Package{Name: "cosmos.staking.v10"}, + want: "staking", + }, + { + name: "test with the version 1 beta 1", + p: Package{Name: "cosmos.staking.v1beta1"}, + want: "staking", + }, + { + name: "test with the version 1 beta 2", + p: Package{Name: "cosmos.staking.v1beta2"}, + want: "staking", + }, + { + name: "test with the version 2 beta 1", + p: Package{Name: "cosmos.staking.v2beta1"}, + want: "staking", + }, + { + name: "test with the version 2 beta 2", + p: Package{Name: "cosmos.staking.v2beta2"}, + want: "staking", + }, + { + name: "test with the version 3 alpha 5", + p: Package{Name: "cosmos.staking.v3alpha5"}, + want: "staking", + }, + { + name: "test with the wrong version", + p: Package{Name: "cosmos.staking.v3bank5"}, + want: "v3bank5", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.p.ModuleName() + require.Equal(t, tt.want, got) + }) + } +} diff --git a/ignite/services/chain/generate.go b/ignite/services/chain/generate.go index 64f5570b6e..2cfbf26efb 100644 --- a/ignite/services/chain/generate.go +++ b/ignite/services/chain/generate.go @@ -179,7 +179,7 @@ func (c *Chain) Generate( openAPIPath = chainconfig.DefaultOpenAPIPath } - // Non absolute OpenAPI paths must be treated as relative to the app directory + // Non-absolute OpenAPI paths must be treated as relative to the app directory if !filepath.IsAbs(openAPIPath) { openAPIPath = filepath.Join(c.app.Path, openAPIPath) } @@ -200,7 +200,7 @@ func (c *Chain) Generate( } } - // Non absolute TS client output paths must be treated as relative to the app directory + // Non-absolute TS client output paths must be treated as relative to the app directory if !filepath.IsAbs(tsClientPath) { tsClientPath = filepath.Join(c.app.Path, tsClientPath) } @@ -225,7 +225,7 @@ func (c *Chain) Generate( updateConfig = true } - // Non absolute Vuex output paths must be treated as relative to the app directory + // Non-absolute Vuex output paths must be treated as relative to the app directory if !filepath.IsAbs(vuexPath) { vuexPath = filepath.Join(c.app.Path, vuexPath) } @@ -251,7 +251,7 @@ func (c *Chain) Generate( } } - // Non absolute Composables output paths must be treated as relative to the app directory + // Non-absolute Composables output paths must be treated as relative to the app directory if !filepath.IsAbs(composablesPath) { composablesPath = filepath.Join(c.app.Path, composablesPath) } @@ -275,7 +275,7 @@ func (c *Chain) Generate( } } - // Non absolute Hooks output paths must be treated as relative to the app directory + // Non-absolute Hooks output paths must be treated as relative to the app directory if !filepath.IsAbs(hooksPath) { hooksPath = filepath.Join(c.app.Path, hooksPath) }