From 8564b7d357ac92726b1e657e695d4e09aea93716 Mon Sep 17 00:00:00 2001 From: Pantani Date: Wed, 6 Dec 2023 02:34:35 +0100 Subject: [PATCH] improve code readbility and add unit tests --- ignite/pkg/goanalysis/goanalysis.go | 56 +++++++++ ignite/pkg/goanalysis/goanalysis_test.go | 118 +++++++++++++++++-- ignite/pkg/goanalysis/testdata/fieldexist.go | 13 ++ ignite/services/scaffolder/configs.go | 76 +++--------- ignite/services/scaffolder/params.go | 76 +++--------- 5 files changed, 204 insertions(+), 135 deletions(-) create mode 100644 ignite/pkg/goanalysis/testdata/fieldexist.go diff --git a/ignite/pkg/goanalysis/goanalysis.go b/ignite/pkg/goanalysis/goanalysis.go index 96115824ce..7dc391585a 100644 --- a/ignite/pkg/goanalysis/goanalysis.go +++ b/ignite/pkg/goanalysis/goanalysis.go @@ -279,3 +279,59 @@ func createUnderscoreImport(imp string) *ast.ImportSpec { }, } } + +// HasStructFieldsInPkg finds the struct into a package folder and checks +// if the field exists inside the struct. +func HasStructFieldsInPkg(pkgPath, structName string, fields []string) (bool, error) { + absPath, err := filepath.Abs(pkgPath) + if err != nil { + return false, err + } + fileSet := token.NewFileSet() + all, err := parser.ParseDir(fileSet, absPath, func(os.FileInfo) bool { return true }, parser.ParseComments) + if err != nil { + return false, err + } + + fieldsNames := make(map[string]struct{}) + for _, field := range fields { + fieldsNames[strings.ToLower(field)] = struct{}{} + } + + exist := false + for _, pkg := range all { + for _, f := range pkg.Files { + ast.Inspect(f, func(x ast.Node) bool { + typeSpec, ok := x.(*ast.TypeSpec) + if !ok { + return true + } + + if _, ok := typeSpec.Type.(*ast.StructType); !ok || + typeSpec.Name.Name != structName || + typeSpec.Type == nil { + return true + } + + // Check if the struct has fields. + structType, ok := typeSpec.Type.(*ast.StructType) + if !ok { + return true + } + + // Iterate through the fields of the struct. + for _, field := range structType.Fields.List { + for _, fieldName := range field.Names { + if _, ok := fieldsNames[strings.ToLower(fieldName.Name)]; !ok { + continue + } + exist = true + return false + } + } + return true + }) + } + } + return exist, nil +} diff --git a/ignite/pkg/goanalysis/goanalysis_test.go b/ignite/pkg/goanalysis/goanalysis_test.go index c562080eb5..a49846df7c 100644 --- a/ignite/pkg/goanalysis/goanalysis_test.go +++ b/ignite/pkg/goanalysis/goanalysis_test.go @@ -144,84 +144,84 @@ func createMainFiles(tmpDir string, mainFiles []string) (pathsWithMain []string, func TestFuncVarExists(t *testing.T) { tests := []struct { name string - testfile string + testFile string goImport string methodSignature string want bool }{ { name: "test a declaration inside a method success", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "Background", goImport: "context", want: true, }, { name: "test global declaration success", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "Join", goImport: "path/filepath", want: true, }, { name: "test a declaration inside an if and inside a method success", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "SplitList", goImport: "path/filepath", want: true, }, { name: "test global variable success assign", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "New", goImport: "errors", want: true, }, { name: "test invalid import", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "Join", goImport: "errors", want: false, }, { name: "test invalid case sensitive assign", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "join", goImport: "context", want: false, }, { name: "test invalid struct assign", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "fooStruct", goImport: "context", want: false, }, { name: "test invalid method signature", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "fooMethod", goImport: "context", want: false, }, { name: "test not found name", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "Invalid", goImport: "context", want: false, }, { name: "test invalid assign with wrong", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "invalid.New", goImport: "context", want: false, }, { name: "test invalid assign with wrong", - testfile: "testdata/varexist", + testFile: "testdata/varexist", methodSignature: "SplitList", goImport: "path/filepath", want: true, @@ -229,7 +229,7 @@ func TestFuncVarExists(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - appPkg, _, err := xast.ParseFile(tt.testfile) + appPkg, _, err := xast.ParseFile(tt.testFile) require.NoError(t, err) got := goanalysis.FuncVarExists(appPkg, tt.goImport, tt.methodSignature) @@ -502,3 +502,95 @@ func TestUpdateInitImports(t *testing.T) { }) } } + +func TestHasStructFieldsInPkg(t *testing.T) { + tests := []struct { + name string + path string + structName string + fields []string + err error + want bool + }{ + { + name: "test a value with an empty struct", + path: "testdata", + structName: "emptyStruct", + fields: []string{"name"}, + want: false, + }, + { + name: "test no value with an empty struct", + path: "testdata", + structName: "emptyStruct", + fields: []string{""}, + want: false, + }, + { + name: "test a valid field into single field struct", + path: "testdata", + structName: "fooStruct", + fields: []string{"name"}, + want: true, + }, + { + name: "test a not valid field into single field struct", + path: "testdata", + structName: "fooStruct", + fields: []string{"baz"}, + want: false, + }, + { + name: "test a not valid field into struct", + path: "testdata", + structName: "bazStruct", + fields: []string{"baz"}, + want: false, + }, + { + name: "test a valid field into struct", + path: "testdata", + structName: "bazStruct", + fields: []string{"name"}, + want: true, + }, + { + name: "test two valid fields into struct", + path: "testdata", + structName: "bazStruct", + fields: []string{"name", "title"}, + want: true, + }, + { + name: "test a valid and a not valid fields into struct", + path: "testdata", + structName: "bazStruct", + fields: []string{"foo", "title"}, + want: true, + }, + { + name: "test three not valid fields into struct", + path: "testdata", + structName: "bazStruct", + fields: []string{"foo", "baz", "bla"}, + want: false, + }, + { + name: "invalid path", + path: "invalid_path", + err: os.ErrNotExist, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := goanalysis.HasStructFieldsInPkg(tt.path, tt.structName, tt.fields) + if tt.err != nil { + require.Error(t, err) + require.ErrorIs(t, err, tt.err) + return + } + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/ignite/pkg/goanalysis/testdata/fieldexist.go b/ignite/pkg/goanalysis/testdata/fieldexist.go new file mode 100644 index 0000000000..34d9e6c6bc --- /dev/null +++ b/ignite/pkg/goanalysis/testdata/fieldexist.go @@ -0,0 +1,13 @@ +package goanalysis + +type ( + emptyStruct struct{} + fooStruct struct { + name string + } + bazStruct struct { + name string + title string + description string + } +) diff --git a/ignite/services/scaffolder/configs.go b/ignite/services/scaffolder/configs.go index a95fc349a9..e533e4eff1 100644 --- a/ignite/services/scaffolder/configs.go +++ b/ignite/services/scaffolder/configs.go @@ -3,16 +3,13 @@ package scaffolder import ( "context" "fmt" - "go/ast" - "go/parser" - "go/token" - "os" "path/filepath" "strings" "github.com/gobuffalo/genny/v2" "github.com/ignite/cli/v28/ignite/pkg/cache" + "github.com/ignite/cli/v28/ignite/pkg/goanalysis" "github.com/ignite/cli/v28/ignite/pkg/multiformatname" "github.com/ignite/cli/v28/ignite/pkg/placeholder" "github.com/ignite/cli/v28/ignite/pkg/xgenny" @@ -48,13 +45,13 @@ func (s Scaffolder) CreateConfigs( return sm, fmt.Errorf("the module %v not exist", moduleName) } - // Parse config with the associated type - configsFields, err := field.ParseFields(configs, checkForbiddenTypeIndex) - if err != nil { + if err := checkConfigCreated(s.path, appName, moduleName, configs); err != nil { return sm, err } - if err := checkConfigCreated(s.path, appName, moduleName, configsFields); err != nil { + // Parse config with the associated type + configsFields, err := field.ParseFields(configs, checkForbiddenTypeIndex) + if err != nil { return sm, err } @@ -80,62 +77,19 @@ func (s Scaffolder) CreateConfigs( } // checkConfigCreated checks if the config has been already created. -func checkConfigCreated(appPath, appName, moduleName string, configs field.Fields) (err error) { - absPath, err := filepath.Abs(filepath.Join(appPath, "api", appName, moduleName, "module")) - if err != nil { - return err - } - fileSet := token.NewFileSet() - all, err := parser.ParseDir(fileSet, absPath, func(os.FileInfo) bool { return true }, parser.ParseComments) +func checkConfigCreated(appPath, appName, moduleName string, configs []string) (err error) { + path := filepath.Join(appPath, "api", appName, moduleName, "module") + ok, err := goanalysis.HasStructFieldsInPkg(path, "Module", configs) if err != nil { return err } - configsName := make(map[string]struct{}) - for _, config := range configs { - configsName[config.Name.LowerCase] = struct{}{} - } - - for _, pkg := range all { - for _, f := range pkg.Files { - ast.Inspect(f, func(x ast.Node) bool { - typeSpec, ok := x.(*ast.TypeSpec) - if !ok { - return true - } - - if _, ok := typeSpec.Type.(*ast.StructType); !ok || - typeSpec.Name.Name != "Module" || - typeSpec.Type == nil { - return true - } - - // Check if the struct has fields. - structType, ok := typeSpec.Type.(*ast.StructType) - if !ok { - return true - } - - // Iterate through the fields of the struct. - for _, configField := range structType.Fields.List { - for _, fieldName := range configField.Names { - if _, ok := configsName[strings.ToLower(fieldName.Name)]; !ok { - continue - } - err = fmt.Errorf( - "config field '%s' already exist for module %s", - fieldName.Name, - moduleName, - ) - return false - } - } - return true - }) - if err != nil { - return err - } - } + if ok { + return fmt.Errorf( + "duplicated configs (%s) module %s", + strings.Join(configs, " "), + moduleName, + ) } - return err + return nil } diff --git a/ignite/services/scaffolder/params.go b/ignite/services/scaffolder/params.go index 6fdb907dcc..19a503ede4 100644 --- a/ignite/services/scaffolder/params.go +++ b/ignite/services/scaffolder/params.go @@ -3,16 +3,13 @@ package scaffolder import ( "context" "fmt" - "go/ast" - "go/parser" - "go/token" - "os" "path/filepath" "strings" "github.com/gobuffalo/genny/v2" "github.com/ignite/cli/v28/ignite/pkg/cache" + "github.com/ignite/cli/v28/ignite/pkg/goanalysis" "github.com/ignite/cli/v28/ignite/pkg/multiformatname" "github.com/ignite/cli/v28/ignite/pkg/placeholder" "github.com/ignite/cli/v28/ignite/pkg/xgenny" @@ -47,13 +44,13 @@ func (s Scaffolder) CreateParams( return sm, fmt.Errorf("the module %v not exist", moduleName) } - // Parse params with the associated type - paramsFields, err := field.ParseFields(params, checkForbiddenTypeIndex) - if err != nil { + if err := checkParamCreated(s.path, moduleName, params); err != nil { return sm, err } - if err := checkParamCreated(s.path, moduleName, paramsFields); err != nil { + // Parse params with the associated type + paramsFields, err := field.ParseFields(params, checkForbiddenTypeIndex) + if err != nil { return sm, err } @@ -79,62 +76,19 @@ func (s Scaffolder) CreateParams( } // checkParamCreated checks if the parameter has been already created. -func checkParamCreated(appPath, moduleName string, params field.Fields) (err error) { - absPath, err := filepath.Abs(filepath.Join(appPath, "x", moduleName, "types")) - if err != nil { - return err - } - fileSet := token.NewFileSet() - all, err := parser.ParseDir(fileSet, absPath, func(os.FileInfo) bool { return true }, parser.ParseComments) +func checkParamCreated(appPath, moduleName string, params []string) error { + path := filepath.Join(appPath, "x", moduleName, "types") + ok, err := goanalysis.HasStructFieldsInPkg(path, "Params", params) if err != nil { return err } - paramsName := make(map[string]struct{}) - for _, param := range params { - paramsName[param.Name.LowerCase] = struct{}{} - } - - for _, pkg := range all { - for _, f := range pkg.Files { - ast.Inspect(f, func(x ast.Node) bool { - typeSpec, ok := x.(*ast.TypeSpec) - if !ok { - return true - } - - if _, ok := typeSpec.Type.(*ast.StructType); !ok || - typeSpec.Name.Name != "Params" || - typeSpec.Type == nil { - return true - } - - // Check if the struct has fields. - structType, ok := typeSpec.Type.(*ast.StructType) - if !ok { - return true - } - - // Iterate through the fields of the struct. - for _, paramField := range structType.Fields.List { - for _, fieldName := range paramField.Names { - if _, ok := paramsName[strings.ToLower(fieldName.Name)]; !ok { - continue - } - err = fmt.Errorf( - "param field '%s' already exist for module %s", - fieldName.Name, - moduleName, - ) - return false - } - } - return true - }) - if err != nil { - return err - } - } + if ok { + return fmt.Errorf( + "duplicated params (%s) module %s", + strings.Join(params, " "), + moduleName, + ) } - return err + return nil }