From 793b7d303377ed5967de541971f5defb58f79f34 Mon Sep 17 00:00:00 2001 From: bufdev Date: Thu, 26 Sep 2024 11:21:12 -0400 Subject: [PATCH] Force usage of private/pkg/protoencoding --- .golangci.yml | 31 +++++++++++++++++++ private/buf/cmd/buf/buf_test.go | 4 +-- private/buf/cmd/buf/workspace_test.go | 4 +-- .../internal/buflintvalidate/numeric.go | 10 +++--- .../bufimageutil/bufimageutil_test.go | 4 +-- private/bufpkg/bufimage/import_tracker.go | 3 +- private/bufpkg/bufimage/util_test.go | 11 ++++--- 7 files changed, 49 insertions(+), 18 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3222e2676e..cd83af2da8 100644 --- 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 index 504ef6d2e7..d577800998 100644 --- a/private/buf/cmd/buf/buf_test.go +++ b/private/buf/cmd/buf/buf_test.go @@ -40,6 +40,7 @@ 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" @@ -47,7 +48,6 @@ import ( "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 index b6cbe6845e..2765cff8f2 100644 --- 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 index cfd7c71a01..0be8a196fa 100644 --- 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 := ×tamppb.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 index 6b501dad2d..13b4d17b4d 100644 --- 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 index 9665a3d576..6d50d8f2b8 100644 --- 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 index 5f0ad13634..46a8e8da7b 100644 --- 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())