From 680c19a058cbbb2ed0f9a4588ed7d50667684b57 Mon Sep 17 00:00:00 2001 From: Vyacheslav Pryimak Date: Fri, 13 Oct 2023 23:49:57 +0300 Subject: [PATCH] [ISSUE-104] Separated groups for "_" and "." imports (#138) * [ISSUE-104] Separated groups for "_" and "." imports --- README.md | 40 ++++++++++++- reviser/file.go | 113 ++++++++++++++--------------------- reviser/file_test.go | 80 ++++++++++++++++++++++++- reviser/import_order.go | 79 ++++++++++++++++++------ reviser/import_order_test.go | 4 +- reviser/model.go | 18 ++++++ 6 files changed, 246 insertions(+), 88 deletions(-) create mode 100644 reviser/model.go diff --git a/README.md b/README.md index 236c1bf..7819884 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,9 @@ Usage of goimports-reviser: std - std import group; general - libs for general purpose; company - inter-org or your company libs(if you set '-company-prefixes'-option, then 4th group will be split separately. In other case, it will be the part of general purpose libs); - project - your local project dependencies. + project - your local project dependencies; + blanked - imports with "_" alias; + dotted - imports with "." alias; Optional parameter. (default "std,general,company,project") -list-diff Option will list files whose formatting differs from goimports-reviser. Optional parameter. @@ -178,6 +180,42 @@ import ( ) ``` +### Example with `-imports-order std,general,company,project,blanked,dotted`-option + +Before usage: + +```go +package testdata // goimports-reviser/testdata + +import ( + _ "github.com/pkg1" + . "github.com/pkg2" + "fmt" //fmt package + "golang.org/x/exp/slices" //custom package + "github.com/incu6us/goimports-reviser/pkg" // this is a company package which is not a part of the project, but is a part of your organization + "goimports-reviser/pkg" +) +``` + +After usage: +```go +package testdata // goimports-reviser/testdata + +import ( + "fmt" // fmt package + + "golang.org/x/exp/slices" // custom package + + "github.com/incu6us/goimports-reviser/pkg" // this is a company package which is not a part of the project, but is a part of your organization + + "goimports-reviser/pkg" + + _ "github.com/pkg1" + + . "github.com/pkg2" +) +``` + ### Example with `-format`-option Before usage: diff --git a/reviser/file.go b/reviser/file.go index 7335ce2..9686895 100644 --- a/reviser/file.go +++ b/reviser/file.go @@ -34,6 +34,7 @@ type SourceFile struct { shouldUseAliasForVersionSuffix bool shouldFormatCode bool shouldSkipAutoGenerated bool + hasSeparateSideEffectGroup bool companyPackagePrefixes []string importsOrders ImportsOrders @@ -85,7 +86,7 @@ func (f *SourceFile) Fix(options ...SourceFileOption) ([]byte, bool, error) { return nil, false, err } - stdImports, generalImports, projectLocalPkgs, projectImports := groupImports( + groups := f.groupImports( f.projectName, f.companyPackagePrefixes, importsWithMetadata, @@ -96,7 +97,7 @@ func (f *SourceFile) Fix(options ...SourceFileOption) ([]byte, bool, error) { pf.Decls = decls } - f.fixImports(pf, stdImports, generalImports, projectLocalPkgs, projectImports, importsWithMetadata) + f.fixImports(pf, groups, importsWithMetadata) f.formatDecls(pf) @@ -157,21 +158,32 @@ func fixCommentGroup(commentGroup *ast.CommentGroup) *ast.CommentGroup { return formattedDoc } -func groupImports( +func (f *SourceFile) groupImports( projectName string, localPkgPrefixes []string, importsWithMetadata map[string]*commentsMetadata, -) ([]string, []string, []string, []string) { +) *groupsImports { var ( stdImports []string projectImports []string projectLocalPkgs []string generalImports []string + blankedImports []string + dottedImports []string ) for imprt := range importsWithMetadata { - pkgWithoutAlias := skipPackageAlias(imprt) + if f.importsOrders.hasBlankedImportOrder() && strings.HasPrefix(imprt, "_") { + blankedImports = append(blankedImports, imprt) + continue + } + + if f.importsOrders.hasDottedImportOrder() && strings.HasPrefix(imprt, ".") { + dottedImports = append(dottedImports, imprt) + continue + } + pkgWithoutAlias := skipPackageAlias(imprt) if _, ok := std.StdPackages[pkgWithoutAlias]; ok { stdImports = append(stdImports, imprt) continue @@ -202,8 +214,20 @@ func groupImports( sort.Strings(generalImports) sort.Strings(projectLocalPkgs) sort.Strings(projectImports) - - return stdImports, generalImports, projectLocalPkgs, projectImports + sort.Strings(blankedImports) + sort.Strings(dottedImports) + + result := &groupsImports{ + common: &common{ + std: stdImports, + general: generalImports, + company: projectLocalPkgs, + project: projectImports, + }, + blanked: blankedImports, + dotted: dottedImports, + } + return result } func skipPackageAlias(pkg string) string { @@ -237,7 +261,7 @@ func isSingleCgoImport(dd *ast.GenDecl) bool { func (f *SourceFile) fixImports( file *ast.File, - stdImports, generalImports, projectLocalPkgs, projectImports []string, + groups *groupsImports, commentsMetadata map[string]*commentsMetadata, ) { var importsPositions []*importPosition @@ -258,8 +282,8 @@ func (f *SourceFile) fixImports( }, ) - one, two, three, four := f.importsOrders.sortImportsByOrder(stdImports, generalImports, projectLocalPkgs, projectImports) - dd.Specs = rebuildImports(dd.Tok, commentsMetadata, one, two, three, four) + imports := f.importsOrders.sortImportsByOrder(groups) + dd.Specs = rebuildImports(dd.Tok, commentsMetadata, imports) } clearImportDocs(file, importsPositions) @@ -275,8 +299,10 @@ func (f *SourceFile) fixImports( // to // ----- // import ( -// "fmt" +// +// "fmt" // "io" +// // ) func hasMultipleImportDecls(f *ast.File) ([]ast.Decl, bool) { importSpecs := make([]ast.Spec, 0, len(f.Imports)) @@ -349,71 +375,24 @@ func removeEmptyImportNode(f *ast.File) { } } -func rebuildImports( - tok token.Token, - commentsMetadata map[string]*commentsMetadata, - firstImportGroup []string, - secondImportsGroup []string, - thirdImportsGroup []string, - fourthImportGroup []string, -) []ast.Spec { +func rebuildImports(tok token.Token, commentsMetadata map[string]*commentsMetadata, imports [][]string) []ast.Spec { var specs []ast.Spec - linesCounter := len(firstImportGroup) - for _, imprt := range firstImportGroup { - spec := &ast.ImportSpec{ - Path: &ast.BasicLit{Value: importWithComment(imprt, commentsMetadata), Kind: tok}, - } - specs = append(specs, spec) - - linesCounter-- - - if linesCounter == 0 && (len(secondImportsGroup) > 0 || len(thirdImportsGroup) > 0 || len(fourthImportGroup) > 0) { - spec = &ast.ImportSpec{Path: &ast.BasicLit{Value: "", Kind: token.STRING}} - - specs = append(specs, spec) - } - } - - linesCounter = len(secondImportsGroup) - for _, imprt := range secondImportsGroup { - spec := &ast.ImportSpec{ - Path: &ast.BasicLit{Value: importWithComment(imprt, commentsMetadata), Kind: tok}, - } - specs = append(specs, spec) - - linesCounter-- - - if linesCounter == 0 && (len(thirdImportsGroup) > 0 || len(fourthImportGroup) > 0) { - spec = &ast.ImportSpec{Path: &ast.BasicLit{Value: "", Kind: token.STRING}} - + amountOfGroups := len(imports) + for i, group := range imports { + for _, imprt := range group { + spec := &ast.ImportSpec{ + Path: &ast.BasicLit{Value: importWithComment(imprt, commentsMetadata), Kind: tok}, + } specs = append(specs, spec) } - } - - linesCounter = len(thirdImportsGroup) - for _, imprt := range thirdImportsGroup { - spec := &ast.ImportSpec{ - Path: &ast.BasicLit{Value: importWithComment(imprt, commentsMetadata), Kind: tok}, - } - specs = append(specs, spec) - - linesCounter-- - - if linesCounter == 0 && len(fourthImportGroup) > 0 { - spec = &ast.ImportSpec{Path: &ast.BasicLit{Value: "", Kind: token.STRING}} + if len(group) != 0 && i != amountOfGroups-1 { + spec := &ast.ImportSpec{Path: &ast.BasicLit{Value: "", Kind: token.STRING}} specs = append(specs, spec) } } - for _, imprt := range fourthImportGroup { - spec := &ast.ImportSpec{ - Path: &ast.BasicLit{Value: importWithComment(imprt, commentsMetadata), Kind: tok}, - } - specs = append(specs, spec) - } - return specs } diff --git a/reviser/file_test.go b/reviser/file_test.go index dc60709..a621f07 100644 --- a/reviser/file_test.go +++ b/reviser/file_test.go @@ -708,6 +708,51 @@ import ( "log" ) +// nolint:gomnd +`, + wantChange: true, + wantErr: false, + }, + { + name: "success project,company,general,std,blanked,dotted", + args: args{ + projectName: "github.com/incu6us/goimports-reviser", + filePath: "./testdata/example.go", + importsOrder: "project,company,general,std,blanked,dotted", + fileContent: `package testdata + +import ( + "log" + + "github.com/incu6us/goimports-reviser/testdata/innderpkg" + + "bytes" + + . "io" + + "golang.org/x/exp/slices" + + _ "fmt" +) + +// nolint:gomnd +`, + }, + want: `package testdata + +import ( + "github.com/incu6us/goimports-reviser/testdata/innderpkg" + + "golang.org/x/exp/slices" + + "bytes" + "log" + + _ "fmt" + + . "io" +) + // nolint:gomnd `, wantChange: true, @@ -960,6 +1005,39 @@ func main() { wantChange: true, wantErr: false, }, + { + name: "skip blanked and dotted import names", + args: args{ + projectName: "github.com/incu6us/goimports-reviser", + filePath: "./testdata/example.go", + fileContent: `// Some comments are here +package testdata + +import ( + _ "fmt" + . "io" +) + +// nolint:gomnd +func main() { +} +`, + }, + want: `// Some comments are here +package testdata + +import ( + _ "fmt" + . "io" +) + +// nolint:gomnd +func main() { +} +`, + wantChange: false, + wantErr: false, + }, { name: `success with "C"`, args: args{ @@ -1020,8 +1098,8 @@ func main() { } assert.NoError(t, err) - assert.Equal(t, tt.wantChange, hasChange) assert.Equal(t, tt.want, string(got)) + assert.Equal(t, tt.wantChange, hasChange) }) } } diff --git a/reviser/import_order.go b/reviser/import_order.go index f6d1f2c..ddf3ed4 100644 --- a/reviser/import_order.go +++ b/reviser/import_order.go @@ -17,6 +17,10 @@ const ( ProjectImportsOrder ImportsOrder = "project" // GeneralImportsOrder is packages that are outside. In other words it is general purpose libraries GeneralImportsOrder ImportsOrder = "general" + // BlankedImportsOrder is separate group for "_" imports + BlankedImportsOrder ImportsOrder = "blanked" + // DottedImportsOrder is separate group for "." imports + DottedImportsOrder ImportsOrder = "dotted" ) const ( @@ -26,14 +30,9 @@ const ( // ImportsOrders alias to []ImportsOrder type ImportsOrders []ImportsOrder -func (o ImportsOrders) sortImportsByOrder( - std []string, - general []string, - company []string, - project []string, -) ([]string, []string, []string, []string) { +func (o ImportsOrders) sortImportsByOrder(importGroups *groupsImports) [][]string { if len(o) == 0 { - return std, general, company, project + return importGroups.defaultSorting() } var result [][]string @@ -41,38 +40,80 @@ func (o ImportsOrders) sortImportsByOrder( var imports []string switch group { case StdImportsOrder: - imports = std + imports = importGroups.std case GeneralImportsOrder: - imports = general + imports = importGroups.general case CompanyImportsOrder: - imports = company + imports = importGroups.company case ProjectImportsOrder: - imports = project + imports = importGroups.project + case BlankedImportsOrder: + imports = importGroups.blanked + case DottedImportsOrder: + imports = importGroups.dotted } result = append(result, imports) } - return result[0], result[1], result[2], result[3] + return result +} + +func (o ImportsOrders) hasBlankedImportOrder() bool { + for _, order := range o { + if order == BlankedImportsOrder { + return true + } + } + return false +} + +func (o ImportsOrders) hasDottedImportOrder() bool { + for _, order := range o { + if order == DottedImportsOrder { + return true + } + } + return false +} + +func (o ImportsOrders) hasRequiredGroups() bool { + var ( + hasStd bool + hasCompany bool + hasGeneral bool + hasProject bool + ) + for _, order := range o { + switch order { + case StdImportsOrder: + hasStd = true + case CompanyImportsOrder: + hasCompany = true + case GeneralImportsOrder: + hasGeneral = true + case ProjectImportsOrder: + hasProject = true + } + } + return hasStd && hasCompany && hasGeneral && hasProject } // StringToImportsOrders will convert string, like "std,general,company,project" to ImportsOrder array type. // Default value for empty string is "std,general,company,project" func StringToImportsOrders(s string) (ImportsOrders, error) { - if len(strings.TrimSpace(s)) == 0 { + if strings.TrimSpace(s) == "" { s = defaultImportsOrder } groups := unique(strings.Split(s, ",")) - if len(groups) != 4 { - return nil, fmt.Errorf(`use this parameters to sort all groups of your imports: %q`, defaultImportsOrder) - } var groupOrder []ImportsOrder for _, g := range groups { group := ImportsOrder(strings.TrimSpace(g)) switch group { - case StdImportsOrder, CompanyImportsOrder, ProjectImportsOrder, GeneralImportsOrder: + case StdImportsOrder, CompanyImportsOrder, ProjectImportsOrder, + GeneralImportsOrder, BlankedImportsOrder, DottedImportsOrder: default: return nil, fmt.Errorf(`unknown order group type: %q`, group) } @@ -80,6 +121,10 @@ func StringToImportsOrders(s string) (ImportsOrders, error) { groupOrder = append(groupOrder, group) } + if !ImportsOrders(groupOrder).hasRequiredGroups() { + return nil, fmt.Errorf(`use default at least 4 parameters to sort groups of your imports: %q`, defaultImportsOrder) + } + return groupOrder, nil } diff --git a/reviser/import_order_test.go b/reviser/import_order_test.go index 94274ab..fe6b7a8 100644 --- a/reviser/import_order_test.go +++ b/reviser/import_order_test.go @@ -19,9 +19,9 @@ func TestStringToImportsOrder(t *testing.T) { wantErr string }{ { - name: "invalid groups count", + name: "invalid groupsImports count", args: args{importsOrder: "std,general"}, - wantErr: `use this parameters to sort all groups of your imports: "std,general,company,project"`, + wantErr: `use default at least 4 parameters to sort groups of your imports: "std,general,company,project"`, }, { name: "unknown group", diff --git a/reviser/model.go b/reviser/model.go new file mode 100644 index 0000000..36f39e4 --- /dev/null +++ b/reviser/model.go @@ -0,0 +1,18 @@ +package reviser + +type groupsImports struct { + *common + blanked []string + dotted []string +} + +type common struct { + std []string + general []string + company []string + project []string +} + +func (c *common) defaultSorting() [][]string { + return [][]string{c.std, c.general, c.company, c.project} +}