Skip to content

Commit 98cb722

Browse files
committed
Fix comment ignores not working for group fields
For proto2 group fields, comments in source get attributed to the synthetic message that holds the group's fields, not the group field itself. This caused buf:lint:ignore comments on group field declarations to not be respected for errors on fields within the group. This fix adds logic to detect when a source path points to a descriptor within a synthetic message created for a group field, and also checks the parent group field's source path for comment ignores. Fixes #4187
1 parent d92ad47 commit 98cb722

File tree

1 file changed

+94
-0
lines changed

1 file changed

+94
-0
lines changed

private/bufpkg/bufcheck/client.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import (
4343
"github.com/bufbuild/buf/private/pkg/protoversion"
4444
"github.com/bufbuild/buf/private/pkg/syserror"
4545
"github.com/google/uuid"
46+
"google.golang.org/protobuf/reflect/protoreflect"
4647
"pluginrpc.com/pluginrpc"
4748
)
4849

@@ -821,6 +822,12 @@ func ignoreFileLocation(
821822
if err != nil {
822823
return false, err
823824
}
825+
// For group fields, comments in source get attributed to the nested message
826+
// (the synthetic message that holds the group's fields), not the group field itself.
827+
// We need to also check the group field's source path for comment ignores.
828+
if groupFieldSourcePath := getGroupFieldSourcePathForNestedMessage(sourcePath, protoreflectFileDescriptor); groupFieldSourcePath != nil {
829+
associatedSourcePaths = append(associatedSourcePaths, groupFieldSourcePath)
830+
}
824831
sourceLocations := protoreflectFileDescriptor.SourceLocations()
825832
for _, associatedSourcePath := range associatedSourcePaths {
826833
sourceLocation := sourceLocations.ByPath(associatedSourcePath)
@@ -836,6 +843,93 @@ func ignoreFileLocation(
836843
return false, nil
837844
}
838845

846+
// getGroupFieldSourcePathForNestedMessage checks if the given source path points to a
847+
// descriptor within a synthetic message that was created for a group field. If so, it
848+
// returns the source path of the parent group field so that comment ignores on the group
849+
// field declaration are properly respected.
850+
//
851+
// Group fields in proto2 create a synthetic message to hold their fields. The source
852+
// path for elements within this synthetic message includes the nested message path
853+
// component. For example, for a group field "Metadata" at index 0 in a message, the
854+
// source path might be [4,0,2,0,3,0,...] where:
855+
// - [4,0] is the parent message
856+
// - [2,0] is the group field
857+
// - [3,0] is the nested message (synthetic message for the group)
858+
// - ... is the rest of the path within the synthetic message
859+
//
860+
// In this case, we want to also check [4,0,2,0] (the group field) for comment ignores.
861+
func getGroupFieldSourcePathForNestedMessage(
862+
sourcePath protoreflect.SourcePath,
863+
fileDescriptor protoreflect.FileDescriptor,
864+
) protoreflect.SourcePath {
865+
// A source path pointing to a descriptor within a synthetic group message will have
866+
// at least 6 elements: [4, msg_idx, 2, field_idx, 3, nested_msg_idx, ...]
867+
// where 3 is the field number for nested_type in DescriptorProto.
868+
//
869+
// We need to check if the message at nested_msg_idx is a synthetic message created
870+
// for a group field. A synthetic message for a group field has:
871+
// 1. The same name as the parent field
872+
// 2. The parent field has type GROUP
873+
//
874+
// We find the parent field by looking at the field at [2, field_idx] within the
875+
// parent message at [4, msg_idx].
876+
if len(sourcePath) < 6 {
877+
return nil
878+
}
879+
// Check if we have the pattern [4, msg_idx, 2, field_idx, 3, nested_msg_idx, ...]
880+
// where 4 = message_type, 2 = field, 3 = nested_type
881+
const (
882+
messageTypeTag = 4 // FieldDescriptorProto.message_type
883+
fieldTypeTag = 2 // DescriptorProto.field
884+
nestedTypeTag = 3 // DescriptorProto.nested_type
885+
fieldNumberTag = 3 // FieldDescriptorProto.number
886+
fieldTypeTag2 = 5 // FieldDescriptorProto.type
887+
fieldTypeNameTag = 6 // FieldDescriptorProto.type_name
888+
)
889+
// Walk through the source path to find the pattern
890+
for i := 0; i < len(sourcePath)-5; i++ {
891+
if sourcePath[i] == messageTypeTag &&
892+
sourcePath[i+2] == fieldTypeTag &&
893+
sourcePath[i+4] == nestedTypeTag {
894+
// Found the pattern [4, msg_idx, 2, field_idx, 3, nested_msg_idx]
895+
msgIdx := int(sourcePath[i+1])
896+
fieldIdx := int(sourcePath[i+3])
897+
nestedMsgIdx := int(sourcePath[i+5])
898+
899+
messages := fileDescriptor.Messages()
900+
if msgIdx >= messages.Len() {
901+
continue
902+
}
903+
message := messages.Get(msgIdx)
904+
fields := message.Fields()
905+
if fieldIdx >= fields.Len() {
906+
continue
907+
}
908+
field := fields.Get(fieldIdx)
909+
// Check if this is a group field
910+
if field.Kind() != protoreflect.GroupKind {
911+
continue
912+
}
913+
// Check if the nested message name matches the field name
914+
// (this confirms it's the synthetic message for the group field)
915+
nestedMessages := message.Messages()
916+
if nestedMsgIdx >= nestedMessages.Len() {
917+
continue
918+
}
919+
nestedMessage := nestedMessages.Get(nestedMsgIdx)
920+
if string(nestedMessage.Name()) != string(field.Name()) {
921+
continue
922+
}
923+
// This is a synthetic message for a group field
924+
// Return the source path of the group field: [4, msg_idx, 2, field_idx]
925+
groupFieldPath := make(protoreflect.SourcePath, i+4)
926+
copy(groupFieldPath, sourcePath[:i+4])
927+
return groupFieldPath
928+
}
929+
}
930+
return nil
931+
}
932+
839933
// checkCommentLineForCheckIgnore checks that the comment line starts with the configured
840934
// comment ignore prefix, a space and the ruleID of the check.
841935
//

0 commit comments

Comments
 (0)