Improve comments & add procedure consts to generated code (#480)

cmd/protoc-gen-connect-go/main.go

@@ -51,6 +51,7 @@ import (
 
  "connectrpc.com/connect"
  "google.golang.org/protobuf/compiler/protogen"
+ "google.golang.org/protobuf/reflect/protoreflect"
  "google.golang.org/protobuf/types/descriptorpb"
  "google.golang.org/protobuf/types/pluginpb"
 )
@@ -68,6 +69,11 @@ const (
  usage = "See https://connectrpc.com/docs/go/getting-started to learn how to use this plugin.\n\nFlags:\n -h, --help\tPrint this help and exit.\n --version\tPrint the version and exit."
 
  commentWidth = 97 // leave room for "// "
+
+ // To propagate top-level comments, we need the field number of the syntax
+ // declaration and the package name in the file descriptor.
+ protoSyntaxFieldNum = 12
+ protoPackageFieldNum = 2
 )
 
 func main() {
@@ -124,6 +130,15 @@ func generate(plugin *protogen.Plugin, file *protogen.File) {
 }
 
 func generatePreamble(g *protogen.GeneratedFile, file *protogen.File) {
+ syntaxPath := protoreflect.SourcePath{protoSyntaxFieldNum}
+ syntaxLocation := file.Desc.SourceLocations().ByPath(syntaxPath)
+ for _, comment := range syntaxLocation.LeadingDetachedComments {
+ leadingComments(g, protogen.Comments(comment), false /* deprecated */)
+ }
+ g.P()
+ leadingComments(g, protogen.Comments(syntaxLocation.LeadingComments), false /* deprecated */)
+ g.P()
+
  g.P("// Code generated by ", filepath.Base(os.Args[0]), ". DO NOT EDIT.")
  g.P("//")
  if file.Proto.GetOptions().GetDeprecated() {
@@ -132,6 +147,15 @@ func generatePreamble(g *protogen.GeneratedFile, file *protogen.File) {
  g.P("// Source: ", file.Desc.Path())
  }
  g.P()
+
+ pkgPath := protoreflect.SourcePath{protoPackageFieldNum}
+ pkgLocation := file.Desc.SourceLocations().ByPath(pkgPath)
+ for _, comment := range pkgLocation.LeadingDetachedComments {
+ leadingComments(g, protogen.Comments(comment), false /* deprecated */)
+ }
+ g.P()
+ leadingComments(g, protogen.Comments(pkgLocation.LeadingComments), false /* deprecated */)
+
  g.P("package ", file.GoPackageName)
  g.P()
  wrapComments(g, "This is a compile-time assertion to ensure that this generated file ",
@@ -144,12 +168,37 @@ func generatePreamble(g *protogen.GeneratedFile, file *protogen.File) {
 }
 
 func generateServiceNameConstants(g *protogen.GeneratedFile, services []*protogen.Service) {
+ var numMethods int
  g.P("const (")
  for _, service := range services {
  constName := fmt.Sprintf("%sName", service.Desc.Name())
  wrapComments(g, constName, " is the fully-qualified name of the ",
  service.Desc.Name(), " service.")
  g.P(constName, ` = "`, service.Desc.FullName(), `"`)
+ numMethods += len(service.Methods)
+ }
+ g.P(")")
+ g.P()
+
+ if numMethods == 0 {
+ return
+ }
+ wrapComments(g, "These constants are the fully-qualified names of the RPCs defined in this package. ",
+ "They're exposed at runtime as Spec.Procedure and as the final two segments of the HTTP route.")
+ g.P("//")
+ wrapComments(g, "Note that these are different from the fully-qualified method names used by ",
+ "google.golang.org/protobuf/reflect/protoreflect. To convert from these constants to ",
+ "reflection-formatted method names, remove the leading slash and convert the ",
+ "remaining slash to a period.")
+ g.P("const (")
+ for _, service := range services {
+ for _, method := range service.Methods {
+ // The runtime exposes this value as Spec.Procedure, so we should use the
+ // same term here.
+ wrapComments(g, procedureConstName(method), " is the fully-qualified name of the ",
+ service.Desc.Name(), "'s ", method.Desc.Name(), " RPC.")
+ g.P(procedureConstName(method), ` = "`, fmt.Sprintf("/%s/%s", service.Desc.FullName(), method.Desc.Name()), `"`)
+ }
  }
  g.P(")")
  g.P()
@@ -212,7 +261,7 @@ func generateClientImplementation(g *protogen.GeneratedFile, service *protogen.S
  "(",
  )
  g.P("httpClient,")
- g.P(`baseURL + "`, procedureName(method), `",`)
+ g.P(`baseURL + `, procedureConstName(method), `,`)
  g.P("opts...,")
  g.P("),")
  }
@@ -329,15 +378,15 @@ func generateServerConstructor(g *protogen.GeneratedFile, service *protogen.Serv
  isStreamingClient := method.Desc.IsStreamingClient()
  switch {
  case isStreamingClient && !isStreamingServer:
- g.P(`mux.Handle("`, procedureName(method), `", `, connectPackage.Ident("NewClientStreamHandler"), "(")
+ g.P(`mux.Handle(`, procedureConstName(method), `, `, connectPackage.Ident("NewClientStreamHandler"), "(")
  case !isStreamingClient && isStreamingServer:
- g.P(`mux.Handle("`, procedureName(method), `", `, connectPackage.Ident("NewServerStreamHandler"), "(")
+ g.P(`mux.Handle(`, procedureConstName(method), `, `, connectPackage.Ident("NewServerStreamHandler"), "(")
  case isStreamingClient && isStreamingServer:
- g.P(`mux.Handle("`, procedureName(method), `", `, connectPackage.Ident("NewBidiStreamHandler"), "(")
+ g.P(`mux.Handle(`, procedureConstName(method), `, `, connectPackage.Ident("NewBidiStreamHandler"), "(")
  default:
- g.P(`mux.Handle("`, procedureName(method), `", `, connectPackage.Ident("NewUnaryHandler"), "(")
+ g.P(`mux.Handle(`, procedureConstName(method), `, `, connectPackage.Ident("NewUnaryHandler"), "(")
  }
- g.P(`"`, procedureName(method), `",`)
+ g.P(procedureConstName(method), `,`)
  g.P("svc.", method.GoName, ",")
  g.P("opts...,")
  g.P("))")
@@ -410,13 +459,8 @@ func serverSignatureParams(g *protogen.GeneratedFile, method *protogen.Method, n
  g.QualifiedGoIdent(method.Output.GoIdent) + "], error)"
 }
 
-func procedureName(method *protogen.Method) string {
- return fmt.Sprintf(
- "/%s.%s/%s",
- method.Parent.Desc.ParentFile().Package(),
- method.Parent.Desc.Name(),
- method.Desc.Name(),
- )
+func procedureConstName(m *protogen.Method) string {
+ return fmt.Sprintf("%s%sProcedure", m.Parent.GoName, m.GoName)
 }
 
 func reflectionName(service *protogen.Service) string {

connect.go

@@ -271,6 +271,9 @@ type HTTPClient interface {
 }
 
 // Spec is a description of a client call or a handler invocation.
+//
+// If you're using Protobuf, protoc-gen-connect-go generates a constant for the
+// fully-qualified Procedure corresponding to each RPC in your schema.
 type Spec struct {
  StreamType StreamType
  Procedure string // for example, "/acme.foo.v1.FooService/Bar"

connect_ext_test.go

@@ -1520,7 +1520,7 @@ func TestStreamForServer(t *testing.T) {
  client, server := newPingServer(&pluggablePingServer{
  cumSum: func(ctx context.Context, stream *connect.BidiStream[pingv1.CumSumRequest, pingv1.CumSumResponse]) error {
  assert.Equal(t, stream.Spec().StreamType, connect.StreamTypeBidi)
- assert.Equal(t, stream.Spec().Procedure, "/connect.ping.v1.PingService/CumSum")
+ assert.Equal(t, stream.Spec().Procedure, pingv1connect.PingServiceCumSumProcedure)
  assert.False(t, stream.Spec().IsClient)
  return nil
  },
@@ -1535,7 +1535,7 @@ func TestStreamForServer(t *testing.T) {
  client, server := newPingServer(&pluggablePingServer{
  countUp: func(ctx context.Context, req *connect.Request[pingv1.CountUpRequest], stream *connect.ServerStream[pingv1.CountUpResponse]) error {
  assert.Equal(t, stream.Conn().Spec().StreamType, connect.StreamTypeServer)
- assert.Equal(t, stream.Conn().Spec().Procedure, "/connect.ping.v1.PingService/CountUp")
+ assert.Equal(t, stream.Conn().Spec().Procedure, pingv1connect.PingServiceCountUpProcedure)
  assert.False(t, stream.Conn().Spec().IsClient)
  assert.Nil(t, stream.Send(&pingv1.CountUpResponse{Number: 1}))
  return nil
@@ -1591,7 +1591,7 @@ func TestStreamForServer(t *testing.T) {
  client, server := newPingServer(&pluggablePingServer{
  sum: func(ctx context.Context, stream *connect.ClientStream[pingv1.SumRequest]) (*connect.Response[pingv1.SumResponse], error) {
  assert.Equal(t, stream.Spec().StreamType, connect.StreamTypeClient)
- assert.Equal(t, stream.Spec().Procedure, "/connect.ping.v1.PingService/Sum")
+ assert.Equal(t, stream.Spec().Procedure, pingv1connect.PingServiceSumProcedure)
  assert.False(t, stream.Spec().IsClient)
  assert.True(t, stream.Receive())
  msg := stream.Msg()
@@ -1847,7 +1847,7 @@ func TestGRPCErrorMetadataIsTrailersOnly(t *testing.T) {
  req, err := http.NewRequestWithContext(
  context.Background(),
  http.MethodPost,
- server.URL+"/connect.ping.v1.PingService/Fail",
+ server.URL+pingv1connect.PingServiceFailProcedure,
  bytes.NewReader(body),
  )
  assert.Nil(t, err)

internal/proto/connect/ping/v1/ping.proto

@@ -12,8 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// The canonical location for this file is
+// https://github.com/connectrpc/connect-go/blob/main/internal/proto/connect/ping/v1/ping.proto.
 syntax = "proto3";
 
+// The connect.ping.v1 package contains an echo service designed to test the
+// connect-go implementation.
 package connect.ping.v1;
 
 message PingRequest {