Fix a few more conformance issues

experimental/fdp/comments.go

@@ -228,9 +228,9 @@ func (ct *commentTracker) handleNonSkippableToken(t token.Token) {
 			donate = true
 		case slicesx.Among(
 			t.Text(),
-			keyword.LParen.String(),
-			keyword.LBracket.String(),
-			keyword.LBrace.String(),
+			keyword.RParen.String(),
+			keyword.RBracket.String(),
+			keyword.RBrace.String(),
 		):
 			donate = true
 		case ct.cursor.NewLinesBetween(ct.tracked[0][len(ct.tracked[0])-1], t, 2) > 1:

experimental/fdp/generator.go

@@ -209,10 +209,10 @@ func (g *generator) message(ty ir.Type, mdp *descriptorpb.DescriptorProto) {
 
 	var extnIndex int32
 	for extend := range seq.Values(ty.Extends()) {
-		g.debug.comments(extend.AST(), tags.File_Extension)
+		g.debug.comments(extend.AST(), tags.Message_Extension)
 
 		for extn := range seq.Values(extend.Extensions()) {
-			g.debug.in(tags.File_Extension, extnIndex)(func() {
+			g.debug.in(tags.Message_Extension, extnIndex)(func() {
 				g.field(extn, slicesx.PushNew(&mdp.Extension))
 			})
 			extnIndex++
@@ -339,9 +339,14 @@ func (g *generator) field(f ir.Member, fdp *descriptorpb.FieldDescriptorProto) {
 	fdp.Number = addr(f.Number())
 	g.debug.span(ast.Tag, tags.Field_Number)
 
-	switch f.Presence() {
+	switch label := f.Presence(); label {
 	case presence.Explicit, presence.Implicit, presence.Shared:
 		fdp.Label = descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum()
+		if label == presence.Explicit && g.currentFile.Syntax() == syntax.Proto3 {
+			// For proto3, if the presence is set explicitly with "optional", we need to set
+			// "proto3_optional" field to true.
+			fdp.Proto3Optional = addr(true)
+		}
 	case presence.Repeated:
 		fdp.Label = descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum()
 	case presence.Required:

experimental/ir/ir_test.go

@@ -239,7 +239,7 @@ func TestIR(t *testing.T) {
 							loc.Leading = &entry.Leading
 						}
 						if entry.Trailing != "" {
-							loc.Leading = &entry.Trailing
+							loc.Trailing = &entry.Trailing
 						}
 
 						// Delete the copyright notice comment, because it's

experimental/ir/ir_value.go

@@ -512,8 +512,7 @@ func (v Value) marshal(buf []byte, r *report.Report, ranges *[][2]int) ([]byte,
 			m := v.AsMessage()
 
 			var k int
-			var group bool // TODO: v.Field().IsGroup()
-			if group {
+			if v.Field().IsGroup() {
 				buf = protowire.AppendTag(buf, protowire.Number(v.Field().Number()), protowire.StartGroupType)
 				buf, k = m.marshal(buf, r, ranges)
 				buf = protowire.AppendTag(buf, protowire.Number(v.Field().Number()), protowire.EndGroupType)

experimental/ir/lower_resolve.go

@@ -85,6 +85,22 @@ func resolveFieldType(field Member, r *report.Report) {
 		// NOTE: Editions features are resolved elsewhere, so we default to
 		// explicit presence here.
 
+		if field.IsGroup() {
+			// Group fields can still have a label, so we check the first prefix, similar to a
+			// prefixed non-group field.
+			prefix, ok := iterx.First(field.AST().Prefixes())
+			if ok {
+				switch prefix.Prefix() {
+				case keyword.Optional:
+					kind = presence.Explicit
+				case keyword.Required:
+					kind = presence.Required
+				case keyword.Repeated:
+					kind = presence.Repeated
+				}
+			}
+		}
+
 		path = ty.AsPath().Path
 
 	case ast.TypeKindPrefixed:

experimental/ir/testdata/comments/attribution.proto.sci.yaml

@@ -8,14 +8,16 @@
 - path: ".package"
   start: { line: 19, column: 0 }
   end: { line: 19, column: 17 }
-  leading: " This is still a trailing comment for the package.\n"
+  leading: " This is a leading comment for the package.\n"
+  trailing: " This is still a trailing comment for the package.\n"
 - path: ".options"
   start: { line: 24, column: 0 }
   end: { line: 24, column: 47 }
 - path: ".options.java_package"
   start: { line: 24, column: 0 }
   end: { line: 24, column: 47 }
-  leading: " Similarly, this is a trailing comment."
+  leading: " This is a leading comment for the java_package option source code info, but not the\n top-level option source code info.\n"
+  trailing: " Similarly, this is a trailing comment."
 - path: ".options"
   start: { line: 25, column: 0 }
   end: { line: 25, column: 34 }
@@ -25,14 +27,16 @@
 - path: ".dependency.0"
   start: { line: 29, column: 0 }
   end: { line: 29, column: 49 }
-  leading: " Same with this trailing comment."
+  leading: " This is a leading comment for the import source code info, but not for the corresponding\n public import source code info.\n"
+  trailing: " Same with this trailing comment."
 - path: ".public_dependency.0"
   start: { line: 29, column: 7 }
   end: { line: 29, column: 13 }
 - path: ".message_type[Foo]"
   start: { line: 32, column: 0 }
   end: { line: 58, column: 1 }
-  leading: " This is the TRAILING comment for Foo. (It is NOT\n a detached comment for baz.)\n"
+  leading: " This, as expected, is a leading comment for Foo.\n"
+  trailing: " This is the TRAILING comment for Foo. (It is NOT\n a detached comment for baz.)\n"
 - path: ".message_type[Foo].name"
   start: { line: 32, column: 32 }
   end: { line: 32, column: 35 }
@@ -49,14 +53,15 @@
 - path: ".message_type[Foo].options.0.message_set_wire_format"
   start: { line: 38, column: 2 }
   end: { line: 38, column: 18 }
-  leading: " Trailing comment for the second message option"
+  trailing: " Trailing comment for the second message option"
 - path: ".message_type[Foo].field[baz].type"
   start: { line: 41, column: 2 }
   end: { line: 41, column: 8 }
 - path: ".message_type[Foo].field[baz]"
   start: { line: 41, column: 2 }
   end: { line: 41, column: 46 }
-  leading: " This is a trailing comment for baz.\n"
+  leading: " This is a leading comment for baz.\n"
+  trailing: " This is a trailing comment for baz.\n"
 - path: ".message_type[Foo].field[baz].name"
   start: { line: 41, column: 38 }
   end: { line: 41, column: 41 }
@@ -66,7 +71,8 @@
 - path: ".message_type[Foo].reserved_range"
   start: { line: 45, column: 2 }
   end: { line: 45, column: 74 }
-  leading: " This is a trailing comment for the reserved range."
+  leading: " This is a leading comment for the reserved range.\n"
+  trailing: " This is a trailing comment for the reserved range."
 - path: ".message_type[Foo].reserved_range[0]"
   start: { line: 45, column: 11 }
   end: { line: 45, column: 12 }
@@ -97,7 +103,8 @@
 - path: ".message_type[Foo].reserved_name"
   start: { line: 48, column: 2 }
   end: { line: 48, column: 42 }
-  leading: " This is a trailing comment for the reserved range."
+  leading: " This is a leading comment for the reserved names declaration.\n"
+  trailing: " This is a trailing comment for the reserved range."
 - path: ".message_type[Foo].reserved_name.0"
   start: { line: 48, column: 11 }
   end: { line: 48, column: 14 }
@@ -170,7 +177,8 @@
 - path: ".enum_type[Baz]"
   start: { line: 82, column: 0 }
   end: { line: 84, column: 1 }
-  leading: " This is a trailing comment for enum Baz."
+  leading: " This is the leading comment for enum Baz.\n"
+  trailing: " This is a trailing comment for enum Baz."
 - path: ".enum_type[Baz].name"
   start: { line: 82, column: 5 }
   end: { line: 82, column: 8 }
@@ -186,14 +194,16 @@
 - path: ".service[FooService]"
   start: { line: 87, column: 0 }
   end: { line: 94, column: 1 }
-  leading: " This is a trailing comment for service FooService."
+  leading: " This is the leading comment for service FooService.\n"
+  trailing: " This is a trailing comment for service FooService."
 - path: ".service[FooService].name"
   start: { line: 87, column: 8 }
   end: { line: 87, column: 18 }
 - path: ".service[FooService].method[FooForBar]"
   start: { line: 89, column: 2 }
   end: { line: 89, column: 35 }
-  leading: " Trailing comment for FooForBar."
+  leading: " Leading comment for method FooForBar.\n"
+  trailing: " Trailing comment for FooForBar."
 - path: ".service[FooService].method[FooForBar].name"
   start: { line: 89, column: 6 }
   end: { line: 89, column: 15 }
@@ -206,7 +216,8 @@
 - path: ".service[FooService].method[BarForBar]"
   start: { line: 92, column: 2 }
   end: { line: 93, column: 3 }
-  leading: " Trailing comment for BarForBar."
+  leading: " Leading comment for method BarForBar.\n"
+  trailing: " Trailing comment for BarForBar."
 - path: ".service[FooService].method[BarForBar].name"
   start: { line: 92, column: 6 }
   end: { line: 92, column: 15 }

experimental/ir/testdata/comments/group.proto.fds.yaml

@@ -6,7 +6,7 @@ file:
     field:
     - name: "group"
       number: 1
-      label: LABEL_OPTIONAL
+      label: LABEL_REQUIRED
       type: TYPE_GROUP
       type_name: ".buf.test.Foo.Group"
       json_name: "group"

experimental/ir/testdata/comments/group.proto.sci.yaml

@@ -11,7 +11,8 @@
 - path: ".message_type[Foo]"
   start: { line: 22, column: 0 }
   end: { line: 32, column: 1 }
-  leading: " This is considered a trailing comment for message Foo.\n"
+  leading: " This is a leading comment for message Foo.\n"
+  trailing: " This is considered a trailing comment for message Foo.\n"
 - path: ".message_type[Foo].name"
   start: { line: 22, column: 8 }
   end: { line: 22, column: 11 }
@@ -24,7 +25,8 @@
 - path: ".message_type[Foo].nested_type[Group]"
   start: { line: 28, column: 2 }
   end: { line: 31, column: 3 }
-  leading: " This is a trailing comment for synthetic message Group"
+  leading: " This is a leading for the synthetic message Group rather than the field.\n"
+  trailing: " This is a trailing comment for synthetic message Group"
   detached:
   - " This is a detached leading comment for synthetic message Group rather than the field.\n"
 - path: ".message_type[Foo].field[group].type_name"

experimental/ir/testdata/fields/groups/ok.proto.fds.yaml

@@ -32,7 +32,7 @@ file:
         json_name: "x"
       - name: "x_y_z"
         number: 2
-        label: LABEL_OPTIONAL
+        label: LABEL_REPEATED
         type: TYPE_GROUP
         type_name: ".test.Foo.FooBar.X_Y_Z"
         json_name: "xYZ"
@@ -42,7 +42,7 @@ file:
         field:
         - name: "baz"
           number: 3
-          label: LABEL_OPTIONAL
+          label: LABEL_REQUIRED
           type: TYPE_GROUP
           type_name: ".test.Foo.FooBar.X_Y_Z.Baz"
           json_name: "baz"