Skip to content

Commit

Permalink
☠️ Fix bones for recursive message types (#42)
Browse files Browse the repository at this point in the history
Fix `jig bones` for recursive message types so that it doesn't cause an
infinite recursion:

	runtime: goroutine stack exceeds 1000000000-byte limit

Add a stack to the formatter tracking the the message names. Rename
FormatOptions to Formatter to better reflect the carried state and type
alias it back to FormatOptions for compatibility and external use.

Pull-Request: #42
  • Loading branch information
juliaogris authored Apr 20, 2022
1 parent be4c9bb commit 7f4dd69
Show file tree
Hide file tree
Showing 13 changed files with 257 additions and 218 deletions.
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ all: build test check-coverage lint lint-proto ## build, test, check coverage an

ci: clean check-uptodate all ## Full clean build and up-to-date checks as run on CI

check-uptodate: proto tidy
test -z "$$(git status --porcelain -- go.mod go.sum '*.pb')" || { git status; false; }
check-uptodate: proto golden tidy
test -z "$$(git status --porcelain -- go.mod go.sum '*.pb' '*.jsonnet' '*.js')" || { git status; false; }

clean:: ## Remove generated files
-rm -rf $(O)
Expand Down Expand Up @@ -49,7 +49,7 @@ cover: test ## Show test coverage in your browser
go tool cover -html=$(COVERFILE)

RUN_BONES = $(O)/jig bones --force --language=$(1) --quote-style=$(2) --proto-set pb/$(3)/$(4) --method-dir bones/testdata/golden/$(3)-$(2)
golden: build ## Generate golden test files
golden: build proto ## Generate golden test files
$(call RUN_BONES,jsonnet,double,exemplar,exemplar.pb)
$(call RUN_BONES,jsonnet,double,greet,greeter.pb)
$(call RUN_BONES,jsonnet,single,exemplar,exemplar.pb)
Expand Down
87 changes: 51 additions & 36 deletions bones/exemplar.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,28 @@ import (
"fmt"
"strings"

"golang.org/x/exp/slices"
"google.golang.org/protobuf/reflect/protoreflect"
)

// FormatOptions is a configurable target language exemplar generator.
type FormatOptions struct {
// Formatter is a configurable target language exemplar generator.
type Formatter struct {
Lang Lang
QuoteStyle QuoteStyle

messageStack []protoreflect.FullName
}

type FormatOptions = Formatter

// Return the file extension for the given language.
func (o *FormatOptions) Extension() string {
return "." + o.Lang.String()
func (f *Formatter) Extension() string {
return "." + f.Lang.String()
}

// Return the file extension for the given language.
func (o *FormatOptions) quote(s string) string {
if o.QuoteStyle == Double {
func (f *Formatter) quote(s string) string {
if f.QuoteStyle == Double {
return `"` + s + `"`
}
return `'` + s + `'`
Expand All @@ -29,9 +34,9 @@ func (o *FormatOptions) quote(s string) string {
// MethodExemplar returns an exemplar for a method, with an exemplar for the
// input message as a comment and a function returning an exemplar of the
// output message as the method implementation.
func (o *FormatOptions) MethodExemplar(md protoreflect.MethodDescriptor) exemplar {
func (f *Formatter) MethodExemplar(md protoreflect.MethodDescriptor) exemplar {
// Format the input message exemplar as a comment
ime := o.MessageExemplar(md.Input())
ime := f.MessageExemplar(md.Input())
ime.append(",")
if md.IsStreamingClient() && !md.IsStreamingServer() {
ime.nest("stream: [", "],")
Expand All @@ -42,14 +47,14 @@ func (o *FormatOptions) MethodExemplar(md protoreflect.MethodDescriptor) exempla
ime.prefix("// ")

// Format the output message exemplar
ome := o.MessageExemplar(md.Output())
ome := f.MessageExemplar(md.Output())
ome.append(",")
if md.IsStreamingServer() {
ome.nest("stream: [", "],")
} else {
ome.prepend("response: ")
}
if o.Lang == Jsonnet {
if f.Lang == Jsonnet {
ome.nest("function(input) {", "}")
} else {
ome.nest("return {", "}")
Expand Down Expand Up @@ -83,23 +88,30 @@ func (o *FormatOptions) MethodExemplar(md protoreflect.MethodDescriptor) exempla
// with a field for every message field. Each field value is an exemplar of the
// type of the field. Oneof fields are emitted as comments as a message should
// not have more than one oneof specified.
func (o *FormatOptions) MessageExemplar(md protoreflect.MessageDescriptor) exemplar {
func (f *Formatter) MessageExemplar(md protoreflect.MessageDescriptor) exemplar {
var e exemplar

if strings.HasPrefix(string(md.FullName()), "google.protobuf.") {
if e = o.WellKnownExemplar(md); len(e.lines) > 0 {
if e = f.WellKnownExemplar(md); len(e.lines) > 0 {
return e
}
}

if slices.Contains(f.messageStack, md.FullName()) {
e.line("{}")
return e
}

f.messageStack = append(f.messageStack, md.FullName())
for _, fd := range fields(md) {
fe := o.FieldExemplar(fd)
fe := f.FieldExemplar(fd)
if fd.ContainingOneof() != nil {
// Comment out one-of fields since they should not all be present.
fe.prefix("// ")
}
e.extend(fe)
}
f.messageStack = f.messageStack[:len(f.messageStack)-1]

e.nest("{", "}")
return e
Expand All @@ -110,52 +122,52 @@ func (o *FormatOptions) MessageExemplar(md protoreflect.MessageDescriptor) exemp
// JSON as a single field, rather than as an object.
//
// https://developers.google.com/protocol-buffers/docs/reference/google.protobuf
func (o *FormatOptions) WellKnownExemplar(md protoreflect.MessageDescriptor) exemplar {
func (f *Formatter) WellKnownExemplar(md protoreflect.MessageDescriptor) exemplar {
var e exemplar
switch string(md.Name()) {
case "Api", "Enum", "EnumValue", "Field", "Method", "Mixin", "Option", "SourceContext", "Type":
return e // empty exemplar. will be formatted as a message
case "Any":
// Emit an Any that can be read back in without modification
// Duration chosen at random, almost. Also for its simplicity.
e.line(o.quote("@type") + ": " + o.quote("type.googleapis.com/google.protobuf.Duration") + ",")
e.line("value: " + o.quote("0s") + ",")
e.line(f.quote("@type") + ": " + f.quote("type.googleapis.com/google.protobuf.Duration") + ",")
e.line("value: " + f.quote("0s") + ",")
e.nest("{", "}")
case "BoolValue", "BytesValue", "DoubleValue", "FloatValue",
"Int32Value", "Int64Value", "StringValue", "UInt32Value", "UInt64Value":
return o.FieldValueExemplar(md.Fields().ByName("value"))
return f.FieldValueExemplar(md.Fields().ByName("value"))
case "Duration":
e.line(o.quote("0s"))
e.line(f.quote("0s"))
case "Empty":
e.line("{}")
case "FieldMask":
e.line(o.quote("field1.field2,field3"))
e.line(f.quote("field1.field2,field3"))
case "ListValue":
return o.FieldValueExemplar(md.Fields().ByName("values"))
return f.FieldValueExemplar(md.Fields().ByName("values"))
case "Struct":
e = o.FieldValueExemplar(md.Fields().Get(0).MapValue())
e = f.FieldValueExemplar(md.Fields().Get(0).MapValue())
e.prepend("structField: ")
e.append(",")
e.nest("{", "}")
case "Timestamp":
e.line(o.quote("2006-01-02T15:04:05.999999999Z"))
e.line(f.quote("2006-01-02T15:04:05.999999999Z"))
case "Value":
e.line(o.quote("https://developers.google.com/protocol-buffers/docs/reference/google.protobuf#value"))
e.line(f.quote("https://developers.google.com/protocol-buffers/docs/reference/google.protobuf#value"))
}
return e
}

// FieldExemplar returns an exemplar for a message field. It has the JSON name
// for the field prefixed and a comment appended to the first line describing
// the type of the field.
func (o *FormatOptions) FieldExemplar(fd protoreflect.FieldDescriptor) exemplar {
e := o.FieldValueExemplar(fd)
func (f *Formatter) FieldExemplar(fd protoreflect.FieldDescriptor) exemplar {
e := f.FieldValueExemplar(fd)
e.prepend(fd.JSONName() + ": ")
e.append(",")

// Add a description of the type to the end of the first line of the
// exemplar as a comment. If part of a oneof, name the oneof too.
if desc := o.typeDescription(fd); desc != "" {
if desc := f.typeDescription(fd); desc != "" {
if od := fd.ContainingOneof(); od != nil {
desc += " (one-of " + string(od.Name()) + ")"
}
Expand All @@ -174,15 +186,15 @@ func (o *FormatOptions) FieldExemplar(fd protoreflect.FieldDescriptor) exemplar
//
// Repeated fields are emitted with a single element exemplar of the repeated
// type.
func (o *FormatOptions) FieldValueExemplar(fd protoreflect.FieldDescriptor) exemplar {
func (f *Formatter) FieldValueExemplar(fd protoreflect.FieldDescriptor) exemplar {
var e exemplar
switch fd.Kind() {
case protoreflect.EnumKind:
e = o.EnumExemplar(fd)
e = f.EnumExemplar(fd)
case protoreflect.MessageKind, protoreflect.GroupKind:
e = o.MessageExemplar(fd.Message())
e = f.MessageExemplar(fd.Message())
default:
e = o.ScalarExemplar(fd.Kind())
e = f.ScalarExemplar(fd.Kind())
}

if fd.Cardinality() == protoreflect.Repeated {
Expand All @@ -198,7 +210,7 @@ func (o *FormatOptions) FieldValueExemplar(fd protoreflect.FieldDescriptor) exem
// Enum exemplars are emitted as a string with the name of the second enum if
// there is more than one enum value, otherwise the first enum. The second enum
// is preferred as often the first enum is the "invalid" value for that enum.
func (o *FormatOptions) EnumExemplar(fd protoreflect.FieldDescriptor) exemplar {
func (f *Formatter) EnumExemplar(fd protoreflect.FieldDescriptor) exemplar {
var e exemplar
if fd.Kind() != protoreflect.EnumKind {
return e
Expand All @@ -215,13 +227,13 @@ func (o *FormatOptions) EnumExemplar(fd protoreflect.FieldDescriptor) exemplar {
if ev.Len() > 1 {
name = ev.Get(1).Name()
}
e.line(o.quote(string(name)))
e.line(f.quote(string(name)))
return e
}

// ScalarExemplar returns an exemplar with a value for basic kinds that have a
// single value (scalars). An empty exemplar is returned for other kinds.
func (o *FormatOptions) ScalarExemplar(kind protoreflect.Kind) exemplar {
func (f *Formatter) ScalarExemplar(kind protoreflect.Kind) exemplar {
var e exemplar
switch kind {
case protoreflect.BoolKind:
Expand All @@ -234,22 +246,25 @@ func (o *FormatOptions) ScalarExemplar(kind protoreflect.Kind) exemplar {
case protoreflect.FloatKind, protoreflect.DoubleKind:
e.line("0.0")
case protoreflect.StringKind, protoreflect.BytesKind:
e.line(o.quote(""))
e.line(f.quote(""))
}
return e
}

// typeDescription returns a string description of a field's type.
func (o *FormatOptions) typeDescription(fd protoreflect.FieldDescriptor) string {
func (f *Formatter) typeDescription(fd protoreflect.FieldDescriptor) string {
if fd.IsMap() {
return fmt.Sprintf("map<%s, %s>", o.typeDescription(fd.MapKey()), o.typeDescription(fd.MapValue()))
return fmt.Sprintf("map<%s, %s>", f.typeDescription(fd.MapKey()), f.typeDescription(fd.MapValue()))
}
result := ""
switch fd.Kind() {
case protoreflect.EnumKind:
result = string(fd.Enum().Name())
case protoreflect.MessageKind, protoreflect.GroupKind:
result = string(fd.Message().Name())
if slices.Contains(f.messageStack, fd.Message().FullName()) {
result += " (recursive)"
}
case protoreflect.BoolKind,
protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Uint32Kind,
protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Uint64Kind,
Expand Down
6 changes: 3 additions & 3 deletions bones/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// slice. Method exemplars are written to stdout if methodDir is empty,
// otherwise each method is written to a separate file in that directory.
// Existing files will not be overwritten unless force is true.
func Generate(fds *descriptorpb.FileDescriptorSet, methodDir string, force bool, targets []string, formatter FormatOptions) error {
func Generate(fds *descriptorpb.FileDescriptorSet, methodDir string, force bool, targets []string, formatter Formatter) error {
files, err := protodesc.NewFiles(fds)
if err != nil {
return err
Expand All @@ -29,7 +29,7 @@ func Generate(fds *descriptorpb.FileDescriptorSet, methodDir string, force bool,
return err
}

func genFile(fd protoreflect.FileDescriptor, methodDir string, force bool, targets []string, formatter FormatOptions) error {
func genFile(fd protoreflect.FileDescriptor, methodDir string, force bool, targets []string, formatter Formatter) error {
for _, sd := range services(fd) {
for _, md := range methods(sd) {
if err := genMethod(md, methodDir, force, targets, formatter); err != nil {
Expand All @@ -40,7 +40,7 @@ func genFile(fd protoreflect.FileDescriptor, methodDir string, force bool, targe
return nil
}

func genMethod(md protoreflect.MethodDescriptor, methodDir string, force bool, targets []string, formatter FormatOptions) error {
func genMethod(md protoreflect.MethodDescriptor, methodDir string, force bool, targets []string, formatter Formatter) error {
var err error
if !match(md, targets) {
return nil
Expand Down
4 changes: 2 additions & 2 deletions bones/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ func TestGenerateGolden(t *testing.T) {
t.Run(name, func(t *testing.T) {
dir := t.TempDir()

err := Generate(getFDS(t, tc.pbFile), dir, false, nil, FormatOptions{QuoteStyle: tc.quoteStyle})
err := Generate(getFDS(t, tc.pbFile), dir, false, nil, Formatter{QuoteStyle: tc.quoteStyle})
require.NoError(t, err)
err = Generate(getFDS(t, tc.pbFile), dir, false, nil, FormatOptions{Lang: JS, QuoteStyle: tc.quoteStyle})
err = Generate(getFDS(t, tc.pbFile), dir, false, nil, Formatter{Lang: JS, QuoteStyle: tc.quoteStyle})
require.NoError(t, err)
requireSameContent(t, tc.goldenDir, dir)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ function Sample(input) {
// field: "", // string
// repeat: [0], // repeated int32
// },
recursive: {}, // SampleResponse (recursive)
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,6 @@ function(input) {
// field: "", // string
// repeat: [0], // repeated int32
// },
recursive: {}, // SampleResponse (recursive)
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ function Sample(input) {
// field: '', // string
// repeat: [0], // repeated int32
// },
recursive: {}, // SampleResponse (recursive)
},
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,6 @@ function(input) {
// field: '', // string
// repeat: [0], // repeated int32
// },
recursive: {}, // SampleResponse (recursive)
},
}
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/alecthomas/protobuf v0.0.0-20220411053735-2a9f60104b87
github.com/google/go-jsonnet v0.17.0
github.com/stretchr/testify v1.7.0
golang.org/x/exp v0.0.0-20220414153411-bcd21879b8fd
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
google.golang.org/genproto v0.0.0-20220407144326-9054f6ed7bac
Expand All @@ -21,7 +22,7 @@ require (
github.com/golang/protobuf v1.5.2 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 // indirect
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 // indirect
golang.org/x/text v0.3.5 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
)
5 changes: 4 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20220414153411-bcd21879b8fd h1:zVFyTKZN/Q7mNRWSs1GOYnHM9NiFSJ54YVRsD0rNWT4=
golang.org/x/exp v0.0.0-20220414153411-bcd21879b8fd/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU=
golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
Expand Down Expand Up @@ -121,8 +123,9 @@ golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 h1:gG67DSER+11cZvqIMb8S8bt0vZtiN6xWYARwirrOSfE=
golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 h1:id054HUawV2/6IGm2IV8KZQjqtwAOo2CYlOToYqa0d0=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
Expand Down
Binary file modified pb/exemplar/exemplar.pb
Binary file not shown.
Loading

0 comments on commit 7f4dd69

Please sign in to comment.