reflect/protoreflect: FieldDescriptor.Kind should never be GroupKind …

…for maps or fields of map entry

Resolves golang/protobuf#1615

The protoc compiler disallows setting the message encoding feature of
map fields to delimited since maps, at least for now (as of edition
2023) should always use normal length-prefixed encoding.

But the field (and a message value field inside the map entry) could
inherit such a feature value if it were set as a file-wide default. At
the point where the code changes the kind from message to group, based
on the field's resolved features, the message type hasn't yet been
resolved.  So this change adds a check after the FieldDescriptor's
message type is resolved, to change the kind back from group to
message if the field is a map field or a field in a map entry message.

Change-Id: I785269a4ecd80d1a17866c08b2afc0b01440e0e3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/588976
Reviewed-by: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Cassondra Foesch <[email protected]>
Reviewed-by: Mike Kruskal <[email protected]>
Reviewed-by: Michael Stapelberg <[email protected]>

cmd/protoc-gen-go/testdata/protoeditions/maps_and_delimited.proto

@@ -0,0 +1,22 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+edition = "2023";
+
+package goproto.protoc.protoeditions;
+
+option go_package = "google.golang.org/protobuf/cmd/protoc-gen-go/testdata/protoeditions";
+option features.message_encoding = DELIMITED;
+
+message MessageWithMaps {
+  map<string, string> map_without_message = 1;
+  map<uint32, bytes> map_without_message_b = 2;
+  map<int64, NestedMessage> map_with_message = 3;
+  message NestedMessage {
+    uint64 id = 1;
+    string name = 2;
+  }
+  NestedMessage nested_message = 4;
+  repeated NestedMessage repeated_message = 5;
+}

internal/filedesc/desc.go

@@ -383,6 +383,10 @@ func (fd *Field) Message() protoreflect.MessageDescriptor {
 	}
 	return fd.L1.Message
 }
+func (fd *Field) IsMapEntry() bool {
+	parent, ok := fd.L0.Parent.(protoreflect.MessageDescriptor)
+	return ok && parent.IsMapEntry()
+}
 func (fd *Field) Format(s fmt.State, r rune)             { descfmt.FormatDesc(s, r, fd) }
 func (fd *Field) ProtoType(protoreflect.FieldDescriptor) {}
 

internal/filedesc/desc_lazy.go

@@ -45,6 +45,11 @@ func (file *File) resolveMessages() {
 			case protoreflect.MessageKind, protoreflect.GroupKind:
 				fd.L1.Message = file.resolveMessageDependency(fd.L1.Message, listFieldDeps, depIdx)
 				depIdx++
+				if fd.L1.Kind == protoreflect.GroupKind && (fd.IsMap() || fd.IsMapEntry()) {
+					// A map field might inherit delimited encoding from a file-wide default feature.
+					// But maps never actually use delimited encoding. (At least for now...)
+					fd.L1.Kind = protoreflect.MessageKind
+				}
 			}
 
 			// Default is resolved here since it depends on Enum being resolved.