Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- 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 `buf convert` panic when converting images that contain files without a `syntax`
declaration or with unused imports.
- Fix `buf generate` with `strategy: directory` and `include_imports: true` adding imports
to the `CodeGeneratorRequest` of the directory that imports them. Imports are now split
by directory into their own requests, the same as non-imports.
Expand Down
24 changes: 24 additions & 0 deletions cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3921,6 +3921,30 @@ func TestConvertInvalidTypeName(t *testing.T) {
)
}

func TestConvertImageWithSourceCodeInfoExtension(t *testing.T) {
t.Parallel()
// The built image carries Buf's SourceCodeInfo extension, which is not part
// of the schema and so must not be resolved when converting.
tempDir := t.TempDir()
schemaDir := filepath.Join(convertTestDataDir, "source_code_info_extension")
imagePath := filepath.Join(tempDir, "image.binpb")
testRunStdout(t, nil, 0, ``, "build", schemaDir, "-o", imagePath)
testRun(
t,
0,
nil,
nil,
"convert",
schemaDir,
"--type",
"google.protobuf.FileDescriptorSet",
"--from",
imagePath,
"--to",
"-#format=json",
)
}

func TestConvert(t *testing.T) {
t.Parallel()
t.Run("binpb-to-json-file-proto", func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// No syntax and an unused import, so that buf build attaches its
// SourceCodeInfo extension to this file.
package a;

import "google/protobuf/descriptor.proto";

message A {}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
version: v2
6 changes: 6 additions & 0 deletions private/buf/bufconvert/bufconvert.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ func checkNoMessageSetWireFormat(descriptor protoreflect.MessageDescriptor, imag
// In order to create a protoreflect.MessageDescriptor, we may have already stripped
// any message_set_wire_format option. So we must examine the file descriptor proto
// in the image to see if the option is set. If it is, we cannot support this message.
if descriptor.IsPlaceholder() {
// The type is not in the image, such as when it was built with
// --exclude-imports, so there is nothing to check. Its fields are
// unknown and so are left as unknown bytes when unmarshaling.
return nil
}
name := descriptor.FullName()
if slices.Contains(checked, name) {
return nil
Expand Down
57 changes: 57 additions & 0 deletions private/buf/bufconvert/bufconvert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,63 @@ func TestImageWithoutMessageSetWireFormatResolution(t *testing.T) {
checker.succeeds(noResolveImage.Resolver().FindEnumByName("foo.bar.Enum"))
}

func TestImageWithoutMessageSetWireFormatResolutionPlaceholder(t *testing.T) {
t.Parallel()
// Types missing from the image, such as when it was built with
// --exclude-imports, resolve to placeholders that cannot be checked.
file := &descriptorpb.FileDescriptorProto{
Name: new("test.proto"),
Package: new("foo.bar"),
Dependency: []string{"missing.proto"},
MessageType: []*descriptorpb.DescriptorProto{
{
Name: new("Baz"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: new("missing"),
Number: new(int32(1)),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: new(".missing.Missing"),
},
},
ExtensionRange: []*descriptorpb.DescriptorProto_ExtensionRange{
{Start: new(int32(100)), End: new(int32(200))},
},
},
},
Extension: []*descriptorpb.FieldDescriptorProto{
{
Name: new("missing_ext"),
Number: new(int32(100)),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: new(".missing.Missing"),
Extendee: new(".foo.bar.Baz"),
},
},
}
imageFile, err := bufimage.NewImageFile(
file,
nil,
uuid.UUID{},
file.GetName(),
file.GetName(),
false,
false,
nil,
)
require.NoError(t, err)
image, err := bufimage.NewImage([]bufimage.ImageFile{imageFile})
require.NoError(t, err)
resolver := ImageWithoutMessageSetWireFormatResolution(image).Resolver()

checker := resultChecker{t}
checker.succeeds(resolver.FindMessageByName("foo.bar.Baz"))
checker.succeeds(resolver.FindExtensionByName("foo.bar.missing_ext"))
checker.succeeds(resolver.FindExtensionByNumber("foo.bar.Baz", 100))
}

func TestFindMessageInFile(t *testing.T) {
t.Parallel()
t.Run("no-package", func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
syntax = "proto3"; import "types/existing_field.proto"; message M { example.types.ExistingField f = 1; }
14 changes: 8 additions & 6 deletions private/bufpkg/bufimage/build_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,9 @@ func BuildImageFromOpener(
}

// resolverForFDS normalizes the given [*descriptorpb.FileDescriptorSet] by
// marshaling and re-parsing it, then builds a [protoencoding.Resolver] with
// Buf's custom descriptor extensions recognised, ready for [fileDescriptorSetToImage].
// marshaling and re-parsing it with Buf's custom descriptor extensions
// recognised, then returns it with a [protoencoding.Resolver] for its files,
// ready for [fileDescriptorSetToImage].
//
// The marshal/unmarshal round-trip is necessary because [fdp.DescriptorProto]
// stores option values as unknown bytes via SetUnknown. Re-parsing converts those
Expand All @@ -301,15 +302,16 @@ func resolverForFDS(fds *descriptorpb.FileDescriptorSet) (*descriptorpb.FileDesc
// input exactly (including any vendored google/protobuf/descriptor.proto).
// Buf's descriptor extensions are supplied by a fallback resolver consulted
// only when the file graph cannot resolve a descriptor, so ReparseExtensions
// can recognise them without altering the graph.
// can recognise them without altering the graph. The fallback is only used
// for reparsing: the returned resolver must resolve only the image's files.
fileResolver := protoencoding.NewLazyResolver(normalizedFDS.File...)
resolver := protoencoding.CombineResolvers(fileResolver, bufDescriptorResolver)
reparseResolver := protoencoding.CombineResolvers(fileResolver, bufDescriptorResolver)
for _, fileDescriptor := range normalizedFDS.File {
if err := protoencoding.ReparseExtensions(resolver, fileDescriptor.ProtoReflect()); err != nil {
if err := protoencoding.ReparseExtensions(reparseResolver, fileDescriptor.ProtoReflect()); err != nil {
return nil, nil, err
}
}
return normalizedFDS, resolver, nil
return normalizedFDS, fileResolver, nil
}

// fileDescriptorSetToImage is a helper function that converts a [descriptorpb.FileDescriptorSet]
Expand Down
Loading