🦴 Update bones: don't reprint previous messages (#44)

Update bones output, don't re-print messages already processed. This is an extension in response to a real world example where the bones files ended up being several hundred mega bytes large. While at it, offer an even more minimal output, setting up files with appropriate file names and fully qualified request and response parameter types without any further zero values. This merges the following commits: * Fix Makefile * Run go mod tidy * Add log messages to bones * Update bones output, don't re-print messages already processed * Add minimal message exemplar Makefile | 28 ++- bones/exemplar.go | 107 ++++++--- bones/generate.go | 15 +- bones/generate_test.go | 18 +- .../exemplar.Exemplar.Sample.js | 25 +- .../exemplar.Exemplar.Sample.jsonnet | 25 +- .../exemplar.Exemplar.WellKnown.js | 110 +++++++++ .../exemplar.Exemplar.WellKnown.jsonnet | 108 +++++++++ .../exemplar.Exemplar.Sample.js | 14 ++ .../exemplar.Exemplar.Sample.jsonnet | 12 + .../exemplar.Exemplar.WellKnown.js | 14 ++ .../exemplar.Exemplar.WellKnown.jsonnet | 12 + .../exemplar.Exemplar.WellKnown.js | 214 ------------------ .../exemplar.Exemplar.WellKnown.jsonnet | 212 ----------------- .../exemplar.Exemplar.Sample.js | 25 +- .../exemplar.Exemplar.Sample.jsonnet | 25 +- .../exemplar.Exemplar.WellKnown.js | 110 +++++++++ .../exemplar.Exemplar.WellKnown.jsonnet | 108 +++++++++ .../exemplar.Exemplar.Sample.js | 14 ++ .../exemplar.Exemplar.Sample.jsonnet | 12 + .../exemplar.Exemplar.WellKnown.js | 14 ++ .../exemplar.Exemplar.WellKnown.jsonnet | 12 + .../exemplar.Exemplar.WellKnown.js | 214 ------------------ .../exemplar.Exemplar.WellKnown.jsonnet | 212 ----------------- .../greet.Greeter.Hello.js | 4 +- .../greet.Greeter.Hello.jsonnet | 4 +- .../greet.Greeter.HelloBidiStream.js | 4 +- .../greet.Greeter.HelloBidiStream.jsonnet | 4 +- .../greet.Greeter.HelloClientStream.js | 4 +- .../greet.Greeter.HelloClientStream.jsonnet | 4 +- .../greet.Greeter.HelloServerStream.js | 4 +- .../greet.Greeter.HelloServerStream.jsonnet | 4 +- .../greet.Greeter.Hello.js | 14 ++ .../greet.Greeter.Hello.jsonnet | 12 + .../greet.Greeter.HelloBidiStream.js | 16 ++ .../greet.Greeter.HelloBidiStream.jsonnet | 14 ++ .../greet.Greeter.HelloClientStream.js | 16 ++ .../greet.Greeter.HelloClientStream.jsonnet | 14 ++ .../greet.Greeter.HelloServerStream.js | 16 ++ .../greet.Greeter.HelloServerStream.jsonnet | 14 ++ .../greet.Greeter.Hello.js | 4 +- .../greet.Greeter.Hello.jsonnet | 4 +- .../greet.Greeter.HelloBidiStream.js | 4 +- .../greet.Greeter.HelloBidiStream.jsonnet | 4 +- .../greet.Greeter.HelloClientStream.js | 4 +- .../greet.Greeter.HelloClientStream.jsonnet | 4 +- .../greet.Greeter.HelloServerStream.js | 4 +- .../greet.Greeter.HelloServerStream.jsonnet | 4 +- .../greet.Greeter.Hello.js | 14 ++ .../greet.Greeter.Hello.jsonnet | 12 + .../greet.Greeter.HelloBidiStream.js | 16 ++ .../greet.Greeter.HelloBidiStream.jsonnet | 14 ++ .../greet.Greeter.HelloClientStream.js | 16 ++ .../greet.Greeter.HelloClientStream.jsonnet | 14 ++ .../greet.Greeter.HelloServerStream.js | 16 ++ .../greet.Greeter.HelloServerStream.jsonnet | 14 ++ go.mod | 1 - go.sum | 2 - log/log.go | 2 + main.go | 38 +++- main_test.go | 5 +- serve/httprule/server_test.go | 5 +- serve/server_test.go | 3 +- 63 files changed, 976 insertions(+), 1036 deletions(-) Pull-Request: #44
foxygoat · May 31, 2022 · d92eee6 · d92eee6
2 parents e8a7489 + bdead89
commit d92eee6
Show file tree

Hide file tree

Showing 63 changed files with 976 additions and 1,036 deletions.
diff --git a/Makefile b/Makefile
@@ -39,7 +39,7 @@ tidy:  ## Tidy go modules with "go mod tidy"
 # --- Test ---------------------------------------------------------------------
 COVERFILE = $(O)/coverage.txt
 
-test: $(O) ## Run tests and generate a coverage file
+test: | $(O) ## Run tests and generate a coverage file
 	go test -coverprofile=$(COVERFILE) ./...
 
 check-coverage: test  ## Check that test coverage meets the required level
@@ -48,16 +48,24 @@ check-coverage: test  ## Check that test coverage meets the required level
 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)
+RUN_BONES = $(O)/jig bones --force --language=$(1) --quote-style=$(2) --proto-set pb/$(3)/$(4) --minimal=$(5) --method-dir bones/testdata/golden/$(3)-$(2)-$(5)-minimal
 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)
-	$(call RUN_BONES,jsonnet,single,greet,greeter.pb)
-	$(call RUN_BONES,js,double,exemplar,exemplar.pb)
-	$(call RUN_BONES,js,double,greet,greeter.pb)
-	$(call RUN_BONES,js,single,exemplar,exemplar.pb)
-	$(call RUN_BONES,js,single,greet,greeter.pb)
+	$(call RUN_BONES,jsonnet,double,exemplar,exemplar.pb,no)
+	$(call RUN_BONES,jsonnet,double,exemplar,exemplar.pb,yes)
+	$(call RUN_BONES,jsonnet,double,greet,greeter.pb,no)
+	$(call RUN_BONES,jsonnet,double,greet,greeter.pb,yes)
+	$(call RUN_BONES,jsonnet,single,exemplar,exemplar.pb,no)
+	$(call RUN_BONES,jsonnet,single,exemplar,exemplar.pb,yes)
+	$(call RUN_BONES,jsonnet,single,greet,greeter.pb,no)
+	$(call RUN_BONES,jsonnet,single,greet,greeter.pb,yes)
+	$(call RUN_BONES,js,double,exemplar,exemplar.pb,no)
+	$(call RUN_BONES,js,double,exemplar,exemplar.pb,yes)
+	$(call RUN_BONES,js,double,greet,greeter.pb,no)
+	$(call RUN_BONES,js,double,greet,greeter.pb,yes)
+	$(call RUN_BONES,js,single,exemplar,exemplar.pb,no)
+	$(call RUN_BONES,js,single,exemplar,exemplar.pb,yes)
+	$(call RUN_BONES,js,single,greet,greeter.pb,no)
+	$(call RUN_BONES,js,single,greet,greeter.pb,yes)
 
 CHECK_COVERAGE = awk -F '[ \t%]+' '/^total:/ {print; if ($$3 < $(COVERAGE)) exit 1}'
 FAIL_COVERAGE = { echo '$(COLOUR_RED)FAIL - Coverage below $(COVERAGE)%$(COLOUR_NORMAL)'; exit 1; }

diff --git a/bones/exemplar.go b/bones/exemplar.go
@@ -4,39 +4,60 @@ import (
 	"fmt"
 	"strings"
 
-	"golang.org/x/exp/slices"
 	"google.golang.org/protobuf/reflect/protoreflect"
 )
 
 // Formatter is a configurable target language exemplar generator.
-type Formatter struct {
+type formatter struct {
+	messagesSeen map[protoreflect.FullName]bool
+	opts         *FormatterOptions
+}
+
+type FormatterOptions struct {
 	Lang       Lang
 	QuoteStyle QuoteStyle
-
-	messageStack []protoreflect.FullName
+	Minimal    bool
 }
 
-type FormatOptions = Formatter
+// NewFormatter creates a new Formatter for given language, quote style,
+// and weather to use just input/output type names or flash out all
+// data structures on their first occurrence.
+func newFormatter(opts *FormatterOptions) *formatter {
+	return &formatter{
+		opts:         opts,
+		messagesSeen: map[protoreflect.FullName]bool{},
+	}
+}
 
 // Return the file extension for the given language.
-func (f *Formatter) Extension() string {
-	return "." + f.Lang.String()
+func (f *formatter) Extension() string {
+	return "." + f.opts.Lang.String()
 }
 
 // Return the file extension for the given language.
-func (f *Formatter) quote(s string) string {
-	if f.QuoteStyle == Double {
+func (f *formatter) quote(s string) string {
+	if f.opts.QuoteStyle == Double {
 		return `"` + s + `"`
 	}
 	return `'` + s + `'`
 }
 
+func (f *formatter) reset() {
+	f.messagesSeen = map[protoreflect.FullName]bool{}
+}
+
 // 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 (f *Formatter) MethodExemplar(md protoreflect.MethodDescriptor) exemplar {
+func (f *formatter) MethodExemplar(md protoreflect.MethodDescriptor) exemplar {
+	f.reset()
 	// Format the input message exemplar as a comment
-	ime := f.MessageExemplar(md.Input())
+	var ime exemplar
+	if f.opts.Minimal {
+		ime = f.minimalMessageExemplar(md.Input())
+	} else {
+		ime = f.MessageExemplar(md.Input(), "  // "+string(md.Input().Name()))
+	}
 	ime.append(",")
 	if md.IsStreamingClient() && !md.IsStreamingServer() {
 		ime.nest("stream: [", "],")
@@ -47,14 +68,20 @@ func (f *Formatter) MethodExemplar(md protoreflect.MethodDescriptor) exemplar {
 	ime.prefix("// ")
 
 	// Format the output message exemplar
-	ome := f.MessageExemplar(md.Output())
+	f.reset()
+	var ome exemplar
+	if f.opts.Minimal {
+		ome = f.minimalMessageExemplar(md.Output())
+	} else {
+		ome = f.MessageExemplar(md.Output(), "  // "+string(md.Output().Name()))
+	}
 	ome.append(",")
 	if md.IsStreamingServer() {
 		ome.nest("stream: [", "],")
 	} else {
 		ome.prepend("response: ")
 	}
-	if f.Lang == Jsonnet {
+	if f.opts.Lang == Jsonnet {
 		ome.nest("function(input) {", "}")
 	} else {
 		ome.nest("return {", "}")
@@ -84,11 +111,18 @@ func (f *Formatter) MethodExemplar(md protoreflect.MethodDescriptor) exemplar {
 	return e
 }
 
+func (f *formatter) minimalMessageExemplar(md protoreflect.MessageDescriptor) exemplar {
+	var e exemplar
+	e.line("{  // " + string(md.FullName()))
+	e.line("}")
+	return e
+}
+
 // MessageExemplar returns an exemplar of a protobuf message as a JSON object
 // 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 (f *Formatter) MessageExemplar(md protoreflect.MessageDescriptor) exemplar {
+func (f *formatter) MessageExemplar(md protoreflect.MessageDescriptor, headerPostfix string) exemplar {
 	var e exemplar
 
 	if strings.HasPrefix(string(md.FullName()), "google.protobuf.") {
@@ -97,12 +131,12 @@ func (f *Formatter) MessageExemplar(md protoreflect.MessageDescriptor) exemplar
 		}
 	}
 
-	if slices.Contains(f.messageStack, md.FullName()) {
+	if f.messagesSeen[md.FullName()] {
 		e.line("{}")
 		return e
 	}
 
-	f.messageStack = append(f.messageStack, md.FullName())
+	f.messagesSeen[md.FullName()] = true
 	for _, fd := range fields(md) {
 		fe := f.FieldExemplar(fd)
 		if fd.ContainingOneof() != nil {
@@ -111,9 +145,8 @@ func (f *Formatter) MessageExemplar(md protoreflect.MessageDescriptor) exemplar
 		}
 		e.extend(fe)
 	}
-	f.messageStack = f.messageStack[:len(f.messageStack)-1]
 
-	e.nest("{", "}")
+	e.nest("{"+headerPostfix, "}")
 	return e
 }
 
@@ -122,7 +155,7 @@ func (f *Formatter) MessageExemplar(md protoreflect.MessageDescriptor) exemplar
 // JSON as a single field, rather than as an object.
 //
 // https://developers.google.com/protocol-buffers/docs/reference/google.protobuf
-func (f *Formatter) 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":
@@ -160,17 +193,31 @@ func (f *Formatter) WellKnownExemplar(md protoreflect.MessageDescriptor) exempla
 // 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 (f *Formatter) FieldExemplar(fd protoreflect.FieldDescriptor) exemplar {
+func (f *formatter) FieldExemplar(fd protoreflect.FieldDescriptor) exemplar {
+	desc := f.typeDescription(fd)
+	seen := false
+	if fd.Kind() == protoreflect.MessageKind || fd.Kind() == protoreflect.GroupKind {
+		seen = f.messagesSeen[fd.Message().FullName()]
+	}
+
 	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 := f.typeDescription(fd); desc != "" {
-		if od := fd.ContainingOneof(); od != nil {
-			desc += " (one-of " + string(od.Name()) + ")"
+	if desc != "" {
+		seenComment := "see example above"
+		if od := fd.ContainingOneof(); od != nil && desc != "" {
+			desc += " (one-of " + string(od.Name())
+			if seen {
+				desc += ", " + seenComment
+			}
+			desc += ")"
+		} else if seen {
+			desc += " (" + seenComment + ")"
 		}
+
 		e.lines[0] += "  // " + desc
 	}
 
@@ -186,13 +233,13 @@ func (f *Formatter) FieldExemplar(fd protoreflect.FieldDescriptor) exemplar {
 //
 // Repeated fields are emitted with a single element exemplar of the repeated
 // type.
-func (f *Formatter) FieldValueExemplar(fd protoreflect.FieldDescriptor) exemplar {
+func (f *formatter) FieldValueExemplar(fd protoreflect.FieldDescriptor) exemplar {
 	var e exemplar
 	switch fd.Kind() {
 	case protoreflect.EnumKind:
 		e = f.EnumExemplar(fd)
 	case protoreflect.MessageKind, protoreflect.GroupKind:
-		e = f.MessageExemplar(fd.Message())
+		e = f.MessageExemplar(fd.Message(), "")
 	default:
 		e = f.ScalarExemplar(fd.Kind())
 	}
@@ -210,7 +257,7 @@ func (f *Formatter) FieldValueExemplar(fd protoreflect.FieldDescriptor) exemplar
 // 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 (f *Formatter) EnumExemplar(fd protoreflect.FieldDescriptor) exemplar {
+func (f *formatter) EnumExemplar(fd protoreflect.FieldDescriptor) exemplar {
 	var e exemplar
 	if fd.Kind() != protoreflect.EnumKind {
 		return e
@@ -233,7 +280,7 @@ func (f *Formatter) EnumExemplar(fd protoreflect.FieldDescriptor) exemplar {
 
 // 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 (f *Formatter) ScalarExemplar(kind protoreflect.Kind) exemplar {
+func (f *formatter) ScalarExemplar(kind protoreflect.Kind) exemplar {
 	var e exemplar
 	switch kind {
 	case protoreflect.BoolKind:
@@ -252,7 +299,7 @@ func (f *Formatter) ScalarExemplar(kind protoreflect.Kind) exemplar {
 }
 
 // typeDescription returns a string description of a field's type.
-func (f *Formatter) typeDescription(fd protoreflect.FieldDescriptor) string {
+func (f *formatter) typeDescription(fd protoreflect.FieldDescriptor) string {
 	if fd.IsMap() {
 		return fmt.Sprintf("map<%s, %s>", f.typeDescription(fd.MapKey()), f.typeDescription(fd.MapValue()))
 	}
@@ -262,9 +309,6 @@ func (f *Formatter) typeDescription(fd protoreflect.FieldDescriptor) string {
 		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,
@@ -278,7 +322,6 @@ func (f *Formatter) typeDescription(fd protoreflect.FieldDescriptor) string {
 	if fd.IsList() && result != "" {
 		result = "repeated " + result
 	}
-
 	return result
 }
 

diff --git a/bones/generate.go b/bones/generate.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"path"
 
+	"foxygo.at/jig/log"
 	"google.golang.org/protobuf/reflect/protodesc"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/types/descriptorpb"
@@ -15,37 +16,38 @@ 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 Formatter) error {
+func Generate(logger log.Logger, fds *descriptorpb.FileDescriptorSet, methodDir string, force bool, targets []string, formatOpts *FormatterOptions) error {
 	files, err := protodesc.NewFiles(fds)
 	if err != nil {
 		return err
 	}
 
 	files.RangeFiles(func(fd protoreflect.FileDescriptor) bool {
-		err = genFile(fd, methodDir, force, targets, formatter)
+		err = genFile(logger, fd, methodDir, force, targets, formatOpts)
 		return err == nil
 	})
 
 	return err
 }
 
-func genFile(fd protoreflect.FileDescriptor, methodDir string, force bool, targets []string, formatter Formatter) error {
+func genFile(logger log.Logger, fd protoreflect.FileDescriptor, methodDir string, force bool, targets []string, formatOpts *FormatterOptions) error {
 	for _, sd := range services(fd) {
 		for _, md := range methods(sd) {
-			if err := genMethod(md, methodDir, force, targets, formatter); err != nil {
+			if err := genMethod(logger, md, methodDir, force, targets, formatOpts); err != nil {
 				return err
 			}
 		}
 	}
 	return nil
 }
 
-func genMethod(md protoreflect.MethodDescriptor, methodDir string, force bool, targets []string, formatter Formatter) error {
+func genMethod(logger log.Logger, md protoreflect.MethodDescriptor, methodDir string, force bool, targets []string, formatOpts *FormatterOptions) error {
 	var err error
 	if !match(md, targets) {
 		return nil
 	}
 
+	formatter := newFormatter(formatOpts)
 	f := os.Stdout
 	if methodDir != "" {
 		flag := os.O_WRONLY | os.O_CREATE | os.O_TRUNC
@@ -59,10 +61,12 @@ func genMethod(md protoreflect.MethodDescriptor, methodDir string, force bool, t
 		if err != nil {
 			if os.IsExist(err) && !force {
 				// skip existing files when not forcing
+				logger.Debugf("skip existing file %q, use --force to override", md.FullName())
 				return nil
 			}
 			return err
 		}
+		logger.Debugf("created file %q", filename)
 		defer func() {
 			cerr := f.Close()
 			if err == nil {
@@ -71,6 +75,7 @@ func genMethod(md protoreflect.MethodDescriptor, methodDir string, force bool, t
 		}()
 	}
 
+	logger.Debugf("writing bones of %q", md.FullName())
 	_, err = fmt.Fprintln(f, formatter.MethodExemplar(md))
 	// Print a separator on stdout to separate multiple exemplars
 	if f == os.Stdout {