client/flagpb: fix unmarshaling of maps

Map type is special and supporting added complexity.

R=vadimsh@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/1940683002

client/flagpb/unmarshal.go

@@ -93,14 +93,20 @@ func (p *parser) parseOneFlag(flags []string, root message) (flagsRest []string,
 		lastMsg := pathMsgs[len(pathMsgs)-1]
 		target = &lastMsg.message
 	}
-
-	// Resolve last field name.
 	name := fieldPath[len(fieldPath)-1]
-	fi := target.desc.FindField(name)
-	if fi == -1 {
-		return nil, fmt.Errorf("field %s not found in message %s", name, target.desc.GetName())
+
+	// Resolve target field.
+	var fieldIndex int
+	if target.desc.GetOptions().GetMapEntry() {
+		if fieldIndex = target.desc.FindField("value"); fieldIndex == -1 {
+			return nil, fmt.Errorf("map entry type %s does not have value field", target.desc.GetName())
+		}
+	} else {
+		if fieldIndex = target.desc.FindField(name); fieldIndex == -1 {
+			return nil, fmt.Errorf("field %s not found in message %s", name, target.desc.GetName())
+		}
 	}
-	field := target.desc.Field[fi]
+	field := target.desc.Field[fieldIndex]
 
 	var value interface{}
 	hasValue := false
@@ -176,12 +182,19 @@ func (p *parser) subMessages(root message, path []string) ([]subMsg, error) {
 	parent := &root
 	for i, name := range path {
 		curPath := path[:i+1]
-		fi := parent.desc.FindField(name)
-		if fi == -1 {
-			return nil, fmt.Errorf("field %q not found in message %s", name, parent.desc.GetName())
+
+		var fieldIndex int
+		if parent.desc.GetOptions().GetMapEntry() {
+			if fieldIndex = parent.desc.FindField("value"); fieldIndex == -1 {
+				return nil, fmt.Errorf("map entry type %s does not have value field", parent.desc.GetName())
+			}
+		} else {
+			if fieldIndex = parent.desc.FindField(name); fieldIndex == -1 {
+				return nil, fmt.Errorf("field %q not found in message %s", name, parent.desc.GetName())
+			}
 		}
 
-		f := parent.desc.Field[fi]
+		f := parent.desc.Field[fieldIndex]
 		if f.GetType() != descriptor.FieldDescriptorProto_TYPE_MESSAGE {
 			return nil, fmt.Errorf("field %s is not a message", strings.Join(curPath, "."))
 		}
@@ -197,7 +210,7 @@ func (p *parser) subMessages(root message, path []string) ([]subMsg, error) {
 
 		sub := subMsg{
 			message:  message{desc: subDesc},
-			repeated: f.Repeated(),
+			repeated: f.Repeated() && !subDesc.GetOptions().GetMapEntry(),
 			path:     curPath,
 		}
 		if value, ok := parent.data[name]; !ok {

client/flagpb/unmarshal_test.go

@@ -172,16 +172,16 @@ func TestUnmarshal(t *testing.T) {
 		Convey("repeated", func() {
 			Convey("int32", func() {
 				So(unmarshalOK("M1", "-ri", "1", "-ri", "2"), ShouldResemble, msg(
-					"ri", []interface{}{int32(1), int32(2)},
+					"ri", repeated(int32(1), int32(2)),
 				))
 			})
 			Convey("submessage string", func() {
 				Convey("works", func() {
 					So(unmarshalOK("M3", "-m1.s", "x", "-m1", "-m1.s", "y"), ShouldResemble, msg(
-						"m1", []interface{}{
+						"m1", repeated(
 							msg("s", "x"),
 							msg("s", "y"),
-						},
+						),
 					))
 				})
 				Convey("reports meaningful error", func() {
@@ -191,5 +191,26 @@ func TestUnmarshal(t *testing.T) {
 				})
 			})
 		})
+
+		Convey("map", func() {
+			Convey("map<string, string>", func() {
+				So(unmarshalOK("MapContainer", "-ss.x", "a", "-ss.y", "b"), ShouldResemble, msg(
+					"ss", msg("x", "a", "y", "b"),
+				))
+			})
+			Convey("map<int32, int32>", func() {
+				So(unmarshalOK("MapContainer", "-ii.1", "10", "-ii.2", "20"), ShouldResemble, msg(
+					"ii", msg("1", int32(10), "2", int32(20)),
+				))
+			})
+			Convey("map<string, M1>", func() {
+				So(unmarshalOK("MapContainer", "-sm1.x.s", "a", "-sm1.y.s", "b"), ShouldResemble, msg(
+					"sm1", msg(
+						"x", msg("s", "a"),
+						"y", msg("s", "b"),
+					),
+				))
+			})
+		})
 	})
 }

client/flagpb/unmarshal_test.proto

@@ -28,3 +28,9 @@ message M3 {
 	string s = 4;
 	bytes bt = 5;
 }
+
+message MapContainer {
+	map<string, string> ss = 1;
+	map<int32, int32> ii = 2;
+	map<string, M1> sm1 = 3;
+}