From da593607ac83982b83d44f71f3663e6646c73d38 Mon Sep 17 00:00:00 2001 From: "Nathan J. Mehl" Date: Wed, 3 Jul 2024 11:42:38 -0400 Subject: [PATCH] Add option to separate named imports This addresses https://github.com/incu6us/goimports-reviser/issues/116 Add a `-separate-named` boolean option. When activated, named/aliased imports are split off into a separate block below the relevant group, e.g. named std below std, named project below project, etc. --- README.md | 32 +++++++++ main.go | 14 +++- reviser/file.go | 68 +++++++++++++++--- reviser/file_option.go | 5 ++ reviser/file_test.go | 136 +++++++++++++++++++++++++++++++++++ reviser/import_order.go | 46 +++++++++--- reviser/import_order_test.go | 37 ++++++++++ reviser/model.go | 16 +++-- 8 files changed, 330 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 5e8e3fc..26313e3 100644 --- a/README.md +++ b/README.md @@ -84,6 +84,8 @@ Usage of goimports-reviser: Apply rules recursively if target is a directory. In case of ./... execution will be recursively applied by default. Optional parameter. -rm-unused Remove unused imports. Optional parameter. + -separate-named + Separate named imports from their group with a new line. Optional parameter. -set-alias Set alias for versioned package names, like 'github.com/go-pg/pg/v9'. In this case import will be set as 'pg "github.com/go-pg/pg/v9"'. Optional parameter. -set-exit-status @@ -238,6 +240,36 @@ func additionalTest(){ } ``` +### Example with `-separate-named`-option + +Before usage: + +```go +package testdata // goimports-reviser/testdata + +import ( + "fmt" + "github.com/incu6us/goimports-reviser/pkg" + extpkg "google.com/golang/pkg" + "golang.org/x/exp/slices" + extslice "github.com/PeterRK/slices" +) +``` + +After usage: +```go +package testdata // goimports-reviser/testdata + +import ( + "fmt" + + "github.com/incu6us/goimports-reviser/pkg" + "golang.org/x/exp/slices" + + extpkg "google.com/golang/pkg" + extslice "github.com/PeterRK/slices" +) +``` --- ## Contributors diff --git a/main.go b/main.go index be02383..2732651 100644 --- a/main.go +++ b/main.go @@ -36,7 +36,8 @@ const ( applyToGeneratedFiles = "apply-to-generated-files" excludesArg = "excludes" // using a regex here so that this will work with forked repos (at least on github.com) - modulePathRegex = `^github.com/[\w-]+/goimports-reviser(/v\d+)?@?` + modulePathRegex = `^github.com/[\w-]+/goimports-reviser(/v\d+)?@?` + separateNamedArg = "separate-named" // Deprecated options localArg = "local" @@ -56,6 +57,7 @@ var ( shouldSetAlias *bool shouldFormat *bool shouldApplyToGeneratedFiles *bool + shouldSeparateNamedImports *bool listFileName *bool setExitStatus *bool isRecursive *bool @@ -158,6 +160,12 @@ Optional parameter.`, "Option will perform additional formatting. Optional parameter.", ) + shouldSeparateNamedImports = flag.Bool( + separateNamedArg, + false, + "Option will separate named imports from the rest of the imports, per group. Optional parameter.", + ) + isRecursive = flag.Bool( recursiveArg, false, @@ -311,6 +319,10 @@ func main() { options = append(options, reviser.WithSkipGeneratedFile) } + if shouldSeparateNamedImports != nil && *shouldSeparateNamedImports { + options = append(options, reviser.WithSeparatedNamedImports) + } + if localPkgPrefixes != "" { if companyPkgPrefixes != "" { companyPkgPrefixes = localPkgPrefixes diff --git a/reviser/file.go b/reviser/file.go index d11ae69..549a64f 100644 --- a/reviser/file.go +++ b/reviser/file.go @@ -35,6 +35,7 @@ type SourceFile struct { shouldUseAliasForVersionSuffix bool shouldFormatCode bool shouldSkipAutoGenerated bool + shouldSeparateNamedImports bool hasSeparateSideEffectGroup bool companyPackagePrefixes []string importsOrders ImportsOrders @@ -165,12 +166,16 @@ func (f *SourceFile) groupImports( importsWithMetadata map[string]*commentsMetadata, ) *groupsImports { var ( - stdImports []string - projectImports []string - projectLocalPkgs []string - generalImports []string - blankedImports []string - dottedImports []string + stdImports []string + projectImports []string + projectLocalPkgs []string + generalImports []string + namedStdImports []string + namedProjectImports []string + namedProjectLocalPkgs []string + namedGeneralImports []string + blankedImports []string + dottedImports []string ) for imprt := range importsWithMetadata { @@ -185,7 +190,17 @@ func (f *SourceFile) groupImports( } pkgWithoutAlias := skipPackageAlias(imprt) + values := strings.Split(imprt, " ") + if _, ok := std.StdPackages[pkgWithoutAlias]; ok { + if f.shouldSeparateNamedImports { + if len(values) > 1 { + namedStdImports = append(namedStdImports, imprt) + } else { + stdImports = append(stdImports, imprt) + } + continue + } stdImports = append(stdImports, imprt) continue } @@ -193,6 +208,15 @@ func (f *SourceFile) groupImports( var isLocalPackageFound bool for _, localPackagePrefix := range localPkgPrefixes { if strings.HasPrefix(pkgWithoutAlias, localPackagePrefix) && !strings.HasPrefix(pkgWithoutAlias, projectName) { + if f.shouldSeparateNamedImports { + if len(values) > 1 { + namedProjectLocalPkgs = append(namedProjectLocalPkgs, imprt) + } else { + projectLocalPkgs = append(projectLocalPkgs, imprt) + } + isLocalPackageFound = true + break + } projectLocalPkgs = append(projectLocalPkgs, imprt) isLocalPackageFound = true break @@ -204,10 +228,26 @@ func (f *SourceFile) groupImports( } if strings.Contains(pkgWithoutAlias, projectName) { + if f.shouldSeparateNamedImports { + if len(values) > 1 { + namedProjectImports = append(namedProjectImports, imprt) + } else { + projectImports = append(projectImports, imprt) + } + continue + } projectImports = append(projectImports, imprt) continue } + if f.shouldSeparateNamedImports { + if len(values) > 1 { + namedGeneralImports = append(namedGeneralImports, imprt) + } else { + generalImports = append(generalImports, imprt) + } + continue + } generalImports = append(generalImports, imprt) } @@ -217,13 +257,21 @@ func (f *SourceFile) groupImports( sort.Strings(projectImports) sort.Strings(blankedImports) sort.Strings(dottedImports) + sort.Strings(namedStdImports) + sort.Strings(namedGeneralImports) + sort.Strings(namedProjectLocalPkgs) + sort.Strings(namedProjectImports) result := &groupsImports{ common: &common{ - std: stdImports, - general: generalImports, - company: projectLocalPkgs, - project: projectImports, + std: stdImports, + namedStd: namedStdImports, + general: generalImports, + namedGeneral: namedGeneralImports, + company: projectLocalPkgs, + namedCompany: namedProjectLocalPkgs, + project: projectImports, + namedProject: namedProjectImports, }, blanked: blankedImports, dotted: dottedImports, diff --git a/reviser/file_option.go b/reviser/file_option.go index b5eff6d..d5c9f2b 100644 --- a/reviser/file_option.go +++ b/reviser/file_option.go @@ -53,3 +53,8 @@ func WithSkipGeneratedFile(f *SourceFile) error { f.shouldSkipAutoGenerated = true return nil } + +func WithSeparatedNamedImports(f *SourceFile) error { + f.shouldSeparateNamedImports = true + return nil +} diff --git a/reviser/file_test.go b/reviser/file_test.go index aba10ee..d2b7641 100644 --- a/reviser/file_test.go +++ b/reviser/file_test.go @@ -1999,3 +1999,139 @@ import ( }) } } + +func TestSourceFile_Fix_WithSeparatedNamedImports(t *testing.T) { + type args struct { + projectName string + filePath string + fileContent string + } + tests := []struct { + name string + args args + want string + wantChange bool + wantErr bool + }{ + { + name: "simple", + args: args{ + projectName: "github.com/incu6us/goimports-reviser", + filePath: "./testdata/example.go", + fileContent: `package testdata +import ( + "fmt" + "github.com/incu6us/goimports-reviser/testdata/innderpkg" + "bytes" + "golang.org/x/exp/slices" +) +`, + }, + want: `package testdata + +import ( + "bytes" + "fmt" + + "golang.org/x/exp/slices" + + "github.com/incu6us/goimports-reviser/testdata/innderpkg" +) +`, + wantChange: true, + wantErr: false, + }, + { + name: "named", + args: args{ + projectName: "github.com/incu6us/goimports-reviser", + filePath: "./testdata/example.go", + fileContent: `package testdata +import ( + "fmt" + second "github.com/incu6us/goimports-reviser/testdata/secondpkg" + by "bytes" + js "encoding/json" + "golang.org/x/exp/slices" + er "golang.org/x/errors" + "github.com/incu6us/goimports-reviser/testdata/innderpkg" +) +`, + }, + want: `package testdata + +import ( + "fmt" + + by "bytes" + js "encoding/json" + + "golang.org/x/exp/slices" + + er "golang.org/x/errors" + + "github.com/incu6us/goimports-reviser/testdata/innderpkg" + + second "github.com/incu6us/goimports-reviser/testdata/secondpkg" +) +`, + wantChange: true, + wantErr: false, + }, + { + name: "named with comments", + args: args{ + projectName: "github.com/incu6us/goimports-reviser", + filePath: "./testdata/example.go", + fileContent: `package testdata +import ( + "fmt" //fmt package + second "github.com/incu6us/goimports-reviser/testdata/secondpkg" //secondpkg package + by "bytes" + js "encoding/json" + "golang.org/x/exp/slices" //slices package + er "golang.org/x/errors" + "github.com/incu6us/goimports-reviser/testdata/innderpkg" +) +`, + }, + want: `package testdata + +import ( + "fmt" //fmt package + + by "bytes" + js "encoding/json" + + "golang.org/x/exp/slices" //slices package + + er "golang.org/x/errors" + + "github.com/incu6us/goimports-reviser/testdata/innderpkg" + + second "github.com/incu6us/goimports-reviser/testdata/secondpkg" //secondpkg package +) +`, + wantChange: true, + wantErr: false, + }, + } + + for _, tt := range tests { + if tt.args.filePath != StandardInput && !strings.Contains(tt.args.filePath, "does-not-exist") { + require.NoError(t, os.WriteFile(tt.args.filePath, []byte(tt.args.fileContent), 0o644)) + } + + t.Run(tt.name, func(t *testing.T) { + got, _, hasChange, err := NewSourceFile(tt.args.projectName, tt.args.filePath).Fix(WithSeparatedNamedImports) + if tt.wantErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.wantChange, hasChange) + assert.Equal(t, tt.want, string(got)) + }) + } +} diff --git a/reviser/import_order.go b/reviser/import_order.go index ddf3ed4..2a1dd1c 100644 --- a/reviser/import_order.go +++ b/reviser/import_order.go @@ -10,13 +10,17 @@ type ImportsOrder string const ( // StdImportsOrder is std libs, e.g. fmt, errors, strings... - StdImportsOrder ImportsOrder = "std" + StdImportsOrder ImportsOrder = "std" + NamedStdImportsOrder ImportsOrder = "namedStd" // CompanyImportsOrder is packages that belong to the same organization - CompanyImportsOrder ImportsOrder = "company" + CompanyImportsOrder ImportsOrder = "company" + NamedCompanyImportsOrder ImportsOrder = "namedCompany" // ProjectImportsOrder is packages that are inside the current project - ProjectImportsOrder ImportsOrder = "project" + ProjectImportsOrder ImportsOrder = "project" + NamedProjectImportsOrder ImportsOrder = "namedProject" // GeneralImportsOrder is packages that are outside. In other words it is general purpose libraries - GeneralImportsOrder ImportsOrder = "general" + GeneralImportsOrder ImportsOrder = "general" + NamedGeneralImportsOrder ImportsOrder = "namedGeneral" // BlankedImportsOrder is separate group for "_" imports BlankedImportsOrder ImportsOrder = "blanked" // DottedImportsOrder is separate group for "." imports @@ -40,13 +44,13 @@ func (o ImportsOrders) sortImportsByOrder(importGroups *groupsImports) [][]strin var imports []string switch group { case StdImportsOrder: - imports = importGroups.std + imports = appendGroups(importGroups.std, importGroups.namedStd) case GeneralImportsOrder: - imports = importGroups.general + imports = appendGroups(importGroups.general, importGroups.namedGeneral) case CompanyImportsOrder: - imports = importGroups.company + imports = appendGroups(importGroups.company, importGroups.namedCompany) case ProjectImportsOrder: - imports = importGroups.project + imports = appendGroups(importGroups.project, importGroups.namedProject) case BlankedImportsOrder: imports = importGroups.blanked case DottedImportsOrder: @@ -138,3 +142,29 @@ func unique(s []string) []string { } return list } + +func appendGroups(input ...[]string) []string { + switch len(input) { + case 0: + return []string{} + case 1: + return input[0] + default: + break + } + separator := []string{"\n", "\n"} + + var output []string + + for idx, block := range input { + if idx == 0 { + output = append(output, block...) + continue + } + if len(block) > 0 { + output = append(output, append(separator, block...)...) + } + } + + return output +} diff --git a/reviser/import_order_test.go b/reviser/import_order_test.go index fe6b7a8..782ed34 100644 --- a/reviser/import_order_test.go +++ b/reviser/import_order_test.go @@ -42,3 +42,40 @@ func TestStringToImportsOrder(t *testing.T) { }) } } + +func Test_appendGroups(t *testing.T) { + type args struct { + input [][]string + } + tests := []struct { + name string + args args + want []string + }{ + { + name: "empty", + args: args{input: [][]string{}}, + want: []string{}, + }, + { + name: "single", + args: args{input: [][]string{{"a", "b", "c"}}}, + want: []string{"a", "b", "c"}, + }, + { + name: "multiple", + args: args{input: [][]string{{"a", "b", "c"}, {"d", "e", "f"}}}, + want: []string{"a", "b", "c", "\n", "\n", "d", "e", "f"}, + }, + { + name: "skip-empty", + args: args{input: [][]string{{"a", "b", "c"}, {}, {"d", "e", "f"}}}, + want: []string{"a", "b", "c", "\n", "\n", "d", "e", "f"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equalf(t, tt.want, appendGroups(tt.args.input...), "appendGroups(%v)", tt.args) + }) + } +} diff --git a/reviser/model.go b/reviser/model.go index 36f39e4..5fa3854 100644 --- a/reviser/model.go +++ b/reviser/model.go @@ -7,12 +7,18 @@ type groupsImports struct { } type common struct { - std []string - general []string - company []string - project []string + std []string + namedStd []string + general []string + namedGeneral []string + company []string + namedCompany []string + project []string + namedProject []string } func (c *common) defaultSorting() [][]string { - return [][]string{c.std, c.general, c.company, c.project} + return [][]string{ + c.std, c.namedStd, c.general, c.namedGeneral, c.company, c.namedCompany, c.project, c.namedProject, + } }