Merge pull request #22794 from wojtek-t/refactor_import_tracker

Auto commit by PR queue bot

cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_clientset.go

@@ -34,7 +34,7 @@ type genClientset struct {
 	groupVersions      []unversioned.GroupVersion
 	typedClientPath    string
 	outputPackage      string
-	imports            *generator.ImportTracker
+	imports            namer.ImportTracker
 	clientsetGenerated bool
 	// the import path of the generated real clientset.
 	clientsetPath string

cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_group.go

@@ -34,7 +34,7 @@ type genFakeForGroup struct {
 	group          string
 	// types in this group
 	types   []*types.Type
-	imports *generator.ImportTracker
+	imports namer.ImportTracker
 }
 
 var _ generator.Generator = &genFakeForGroup{}

cmd/libs/go2idl/client-gen/generators/fake/generator_fake_for_type.go

@@ -32,7 +32,7 @@ type genFakeForType struct {
 	outputPackage string
 	group         string
 	typeToMatch   *types.Type
-	imports       *generator.ImportTracker
+	imports       namer.ImportTracker
 }
 
 var _ generator.Generator = &genFakeForType{}

cmd/libs/go2idl/client-gen/generators/generator_for_clientset.go

@@ -34,7 +34,7 @@ type genClientset struct {
 	groupVersions      []unversioned.GroupVersion
 	typedClientPath    string
 	outputPackage      string
-	imports            *generator.ImportTracker
+	imports            namer.ImportTracker
 	clientsetGenerated bool
 }
 

cmd/libs/go2idl/client-gen/generators/generator_for_group.go

@@ -31,7 +31,7 @@ type genGroup struct {
 	group         string
 	// types in this group
 	types   []*types.Type
-	imports *generator.ImportTracker
+	imports namer.ImportTracker
 }
 
 var _ generator.Generator = &genGroup{}

cmd/libs/go2idl/client-gen/generators/generator_for_type.go

@@ -32,7 +32,7 @@ type genClientForType struct {
 	outputPackage string
 	group         string
 	typeToMatch   *types.Type
-	imports       *generator.ImportTracker
+	imports       namer.ImportTracker
 }
 
 var _ generator.Generator = &genClientForType{}

cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go

@@ -110,7 +110,7 @@ const (
 type genDeepCopy struct {
 	generator.DefaultGen
 	targetPackage string
-	imports       *generator.ImportTracker
+	imports       namer.ImportTracker
 	typesForInit  []*types.Type
 }
 

cmd/libs/go2idl/generator/import_tracker.go

@@ -20,48 +20,23 @@ import (
 	"path/filepath"
 	"strings"
 
+	"k8s.io/kubernetes/cmd/libs/go2idl/namer"
 	"k8s.io/kubernetes/cmd/libs/go2idl/types"
 )
 
-// ImportTracker may be passed to a namer.RawNamer, to track the imports needed
+func NewImportTracker(typesToAdd ...*types.Type) namer.ImportTracker {
-// for the types it names.
+	tracker := namer.NewDefaultImportTracker(types.Name{})
-//
+	tracker.IsInvalidType = func(*types.Type) bool { return false }
-// TODO: pay attention to the package name (instead of renaming every package).
+	tracker.LocalName = func(name types.Name) string { return golangTrackerLocalName(&tracker, name) }
-// TODO: Figure out the best way to make names for packages that collide.
+	tracker.PrintImport = func(path, name string) string { return name + " \"" + path + "\"" }
-type ImportTracker struct {
+
-	pathToName map[string]string
+	tracker.AddTypes(typesToAdd...)
-	// forbidden names are in here. (e.g. "go" is a directory in which
+	return &tracker
-	// there is code, but "go" is not a legal name for a package, so we put
+
-	// it here to prevent us from naming any package "go")
-	nameToPath map[string]string
 }
 
-func NewImportTracker(types ...*types.Type) *ImportTracker {
+func golangTrackerLocalName(tracker namer.ImportTracker, t types.Name) string {
-	tracker := &ImportTracker{
+	path := t.Package
-		pathToName: map[string]string{},
-		nameToPath: map[string]string{
-			"go": "",
-			// Add other forbidden keywords that also happen to be
-			// package names here.
-		},
-	}
-	tracker.AddTypes(types...)
-	return tracker
-}
-
-func (tracker *ImportTracker) AddTypes(types ...*types.Type) {
-	for _, t := range types {
-		tracker.AddType(t)
-	}
-}
-func (tracker *ImportTracker) AddType(t *types.Type) {
-	path := t.Name.Package
-	if path == "" {
-		return
-	}
-	if _, ok := tracker.pathToName[path]; ok {
-		return
-	}
 	dirs := strings.Split(path, string(filepath.Separator))
 	for n := len(dirs) - 1; n >= 0; n-- {
 		// TODO: bikeshed about whether it's more readable to have an
@@ -71,27 +46,11 @@ func (tracker *ImportTracker) AddType(t *types.Type) {
 		// packages, but aren't legal go names. So we'll sanitize.
 		name = strings.Replace(name, ".", "_", -1)
 		name = strings.Replace(name, "-", "_", -1)
-		if _, found := tracker.nameToPath[name]; found {
+		if _, found := tracker.PathOf(name); found {
 			// This name collides with some other package
 			continue
 		}
-		tracker.nameToPath[name] = path
+		return name
-		tracker.pathToName[path] = name
-		return
 	}
 	panic("can't find import for " + path)
 }
-
-func (tracker *ImportTracker) ImportLines() []string {
-	out := []string{}
-	for path, name := range tracker.pathToName {
-		out = append(out, name+" \""+path+"\"")
-	}
-	return out
-}
-
-// LocalNameOf returns the name you would use to refer to the package at the
-// specified path within the body of a file.
-func (tracker *ImportTracker) LocalNameOf(path string) string {
-	return tracker.pathToName[path]
-}

cmd/libs/go2idl/go-to-protobuf/protobuf/cmd.go

@@ -83,10 +83,6 @@ func (g *Generator) BindFlags(flag *flag.FlagSet) {
 	flag.StringVar(&g.DropEmbeddedFields, "drop-embedded-fields", g.DropEmbeddedFields, "Comma-delimited list of embedded Go types to omit from generated protobufs")
 }
 
-const (
-	typesKindProtobuf = "Protobuf"
-)
-
 func Run(g *Generator) {
 	if g.Common.VerifyOnly {
 		g.OnlyIDL = true

cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go

@@ -35,7 +35,7 @@ type genProtoIDL struct {
 	generator.DefaultGen
 	localPackage   types.Name
 	localGoPackage types.Name
-	imports        *ImportTracker
+	imports        namer.ImportTracker
 
 	generateAll    bool
 	omitGogo       bool
@@ -187,7 +187,7 @@ func (p protobufLocator) CastTypeName(name types.Name) string {
 func (p protobufLocator) ProtoTypeFor(t *types.Type) (*types.Type, error) {
 	switch {
 	// we've already converted the type, or it's a map
-	case t.Kind == typesKindProtobuf || t.Kind == types.Map:
+	case t.Kind == types.Protobuf || t.Kind == types.Map:
 		p.tracker.AddType(t)
 		return t, nil
 	}
@@ -200,7 +200,7 @@ func (p protobufLocator) ProtoTypeFor(t *types.Type) (*types.Type, error) {
 	if t.Kind == types.Struct {
 		t := &types.Type{
 			Name: p.namer.GoNameToProtoName(t.Name),
-			Kind: typesKindProtobuf,
+			Kind: types.Protobuf,
 
 			CommentLines: t.CommentLines,
 		}
@@ -354,29 +354,29 @@ func isFundamentalProtoType(t *types.Type) (*types.Type, bool) {
 	// switch {
 	// case t.Kind == types.Struct && t.Name == types.Name{Package: "time", Name: "Time"}:
 	// 	return &types.Type{
-	// 		Kind: typesKindProtobuf,
+	// 		Kind: types.Protobuf,
 	// 		Name: types.Name{Path: "google/protobuf/timestamp.proto", Package: "google.protobuf", Name: "Timestamp"},
 	// 	}, true
 	// }
 	switch t.Kind {
 	case types.Slice:
 		if t.Elem.Name.Name == "byte" && len(t.Elem.Name.Package) == 0 {
-			return &types.Type{Name: types.Name{Name: "bytes"}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: "bytes"}, Kind: types.Protobuf}, true
 		}
 	case types.Builtin:
 		switch t.Name.Name {
 		case "string", "uint32", "int32", "uint64", "int64", "bool":
-			return &types.Type{Name: types.Name{Name: t.Name.Name}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: t.Name.Name}, Kind: types.Protobuf}, true
 		case "int":
-			return &types.Type{Name: types.Name{Name: "int64"}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: "int64"}, Kind: types.Protobuf}, true
 		case "uint":
-			return &types.Type{Name: types.Name{Name: "uint64"}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: "uint64"}, Kind: types.Protobuf}, true
 		case "float64", "float":
-			return &types.Type{Name: types.Name{Name: "double"}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: "double"}, Kind: types.Protobuf}, true
 		case "float32":
-			return &types.Type{Name: types.Name{Name: "float"}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: "float"}, Kind: types.Protobuf}, true
 		case "uintptr":
-			return &types.Type{Name: types.Name{Name: "uint64"}, Kind: typesKindProtobuf}, true
+			return &types.Type{Name: types.Name{Name: "uint64"}, Kind: types.Protobuf}, true
 		}
 		// TODO: complex?
 	}
@@ -386,7 +386,7 @@ func isFundamentalProtoType(t *types.Type) (*types.Type, bool) {
 func memberTypeToProtobufField(locator ProtobufLocator, field *protoField, t *types.Type) error {
 	var err error
 	switch t.Kind {
-	case typesKindProtobuf:
+	case types.Protobuf:
 		field.Type, err = locator.ProtoTypeFor(t)
 	case types.Builtin:
 		field.Type, err = locator.ProtoTypeFor(t)
@@ -430,7 +430,7 @@ func memberTypeToProtobufField(locator ProtobufLocator, field *protoField, t *ty
 		field.Extras["(gogoproto.casttype)"] = strconv.Quote(locator.CastTypeName(t.Name))
 	case types.Slice:
 		if t.Elem.Name.Name == "byte" && len(t.Elem.Name.Package) == 0 {
-			field.Type = &types.Type{Name: types.Name{Name: "bytes"}, Kind: typesKindProtobuf}
+			field.Type = &types.Type{Name: types.Name{Name: "bytes"}, Kind: types.Protobuf}
 			return nil
 		}
 		if err := memberTypeToProtobufField(locator, field, t.Elem); err != nil {
@@ -477,7 +477,7 @@ func protobufTagToField(tag string, field *protoField, m types.Member, t *types.
 				// TODO: this probably needs to be a lookup into a namer
 				Path: strings.Replace(prefix, ".", "/", -1),
 			},
-			Kind: typesKindProtobuf,
+			Kind: types.Protobuf,
 		}
 	} else {
 		switch parts[0] {
@@ -489,7 +489,7 @@ func protobufTagToField(tag string, field *protoField, m types.Member, t *types.
 					Package: localPackage.Package,
 					Path:    localPackage.Path,
 				},
-				Kind: typesKindProtobuf,
+				Kind: types.Protobuf,
 			}
 		}
 	}

cmd/libs/go2idl/go-to-protobuf/protobuf/import_tracker.go

@@ -17,44 +17,30 @@ limitations under the License.
 package protobuf
 
 import (
-	"fmt"
+	"k8s.io/kubernetes/cmd/libs/go2idl/namer"
-	"sort"
-
 	"k8s.io/kubernetes/cmd/libs/go2idl/types"
 )
 
-// ImportTracker handles Protobuf package imports
-//
-// TODO: pay attention to the package name (instead of renaming every package).
-// TODO: Figure out the best way to make names for packages that collide.
-//
-// TODO: Try to merge this into generator.ImportTracker (or refactor common parts).
 type ImportTracker struct {
-	pathToName map[string]string
+	namer.DefaultImportTracker
-	// forbidden names are in here. (e.g. "go" is a directory in which
-	// there is code, but "go" is not a legal name for a package, so we put
-	// it here to prevent us from naming any package "go")
-	nameToPath map[string]string
-	local      types.Name
 }
 
-func NewImportTracker(local types.Name, types ...*types.Type) *ImportTracker {
+func NewImportTracker(local types.Name, typesToAdd ...*types.Type) *ImportTracker {
-	tracker := &ImportTracker{
+	tracker := namer.NewDefaultImportTracker(local)
-		local:      local,
+	tracker.IsInvalidType = func(t *types.Type) bool { return t.Kind != types.Protobuf }
-		pathToName: map[string]string{},
+	tracker.LocalName = func(name types.Name) string { return name.Package }
-		nameToPath: map[string]string{
+	tracker.PrintImport = func(path, name string) string { return path }
-		// Add other forbidden keywords that also happen to be
+
-		// package names here.
+	tracker.AddTypes(typesToAdd...)
-		},
+	return &ImportTracker{
+		DefaultImportTracker: tracker,
 	}
-	tracker.AddTypes(types...)
-	return tracker
 }
 
 // AddNullable ensures that support for the nullable Gogo-protobuf extension is added.
 func (tracker *ImportTracker) AddNullable() {
 	tracker.AddType(&types.Type{
-		Kind: typesKindProtobuf,
+		Kind: types.Protobuf,
 		Name: types.Name{
 			Name:    "nullable",
 			Package: "gogoproto",
@@ -62,58 +48,3 @@ func (tracker *ImportTracker) AddNullable() {
 		},
 	})
 }
-
-func (tracker *ImportTracker) AddTypes(types ...*types.Type) {
-	for _, t := range types {
-		tracker.AddType(t)
-	}
-}
-func (tracker *ImportTracker) AddType(t *types.Type) {
-	if tracker.local.Package == t.Name.Package {
-		return
-	}
-	// Golang type
-	if t.Kind != typesKindProtobuf {
-		// ignore built in package
-		if t.Kind == types.Builtin {
-			return
-		}
-		if _, ok := tracker.nameToPath[t.Name.Package]; !ok {
-			tracker.nameToPath[t.Name.Package] = ""
-		}
-		return
-	}
-	// ignore default proto package
-	if len(t.Name.Package) == 0 {
-		return
-	}
-	path := t.Name.Path
-	if len(path) == 0 {
-		panic(fmt.Sprintf("imported proto package %s must also have a path", t.Name.Package))
-	}
-	if _, ok := tracker.pathToName[path]; ok {
-		return
-	}
-	tracker.nameToPath[t.Name.Package] = path
-	tracker.pathToName[path] = t.Name.Package
-}
-
-func (tracker *ImportTracker) ImportLines() []string {
-	for k, v := range tracker.nameToPath {
-		if len(v) == 0 {
-			panic(fmt.Sprintf("tracking import Go package %s from %s, but no matching proto path set", k, tracker.local.Package))
-		}
-	}
-	out := []string{}
-	for path := range tracker.pathToName {
-		out = append(out, path)
-	}
-	sort.Sort(sort.StringSlice(out))
-	return out
-}
-
-// LocalNameOf returns the name you would use to refer to the package at the
-// specified path within the body of a file.
-func (tracker *ImportTracker) LocalNameOf(path string) string {
-	return tracker.pathToName[path]
-}

cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go

@@ -107,7 +107,7 @@ func assignGoTypeToProtoPackage(p *protobufPackage, t *types.Type, local, global
 	if otherP, ok := global[t.Name]; ok {
 		if _, ok := local[t.Name]; !ok {
 			p.Imports.AddType(&types.Type{
-				Kind: typesKindProtobuf,
+				Kind: types.Protobuf,
 				Name: otherP.ProtoTypeName(),
 			})
 		}

cmd/libs/go2idl/import-boss/generators/import_restrict.go

@@ -231,7 +231,7 @@ func (importRuleFile) VerifyFile(f *generator.File, path string) error {
 // importRules produces a file with a set for a single type.
 type importRules struct {
 	myPackage *types.Package
-	imports   *generator.ImportTracker
+	imports   namer.ImportTracker
 }
 
 var (

cmd/libs/go2idl/namer/import_tracker.go

@@ -0,0 +1,112 @@
+/*
+Copyright 2015 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package namer
+
+import (
+	"sort"
+
+	"k8s.io/kubernetes/cmd/libs/go2idl/types"
+)
+
+// ImportTracker may be passed to a namer.RawNamer, to track the imports needed
+// for the types it names.
+//
+// TODO: pay attention to the package name (instead of renaming every package).
+type DefaultImportTracker struct {
+	pathToName map[string]string
+	// forbidden names are in here. (e.g. "go" is a directory in which
+	// there is code, but "go" is not a legal name for a package, so we put
+	// it here to prevent us from naming any package "go")
+	nameToPath map[string]string
+	local      types.Name
+
+	// Returns true if a given types is an invalid type and should be ignored.
+	IsInvalidType func(*types.Type) bool
+	// Returns the final local name for the given name
+	LocalName func(types.Name) string
+	// Returns the "import" line for a given (path, name).
+	PrintImport func(string, string) string
+}
+
+func NewDefaultImportTracker(local types.Name) DefaultImportTracker {
+	return DefaultImportTracker{
+		pathToName: map[string]string{},
+		nameToPath: map[string]string{},
+		local:      local,
+	}
+}
+
+func (tracker *DefaultImportTracker) AddTypes(types ...*types.Type) {
+	for _, t := range types {
+		tracker.AddType(t)
+	}
+}
+func (tracker *DefaultImportTracker) AddType(t *types.Type) {
+	if tracker.local.Package == t.Name.Package {
+		return
+	}
+
+	if tracker.IsInvalidType(t) {
+		if t.Kind == types.Builtin {
+			return
+		}
+		if _, ok := tracker.nameToPath[t.Name.Package]; !ok {
+			tracker.nameToPath[t.Name.Package] = ""
+		}
+		return
+	}
+
+	if len(t.Name.Package) == 0 {
+		return
+	}
+	path := t.Name.Path
+	if len(path) == 0 {
+		path = t.Name.Package
+	}
+	if _, ok := tracker.pathToName[path]; ok {
+		return
+	}
+	name := tracker.LocalName(t.Name)
+	tracker.nameToPath[name] = path
+	tracker.pathToName[path] = name
+}
+
+func (tracker *DefaultImportTracker) ImportLines() []string {
+	importPaths := []string{}
+	for path := range tracker.pathToName {
+		importPaths = append(importPaths, path)
+	}
+	sort.Sort(sort.StringSlice(importPaths))
+	out := []string{}
+	for _, path := range importPaths {
+		out = append(out, tracker.PrintImport(path, tracker.pathToName[path]))
+	}
+	return out
+}
+
+// LocalNameOf returns the name you would use to refer to the package at the
+// specified path within the body of a file.
+func (tracker *DefaultImportTracker) LocalNameOf(path string) string {
+	return tracker.pathToName[path]
+}
+
+// PathOf returns the path that a given localName is referring to within the
+// body of a file.
+func (tracker *DefaultImportTracker) PathOf(localName string) (string, bool) {
+	name, ok := tracker.nameToPath[localName]
+	return name, ok
+}

cmd/libs/go2idl/namer/namer.go

@@ -278,6 +278,8 @@ func (ns *NameStrategy) Name(t *types.Type) string {
 type ImportTracker interface {
 	AddType(*types.Type)
 	LocalNameOf(packagePath string) string
+	PathOf(localName string) (string, bool)
+	ImportLines() []string
 }
 
 type rawNamer struct {

cmd/libs/go2idl/set-gen/generators/sets.go

@@ -120,7 +120,7 @@ type genSet struct {
 	generator.DefaultGen
 	outputPackage string
 	typeToMatch   *types.Type
-	imports       *generator.ImportTracker
+	imports       namer.ImportTracker
 }
 
 // Filter ignores all but one type because we're making a single file per type.

cmd/libs/go2idl/types/types.go

@@ -72,6 +72,9 @@ const (
 	DeclarationOf Kind = "DeclarationOf"
 	Unknown       Kind = ""
 	Unsupported   Kind = "Unsupported"
+
+	// Protobuf is protobuf type.
+	Protobuf Kind = "Protobuf"
 )
 
 // Package holds package-level information.