Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor import tracker to unify protobuf & golang implementations #22794

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type genFakeForType struct {
outputPackage string
group string
typeToMatch *types.Type
imports *generator.ImportTracker
imports namer.ImportTracker
}

var _ generator.Generator = &genFakeForType{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type genClientset struct {
groupVersions []unversioned.GroupVersion
typedClientPath string
outputPackage string
imports *generator.ImportTracker
imports namer.ImportTracker
clientsetGenerated bool
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type genClientForType struct {
outputPackage string
group string
typeToMatch *types.Type
imports *generator.ImportTracker
imports namer.ImportTracker
}

var _ generator.Generator = &genClientForType{}
Expand Down
2 changes: 1 addition & 1 deletion cmd/libs/go2idl/deepcopy-gen/generators/deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ const (
type genDeepCopy struct {
generator.DefaultGen
targetPackage string
imports *generator.ImportTracker
imports namer.ImportTracker
typesForInit []*types.Type
}

Expand Down
67 changes: 13 additions & 54 deletions cmd/libs/go2idl/generator/import_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
// for the types it names.
//
// 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.
type ImportTracker 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
}
func NewImportTracker(typesToAdd ...*types.Type) namer.ImportTracker {
tracker := namer.NewDefaultImportTracker(types.Name{})
tracker.IsInvalidType = func(*types.Type) bool { return false }
tracker.LocalName = func(name types.Name) string { return golangTrackerLocalName(&tracker, name) }
tracker.PrintImport = func(path, name string) string { return name + " \"" + path + "\"" }

func NewImportTracker(types ...*types.Type) *ImportTracker {
tracker := &ImportTracker{
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
}
tracker.AddTypes(typesToAdd...)
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
}

func golangTrackerLocalName(tracker namer.ImportTracker, t types.Name) string {
path := t.Package
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
Expand All @@ -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
tracker.pathToName[path] = name
return
return name
}
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]
}
4 changes: 0 additions & 4 deletions cmd/libs/go2idl/go-to-protobuf/protobuf/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 15 additions & 15 deletions cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type genProtoIDL struct {
generator.DefaultGen
localPackage types.Name
localGoPackage types.Name
imports *ImportTracker
imports namer.ImportTracker

generateAll bool
omitGogo bool
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
}
Expand Down Expand Up @@ -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?
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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] {
Expand All @@ -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,
}
}
}
Expand Down
93 changes: 12 additions & 81 deletions cmd/libs/go2idl/go-to-protobuf/protobuf/import_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,103 +17,34 @@ limitations under the License.
package protobuf

import (
"fmt"
"sort"

"k8s.io/kubernetes/cmd/libs/go2idl/namer"
"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
// 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
namer.DefaultImportTracker
}

func NewImportTracker(local types.Name, types ...*types.Type) *ImportTracker {
tracker := &ImportTracker{
local: local,
pathToName: map[string]string{},
nameToPath: map[string]string{
// Add other forbidden keywords that also happen to be
// package names here.
},
func NewImportTracker(local types.Name, typesToAdd ...*types.Type) *ImportTracker {
tracker := namer.NewDefaultImportTracker(local)
tracker.IsInvalidType = func(t *types.Type) bool { return t.Kind != types.Protobuf }
tracker.LocalName = func(name types.Name) string { return name.Package }
tracker.PrintImport = func(path, name string) string { return path }

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",
Path: "github.com/gogo/protobuf/gogoproto/gogo.proto",
},
})
}

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]
}
2 changes: 1 addition & 1 deletion cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/libs/go2idl/import-boss/generators/import_restrict.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down