Repository: googleapis/gapic-generator-go
Pull request #216 (2019-09-30 18:53:35)

SampleGen: Add function-level documentation
Generates function-level comments.
* a comment describing what the sample does. Generated from `description` field in sample config.
* comments describing what each parameter is. Generated from `request.comment` field in sample config.

Open questions:
* Not following GoDoc style?
  * @jadekler mentioned in Chat that samples don't necessarily need to follow GoDoc styles.
  * It's hard to implement because that will require either the generator to construct a grammar correct English sentence from user-specified sample config or enforcing some conventions to the `request.comment` field in sample config.
* Is it OK to use a `:` between the parameter name and the comment?

hzyi-google: PTAL
hzyi-google: cc/ @googleapis/samplegen 
hzyi-google: PTAL

Patch:
diff --git a/cmd/gen-go-sample/inittree.go b/cmd/gen-go-sample/inittree.go
index 7846e08..93c5aca 100644
--- a/cmd/gen-go-sample/inittree.go
+++ b/cmd/gen-go-sample/inittree.go
@@ -522,19 +522,19 @@ func (t *initTree) print(w *bufio.Writer, g *generator, ind int) error {
 	}
 
 	for i, k := range t.keys {
-		printCommentLines(w, t.vals[i].comment, ind + 1)
+		printCommentLines(w, t.vals[i].comment, ind+1)
 		if t.vals[i].typ.namePat != nil {
 			for _, v := range t.vals[i].vals {
-				printCommentLines(w, v.comment, ind + 1)
+				printCommentLines(w, v.comment, ind+1)
 			}
 		}
-		indent(w, ind + 1)
+		indent(w, ind+1)
 
 		var closeBrace bool
 		if oneof, ok := oneofs[k]; ok {
 			fmt.Fprintf(w, "%s: &%s.%s_%s{\n", snakeToPascal(oneof), impSpec.Name, typName, snakeToPascal(k))
 			closeBrace = true
-			indent(w, ind + 2)
+			indent(w, ind+2)
 		}
 
 		if writeKey {
@@ -548,7 +548,7 @@ func (t *initTree) print(w *bufio.Writer, g *generator, ind int) error {
 
 		if closeBrace {
 			w.WriteString(",\n")
-			indent(w, ind + 1)
+			indent(w, ind+1)
 			w.WriteByte('}')
 		}
 		w.WriteString(",\n")
diff --git a/cmd/gen-go-sample/main.go b/cmd/gen-go-sample/main.go
index 03f8c9e..f700d32 100644
--- a/cmd/gen-go-sample/main.go
+++ b/cmd/gen-go-sample/main.go
@@ -313,6 +313,7 @@ func (g *generator) disambiguateSampleIDs() error {
 	return nil
 }
 
+// TODO(hzyi): this method is getting long. Split it up.
 func (g *generator) genSample(sampConf schema_v1p2.Sample, methConf GAPICMethod) error {
 	ifaceName := sampConf.Service
 
@@ -353,7 +354,7 @@ func (g *generator) genSample(sampConf schema_v1p2.Sample, methConf GAPICMethod)
 
 	p := g.pt.Printf
 
-	// Region tag, function signature and new client
+	// Region tag
 	argStr, err := argListStr(initInfo, g)
 	if err != nil {
 		return err
@@ -361,6 +362,33 @@ func (g *generator) genSample(sampConf schema_v1p2.Sample, methConf GAPICMethod)
 	p("")
 	p("// [START %s]", sampConf.RegionTag)
 	p("")
+
+	// comments above the sample function
+	requiresNewLine := false
+
+	writeCommentLines := func(comment string) {
+		comment = strings.TrimSpace(comment)
+		writeComment(comment, nil, g)
+	}
+
+	if sampConf.Description != "" {
+		writeCommentLines(fmt.Sprintf("sample%s: %s", meth.GetName(), sampConf.Description))
+		requiresNewLine = true
+	}
+
+	for i, argName := range initInfo.argNames {
+		comment := initInfo.argTrees[i].comment
+		if comment == "" {
+			continue
+		}
+		if requiresNewLine {
+			writeCommentLines("")
+		}
+		requiresNewLine = true
+		writeCommentLines(fmt.Sprintf("%s: %s", argName, comment))
+	}
+
+	// function signature and client initialization
 	p("func sample%s(%s) error {", meth.GetName(), argStr)
 	p("  ctx := context.Background()")
 	p("  c, err := %s.New%sClient(ctx)", g.clientPkg.Name, pbinfo.ReduceServName(serv.GetName(), g.clientPkg.Name))
@@ -689,3 +717,42 @@ func prependLines(b *bytes.Buffer, prefix string, skipEmptyLine bool) {
 		b.WriteString(l)
 	}
 }
+
+// wrapComment wraps comment at 100 characters, iff comment does not contain any newline characters
+// and comment has more than 110 characters.
+//
+// comment cannot have leading or trailing white spaces.
+func wrapComment(comment string) string {
+	if strings.ContainsRune(comment, '\n') || len(comment) < 110 {
+		return comment
+	}
+
+	var output strings.Builder
+	s := 0
+	prev := -1
+
+	for true {
+		fmt.Println("what")
+		p := strings.IndexByte(comment[prev+1:], ' ')
+		// we reached the end of comment
+		if p < 0 {
+			output.WriteString(comment[s:])
+			return output.String()
+		}
+
+		p = p + prev + 1
+		if p-s > 100 {
+			// a single word has more than 100 characters
+			if prev == s-1 {
+				prev = p
+			}
+			// break the line at prev
+			output.WriteString(comment[s:prev])
+			output.WriteByte('\n')
+			s = prev + 1
+		}
+		prev = p
+	}
+
+	return ""
+}
diff --git a/internal/gengapic/client_init.go b/internal/gengapic/client_init.go
index c9f2262..e3453b5 100644
--- a/internal/gengapic/client_init.go
+++ b/internal/gengapic/client_init.go
@@ -97,7 +97,7 @@ func (g *generator) clientOptions(serv *descriptor.ServiceDescriptorProto, servN
 				if name.GetMethod() != "" {
 					base = base + "." + name.GetMethod()
 					policies[base] = mc.GetRetryPolicy()
-					
+
 					if maxReq := mc.GetMaxRequestMessageBytes(); maxReq != nil {
 						reqLimits[base] = int(maxReq.GetValue())
 					}
@@ -148,7 +148,7 @@ func (g *generator) clientOptions(serv *descriptor.ServiceDescriptorProto, servN
 			mFQN := sFQN + "." + m.GetName()
 			p("%s: []gax.CallOption{", m.GetName())
 
-			if maxReq, ok := reqLimits[mFQN]; ok  {
+			if maxReq, ok := reqLimits[mFQN]; ok {
 				p("gax.WithGRPCOptions(grpc.MaxCallSendMsgSize(%d)),", maxReq)
 			}
 


Test patch:
diff --git a/cmd/gen-go-sample/main_test.go b/cmd/gen-go-sample/main_test.go
index 2161d99..135061f 100644
--- a/cmd/gen-go-sample/main_test.go
+++ b/cmd/gen-go-sample/main_test.go
@@ -33,14 +33,15 @@ func TestUnary(t *testing.T) {
 	g := initTestGenerator()
 
 	sp := schema_v1p2.Sample{
-		ID:        "my_sample_config",
-		Rpc:       "UnaryMethod",
-		Service:   "foo.FooService",
-		RegionTag: "awesome_region",
+		ID:          "my_sample_config",
+		Rpc:         "UnaryMethod",
+		Service:     "foo.FooService",
+		RegionTag:   "awesome_region",
+		Description: "Construct a complex request object,\nsend it to the server,\nand inspect the response.\n",
 		Request: []schema_v1p2.RequestConfig{
-			{Field: "a.x", Value: "42", InputParameter: "the_x"},
+			{Field: "a.x", Value: "42", InputParameter: "the_x", Comment: "a single-line comment for an input parameter"},
 			{Field: "a.y", Value: "3.14159", Comment: "approximation of Pi"},
-			{Field: "b", Value: "foobar", InputParameter: "the_b"},
+			{Field: "b", Value: "foobar", InputParameter: "the_b", Comment: "a multi-line comment for\nan input parameter\n"},
 			{Field: "e", Value: "BANANA"},
 			{Field: "f", Value: "in a oneof"},
 			{Field: "bytes", Value: "mybytes"},
@@ -427,6 +428,29 @@ func TestEnum(t *testing.T) {
 	compare(t, g, filepath.Join("testdata", "sample_enum.want"))
 }
 
+func TestWrapCommentNotWrapping(t *testing.T) {
+	comment := "some short comments"
+	if c := wrapComment(comment); c != comment {
+		t.Fatal(errors.E(nil, "want %q, got %q", comment, c))
+	}
+
+	comment = "some\nmulti-line\ncomments"
+	if c := wrapComment(comment); c != comment {
+		t.Fatal(errors.E(nil, "want %q, got %q", comment, c))
+	}
+}
+
+func TestWrapCommentWrapping(t *testing.T) {
+	raw := "The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog again. The quick brown fox jumps over the lazy dog again and again."
+	wrapped :=
+		`The quick brown fox jumps over the lazy dog. The quick brown fox jumps over the lazy dog again. The
+quick brown fox jumps over the lazy dog again and again.`
+
+	if c := wrapComment(raw); c != wrapped {
+		t.Fatal(errors.E(nil, "want %q, got %q", wrapped, c))
+	}
+}
+
 func initTestGenerator() *generator {
 	eType := &descriptor.EnumDescriptorProto{
 		Name: proto.String("FruitEnum"),
diff --git a/cmd/gen-go-sample/testdata/sample_unary.want b/cmd/gen-go-sample/testdata/sample_unary.want
index ad22a6c..a47dc10 100644
--- a/cmd/gen-go-sample/testdata/sample_unary.want
+++ b/cmd/gen-go-sample/testdata/sample_unary.want
@@ -28,6 +28,14 @@ foopb "path.to/pb/foo"
 
 // [START awesome_region]
 
+// sampleUnaryMethod: Construct a complex request object,
+// send it to the server,
+// and inspect the response.
+//
+// theX: a single-line comment for an input parameter
+//
+// theB: a multi-line comment for
+// an input parameter
 func sampleUnaryMethod(theX int64, theB string, theFoo string, bobFile string) error {
 	ctx := context.Background()
 	c, err := foo.NewClient(ctx)
diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go
index c37cbaf..def786d 100644
--- a/internal/gengapic/client_init_test.go
+++ b/internal/gengapic/client_init_test.go
@@ -21,12 +21,12 @@ import (
 	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/protoc-gen-go/descriptor"
 	"github.com/golang/protobuf/ptypes/duration"
+	wrappers "github.com/golang/protobuf/ptypes/wrappers"
 	conf "github.com/googleapis/gapic-generator-go/internal/grpc_service_config"
 	"github.com/googleapis/gapic-generator-go/internal/pbinfo"
 	"github.com/googleapis/gapic-generator-go/internal/txtdiff"
 	"google.golang.org/genproto/googleapis/api/annotations"
 	code "google.golang.org/genproto/googleapis/rpc/code"
-	wrappers "github.com/golang/protobuf/ptypes/wrappers"
 )
 
 func TestClientOpt(t *testing.T) {
@@ -41,7 +41,7 @@ func TestClientOpt(t *testing.T) {
 						Method:  "Zip",
 					},
 				},
-				MaxRequestMessageBytes: &wrappers.UInt32Value{Value: 123456},
+				MaxRequestMessageBytes:  &wrappers.UInt32Value{Value: 123456},
 				MaxResponseMessageBytes: &wrappers.UInt32Value{Value: 123456},
 				RetryOrHedgingPolicy: &conf.MethodConfig_RetryPolicy_{
 					RetryPolicy: &conf.MethodConfig_RetryPolicy{
@@ -60,7 +60,7 @@ func TestClientOpt(t *testing.T) {
 						Service: "bar.FooService",
 					},
 				},
-				MaxRequestMessageBytes: &wrappers.UInt32Value{Value: 654321},
+				MaxRequestMessageBytes:  &wrappers.UInt32Value{Value: 654321},
 				MaxResponseMessageBytes: &wrappers.UInt32Value{Value: 654321},
 				RetryOrHedgingPolicy: &conf.MethodConfig_RetryPolicy_{
 					RetryPolicy: &conf.MethodConfig_RetryPolicy{