lint

decode.go

@@ -393,7 +393,7 @@ func (u *unmarshaler) unmarshalInteger(node *yaml.Node, bits int) int64 {
 
 func getFieldNames(fields protoreflect.FieldDescriptors) []protoreflect.Name {
 	names := make([]protoreflect.Name, 0, fields.Len())
-	for i := 0; i < fields.Len(); i++ {
+	for i := range fields.Len() {
 		names = append(names, fields.Get(i).Name())
 		if i > 5 {
 			names = append(names, protoreflect.Name("..."))
@@ -405,7 +405,7 @@ func getFieldNames(fields protoreflect.FieldDescriptors) []protoreflect.Name {
 
 func getEnumValueNames(values protoreflect.EnumValueDescriptors) []protoreflect.Name {
 	names := make([]protoreflect.Name, 0, values.Len())
-	for i := 0; i < values.Len(); i++ {
+	for i := range values.Len() {
 		names = append(names, values.Get(i).Name())
 		if i > 5 {
 			names = append(names, protoreflect.Name("..."))
@@ -1074,7 +1074,7 @@ func parseFieldPath(path string) ([]string, error) {
 }
 
 func parseNextFieldName(path string) (string, string) {
-	for i := 0; i < len(path); i++ {
+	for i := range len(path) {
 		switch path[i] {
 		case '.':
 			return path[:i], path[i:]
@@ -1105,7 +1105,7 @@ func parseNextValue(path string) (string, string) {
 		return path, ""
 	}
 	// Go til the trailing ']'
-	for i := 0; i < len(path); i++ {
+	for i := range len(path) {
 		if path[i] == ']' {
 			return path[:i], path[i+1:]
 		}

decode_test.go

@@ -15,40 +15,15 @@
 package protoyaml
 
 import (
-	"io"
-	"os"
-	"path/filepath"
-	"strings"
 	"testing"
 
-	"github.com/bufbuild/protovalidate-go"
 	testv1 "github.com/bufbuild/protoyaml-go/internal/gen/proto/buf/protoyaml/test/v1"
-	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/durationpb"
 )
 
-func TestGoldenFiles(t *testing.T) {
-	t.Parallel()
-	// Walk the test data directory for .yaml files
-	if err := filepath.Walk("internal/testdata", func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if !info.IsDir() && strings.HasSuffix(path, ".yaml") {
-			t.Run(path, func(t *testing.T) {
-				t.Parallel()
-				testRunYAMLFile(t, path)
-			})
-		}
-		return nil
-	}); err != nil {
-		t.Fatal(err)
-	}
-}
-
 func TestParseDuration(t *testing.T) {
 	t.Parallel()
 	for _, testCase := range []struct {
@@ -99,7 +74,6 @@ func TestParseDuration(t *testing.T) {
 		{Input: "1.5h1m1.5s1.5h1m1.5s", Expected: &durationpb.Duration{Seconds: 10923}},
 		{Input: "1h1m1s1ms1us1μs1µs1ns", Expected: &durationpb.Duration{Seconds: 3661, Nanos: 1003001}},
 	} {
-		testCase := testCase
 		t.Run(testCase.Input, func(t *testing.T) {
 			t.Parallel()
 			actual, err := ParseDuration(testCase.Input)
@@ -122,66 +96,3 @@ func TestExtension(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "hi", proto.GetExtension(actual, testv1.E_P2TStringExt))
 }
-
-func testRunYAML(path string, msg proto.Message) error {
-	// Read the test file
-	file, err := os.Open(path)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-	data, err := io.ReadAll(file)
-	if err != nil {
-		return err
-	}
-
-	validator, err := protovalidate.New()
-	if err != nil {
-		return err
-	}
-
-	return UnmarshalOptions{
-		Validator: validator,
-		Path:      path,
-	}.Unmarshal(data, msg)
-}
-
-func testRunYAMLFile(t *testing.T, testFile string) {
-	t.Helper()
-
-	var err error
-	switch {
-	case strings.HasSuffix(testFile, ".proto2test.yaml"):
-		err = testRunYAML(testFile, &testv1.Proto2Test{})
-	case strings.HasSuffix(testFile, ".proto3test.yaml"):
-		err = testRunYAML(testFile, &testv1.Proto3Test{})
-	case strings.HasSuffix(testFile, ".const.yaml"):
-		err = testRunYAML(testFile, &testv1.ConstValues{})
-	case strings.HasSuffix(testFile, ".validate.yaml"):
-		err = testRunYAML(testFile, &testv1.ValidateTest{})
-	default:
-		t.Fatalf("Unknown test file extension: %s", testFile)
-	}
-	var errorText string
-	if err != nil {
-		errorText = err.Error()
-	}
-	// Read the expected file
-	expectedFileName := strings.TrimSuffix(testFile, ".yaml") + ".txt"
-	expectedFile, err := os.Open(expectedFileName)
-	var expectedData []byte
-	if err != nil {
-		t.Fatal(err)
-	} else {
-		defer expectedFile.Close()
-		expectedData, err = io.ReadAll(expectedFile)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-	expectedText := string(expectedData)
-	if expectedText != errorText {
-		diff := cmp.Diff(expectedText, errorText)
-		t.Errorf("%s: Test %s failed:\nExpected:\n%s\nActual:\n%s\nDiff:\n%s", expectedFileName, testFile, expectedText, errorText, diff)
-	}
-}

encode.go

@@ -16,6 +16,7 @@ package protoyaml
 
 import (
 	"bytes"
+	"encoding/json"
 
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
@@ -66,28 +67,42 @@ func (o MarshalOptions) Marshal(message proto.Message) ([]byte, error) {
 	if err != nil {
 		return nil, err
 	}
-	// Use a json.Node to preserve the order of fields.
-	jsonNode := &yaml.Node{}
-	if err := yaml.Unmarshal(data, jsonNode); err != nil {
+	yamlVal, err := jsonDataToYAML(data)
+	if err != nil {
 		return nil, err
 	}
 
-	// Clear the style of all nodes to avoid emitting style information in the output.
-	clearStyle(jsonNode)
-	if jsonNode.Kind == yaml.DocumentNode {
-		jsonNode = jsonNode.Content[0]
-	}
-
 	// Write the JSON back out as YAML
 	buffer := &bytes.Buffer{}
 	encoder := yaml.NewEncoder(buffer)
 	encoder.SetIndent(o.Indent)
-	if err := encoder.Encode(jsonNode); err != nil {
+	if err := encoder.Encode(yamlVal); err != nil {
 		return nil, err
 	}
 	return buffer.Bytes(), nil
 }
 
+func jsonDataToYAML(data []byte) (interface{}, error) {
+	// YAML unmarshal preserves the order of fields, but is more restrictive than JSON.
+	// Prefer it if the data is valid YAML.
+	jsonNode := &yaml.Node{}
+	if err := yaml.Unmarshal(data, jsonNode); err == nil {
+		if jsonNode.Kind == yaml.DocumentNode {
+			jsonNode = jsonNode.Content[0]
+		}
+		clearStyle(jsonNode)
+		return jsonNode, nil
+	}
+
+	// If the data is not valid YAML (e.g. a string contains control characters),
+	// fall back to JSON unmarshal, which loses field order, but is more permissive.
+	var jsonValue interface{}
+	if err := json.Unmarshal(data, &jsonValue); err != nil {
+		return nil, err
+	}
+	return jsonValue, nil
+}
+
 func clearStyle(node *yaml.Node) {
 	node.Style = 0
 	for _, child := range node.Content {

go.mod

@@ -1,6 +1,6 @@
 module github.com/bufbuild/protoyaml-go
 
-go 1.20
+go 1.22
 
 require (
 	buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.33.0-20240401165935-b983156c5e99.1

go.sum

@@ -8,14 +8,17 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/envoyproxy/protoc-gen-validate v1.0.4 h1:gVPz/FMfvh57HdSJQyvBtF00j8JU4zdyUgIUNhlgg0A=
+github.com/envoyproxy/protoc-gen-validate v1.0.4/go.mod h1:qys6tmnRsYrQqIhm2bvKZH4Blx/1gTIZ2UKVY1M+Yew=
 github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
 github.com/google/cel-go v0.20.1 h1:nDx9r8S3L4pE61eDdt8igGj8rf5kjYR3ILxWIpWNi84=
 github.com/google/cel-go v0.20.1/go.mod h1:kWcIzTsPX0zmQ+H3TirHstLLf9ep5QTsZBN9u4dOYLg=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
+github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
+github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stoewer/go-strcase v1.3.0 h1:g0eASXYtp+yvN9fK8sH94oCIk0fau9uV1/ZdJ0AVEzs=
@@ -42,6 +45,7 @@ google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGm
 google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
+gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

internal/cmd/generate-txt-testdata/main.go

@@ -21,10 +21,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/bufbuild/protovalidate-go"
-	"github.com/bufbuild/protoyaml-go"
-	testv1 "github.com/bufbuild/protoyaml-go/internal/gen/proto/buf/protoyaml/test/v1"
-	"google.golang.org/protobuf/proto"
+	"github.com/bufbuild/protoyaml-go/internal/protoyamltest/golden"
 )
 
 func main() {
@@ -86,46 +83,5 @@ func tryParse(filePath string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-
-	validator, err := protovalidate.New()
-	if err != nil {
-		return "", err
-	}
-
-	options := protoyaml.UnmarshalOptions{
-		Validator: validator,
-		Path:      filePath,
-	}
-	var val proto.Message
-	switch {
-	case strings.HasSuffix(filePath, ".proto2test.yaml"):
-		testCase := &testv1.Proto2Test{}
-		err = options.Unmarshal(data, testCase)
-		val = testCase
-	case strings.HasSuffix(filePath, ".proto3test.yaml"):
-		testCase := &testv1.Proto3Test{}
-		err = options.Unmarshal(data, testCase)
-		val = testCase
-	case strings.HasSuffix(filePath, ".const.yaml"):
-		testCase := &testv1.ConstValues{}
-		err = options.Unmarshal(data, testCase)
-		val = testCase
-	case strings.HasSuffix(filePath, ".validate.yaml"):
-		testCase := &testv1.ValidateTest{}
-		err = options.Unmarshal(data, testCase)
-		val = testCase
-	default:
-		return "", fmt.Errorf("unknown file type: %s", filePath)
-	}
-	if err != nil {
-		return err.Error(), nil
-	}
-	opts := protoyaml.MarshalOptions{
-		UseProtoNames: true,
-	}
-	yamlData, err := opts.Marshal(val)
-	if err != nil {
-		return "", err
-	}
-	return string(yamlData), nil
+	return golden.GenGoldenContent(filePath, data)
 }

internal/protoyamltest/combine.go

@@ -24,6 +24,9 @@ import (
 	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
+// InterestingTestValues returns a list of interesting values for testing.
+//
+// For example extrema, zero values, and other values that exercise edge cases.
 func InterestingTestValues() []*proto3.TestAllTypes {
 	var interestingValues []*proto3.TestAllTypes
 	for _, value := range []bool{true, false} {
@@ -63,7 +66,7 @@ func InterestingTestValues() []*proto3.TestAllTypes {
 		)
 	}
 	fields := (&proto3.TestAllTypes{}).ProtoReflect().Descriptor().Fields()
-	for i := 0; i < fields.Len(); i++ {
+	for i := range fields.Len() {
 		interestingValues = append(interestingValues,
 			interestingFieldValues(fields.Get(i))...)
 	}
@@ -75,7 +78,7 @@ func interestingMessageFieldValues(field protoreflect.FieldDescriptor) []*proto3
 	switch {
 	case field.IsList():
 		listVal := &proto3.TestAllTypes{}
-		for j := 0; j < 3; j++ {
+		for j := range 3 {
 			newVal := listVal.ProtoReflect().Get(field).List().NewElement()
 			PopulateMessage(newVal.Message().Interface(), int64(j))
 			listVal.ProtoReflect().Mutable(field).List().Append(newVal)
@@ -291,7 +294,7 @@ func interestingFieldValues(field protoreflect.FieldDescriptor) []*proto3.TestAl
 func interestingEnumValues(enum protoreflect.EnumDescriptor) []protoreflect.EnumNumber {
 	values := enum.Values()
 	result := []protoreflect.EnumNumber{}
-	for i := 0; i < values.Len(); i++ {
+	for i := range values.Len() {
 		result = append(result, values.Get(i).Number())
 	}
 	if enum.FullName() != "google.protobuf.NullValue" {