Support recently added source_file_descriptors field of CodeGenerator…

…Request (#2792)

.golangci.yml

@@ -263,3 +263,8 @@ issues:
         - staticcheck
       # We need to handle this field in descriptor.proto.
       text: "GetDeprecatedLegacyJsonFieldConflicts is deprecated"
+    - linters:
+        - forcetypeassert
+      # This greatly simplifies creation of descriptors, and it's safe enough since
+      # it's just test code.
+      path: private/bufpkg/bufimage/source_retention_options_test\.go

CHANGELOG.md

@@ -14,6 +14,12 @@
 - Move `buf mod {clear-cache,cc}` to `buf registry cc`. `buf mod {clear-cache,cc}` is now deprecated.
 - Deprecate `buf mod open`.
 - Delete `buf beta migrate-v1beta1`.
+- Update `buf generate` so it populates the recently-added
+  [`source_file_descriptors`](https://github.com/protocolbuffers/protobuf/blob/v24.0/src/google/protobuf/compiler/plugin.proto#L96-L99)
+  field of the `CodeGeneratorRequest` message. This provides the plugin with access to options
+  that are configured to only be retained in source and not at runtime (via
+  [field option](https://github.com/protocolbuffers/protobuf/blob/v24.0/src/google/protobuf/descriptor.proto#L693-L702)).
+  Descriptors in the `proto_file` field will not include any options configured this way.
 
 ## [v1.29.0] - 2024-01-24
 

go.mod

@@ -8,7 +8,7 @@ require (
 	buf.build/gen/go/bufbuild/registry/protocolbuffers/go v1.32.0-20240229184805-518c7bc6bd0a.1
 	connectrpc.com/connect v1.15.0
 	connectrpc.com/otelconnect v0.7.0
-	github.com/bufbuild/protocompile v0.8.0
+	github.com/bufbuild/protocompile v0.9.0
 	github.com/bufbuild/protovalidate-go v0.6.0
 	github.com/bufbuild/protoyaml-go v0.1.8
 	github.com/docker/docker v25.0.3+incompatible

go.sum

@@ -14,8 +14,8 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
 github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
 github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
 github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
-github.com/bufbuild/protocompile v0.8.0 h1:9Kp1q6OkS9L4nM3FYbr8vlJnEwtbpDPQlQOVXfR+78s=
-github.com/bufbuild/protocompile v0.8.0/go.mod h1:+Etjg4guZoAqzVk2czwEQP12yaxLJ8DxuqCJ9qHdH94=
+github.com/bufbuild/protocompile v0.9.0 h1:DI8qLG5PEO0Mu1Oj51YFPqtx6I3qYXUAhJVJ/IzAVl0=
+github.com/bufbuild/protocompile v0.9.0/go.mod h1:s89m1O8CqSYpyE/YaSGtg1r1YFMF5nLTwh4vlj6O444=
 github.com/bufbuild/protovalidate-go v0.6.0 h1:Jgs1kFuZ2LHvvdj8SpCLA1W/+pXS8QSM3F/E2l3InPY=
 github.com/bufbuild/protovalidate-go v0.6.0/go.mod h1:1LamgoYHZ2NdIQH0XGczGTc6Z8YrTHjcJVmiBaar4t4=
 github.com/bufbuild/protoyaml-go v0.1.8 h1:X9QDLfl9uEllh4gsXUGqPanZYCOKzd92uniRtW2OnAQ=

private/buf/bufgen/generator.go

@@ -287,22 +287,23 @@ func (g *generator) execLocalPlugin(
 	if err != nil {
 		return nil, err
 	}
-	generateOptions := []bufpluginexec.GenerateOption{
-		bufpluginexec.GenerateWithPluginPath(pluginConfig.Path()...),
-		bufpluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath()),
+	requests, err := bufimage.ImagesToCodeGeneratorRequests(
+		pluginImages,
+		pluginConfig.Opt(),
+		nil,
+		includeImports,
+		includeWellKnownTypes,
+	)
+	if err != nil {
+		return nil, err
 	}
 	response, err := g.pluginexecGenerator.Generate(
 		ctx,
 		container,
 		pluginConfig.Name(),
-		bufimage.ImagesToCodeGeneratorRequests(
-			pluginImages,
-			pluginConfig.Opt(),
-			nil,
-			includeImports,
-			includeWellKnownTypes,
-		),
-		generateOptions...,
+		requests,
+		bufpluginexec.GenerateWithPluginPath(pluginConfig.Path()...),
+		bufpluginexec.GenerateWithProtocPath(pluginConfig.ProtocPath()),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("plugin %s: %v", pluginConfig.Name(), err)

private/buf/cmd/buf/command/alpha/protoc/plugin.go

@@ -59,6 +59,16 @@ func executePlugin(
 		storageosProvider,
 		runner,
 	)
+	requests, err := bufimage.ImagesToCodeGeneratorRequests(
+		images,
+		strings.Join(pluginInfo.Opt, ","),
+		bufpluginexec.DefaultVersion,
+		false,
+		false,
+	)
+	if err != nil {
+		return nil, err
+	}
 	var options []bufpluginexec.GenerateOption
 	if pluginInfo.Path != "" {
 		options = append(options, bufpluginexec.GenerateWithPluginPath(pluginInfo.Path))
@@ -67,13 +77,7 @@ func executePlugin(
 		ctx,
 		container,
 		pluginName,
-		bufimage.ImagesToCodeGeneratorRequests(
-			images,
-			strings.Join(pluginInfo.Opt, ","),
-			bufpluginexec.DefaultVersion,
-			false,
-			false,
-		),
+		requests,
 		options...,
 	)
 	if err != nil {

private/buf/cmd/protoc-gen-buf-lint/lint_test.go

@@ -344,5 +344,7 @@ func testBuildCodeGeneratorRequest(
 	}
 	image, err := bufimage.NewImage(imageFiles)
 	require.NoError(t, err)
-	return bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false)
+	codeGenReq, err := bufimage.ImageToCodeGeneratorRequest(image, parameter, nil, false, false)
+	require.NoError(t, err)
+	return codeGenReq
 }

private/bufpkg/bufimage/bufimage.go

@@ -630,7 +630,7 @@ func ImageToCodeGeneratorRequest(
 	compilerVersion *pluginpb.Version,
 	includeImports bool,
 	includeWellKnownTypes bool,
-) *pluginpb.CodeGeneratorRequest {
+) (*pluginpb.CodeGeneratorRequest, error) {
 	return imageToCodeGeneratorRequest(
 		image,
 		parameter,
@@ -655,7 +655,7 @@ func ImagesToCodeGeneratorRequests(
 	compilerVersion *pluginpb.Version,
 	includeImports bool,
 	includeWellKnownTypes bool,
-) []*pluginpb.CodeGeneratorRequest {
+) ([]*pluginpb.CodeGeneratorRequest, error) {
 	requests := make([]*pluginpb.CodeGeneratorRequest, len(images))
 	// alreadyUsedPaths is a map of paths that have already been added to an image.
 	//
@@ -689,7 +689,8 @@ func ImagesToCodeGeneratorRequests(
 		}
 	}
 	for i, image := range images {
-		requests[i] = imageToCodeGeneratorRequest(
+		var err error
+		requests[i], err = imageToCodeGeneratorRequest(
 			image,
 			parameter,
 			compilerVersion,
@@ -698,8 +699,11 @@ func ImagesToCodeGeneratorRequests(
 			alreadyUsedPaths,
 			nonImportPaths,
 		)
+		if err != nil {
+			return nil, err
+		}
 	}
-	return requests
+	return requests, nil
 }
 
 type newImageForProtoOptions struct {

private/bufpkg/bufimage/bufimagetesting/bufimagetesting_test.go

@@ -470,7 +470,7 @@ func TestBasic(t *testing.T) {
 	newProtoImage, err := bufimage.ImageToProtoImage(newImage)
 	require.NoError(t, err)
 	diff := cmp.Diff(protoImage, newProtoImage, protocmp.Transform())
-	require.Equal(t, "", diff)
+	require.Empty(t, diff)
 	fileDescriptorSet := &descriptorpb.FileDescriptorSet{
 		File: []*descriptorpb.FileDescriptorProto{
 			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
@@ -483,7 +483,7 @@ func TestBasic(t *testing.T) {
 		},
 	}
 	diff = cmp.Diff(fileDescriptorSet, bufimage.ImageToFileDescriptorSet(image), protocmp.Transform())
-	require.Equal(t, "", diff)
+	require.Empty(t, diff)
 	codeGeneratorRequest := &pluginpb.CodeGeneratorRequest{
 		ProtoFile: []*descriptorpb.FileDescriptorProto{
 			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
@@ -502,21 +502,32 @@ func TestBasic(t *testing.T) {
 			"b/b.proto",
 			"d/d.proto/d.proto",
 		},
+		SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileAB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
+		},
 	}
+	actualRequest, err := bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false)
+	require.NoError(t, err)
 	diff = cmp.Diff(
 		codeGeneratorRequest,
-		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, false),
+		actualRequest,
 		protocmp.Transform(),
 	)
-	require.Equal(t, "", diff)
+	require.Empty(t, diff)
 
 	// verify that includeWellKnownTypes is a no-op if includeImports is false
+	actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true)
+	require.NoError(t, err)
 	diff = cmp.Diff(
 		codeGeneratorRequest,
-		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, false, true),
+		actualRequest,
 		protocmp.Transform(),
 	)
-	require.Equal(t, "", diff)
+	require.Empty(t, diff)
 
 	codeGeneratorRequestIncludeImports := &pluginpb.CodeGeneratorRequest{
 		ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -538,13 +549,23 @@ func TestBasic(t *testing.T) {
 			"b/b.proto",
 			"d/d.proto/d.proto",
 		},
+		SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileImport),
+			testProtoImageFileToFileDescriptorProto(protoImageFileAB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
+		},
 	}
+	actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false)
+	require.NoError(t, err)
 	diff = cmp.Diff(
 		codeGeneratorRequestIncludeImports,
-		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, false),
+		actualRequest,
 		protocmp.Transform(),
 	)
-	require.Equal(t, "", diff)
+	require.Empty(t, diff)
 	newImage, err = bufimage.NewImageForCodeGeneratorRequest(codeGeneratorRequest)
 	require.NoError(t, err)
 	AssertImageFilesEqual(
@@ -580,13 +601,24 @@ func TestBasic(t *testing.T) {
 			"b/b.proto",
 			"d/d.proto/d.proto",
 		},
+		SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+			testProtoImageFileToFileDescriptorProto(protoImageFileAA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileImport),
+			testProtoImageFileToFileDescriptorProto(protoImageFileWellKnownTypeImport),
+			testProtoImageFileToFileDescriptorProto(protoImageFileAB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBA),
+			testProtoImageFileToFileDescriptorProto(protoImageFileBB),
+			testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
+		},
 	}
+	actualRequest, err = bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true)
+	require.NoError(t, err)
 	diff = cmp.Diff(
 		codeGeneratorRequestIncludeImportsAndWellKnownTypes,
-		bufimage.ImageToCodeGeneratorRequest(image, "foo", nil, true, true),
+		actualRequest,
 		protocmp.Transform(),
 	)
-	require.Equal(t, "", diff)
+	require.Empty(t, diff)
 	// imagesByDir and multiple Image tests
 	imagesByDir, err := bufimage.ImageByDir(image)
 	require.NoError(t, err)
@@ -634,6 +666,10 @@ func TestBasic(t *testing.T) {
 				"a/a.proto",
 				"a/b.proto",
 			},
+			SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+				testProtoImageFileToFileDescriptorProto(protoImageFileAA),
+				testProtoImageFileToFileDescriptorProto(protoImageFileAB),
+			},
 		},
 		{
 			ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -649,6 +685,10 @@ func TestBasic(t *testing.T) {
 				"b/a.proto",
 				"b/b.proto",
 			},
+			SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+				testProtoImageFileToFileDescriptorProto(protoImageFileBA),
+				testProtoImageFileToFileDescriptorProto(protoImageFileBB),
+			},
 		},
 		{
 			ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -659,13 +699,17 @@ func TestBasic(t *testing.T) {
 			FileToGenerate: []string{
 				"d/d.proto/d.proto",
 			},
+			SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+				testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
+			},
 		},
 	}
-	requestsFromImages := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
+	requestsFromImages, err := bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, false, false)
+	require.NoError(t, err)
 	require.Equal(t, len(codeGeneratorRequests), len(requestsFromImages))
 	for i := range codeGeneratorRequests {
 		diff = cmp.Diff(codeGeneratorRequests[i], requestsFromImages[i], protocmp.Transform())
-		require.Equal(t, "", diff)
+		require.Empty(t, diff)
 	}
 	codeGeneratorRequestsIncludeImports := []*pluginpb.CodeGeneratorRequest{
 		{
@@ -681,6 +725,11 @@ func TestBasic(t *testing.T) {
 				"import.proto",
 				"a/b.proto",
 			},
+			SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+				testProtoImageFileToFileDescriptorProto(protoImageFileAA),
+				testProtoImageFileToFileDescriptorProto(protoImageFileImport),
+				testProtoImageFileToFileDescriptorProto(protoImageFileAB),
+			},
 		},
 		{
 			ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -696,6 +745,10 @@ func TestBasic(t *testing.T) {
 				"b/a.proto",
 				"b/b.proto",
 			},
+			SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+				testProtoImageFileToFileDescriptorProto(protoImageFileBA),
+				testProtoImageFileToFileDescriptorProto(protoImageFileBB),
+			},
 		},
 		{
 			ProtoFile: []*descriptorpb.FileDescriptorProto{
@@ -706,13 +759,17 @@ func TestBasic(t *testing.T) {
 			FileToGenerate: []string{
 				"d/d.proto/d.proto",
 			},
+			SourceFileDescriptors: []*descriptorpb.FileDescriptorProto{
+				testProtoImageFileToFileDescriptorProto(protoImageFileOutlandishDirectoryName),
+			},
 		},
 	}
-	requestsFromImages = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
+	requestsFromImages, err = bufimage.ImagesToCodeGeneratorRequests(imagesByDir, "foo", nil, true, false)
+	require.NoError(t, err)
 	require.Equal(t, len(codeGeneratorRequestsIncludeImports), len(requestsFromImages))
 	for i := range codeGeneratorRequestsIncludeImports {
 		diff = cmp.Diff(codeGeneratorRequestsIncludeImports[i], requestsFromImages[i], protocmp.Transform())
-		require.Equal(t, "", diff)
+		require.Empty(t, diff)
 	}
 }
 

private/bufpkg/bufimage/util.go

@@ -24,6 +24,7 @@ import (
 	"github.com/bufbuild/buf/private/pkg/normalpath"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/uuidutil"
+	"github.com/bufbuild/protocompile/options"
 	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
@@ -416,7 +417,7 @@ func imageToCodeGeneratorRequest(
 	includeWellKnownTypes bool,
 	alreadyUsedPaths map[string]struct{},
 	nonImportPaths map[string]struct{},
-) *pluginpb.CodeGeneratorRequest {
+) (*pluginpb.CodeGeneratorRequest, error) {
 	imageFiles := image.Files()
 	request := &pluginpb.CodeGeneratorRequest{
 		ProtoFile:       make([]*descriptorpb.FileDescriptorProto, len(imageFiles)),
@@ -426,7 +427,8 @@ func imageToCodeGeneratorRequest(
 		request.Parameter = proto.String(parameter)
 	}
 	for i, imageFile := range imageFiles {
-		request.ProtoFile[i] = imageFile.FileDescriptorProto()
+		fileDescriptorProto := imageFile.FileDescriptorProto()
+		// ProtoFile should include only runtime-retained options for files to generate.
 		if isFileToGenerate(
 			imageFile,
 			alreadyUsedPaths,
@@ -435,9 +437,18 @@ func imageToCodeGeneratorRequest(
 			includeWellKnownTypes,
 		) {
 			request.FileToGenerate = append(request.FileToGenerate, imageFile.Path())
+			// Source-retention options for items in FileToGenerate are provided in SourceFileDescriptors.
+			request.SourceFileDescriptors = append(request.SourceFileDescriptors, fileDescriptorProto)
+			// And the corresponding descriptor in ProtoFile will have source-retention options stripped.
+			var err error
+			fileDescriptorProto, err = options.StripSourceRetentionOptionsFromFile(fileDescriptorProto)
+			if err != nil {
+				return nil, fmt.Errorf("failed to strip source-retention options for file %q when constructing a CodeGeneratorRequest: %w", imageFile.Path(), err)
+			}
 		}
+		request.ProtoFile[i] = fileDescriptorProto
 	}
-	return request
+	return request, nil
 }
 
 func isFileToGenerate(