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.

Sign up for GitHub

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

Jump to bottom

Minor cleanups & extended comments in protobuf generator #22982

Merged
merged 1 commit into from
Mar 16, 2016
Merged

Minor cleanups & extended comments in protobuf generator #22982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 32 additions & 26 deletions cmd/libs/go2idl/go-to-protobuf/protobuf/generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func (b bodyGen) doStruct(sw *generator.SnippetWriter) error {
}
}

// If we don't explicitly embed anything, generate fields by traversing fields.
if fields == nil {
memberFields, err := membersToFields(b.locator, b.t, b.localPackage, b.omitFieldTypes)
if err != nil {
Expand Down Expand Up @@ -399,8 +400,10 @@ func memberTypeToProtobufField(locator ProtobufLocator, field *protoField, t *ty
if err := memberTypeToProtobufField(locator, keyField, t.Key); err != nil {
return err
}
// All other protobuf types has kind types.Protobuf, so setting types.Map
// here would be very misleading.
field.Type = &types.Type{
Kind: types.Map,
Kind: types.Protobuf,
Key: keyField.Type,
Elem: valueField.Type,
}
Expand Down Expand Up @@ -450,7 +453,6 @@ func memberTypeToProtobufField(locator ProtobufLocator, field *protoField, t *ty
}

// protobufTagToField extracts information from an existing protobuf tag
// TODO: take a current package
func protobufTagToField(tag string, field *protoField, m types.Member, t *types.Type, localPackage types.Name) error {
if len(tag) == 0 {
return nil
Expand All @@ -466,32 +468,37 @@ func protobufTagToField(tag string, field *protoField, m types.Member, t *types.
return fmt.Errorf("member %q of %q malformed 'protobuf' tag, field ID is %q which is not an integer: %v\n", m.Name, t.Name, parts[1], err)
}
field.Tag = protoTag
// TODO: we are converting a Protobuf type back into an internal type, which is questionable
// TODO; Allow a way to unambiguously represent a type into two systems at the same time, like Go and Protobuf.
if last := strings.LastIndex(parts[0], "."); last != -1 {
prefix := parts[0][:last]
field.Type = &types.Type{
Name: types.Name{

// In general there is doesn't make sense to parse the protobuf tags to get the type,
// as all auto-generated once will have wire type "bytes", "varint" or "fixed64".
// However, sometimes we explicitly set them to have a custom serialization, e.g.:
// type Time struct {
// time.Time `protobuf:"Timestamp,1,req,name=time"`
// }
// to force the generator to use a given type (that we manually wrote serialization &
// deserialization methods for).
switch parts[0] {
case "varint", "fixed32", "fixed64", "bytes", "group":
default:
name := types.Name{}
if last := strings.LastIndex(parts[0], "."); last != -1 {
prefix := parts[0][:last]
name = types.Name{
Name: parts[0][last+1:],
Package: prefix,
// TODO: this probably needs to be a lookup into a namer
Path: strings.Replace(prefix, ".", "/", -1),
},
Kind: types.Protobuf,
}
} else {
switch parts[0] {
case "varint", "bytes", "fixed64":
default:
field.Type = &types.Type{
Name: types.Name{
Name: parts[0],
Package: localPackage.Package,
Path: localPackage.Path,
},
Kind: types.Protobuf,
Path: strings.Replace(prefix, ".", "/", -1),
}
} else {
name = types.Name{
Name: parts[0],
Package: localPackage.Package,
Path: localPackage.Path,
}
}
field.Type = &types.Type{
Name: name,
Kind: types.Protobuf,
}
}

protoExtra := make(map[string]string)
Expand All @@ -501,11 +508,10 @@ func protobufTagToField(tag string, field *protoField, m types.Member, t *types.
return fmt.Errorf("member %q of %q malformed 'protobuf' tag, tag %d should be key=value, got %q\n", m.Name, t.Name, i+4, extra)
}
switch parts[0] {
case "casttype":
case "casttype", "castkey", "castvalue":
parts[0] = fmt.Sprintf("(gogoproto.%s)", parts[0])
protoExtra[parts[0]] = parts[1]
}
// TODO: Should we parse castkey and castvalue too?
}

field.Extras = protoExtra
Expand Down
8 changes: 5 additions & 3 deletions cmd/libs/go2idl/go-to-protobuf/protobuf/namer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type localNamer struct {
}

func (n localNamer) Name(t *types.Type) string {
if t.Kind == types.Map {
if t.Key != nil && t.Elem != nil {
return fmt.Sprintf("map<%s, %s>", n.Name(t.Key), n.Name(t.Elem))
}
if len(n.localPackage.Package) != 0 && n.localPackage.Package == t.Name.Package {
Expand Down Expand Up @@ -67,8 +67,10 @@ func (n *protobufNamer) List() []generator.Package {
}

func (n *protobufNamer) Add(p *protobufPackage) {
n.packagesByPath[p.PackagePath] = p
n.packages = append(n.packages, p)
if _, ok := n.packagesByPath[p.PackagePath]; !ok {
n.packagesByPath[p.PackagePath] = p
n.packages = append(n.packages, p)
}
}

func (n *protobufNamer) GoNameToProtoName(name types.Name) types.Name {
Expand Down