Skip to content

Commit

Permalink
Force usage of private/pkg/protoencoding
Browse files Browse the repository at this point in the history
  • Loading branch information
bufdev committed Sep 26, 2024
1 parent dacb299 commit 793b7d3
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 18 deletions.
31 changes: 31 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
4 changes: 2 additions & 2 deletions private/buf/cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions private/buf/cmd/buf/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions private/bufpkg/bufimage/bufimageutil/bufimageutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions private/bufpkg/bufimage/import_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
11 changes: 6 additions & 5 deletions private/bufpkg/bufimage/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 793b7d3

Please sign in to comment.