Switch to proto.CloneOf (#3757)

Author: pkwarren

.golangci.yml

@@ -70,6 +70,8 @@ linters:
         - pattern: ^(proto|prototext|protojson|protoyaml).Unmarshal$
         - pattern: ^(proto|prototext|protojson|protoyaml).MarshalOptions$
         - pattern: ^(proto|prototext|protojson|protoyaml).UnmarshalOptions$
+        - pattern: ^proto\.Clone$
+          msg: please use proto.CloneOf
     govet:
       enable:
         - nilness
@@ -419,6 +421,8 @@ linters:
         # to set the source path for the location, this operation should be safe.
         path: private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/predefined_rules.go
         text: 'G115:'
+issues:
+  max-same-issues: 0
 formatters:
   enable:
     - gci

make/go/dep_buf.mk

@@ -7,8 +7,8 @@ $(call _assert_var,CACHE_VERSIONS)
 $(call _assert_var,CACHE_BIN)
 
 # Settable
-# https://github.com/bufbuild/buf/releases 20250328 checked 20250328
-BUF_VERSION ?= v1.51.0
+# https://github.com/bufbuild/buf/releases 20250408 checked 20250408
+BUF_VERSION ?= v1.52.1
 # Settable
 #
 # If set, this path will be installed every time someone depends on $(BUF)

private/bufpkg/bufimage/bufimage.go

@@ -325,13 +325,7 @@ func CloneImage(image Image) (Image, error) {
 
 // CloneImageFile returns a deep copy of the given image file.
 func CloneImageFile(imageFile ImageFile) (ImageFile, error) {
-	clonedProto := proto.Clone(imageFile.FileDescriptorProto())
-	clonedDescriptor, ok := clonedProto.(*descriptorpb.FileDescriptorProto)
-	if !ok {
-		// Shouldn't actually be possible...
-		return nil, fmt.Errorf("failed to clone image file %q: input %T; clone is %T but expecting %T",
-			imageFile.Path(), imageFile, clonedProto, (*descriptorpb.FileDescriptorProto)(nil))
-	}
+	clonedDescriptor := proto.CloneOf(imageFile.FileDescriptorProto())
 	originalUnusedDeps := imageFile.UnusedDependencyIndexes()
 	unusedDeps := make([]int32, len(originalUnusedDeps))
 	copy(unusedDeps, originalUnusedDeps)

private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go

@@ -26,7 +26,6 @@ import (
 	"github.com/bufbuild/buf/private/bufpkg/bufimage"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule/bufmoduletesting"
-	imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1"
 	"github.com/bufbuild/buf/private/pkg/app/appext"
 	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
@@ -545,7 +544,7 @@ func runFilterImage(t *testing.T, image bufimage.Image, opts ...ImageFilterOptio
 	protoImage, err := bufimage.ImageToProtoImage(filteredImage)
 	require.NoError(t, err)
 	// Clone here as `bufimage.NewImageForProto` mutates protoImage.
-	protoImage, _ = proto.Clone(protoImage).(*imagev1.Image) // Safe to assert.
+	protoImage = proto.CloneOf(protoImage)
 	filteredImage, err = bufimage.NewImageForProto(protoImage)
 	require.NoError(t, err)
 
@@ -751,8 +750,7 @@ func benchmarkFilterImage(b *testing.B, opts ...bufimage.BuildImageOption) {
 				b.StopTimer()
 				imageFiles := make([]bufimage.ImageFile, len(benchmarkCase.image.Files()))
 				for j, imageFile := range benchmarkCase.image.Files() {
-					clone, ok := proto.Clone(imageFile.FileDescriptorProto()).(*descriptorpb.FileDescriptorProto)
-					require.True(b, ok)
+					clone := proto.CloneOf(imageFile.FileDescriptorProto())
 					var err error
 					imageFiles[j], err = bufimage.NewImageFile(clone, nil, uuid.Nil, "", "", false, false, nil)
 					require.NoError(b, err)

private/pkg/protoencoding/strip_legacy_options.go

@@ -15,8 +15,6 @@
 package protoencoding
 
 import (
-	"fmt"
-
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/descriptorpb"
 )
@@ -62,11 +60,7 @@ func stripLegacyOptionsFromFile(file *descriptorpb.FileDescriptorProto) (*descri
 			continue
 		}
 		if !cloned {
-			newFile, err := clone(file)
-			if err != nil {
-				return nil, err
-			}
-			file = newFile
+			file = proto.CloneOf(file)
 			cloned = true
 		}
 		file.MessageType[i] = newDescriptor
@@ -77,11 +71,7 @@ func stripLegacyOptionsFromFile(file *descriptorpb.FileDescriptorProto) (*descri
 	}
 	if newExts != nil {
 		if !cloned {
-			newFile, err := clone(file)
-			if err != nil {
-				return nil, err
-			}
-			file = newFile
+			file = proto.CloneOf(file)
 			cloned = true
 		}
 		file.Extension = newExts
@@ -100,11 +90,7 @@ func stripLegacyOptionsFromMessage(message *descriptorpb.DescriptorProto) (*desc
 	if message.GetOptions().GetMessageSetWireFormat() {
 		// Strip this option since the Go runtime does not support
 		// creating protoreflect.Descriptor instances with this set.
-		newMessage, err := clone(message)
-		if err != nil {
-			return nil, err
-		}
-		message = newMessage
+		message = proto.CloneOf(message)
 		cloned = true
 		message.Options.MessageSetWireFormat = nil
 	}
@@ -117,11 +103,7 @@ func stripLegacyOptionsFromMessage(message *descriptorpb.DescriptorProto) (*desc
 			continue
 		}
 		if !cloned {
-			newMessage, err := clone(message)
-			if err != nil {
-				return nil, err
-			}
-			message = newMessage
+			message = proto.CloneOf(message)
 			cloned = true
 		}
 		message.Field[i] = newDescriptor
@@ -136,11 +118,7 @@ func stripLegacyOptionsFromMessage(message *descriptorpb.DescriptorProto) (*desc
 			continue
 		}
 		if !cloned {
-			newMessage, err := clone(message)
-			if err != nil {
-				return nil, err
-			}
-			message = newMessage
+			message = proto.CloneOf(message)
 			cloned = true
 		}
 		message.NestedType[i] = newDescriptor
@@ -151,11 +129,7 @@ func stripLegacyOptionsFromMessage(message *descriptorpb.DescriptorProto) (*desc
 	}
 	if newExtRanges != nil {
 		if !cloned {
-			newMessage, err := clone(message)
-			if err != nil {
-				return nil, err
-			}
-			message = newMessage
+			message = proto.CloneOf(message)
 			cloned = true
 		}
 		message.ExtensionRange = newExtRanges
@@ -166,11 +140,7 @@ func stripLegacyOptionsFromMessage(message *descriptorpb.DescriptorProto) (*desc
 	}
 	if newExts != nil {
 		if !cloned {
-			newMessage, err := clone(message)
-			if err != nil {
-				return nil, err
-			}
-			message = newMessage
+			message = proto.CloneOf(message)
 			cloned = true
 		}
 		message.Extension = newExts
@@ -192,10 +162,7 @@ func stripLegacyOptionsFromField(field *descriptorpb.FieldDescriptorProto) (*des
 	// creating protoreflect.Descriptor instances with this set.
 	// Buf CLI doesn't actually support weak dependencies, so
 	// there should be no practical consequences of removing this.
-	newField, err := clone(field)
-	if err != nil {
-		return nil, err
-	}
+	newField := proto.CloneOf(field)
 	newField.Options.Weak = nil
 	return newField, nil
 }
@@ -267,10 +234,7 @@ func stripLegacyOptionsFromExtensionRanges(extRanges []*descriptorpb.DescriptorP
 			continue
 		}
 		if extRange.GetEnd() > maxTagNumber+1 /* extension range end is exclusive */ {
-			newExtRange, err := clone(extRange)
-			if err != nil {
-				return nil, err
-			}
+			newExtRange := proto.CloneOf(extRange)
 			newExtRange.End = proto.Int32(maxTagNumber + 1)
 			if newExtRanges == nil {
 				// initialize to everything so far except current item (that we're replacing)
@@ -286,13 +250,3 @@ func stripLegacyOptionsFromExtensionRanges(extRanges []*descriptorpb.DescriptorP
 	}
 	return newExtRanges, nil
 }
-
-func clone[M proto.Message](message M) (M, error) {
-	clone := proto.Clone(message)
-	newMessage, isFileProto := clone.(M)
-	if !isFileProto {
-		var zero M
-		return zero, fmt.Errorf("proto.Clone returned unexpected value: %T instead of %T", clone, message)
-	}
-	return newMessage, nil
-}