protoc-gen-go-grpc: add requirement of embedding UnimplementedServer …

…in services (grpc#3657)

balancer/grpclb/grpclb_test.go

@@ -187,6 +187,7 @@ func (s *rpcStats) String() string {
 }
 
 type remoteBalancer struct {
+	lbgrpc.UnimplementedLoadBalancerServer
 	sls       chan *lbpb.ServerList
 	statsDura time.Duration
 	done      chan struct{}

balancer/rls/internal/testutils/fakeserver/fakeserver.go

@@ -49,6 +49,7 @@ type Response struct {
 // Server is a fake implementation of RLS. It exposes channels to send/receive
 // RLS requests and responses.
 type Server struct {
+	rlsgrpc.UnimplementedRouteLookupServiceServer
 	RequestChan  *testutils.Channel
 	ResponseChan chan Response
 	Address      string

benchmark/benchmark.go

@@ -60,6 +60,7 @@ func NewPayload(t testpb.PayloadType, size int) *testpb.Payload {
 }
 
 type testServer struct {
+	testpb.UnimplementedBenchmarkServiceServer
 }
 
 func (s *testServer) UnaryCall(ctx context.Context, in *testpb.SimpleRequest) (*testpb.SimpleResponse, error) {
@@ -141,6 +142,7 @@ func (s *testServer) UnconstrainedStreamingCall(stream testpb.BenchmarkService_U
 // byteBufServer is a gRPC server that sends and receives byte buffer.
 // The purpose is to benchmark the gRPC performance without protobuf serialization/deserialization overhead.
 type byteBufServer struct {
+	testpb.UnimplementedBenchmarkServiceServer
 	respSize int32
 }
 

benchmark/worker/main.go

@@ -73,6 +73,7 @@ func (byteBufCodec) String() string {
 // workerServer implements WorkerService rpc handlers.
 // It can create benchmarkServer or benchmarkClient on demand.
 type workerServer struct {
+	testpb.UnimplementedWorkerServiceServer
 	stop       chan<- bool
 	serverPort int
 }

channelz/service/service.go

@@ -48,7 +48,9 @@ func newCZServer() channelzgrpc.ChannelzServer {
 	return &serverImpl{}
 }
 
-type serverImpl struct{}
+type serverImpl struct {
+	channelzgrpc.UnimplementedChannelzServer
+}
 
 func connectivityStateToProto(s connectivity.State) *channelzpb.ChannelConnectivityState {
 	switch s {

cmd/protoc-gen-go-grpc/README.md

@@ -0,0 +1,21 @@
+# protoc-gen-go-grpc
+
+This tool generates Go language bindings of `service`s in protobuf definition
+files for gRPC.  For usage information, please see our [quick start
+guide](https://grpc.io/docs/languages/go/quickstart/).
+
+## Future-proofing services
+
+By default, to register services using the methods generated by this tool, the
+service implementations must embed the corresponding
+`Unimplemented<ServiceName>Server` for future compatibility.  This is a behavior
+change from the grpc code generator previously included with `protoc-gen-go`.
+To restore this behavior, set the option `requireUnimplementedServers=false`.
+E.g.:
+
+```
+  protoc --go-grpc_out=requireUnimplementedServers=false[,other options...]:. \
+```
+
+Note that this is not recommended, and the option is only provided to restore
+backward compatibility with previously-generated code.

cmd/protoc-gen-go-grpc/grpc.go

@@ -119,9 +119,16 @@ func genService(gen *protogen.Plugin, file *protogen.File, g *protogen.Generated
 		}
 	}
 
+	mustOrShould := "must"
+	if !*requireUnimplemented {
+		mustOrShould = "should"
+	}
+
 	// Server interface.
 	serverType := service.GoName + "Server"
 	g.P("// ", serverType, " is the server API for ", service.GoName, " service.")
+	g.P("// All implementations ", mustOrShould, " embed Unimplemented", serverType)
+	g.P("// for forward compatibility")
 	if service.Desc.Options().(*descriptorpb.ServiceOptions).GetDeprecated() {
 		g.P("//")
 		g.P(deprecationComment)
@@ -136,11 +143,14 @@ func genService(gen *protogen.Plugin, file *protogen.File, g *protogen.Generated
 		g.P(method.Comments.Leading,
 			serverSignature(g, method))
 	}
+	if *requireUnimplemented {
+		g.P("mustEmbedUnimplemented", serverType, "()")
+	}
 	g.P("}")
 	g.P()
 
 	// Server Unimplemented struct for forward compatibility.
-	g.P("// Unimplemented", serverType, " can be embedded to have forward compatible implementations.")
+	g.P("// Unimplemented", serverType, " ", mustOrShould, " be embedded to have forward compatible implementations.")
 	g.P("type Unimplemented", serverType, " struct {")
 	g.P("}")
 	g.P()
@@ -153,6 +163,9 @@ func genService(gen *protogen.Plugin, file *protogen.File, g *protogen.Generated
 		g.P("return ", nilArg, statusPackage.Ident("Errorf"), "(", codesPackage.Ident("Unimplemented"), `, "method `, method.GoName, ` not implemented")`)
 		g.P("}")
 	}
+	if *requireUnimplemented {
+		g.P("func (*Unimplemented", serverType, ") mustEmbedUnimplemented", serverType, "() {}")
+	}
 	g.P()
 
 	// Server registration.

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

@@ -31,11 +31,20 @@
 package main
 
 import (
+	"flag"
+
 	"google.golang.org/protobuf/compiler/protogen"
 )
 
+var requireUnimplemented *bool
+
 func main() {
-	protogen.Options{}.Run(func(gen *protogen.Plugin) error {
+	var flags flag.FlagSet
+	requireUnimplemented = flags.Bool("requireUnimplementedServers", true, "unset to match legacy behavior")
+
+	protogen.Options{
+		ParamFunc: flags.Set,
+	}.Run(func(gen *protogen.Plugin) error {
 		for _, f := range gen.Files {
 			if !f.Generate {
 				continue

health/server.go

@@ -33,6 +33,7 @@ import (
 
 // Server implements `service Health`.
 type Server struct {
+	healthgrpc.UnimplementedHealthServer
 	mu sync.RWMutex
 	// If shutdown is true, it's expected all serving status is NOT_SERVING, and
 	// will stay in NOT_SERVING.

interop/fake_grpclb/fake_grpclb.go

@@ -48,6 +48,7 @@ var (
 )
 
 type loadBalancerServer struct {
+	lbpb.UnimplementedLoadBalancerServer
 	serverListResponse *lbpb.LoadBalanceResponse
 }
 

interop/test_utils.go

@@ -672,6 +672,7 @@ func DoPickFirstUnary(tc testpb.TestServiceClient) {
 }
 
 type testServer struct {
+	testpb.UnimplementedTestServiceServer
 }
 
 // NewTestServer creates a test server for test service.

interop/xds/client/client.go

@@ -60,7 +60,9 @@ var (
 	watchers         = make(map[statsWatcherKey]*statsWatcher)
 )
 
-type statsService struct{}
+type statsService struct {
+	testpb.UnimplementedLoadBalancerStatsServiceServer
+}
 
 // Wait for the next LoadBalancerStatsRequest.GetNumRpcs to start and complete,
 // and return the distribution of remote peers. This is essentially a clientside