Resolve ignore comments for group fields

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Add `--stdin-filepath` flag to `buf format`, which reads a single `.proto` file from
   stdin and writes the formatted result to stdout. The path is not read from disk, and is
   only used to report parse errors and diffs.
+- Fix lint comment ignores on proto2 `group` fields being ignored.
 - Improve the `buf curl` error message for methods that accept a single request message.
 
 ## [v1.73.0] - 2026-09-11

private/bufpkg/bufcheck/client.go

@@ -43,6 +43,7 @@ import (
 	"github.com/bufbuild/buf/private/pkg/protoversion"
 	"github.com/bufbuild/buf/private/pkg/syserror"
 	"github.com/google/uuid"
+	"google.golang.org/protobuf/reflect/protoreflect"
 	"pluginrpc.com/pluginrpc"
 )
 
@@ -824,18 +825,52 @@ func ignoreFileLocation(
 		sourceLocations := protoreflectFileDescriptor.SourceLocations()
 		for _, associatedSourcePath := range associatedSourcePaths {
 			sourceLocation := sourceLocations.ByPath(associatedSourcePath)
-			if leadingComments := sourceLocation.LeadingComments; leadingComments != "" {
-				for _, line := range xstrings.SplitTrimLinesNoEmpty(leadingComments) {
-					if checkCommentLineForCheckIgnore(line, config.CommentIgnorePrefix, ruleID) {
-						return true, nil
-					}
+			if leadingCommentsHaveCheckIgnore(sourceLocation.LeadingComments, config.CommentIgnorePrefix, ruleID) {
+				return true, nil
+			}
+			// A group field has both a field and synthetic message in the descriptor, with comments
+			// that appear to be on the field actually assigned to the synthetic message.
+			// So we need to see if the path is a group, resolve its synthetic message, and check the comments
+			// there if so.
+			if syntheticMessage := groupFieldSyntheticMessage(protoreflectFileDescriptor, associatedSourcePath); syntheticMessage != nil {
+				syntheticMessageSourceLocation := sourceLocations.ByDescriptor(syntheticMessage)
+				if leadingCommentsHaveCheckIgnore(syntheticMessageSourceLocation.LeadingComments, config.CommentIgnorePrefix, ruleID) {
+					return true, nil
 				}
 			}
 		}
 	}
 	return false, nil
 }
 
+// groupFieldSyntheticMessage returns the synthetic message declaration for the group field
+// at the given source path, or nil if the source path does not point to a group field.
+func groupFieldSyntheticMessage(
+	fileDescriptor protoreflect.FileDescriptor,
+	sourcePath protoreflect.SourcePath,
+) protoreflect.MessageDescriptor {
+	fieldDescriptor, ok := protosourcepath.DescriptorForSourcePath(fileDescriptor, sourcePath).(protoreflect.FieldDescriptor)
+	if !ok || fieldDescriptor.Kind() != protoreflect.GroupKind {
+		return nil
+	}
+	return fieldDescriptor.Message()
+}
+
+// leadingCommentsHaveCheckIgnore checks if any line of the given leading comments is a
+// comment ignore for the given rule.
+func leadingCommentsHaveCheckIgnore(
+	leadingComments string,
+	commentIgnorePrefix string,
+	ruleID string,
+) bool {
+	for _, line := range xstrings.SplitTrimLinesNoEmpty(leadingComments) {
+		if checkCommentLineForCheckIgnore(line, commentIgnorePrefix, ruleID) {
+			return true
+		}
+	}
+	return false
+}
+
 // checkCommentLineForCheckIgnore checks that the comment line starts with the configured
 // comment ignore prefix, a space and the ruleID of the check.
 //

private/bufpkg/bufcheck/client_test.go

@@ -0,0 +1,149 @@
+// Copyright 2020-2026 Buf Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package bufcheck
+
+import (
+	"testing"
+
+	"github.com/bufbuild/protocompile"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/reflect/protoreflect"
+)
+
+const groupsProtoFileContent = `syntax = "proto2";
+
+package a;
+
+message Foo {
+  optional group Group = 1 {
+    optional string id = 1;
+  }
+  optional string scalar = 2;
+  message Nested {
+    optional group NestedGroup = 1 {
+      optional string id = 1;
+    }
+  }
+  extend Bar {
+    optional group MessageExtensionGroup = 101 {
+      optional string id = 1;
+    }
+  }
+}
+
+message Bar {
+  extensions 100 to 200;
+}
+
+extend Bar {
+  optional group FileExtensionGroup = 100 {
+    optional string id = 1;
+  }
+}
+`
+
+func TestGroupFieldSyntheticMessage(t *testing.T) {
+	t.Parallel()
+	fileDescriptor := testCompileFileDescriptor(t, groupsProtoFileContent)
+	sourceLocations := fileDescriptor.SourceLocations()
+	fooMessageDescriptor := fileDescriptor.Messages().ByName("Foo")
+	require.NotNil(t, fooMessageDescriptor)
+	nestedMessageDescriptor := fooMessageDescriptor.Messages().ByName("Nested")
+	require.NotNil(t, nestedMessageDescriptor)
+	for _, testCase := range []struct {
+		name                            string
+		fieldDescriptor                 protoreflect.FieldDescriptor
+		expectedSyntheticMessageName    protoreflect.FullName
+		expectedNoSyntheticMessageFound bool
+	}{
+		{
+			name:                         "group field",
+			fieldDescriptor:              fooMessageDescriptor.Fields().ByName("group"),
+			expectedSyntheticMessageName: "a.Foo.Group",
+		},
+		{
+			name:                         "group field in nested message",
+			fieldDescriptor:              nestedMessageDescriptor.Fields().ByName("nestedgroup"),
+			expectedSyntheticMessageName: "a.Foo.Nested.NestedGroup",
+		},
+		{
+			name:                         "group extension field in message",
+			fieldDescriptor:              fooMessageDescriptor.Extensions().ByName("messageextensiongroup"),
+			expectedSyntheticMessageName: "a.Foo.MessageExtensionGroup",
+		},
+		{
+			name:                         "group extension field in file",
+			fieldDescriptor:              fileDescriptor.Extensions().ByName("fileextensiongroup"),
+			expectedSyntheticMessageName: "a.FileExtensionGroup",
+		},
+		{
+			name:                            "non-group field",
+			fieldDescriptor:                 fooMessageDescriptor.Fields().ByName("scalar"),
+			expectedNoSyntheticMessageFound: true,
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			require.NotNil(t, testCase.fieldDescriptor)
+			sourcePath := sourceLocations.ByDescriptor(testCase.fieldDescriptor).Path
+			require.NotEmpty(t, sourcePath)
+			syntheticMessageDescriptor := groupFieldSyntheticMessage(fileDescriptor, sourcePath)
+			if testCase.expectedNoSyntheticMessageFound {
+				require.Nil(t, syntheticMessageDescriptor)
+				return
+			}
+			require.NotNil(t, syntheticMessageDescriptor)
+			require.Equal(t, testCase.expectedSyntheticMessageName, syntheticMessageDescriptor.FullName())
+		})
+	}
+}
+
+// Source path traversal itself is covered by protosourcepath, this covers the source paths
+// that resolve to something other than a group field.
+func TestGroupFieldSyntheticMessageNonGroupFieldSourcePaths(t *testing.T) {
+	t.Parallel()
+	fileDescriptor := testCompileFileDescriptor(t, groupsProtoFileContent)
+	for _, sourcePath := range []protoreflect.SourcePath{
+		// The file.
+		nil,
+		{},
+		// .message_type(0), a message.
+		{4, 0},
+		// .message_type(0).nested_type(0), the synthetic message of a group field.
+		{4, 0, 3, 0},
+		// .message_type(0).field(0).name, an attribute of a field.
+		{4, 0, 2, 0, 1},
+	} {
+		require.Nil(
+			t,
+			groupFieldSyntheticMessage(fileDescriptor, sourcePath),
+			"source path %v", sourcePath,
+		)
+	}
+}
+
+func testCompileFileDescriptor(t *testing.T, fileContent string) protoreflect.FileDescriptor {
+	t.Helper()
+	compiler := protocompile.Compiler{
+		Resolver: &protocompile.SourceResolver{
+			Accessor: protocompile.SourceAccessorFromMap(map[string]string{"a.proto": fileContent}),
+		},
+		SourceInfoMode: protocompile.SourceInfoStandard,
+	}
+	files, err := compiler.Compile(t.Context(), "a.proto")
+	require.NoError(t, err)
+	require.Len(t, files, 1)
+	return files[0]
+}

private/bufpkg/bufcheck/lint_test.go

@@ -1310,6 +1310,16 @@ func TestCommentIgnoresOnlyRule(t *testing.T) {
 	)
 }
 
+func TestCommentIgnoresGroup(t *testing.T) {
+	t.Parallel()
+	testLint(
+		t,
+		"comment_ignores_group",
+		bufanalysistesting.NewFileAnnotation(t, "a.proto", 10, 18, 10, 28, "FIELD_NOT_REQUIRED"),
+		bufanalysistesting.NewFileAnnotation(t, "a.proto", 18, 20, 18, 30, "FIELD_NOT_REQUIRED"),
+	)
+}
+
 func TestCommentIgnoresWithTrailingComment(t *testing.T) {
 	t.Parallel()
 	testLint(

private/bufpkg/bufcheck/testdata/lint/comment_ignores_group/a.proto

@@ -0,0 +1,22 @@
+syntax = "proto2";
+
+package a;
+
+message Foo {
+  // buf:lint:ignore FIELD_NOT_REQUIRED
+  required group Ignored = 1 {
+    optional string id = 1;
+  }
+  required group NotIgnored = 2 {
+    optional string id = 1;
+  }
+  message Nested {
+    // buf:lint:ignore FIELD_NOT_REQUIRED
+    required group Ignored = 1 {
+      optional string id = 1;
+    }
+    required group NotIgnored = 2 {
+      optional string id = 1;
+    }
+  }
+}

private/bufpkg/bufcheck/testdata/lint/comment_ignores_group/buf.yaml

@@ -0,0 +1,4 @@
+version: v2
+lint:
+  use:
+    - FIELD_NOT_REQUIRED

private/pkg/protosourcepath/README.md

@@ -80,8 +80,8 @@ Details examples for associated paths can be found through the tests.
 
 ## API
 
-There is a single function, `GetAssociatedSourcePaths`, that takes a `protoreflect.SourcePath`
-and returns a list of associated paths.
+`GetAssociatedSourcePaths` takes a `protoreflect.SourcePath` and returns a list of associated
+paths.
 
 ```go
 func GetAssociatedSourcePaths(
@@ -91,6 +91,20 @@ func GetAssociatedSourcePaths(
 
 We expect there always to be at least one associated path, the path itself.
 
+`DescriptorForSourcePath` resolves a `protoreflect.SourcePath` against a file to the descriptor
+of the Protobuf declaration that the source path points to.
+
+```go
+func DescriptorForSourcePath(
+	fileDescriptor protoreflect.FileDescriptor,
+	sourcePath protoreflect.SourcePath,
+) protoreflect.Descriptor
+```
+
+Source paths that point to an attribute of a declaration rather than to the declaration itself,
+such as `[4, 0, 1]` for `.message_type(0).name`, return `nil`. `GetAssociatedSourcePaths` can be
+used to resolve such a source path to the source paths of the declarations it belongs to.
+
 ## Future
 
 We are currently returning all associated source paths, but we have the option to exclude