encoding/protobuf: always include type as second argument

This significanty simplifies finding the right mapping from CUE to protobuf. Also changed map type from map<A,B> to map[A]B. The former caused issues as the comma caused it to be a separate attribute argument (<> is not matched). Encosing it in quotes was also annoying. We considered using [A]:B, but using the map notation allows for more extendibility and less ambiguity in case we must support lists for some reason. This is a backwards incompatible change. Issue #5 Change-Id: Ief64d0c91d481fd7b03a4a8842faf377fd364a26
cuelang · Apr 11, 2021 · 2f89a0b · 2f89a0b
1 parent ea46072
commit 2f89a0b
Show file tree

Hide file tree

Showing 22 changed files with 472 additions and 448 deletions.
diff --git a/cmd/cue/cmd/get_go.go b/cmd/cue/cmd/get_go.go
@@ -1210,7 +1210,10 @@ func (e *extractor) addFields(x *types.Struct, st *cueast.StructLit) {
 		}
 
 		// Carry over protobuf field tags with modifications.
-		if t := reflect.StructTag(tag).Get("protobuf"); t != "" {
+		// TODO: consider trashing the protobuf tag, as the Go versions are
+		// lossy and will not allow for an accurate translation in some cases.
+		tags := reflect.StructTag(tag)
+		if t := tags.Get("protobuf"); t != "" {
 			split := strings.Split(t, ",")
 			k := 0
 			for _, s := range split {
@@ -1227,6 +1230,18 @@ func (e *extractor) addFields(x *types.Struct, st *cueast.StructLit) {
 			if len(split) >= 2 {
 				split[0], split[1] = split[1], split[0]
 			}
+
+			// Interpret as map?
+			if len(split) > 2 && split[1] == "bytes" {
+				tk := tags.Get("protobuf_key")
+				tv := tags.Get("protobuf_val")
+				if tk != "" && tv != "" {
+					tk = strings.SplitN(tk, ",", 2)[0]
+					tv = strings.SplitN(tv, ",", 2)[0]
+					split[1] = fmt.Sprintf("map[%s]%s", tk, tv)
+				}
+			}
+
 			e.addAttr(field, "protobuf", strings.Join(split, ","))
 		}
 

diff --git a/cmd/cue/cmd/testdata/script/def_proto.txt b/cmd/cue/cmd/testdata/script/def_proto.txt
@@ -11,32 +11,32 @@ import "time"
 	// A map of attribute name to its value.
 	attributes?: {
 		[string]: #AttributeValue
-	} @protobuf(1,type=map<string,AttributeValue>)
+	} @protobuf(1,map[string]AttributeValue)
 
 	// Specifies one attribute value with different type.
 	#AttributeValue: {} | {
-		stringValue: string @protobuf(2,name=string_value)
+		stringValue: string @protobuf(2,string,name=string_value)
 	} | {
-		int64Value: int64 @protobuf(3,name=int64_value)
+		int64Value: int64 @protobuf(3,int64,name=int64_value)
 	} | {
-		doubleValue: float64 @protobuf(4,type=double,name=double_value)
+		doubleValue: float64 @protobuf(4,double,name=double_value)
 	} | {
-		boolValue: bool @protobuf(5,name=bool_value)
+		boolValue: bool @protobuf(5,bool,name=bool_value)
 	} | {
-		bytesValue: bytes @protobuf(6,name=bytes_value)
+		bytesValue: bytes @protobuf(6,bytes,name=bytes_value)
 	} | {
-		timestampValue: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
+		timestampValue: time.Time @protobuf(7,google.protobuf.Timestamp,name=timestamp_value)
 	} | {
 		// Used for values of type STRING_MAP
-		stringMapValue: #StringMap @protobuf(9,name=string_map_value)
+		stringMapValue: #StringMap @protobuf(9,StringMap,name=string_map_value)
 	}
 
 	// Defines a string map.
 	#StringMap: {
 		// Holds a set of name/value pairs.
 		entries?: {
 			[string]: string
-		} @protobuf(1,type=map<string,string>)
+		} @protobuf(1,map[string]string)
 	}
 }
 -- policy.proto --

diff --git a/cmd/cue/cmd/testdata/script/get_go_types.txt b/cmd/cue/cmd/testdata/script/get_go_types.txt
@@ -70,7 +70,9 @@ type Foozer struct {
 
 	Alias1 *MyBarzer
 
-	Map    map[string]*CustomJSON
+	// Note: Go encodings of protobuf tags are lossy. So this is a best-effort
+	// thing.
+	Map    map[string]*CustomJSON `protobuf:"bytes,1,name=intf" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"`
 	Slice1 []int
 	Slice2 []interface{}
 	Slice3 *[]json.Unmarshaler
@@ -222,63 +224,63 @@ import "example.com/pkg2"
 package pkg1
 
 import (
- "time"
- p2 "example.com/pkg2:pkgtwo"
+	"time"
+	p2 "example.com/pkg2:pkgtwo"
 )
 
 // Foozer foozes a jaman.
 #Foozer: {
- Int:    int
- String: string
-
- #Inline
- NoInline:    #NoInline
- CustomJSON:  #CustomJSON
- CustomYAML?: null | #CustomYAML @go(,*CustomYAML)
- AnyJSON:     _                  @go(,json.Marshaler)
- AnyText:     string             @go(,encoding.TextMarshaler)
- bar?:        int & >10          @go(Bar)
-
- // Time is mapped to CUE's internal type.
- Time:    time.Time
- Barzer:  p2.#Barzer
- Alias1?: null | p2.#Barzer @go(,*p2.Barzer)
- Map: {[string]: null | #CustomJSON} @go(,map[string]*CustomJSON)
- Slice1: [...int] @go(,[]int)
- Slice2: [...] @go(,[]interface{})
- Slice3?: null | [...] @go(,*[]json.Unmarshaler)
- Array1:  5 * [int]    @go(,[5]int)
- Array2:  5 * [_]      @go(,[5]interface{})
- Array3?: null | 5*[_] @go(,*[5]json.Marshaler)
- Intf:    #Interface   @protobuf(2,varint,name=intf)
- Intf2:   _            @go(,interface{})
- Intf3: {
-  Interface: #Interface
- } @go(,struct{Interface})
- Intf4: _ @go(,"interface{Foo()}")
-
- // Even though this struct as a type implements MarshalJSON, it is known
- // that it is really only implemented by the embedded field.
- Embed: {
-  CustomJSON: #CustomJSON
- } @go(,struct{CustomJSON})
+	Int:    int
+	String: string
+
+	#Inline
+	NoInline:    #NoInline
+	CustomJSON:  #CustomJSON
+	CustomYAML?: null | #CustomYAML @go(,*CustomYAML)
+	AnyJSON:     _                  @go(,json.Marshaler)
+	AnyText:     string             @go(,encoding.TextMarshaler)
+	bar?:        int & >10          @go(Bar)
+
+	// Time is mapped to CUE's internal type.
+	Time:    time.Time
+	Barzer:  p2.#Barzer
+	Alias1?: null | p2.#Barzer @go(,*p2.Barzer)
+	Map: {[string]: null | #CustomJSON} @go(,map[string]*CustomJSON)
+	Slice1: [...int] @go(,[]int)
+	Slice2: [...] @go(,[]interface{})
+	Slice3?: null | [...] @go(,*[]json.Unmarshaler)
+	Array1:  5 * [int]    @go(,[5]int)
+	Array2:  5 * [_]      @go(,[5]interface{})
+	Array3?: null | 5*[_] @go(,*[5]json.Marshaler)
+	Intf:    #Interface   @protobuf(2,varint,name=intf)
+	Intf2:   _            @go(,interface{})
+	Intf3: {
+		Interface: #Interface
+	} @go(,struct{Interface})
+	Intf4: _ @go(,"interface{Foo()}")
+
+	// Even though this struct as a type implements MarshalJSON, it is known
+	// that it is really only implemented by the embedded field.
+	Embed: {
+		CustomJSON: #CustomJSON
+	} @go(,struct{CustomJSON})
 }
 
 #Identifier: string // #enumIdentifier
 
 #enumIdentifier:
- _#internalIdentifier
+	_#internalIdentifier
 
 _#internalIdentifier: #Identifier & "internal"
 
 // Level gives an indication of the extent of stuff.
 #Level: int // #enumLevel
 
 #enumLevel:
- #Unknown |
- #Low |
- #Medium |
- #High
+	#Unknown |
+	#Low |
+	#Medium |
+	#High
 
 // Block comment.
 //  Indented.
@@ -317,17 +319,17 @@ import t "time"
 
 // A Barzer barzes.
 #Barzer: {
- a:     int @go(A) @protobuf(2,varint,)
- T:     t.Time
- B?:    null | int    @go(,*big.Int)
- C:     int           @go(,big.Int)
- F:     string        @go(,big.Float) @xml(,attr)
- G?:    null | string @go(,*big.Float)
- S:     string
- "x-y": bool @go(XY)
- Err:   _    @go(,error)
-
- #Inline
+	a:     int @go(A) @protobuf(2,varint,)
+	T:     t.Time
+	B?:    null | int    @go(,*big.Int)
+	C:     int           @go(,big.Int)
+	F:     string        @go(,big.Float) @xml(,attr)
+	G?:    null | string @go(,*big.Float)
+	S:     string
+	"x-y": bool @go(XY)
+	Err:   _    @go(,error)
+
+	#Inline
 }
 
 #Perm: 0o755
@@ -336,6 +338,8 @@ import t "time"
 
 #Couple: int & 2
 
+#LongStringConst: "This is a really long string. Why are we using a long string? Because that way it ensures we are using go/constant.Value.ExactString() instead of go/constant.Value.String()"
+
 #Inline: A: int
 -- pkg3/pkg3_go_gen.cue --
 // Code generated by cue get go. DO NOT EDIT.
@@ -388,7 +392,10 @@ import (
 	Time:    time.Time
 	Barzer:  p2.#Barzer
 	Alias1?: null | p2.#Barzer @go(,*p2.Barzer)
-	Map: {[string]: null | #CustomJSON} @go(,map[string]*CustomJSON)
+
+	// Note: Go encodings of protobuf tags are lossy. So this is a best-effort
+	// thing.
+	Map: {[string]: null | #CustomJSON} @go(,map[string]*CustomJSON) @protobuf(1,map[bytes]bytes,name=intf)
 	Slice1: [...int] @go(,[]int)
 	Slice2: [...] @go(,[]interface{})
 	Slice3?: null | [...] @go(,*[]json.Unmarshaler)

diff --git a/cmd/cue/cmd/testdata/script/import_proto.txt b/cmd/cue/cmd/testdata/script/import_proto.txt
@@ -4,9 +4,9 @@ cd ..
 
 cmp stderr expect-stderr
 cmp stdout expect-stdout
-cmp expect-attributes_proto_gen.cue root/mixer/v1/attributes_proto_gen.cue
-cmp expect-client_config_proto_gen.cue root/mixer/v1/config/client/client_config_proto_gen.cue
-cmp expect-test_proto_gen.cue root/cue.mod/gen/googleapis.com/acme/test/test_proto_gen.cue
+cmp root/mixer/v1/attributes_proto_gen.cue expect-attributes_proto_gen.cue
+cmp root/mixer/v1/config/client/client_config_proto_gen.cue expect-client_config_proto_gen.cue
+cmp root/cue.mod/gen/googleapis.com/acme/test/test_proto_gen.cue expect-test_proto_gen.cue
 
 -- expect-stdout --
 -- expect-stderr --
@@ -174,30 +174,30 @@ import (
 	// A map of attribute name to its value.
 	attributes?: {
 		[string]: #AttributeValue
-	} @protobuf(1,type=map<string,AttributeValue>)
+	} @protobuf(1,map[string]AttributeValue)
 
 	// Specifies one attribute value with different type.
 	#AttributeValue: {
 		// The attribute value.
 		{} | {
-			stringValue: string @protobuf(2,name=string_value)
+			stringValue: string @protobuf(2,string,name=string_value)
 		} | {
-			int64Value: int64 @protobuf(3,name=int64_value)
+			int64Value: int64 @protobuf(3,int64,name=int64_value)
 		} | {
-			doubleValue: float64 @protobuf(4,type=double,name=double_value)
+			doubleValue: float64 @protobuf(4,double,name=double_value)
 		} | {
-			boolValue: bool @protobuf(5,name=bool_value)
+			boolValue: bool @protobuf(5,bool,name=bool_value)
 		} | {
-			bytesValue: bytes @protobuf(6,name=bytes_value)
+			bytesValue: bytes @protobuf(6,bytes,name=bytes_value)
 		} | {
-			timestampValue: time.Time @protobuf(7,type=google.protobuf.Timestamp,name=timestamp_value)
+			timestampValue: time.Time @protobuf(7,google.protobuf.Timestamp,name=timestamp_value)
 		} | {
 			// Used for values of type STRING_MAP
-			stringMapValue: #StringMap @protobuf(9,name=string_map_value)
+			stringMapValue: #StringMap @protobuf(9,StringMap,name=string_map_value)
 		} | {
-			testValue: test.#Test @protobuf(10,type=acme.test.Test,name=test_value)
+			testValue: test.#Test @protobuf(10,acme.test.Test,name=test_value)
 		} | {
-			testValue: test_test.#AnotherTest @protobuf(11,type=acme.test.test.AnotherTest,name=test_value)
+			testValue: test_test.#AnotherTest @protobuf(11,acme.test.test.AnotherTest,name=test_value)
 		}
 	}
 
@@ -206,7 +206,7 @@ import (
 		// Holds a set of name/value pairs.
 		entries?: {
 			[string]: string
-		} @protobuf(1,type=map<string,string>)
+		} @protobuf(1,map[string]string)
 	}
 }
 -- expect-client_config_proto_gen.cue --
@@ -217,13 +217,13 @@ import "acme.com/api/mixer/v1"
 
 // Defines the per-service client configuration.
 #ServiceConfig: {
-	disableCheckCalls?:  bool           @protobuf(1,name=disable_check_calls)
-	disableReportCalls?: bool           @protobuf(2,name=disable_report_calls)
-	mixerAttributes?:    v1.#Attributes @protobuf(3,type=Attributes,name=mixer_attributes)
+	disableCheckCalls?:  bool           @protobuf(1,bool,name=disable_check_calls)
+	disableReportCalls?: bool           @protobuf(2,bool,name=disable_report_calls)
+	mixerAttributes?:    v1.#Attributes @protobuf(3,Attributes,name=mixer_attributes)
 }
 -- expect-test_proto_gen.cue --
 package test
 
 #Test: {
-	test?: int32 @protobuf(1)
+	test?: int32 @protobuf(1,int32)
 }
diff --git a/cue/types.go b/cue/types.go
@@ -2266,7 +2266,15 @@ func (a *Attribute) Flag(pos int, key string) (bool, error) {
 // and reports the value if found. It reports an error if the attribute is
 // invalid or if the first pos-1 entries are not defined.
 func (a *Attribute) Lookup(pos int, key string) (val string, found bool, err error) {
-	return a.attr.Lookup(pos, key)
+	val, found, err = a.attr.Lookup(pos, key)
+
+	// TODO: remove at some point. This is an ugly hack to simulate the old
+	// behavior of protobufs.
+	if !found && a.attr.Name == "protobuf" && key == "type" {
+		val, err = a.String(1)
+		found = err == nil
+	}
+	return val, found, err
 }
 
 // Expr reports the operation of the underlying expression and the values it

diff --git a/encoding/protobuf/examples_test.go b/encoding/protobuf/examples_test.go
@@ -47,13 +47,13 @@ func ExampleExtract() {
 	//
 	// // This is my type.
 	// #MyType: {
-	// 	stringValue?: string @protobuf(1,name=string_value) // just any 'ole string
+	// 	stringValue?: string @protobuf(1,string,name=string_value) // just any 'ole string
 	//
 	// 	// A method must start with a capital letter.
-	// 	method?: [...string] @protobuf(2)
+	// 	method?: [...string] @protobuf(2,string)
 	// 	method?: [...=~"^[A-Z]"]
 	// 	exampleMap?: {
 	// 		[string]: string
-	// 	} @protobuf(3,type=map<string,string>,example_map)
+	// 	} @protobuf(3,map[string]string,example_map)
 	// }
 }
diff --git a/encoding/protobuf/parse.go b/encoding/protobuf/parse.go
@@ -30,7 +30,6 @@ import (
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/ast/astutil"
 	"cuelang.org/go/cue/errors"
-	"cuelang.org/go/cue/format"
 	"cuelang.org/go/cue/literal"
 	"cuelang.org/go/cue/parser"
 	"cuelang.org/go/cue/token"
@@ -520,7 +519,7 @@ func (p *protoConverter) messageField(s *ast.StructLit, i int, v proto.Visitee)
 		addComments(f, i, x.Comment, x.InlineComment)
 
 		o := optionParser{message: s, field: f}
-		o.tags = fmt.Sprintf("%d,type=map<%s,%s>", x.Sequence, x.KeyType, x.Type)
+		o.tags = fmt.Sprintf(`%d,map[%s]%s`, x.Sequence, x.KeyType, x.Type)
 		if x.Name != name.Name {
 			o.tags += "," + x.Name
 		}
@@ -718,13 +717,8 @@ func (p *protoConverter) parseField(s *ast.StructLit, i int, x *proto.Field) *as
 
 	o := optionParser{message: s, field: f}
 
-	// body of @protobuf tag: sequence[,type][,name=<name>][,...]
-	o.tags += fmt.Sprint(x.Sequence)
-	b, _ := format.Node(typ)
-	str := string(b)
-	if x.Type != strings.TrimLeft(str, "#") {
-		o.tags += ",type=" + x.Type
-	}
+	// body of @protobuf tag: sequence,type[,name=<name>][,...]
+	o.tags += fmt.Sprintf("%v,%s", x.Sequence, x.Type)
 	if x.Name != name.Name {
 		o.tags += ",name=" + x.Name
 	}