From 13ad52c8e22fd94e10cd89e0c2b61279b9f747da Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 29 Aug 2024 11:07:07 -0400 Subject: [PATCH 1/3] Disable the insertion and deletion of imports Co-authored-by: Tim Hockin --- v2/generator/execute.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/v2/generator/execute.go b/v2/generator/execute.go index 02b4c331..a1e052f5 100644 --- a/v2/generator/execute.go +++ b/v2/generator/execute.go @@ -26,6 +26,7 @@ import ( "strings" "golang.org/x/tools/imports" + "k8s.io/gengo/v2/namer" "k8s.io/gengo/v2/types" "k8s.io/klog/v2" @@ -114,7 +115,13 @@ func assembleGoFile(w io.Writer, f *File) { } func importsWrapper(src []byte) ([]byte, error) { - return imports.Process("", src, nil) + opt := imports.Options{ + Comments: true, + TabIndent: true, + TabWidth: 8, + FormatOnly: true, // Disable the insertion and deletion of imports + } + return imports.Process("", src, &opt) } func NewGoFile() *DefaultFileType { From df354888d218525e06b14f7c9b22f7ee7e3621d3 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 2 Sep 2024 18:11:19 -0400 Subject: [PATCH 2/3] Avoid imports of same name This primarily benefits applyconfiguration-gen today, which accidentaly got this behavior by including the local package name in imports, which caused goimports to deconflict with it before removing it since it was unused. This commit effictively preserves that behavior now that we will not be relying on goimports to add/rename/remove imports. Co-authored-by: Tim Hockin --- v2/generator/import_tracker.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/v2/generator/import_tracker.go b/v2/generator/import_tracker.go index 70b86cf5..22393e4d 100644 --- a/v2/generator/import_tracker.go +++ b/v2/generator/import_tracker.go @@ -18,6 +18,7 @@ package generator import ( "go/token" + "path/filepath" "strings" "k8s.io/klog/v2" @@ -45,7 +46,7 @@ import ( func NewImportTrackerForPackage(local string, typesToAdd ...*types.Type) *namer.DefaultImportTracker { tracker := namer.NewDefaultImportTracker(types.Name{Package: local}) tracker.IsInvalidType = func(*types.Type) bool { return false } - tracker.LocalName = func(name types.Name) string { return goTrackerLocalName(&tracker, name) } + tracker.LocalName = func(name types.Name) string { return goTrackerLocalName(&tracker, local, name) } tracker.PrintImport = func(path, name string) string { return name + " \"" + path + "\"" } tracker.AddTypes(typesToAdd...) @@ -56,7 +57,7 @@ func NewImportTracker(typesToAdd ...*types.Type) *namer.DefaultImportTracker { return NewImportTrackerForPackage("", typesToAdd...) } -func goTrackerLocalName(tracker namer.ImportTracker, t types.Name) string { +func goTrackerLocalName(tracker namer.ImportTracker, localPkg string, t types.Name) string { path := t.Package // Using backslashes in package names causes gengo to produce Go code which @@ -64,6 +65,7 @@ func goTrackerLocalName(tracker namer.ImportTracker, t types.Name) string { if strings.ContainsRune(path, '\\') { klog.Warningf("Warning: backslash used in import path '%v', this is unsupported.\n", path) } + localLeaf := filepath.Base(localPkg) dirs := strings.Split(path, namer.GoSeperator) for n := len(dirs) - 1; n >= 0; n-- { @@ -74,8 +76,13 @@ func goTrackerLocalName(tracker namer.ImportTracker, t types.Name) string { // packages, but aren't legal go names. So we'll sanitize. name = strings.ReplaceAll(name, ".", "") name = strings.ReplaceAll(name, "-", "") - if _, found := tracker.PathOf(name); found { - // This name collides with some other package + if _, found := tracker.PathOf(name); found || name == localLeaf { + // This name collides with some other package. + // Or, this name is tne same name as the local package, + // which we avoid because it can be confusing. For example, + // if the local package is v1, we to avoid importing + // another package using the v1 name, and instead import + // it with a more qualified name, such as metav1. continue } From 4ef5de9946b5072470109290023df8a239d27d7c Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 2 Sep 2024 18:49:34 -0400 Subject: [PATCH 3/3] Update local package test to match behavior change --- v2/generator/import_tracker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/v2/generator/import_tracker_test.go b/v2/generator/import_tracker_test.go index bdd2afe7..56087da8 100644 --- a/v2/generator/import_tracker_test.go +++ b/v2/generator/import_tracker_test.go @@ -72,7 +72,7 @@ func TestNewImportTracker(t *testing.T) { {Name: types.Name{Package: "bar.com/external/pkg"}}, }, expectedImports: []string{ - `pkg "bar.com/external/pkg"`, + `externalpkg "bar.com/external/pkg"`, }, }, }