Force usage of private/pkg/protoencoding (#3346)

bufbuild · Sep 26, 2024 · a4fb701 · a4fb701
1 parent dacb299
commit a4fb701
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 18 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -21,6 +21,15 @@ linters-settings:
       - '^log\.'
       - '^print$'
       - '^println$'
+      # Use private/pkg/protoencoding Marshalers and Unmarshalers
+      - '^proto.Marshal$'
+      - '^proto.Unmarshal$'
+      - '^proto.MarshalOptions$'
+      - '^proto.UnmarshalOptions$'
+      - '^protojson.Marshal$'
+      - '^protojson.Unmarshal$'
+      - '^protojson.MarshalOptions$'
+      - '^protojson.UnmarshalOptions$'
   govet:
     enable:
       - nilness
@@ -342,3 +351,25 @@ issues:
       # to set the source path for the location, this operation should be safe.
       path: private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/cel.go
       text: "G115:"
+    # No obvious deprecated replacement.
+    - linters:
+        - staticcheck
+      path: private/pkg/protoencoding/reparse_extensions_test.go
+      text: "SA1019:"
+    # Allow marshal and unmarshal functions in protoencoding only
+    - linters:
+      - forbidigo
+      path: private/pkg/protoencoding
+      text: "proto.Marshal"
+    - linters:
+      - forbidigo
+      path: private/pkg/protoencoding
+      text: "proto.Unmarshal"
+    - linters:
+      - forbidigo
+      path: private/pkg/protoencoding
+      text: "protojson.Marshal"
+    - linters:
+      - forbidigo
+      path: private/pkg/protoencoding
+      text: "protojson.Unmarshal"
diff --git a/private/buf/cmd/buf/buf_test.go b/private/buf/cmd/buf/buf_test.go
@@ -40,14 +40,14 @@ import (
 	"github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting"
 	"github.com/bufbuild/buf/private/pkg/command"
 	"github.com/bufbuild/buf/private/pkg/osext"
+	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
 	"github.com/bufbuild/buf/private/pkg/storage/storagetesting"
 	"github.com/bufbuild/buf/private/pkg/tracing"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
-	"google.golang.org/protobuf/proto"
 )
 
 var (
@@ -4352,7 +4352,7 @@ func testBuildLsFilesFormatImport(t *testing.T, expectedExitCode int, expectedFi
 	buffer := bytes.NewBuffer(nil)
 	testRun(t, expectedExitCode, nil, buffer, append([]string{"build", "-o", "-"}, buildArgs...)...)
 	protoImage := &imagev1.Image{}
-	err := proto.Unmarshal(buffer.Bytes(), protoImage)
+	err := protoencoding.NewWireUnmarshaler(nil).Unmarshal(buffer.Bytes(), protoImage)
 	require.NoError(t, err)
 	image, err := bufimage.NewImageForProto(protoImage)
 	require.NoError(t, err)

diff --git a/private/buf/cmd/buf/workspace_test.go b/private/buf/cmd/buf/workspace_test.go
@@ -30,11 +30,11 @@ import (
 	"github.com/bufbuild/buf/private/pkg/app/appcmd"
 	"github.com/bufbuild/buf/private/pkg/app/appcmd/appcmdtesting"
 	"github.com/bufbuild/buf/private/pkg/osext"
+	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"github.com/bufbuild/buf/private/pkg/slicesext"
 	"github.com/bufbuild/buf/private/pkg/storage/storagearchive"
 	"github.com/bufbuild/buf/private/pkg/storage/storageos"
 	"github.com/stretchr/testify/require"
-	"google.golang.org/protobuf/proto"
 )
 
 func TestWorkspaceDir(t *testing.T) {
@@ -1727,7 +1727,7 @@ func requireBuildOutputFilePaths(t *testing.T, expectedFilePathToInfo map[string
 		)...,
 	)
 	outputImage := &imagev1.Image{}
-	require.NoError(t, proto.Unmarshal(stdout.Bytes(), outputImage))
+	require.NoError(t, protoencoding.NewWireUnmarshaler(nil).Unmarshal(stdout.Bytes(), outputImage))
 
 	filesToCheck := slicesext.ToStructMap(slicesext.MapKeysToSlice(expectedFilePathToInfo))
 

diff --git a/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go b/private/bufpkg/bufcheck/bufcheckserver/internal/buflintvalidate/numeric.go
@@ -17,7 +17,7 @@ package buflintvalidate
 import (
 	"fmt"
 
-	"google.golang.org/protobuf/proto"
+	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/types/known/durationpb"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -165,12 +165,12 @@ func getNumericPointerFromValue[
 }
 
 func getTimestampFromValue(value protoreflect.Value) (*timestamppb.Timestamp, string, error) {
-	bytes, err := proto.Marshal(value.Message().Interface())
+	bytes, err := protoencoding.NewWireMarshaler().Marshal(value.Message().Interface())
 	if err != nil {
 		return nil, "", err
 	}
 	timestamp := &timestamppb.Timestamp{}
-	err = proto.Unmarshal(bytes, timestamp)
+	err = protoencoding.NewWireUnmarshaler(nil).Unmarshal(bytes, timestamp)
 	if err != nil {
 		return nil, "", err
 	}
@@ -182,12 +182,12 @@ func getTimestampFromValue(value protoreflect.Value) (*timestamppb.Timestamp, st
 }
 
 func getDurationFromValue(value protoreflect.Value) (*durationpb.Duration, string, error) {
-	bytes, err := proto.Marshal(value.Message().Interface())
+	bytes, err := protoencoding.NewWireMarshaler().Marshal(value.Message().Interface())
 	if err != nil {
 		return nil, "", err
 	}
 	duration := &durationpb.Duration{}
-	err = proto.Unmarshal(bytes, duration)
+	err = protoencoding.NewWireUnmarshaler(nil).Unmarshal(bytes, duration)
 	if err != nil {
 		return nil, "", err
 	}

diff --git a/private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go b/private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go
@@ -270,10 +270,10 @@ func runDiffTest(t *testing.T, testdataDir string, typenames []string, expectedF
 	// So we serialize and then de-serialize, and use only the filtered results to parse extensions. That
 	// way, the result will omit custom options that aren't present in the filtered set (as they will be
 	// considered unrecognized fields).
-	data, err := proto.Marshal(bufimage.ImageToFileDescriptorSet(filteredImage))
+	data, err := protoencoding.NewWireMarshaler().Marshal(bufimage.ImageToFileDescriptorSet(filteredImage))
 	require.NoError(t, err)
 	fileDescriptorSet := &descriptorpb.FileDescriptorSet{}
-	err = proto.UnmarshalOptions{Resolver: filteredImage.Resolver()}.Unmarshal(data, fileDescriptorSet)
+	err = protoencoding.NewWireUnmarshaler(filteredImage.Resolver()).Unmarshal(data, fileDescriptorSet)
 	require.NoError(t, err)
 
 	files, err := protodesc.NewFiles(fileDescriptorSet)

diff --git a/private/bufpkg/bufimage/import_tracker.go b/private/bufpkg/bufimage/import_tracker.go
@@ -212,8 +212,7 @@ func (t *importTracker) findUsedImportsInMessageValue(file *imagev1.ImageFile, m
 		// process Any messages that might be nested inside this one
 		value := msg.Get(valueField).Bytes()
 		nestedMessage := msgType.New()
-		err = proto.UnmarshalOptions{Resolver: t.resolver}.Unmarshal(value, nestedMessage.Interface())
-		if err != nil {
+		if err := protoencoding.NewWireUnmarshaler(t.resolver).Unmarshal(value, nestedMessage.Interface()); err != nil {
 			// bytes are not valid; skip it
 			return
 		}

diff --git a/private/bufpkg/bufimage/util_test.go b/private/bufpkg/bufimage/util_test.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
 	imagev1 "github.com/bufbuild/buf/private/gen/proto/go/buf/alpha/image/v1"
+	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"github.com/bufbuild/buf/private/pkg/uuidutil"
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/require"
@@ -45,7 +46,7 @@ func TestStripBufExtensionField(t *testing.T) {
 			},
 		},
 	}
-	dataToBeStripped, err := proto.Marshal(file)
+	dataToBeStripped, err := protoencoding.NewWireMarshaler().Marshal(file)
 	require.NoError(t, err)
 
 	otherData := protowire.AppendTag(nil, 122, protowire.BytesType)
@@ -164,11 +165,11 @@ func TestImageToProtoPreservesUnrecognizedFields(t *testing.T) {
 	require.Equal(t, otherData, []byte(protoImageFile.ProtoReflect().GetUnknown()))
 
 	// now round-trip it back through
-	imageFileBytes, err := proto.Marshal(protoImageFile)
+	imageFileBytes, err := protoencoding.NewWireMarshaler().Marshal(protoImageFile)
 	require.NoError(t, err)
 
 	roundTrippedFileDescriptor := &descriptorpb.FileDescriptorProto{}
-	err = proto.Unmarshal(imageFileBytes, roundTrippedFileDescriptor)
+	err = protoencoding.NewWireUnmarshaler(nil).Unmarshal(imageFileBytes, roundTrippedFileDescriptor)
 	require.NoError(t, err)
 	// unrecognized now includes image file's buf extension
 	require.Greater(t, len(roundTrippedFileDescriptor.ProtoReflect().GetUnknown()), len(otherData))
@@ -199,11 +200,11 @@ func TestImageToProtoPreservesUnrecognizedFields(t *testing.T) {
 
 	// double-check via round-trip, to make sure resulting image file equals the input
 	// (to verify that the original unknown bytes byf extension didn't interfere)
-	imageFileBytes, err = proto.Marshal(protoImageFile)
+	imageFileBytes, err = protoencoding.NewWireMarshaler().Marshal(protoImageFile)
 	require.NoError(t, err)
 
 	roundTrippedImageFile := &imagev1.ImageFile{}
-	err = proto.Unmarshal(imageFileBytes, roundTrippedImageFile)
+	err = protoencoding.NewWireUnmarshaler(nil).Unmarshal(imageFileBytes, roundTrippedImageFile)
 	require.NoError(t, err)
 
 	diff := cmp.Diff(protoImageFile, roundTrippedImageFile, protocmp.Transform())