Remove dependency on grpc-go in internal/testprotos (#519)

* Cleanup CI config to use newer Go versions and newer circleci images
* Use newer protoc plugins (separate for normal code vs. gRPC-related code)
* Remove dependency on grpc-go in internal/testprotos by not generating gRPC stubs for that package
* Copy testprotos.TestService into DummyService in grpc sub-package, since we no longer generate gRPC stubs for testprotos
* Update tests to look for DummyService; fix other related errors that fall out of using new protoc grpc plugin

Co-authored-by: bufdev <bufdev-github@buf.build>

.circleci/config.yml

@@ -11,31 +11,51 @@ shared_configs:
 version: 2.1
 
 orbs:
-  win: circleci/windows@2.2.0
+  win: circleci/windows@4.1.1
 
 jobs:
   build-1-13:
     working_directory: ~/repo
     docker:
-      - image: circleci/golang:1.13
+      - image: cimg/go:1.13
     steps: *simple_job_steps
 
   build-1-14:
     working_directory: ~/repo
     docker:
-      - image: circleci/golang:1.14
+      - image: cimg/go:1.14
     steps: *simple_job_steps
 
   build-1-15:
     working_directory: ~/repo
     docker:
-      - image: circleci/golang:1.15
+      - image: cimg/go:1.15
     steps: *simple_job_steps
 
   build-1-16:
     working_directory: ~/repo
     docker:
-      - image: circleci/golang:1.16
+      - image: cimg/go:1.16
+    steps: *simple_job_steps
+
+  build-1-17:
+    working_directory: ~/repo
+    docker:
+      - image: cimg/go:1.17
+    steps: *simple_job_steps
+
+  build-windows:  # as of Feb 2022, Go 1.17
+    executor:
+      name: win/default
+    steps:
+      - run: git config --global core.autocrlf false
+      - checkout
+      - run: go test ./...
+
+  build-1-18:
+    working_directory: ~/repo
+    docker:
+      - image: cimg/go:1.18
     steps:
       - checkout
       - restore_cache:
@@ -57,24 +77,10 @@ jobs:
     #- store_test_results:
     #    path: /tmp/test-reports
 
-  build-1-16-windows:
-    executor:
-      name: win/default
-    steps:
-      - run: git config --global core.autocrlf false
-      - checkout
-      - run: go test ./...
-
-  build-1-17:
-    working_directory: ~/repo
-    docker:
-      - image: circleci/golang:1.17
-    steps: *simple_job_steps
-
-  build-1-17-u:
+  build-1-18-u:
     working_directory: ~/repo
     docker:
-      - image: circleci/golang:1.17
+      - image: cimg/go:1.18
     steps:
       - checkout
       - run:
@@ -93,6 +99,7 @@ workflows:
       - build-1-14
       - build-1-15
       - build-1-16
-      - build-1-16-windows
       - build-1-17
-      - build-1-17-u
+      - build-windows
+      - build-1-18
+      - build-1-18-u

Makefile

@@ -53,7 +53,7 @@ ineffassign:
 
 .PHONY: predeclared
 predeclared:
-	@go install github.com/nishanths/predeclared@v0.0.0-20200524104333-86fad755b4d3
+	@go install github.com/nishanths/predeclared@v0.2.2
 	predeclared ./...
 
 # Intentionally omitted from CI, but target here for ad-hoc reports.
@@ -76,7 +76,6 @@ test:
 .PHONY: generate
 generate:
 	@go install golang.org/x/tools/cmd/goyacc@v0.0.0-20200717024301-6ddee64345a6
-	@go install github.com/golang/protobuf/protoc-gen-go
 	go generate ./...
 
 .PHONY: testcover

dynamic/msgregistry/message_registry.go

@@ -341,12 +341,12 @@ func (r *MessageRegistry) ResolveApiIntoServiceDescriptor(a *api.Api) (*desc.Ser
 // UnmarshalAny will unmarshal the value embedded in the given Any value. This will use this
 // registry to resolve the given value's type URL. Use this instead of ptypes.UnmarshalAny for
 // cases where the type might not be statically linked into the current program.
-func (r *MessageRegistry) UnmarshalAny(any *any.Any) (proto.Message, error) {
-	return r.unmarshalAny(any, r.FindMessageTypeByUrl)
+func (r *MessageRegistry) UnmarshalAny(a *any.Any) (proto.Message, error) {
+	return r.unmarshalAny(a, r.FindMessageTypeByUrl)
 }
 
-func (r *MessageRegistry) unmarshalAny(any *any.Any, fetch func(string) (*desc.MessageDescriptor, error)) (proto.Message, error) {
-	name, err := ptypes.AnyMessageName(any)
+func (r *MessageRegistry) unmarshalAny(a *any.Any, fetch func(string) (*desc.MessageDescriptor, error)) (proto.Message, error) {
+	name, err := ptypes.AnyMessageName(a)
 	if err != nil {
 		return nil, err
 	}
@@ -360,16 +360,16 @@ func (r *MessageRegistry) unmarshalAny(any *any.Any, fetch func(string) (*desc.M
 		ktr = r.mf.GetKnownTypeRegistry()
 	}
 	if msg = ktr.CreateIfKnown(name); msg == nil {
-		if md, err := fetch(any.TypeUrl); err != nil {
+		if md, err := fetch(a.TypeUrl); err != nil {
 			return nil, err
 		} else if md == nil {
-			return nil, fmt.Errorf("unknown message type: %s", any.TypeUrl)
+			return nil, fmt.Errorf("unknown message type: %s", a.TypeUrl)
 		} else {
 			msg = mf.NewDynamicMessage(md)
 		}
 	}
 
-	err = proto.Unmarshal(any.Value, msg)
+	err = proto.Unmarshal(a.Value, msg)
 	if err != nil {
 		return nil, err
 	} else {

grpcreflect/client_test.go

@@ -25,7 +25,7 @@ import (
 
 	"github.com/jhump/protoreflect/desc"
 	"github.com/jhump/protoreflect/internal"
-	"github.com/jhump/protoreflect/internal/testprotos"
+	testprotosgrpc "github.com/jhump/protoreflect/internal/testprotos/grpc"
 	"github.com/jhump/protoreflect/internal/testutil"
 )
 
@@ -42,7 +42,7 @@ func TestMain(m *testing.M) {
 	}()
 
 	svr := grpc.NewServer()
-	testprotos.RegisterTestServiceServer(svr, testService{})
+	testprotosgrpc.RegisterDummyServiceServer(svr, testService{})
 	reflection.Register(svr)
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
@@ -203,7 +203,7 @@ func TestListServices(t *testing.T) {
 	testutil.Ok(t, err)
 
 	sort.Strings(s)
-	testutil.Eq(t, []string{"grpc.reflection.v1alpha.ServerReflection", "testprotos.TestService"}, s)
+	testutil.Eq(t, []string{"grpc.reflection.v1alpha.ServerReflection", "testprotos.DummyService"}, s)
 }
 
 func TestReset(t *testing.T) {

grpcreflect/server_test.go

@@ -5,27 +5,27 @@ import (
 
 	"google.golang.org/grpc"
 
-	"github.com/jhump/protoreflect/internal/testprotos"
+	testprotosgrpc "github.com/jhump/protoreflect/internal/testprotos/grpc"
 	"github.com/jhump/protoreflect/internal/testutil"
 )
 
 type testService struct {
-	testprotos.TestServiceServer
+	testprotosgrpc.DummyServiceServer
 }
 
 func TestLoadServiceDescriptors(t *testing.T) {
 	s := grpc.NewServer()
-	testprotos.RegisterTestServiceServer(s, testService{})
+	testprotosgrpc.RegisterDummyServiceServer(s, testService{})
 	sds, err := LoadServiceDescriptors(s)
 	testutil.Ok(t, err)
 	testutil.Eq(t, 1, len(sds))
-	sd := sds["testprotos.TestService"]
+	sd := sds["testprotos.DummyService"]
 
 	cases := []struct{ method, request, response string }{
-		{"DoSomething", "testprotos.TestRequest", "jhump.protoreflect.desc.Bar"},
-		{"DoSomethingElse", "testprotos.TestMessage", "testprotos.TestResponse"},
+		{"DoSomething", "testprotos.DummyRequest", "jhump.protoreflect.desc.Bar"},
+		{"DoSomethingElse", "testprotos.TestMessage", "testprotos.DummyResponse"},
 		{"DoSomethingAgain", "jhump.protoreflect.desc.Bar", "testprotos.AnotherTestMessage"},
-		{"DoSomethingForever", "testprotos.TestRequest", "testprotos.TestResponse"},
+		{"DoSomethingForever", "testprotos.DummyRequest", "testprotos.DummyResponse"},
 	}
 
 	testutil.Eq(t, len(cases), len(sd.GetMethods()))
@@ -39,14 +39,14 @@ func TestLoadServiceDescriptors(t *testing.T) {
 }
 
 func TestLoadServiceDescriptor(t *testing.T) {
-	sd, err := LoadServiceDescriptor(testprotos.TestService_ServiceDesc)
+	sd, err := LoadServiceDescriptor(&testprotosgrpc.DummyService_ServiceDesc)
 	testutil.Ok(t, err)
 
 	cases := []struct{ method, request, response string }{
-		{"DoSomething", "testprotos.TestRequest", "jhump.protoreflect.desc.Bar"},
-		{"DoSomethingElse", "testprotos.TestMessage", "testprotos.TestResponse"},
+		{"DoSomething", "testprotos.DummyRequest", "jhump.protoreflect.desc.Bar"},
+		{"DoSomethingElse", "testprotos.TestMessage", "testprotos.DummyResponse"},
 		{"DoSomethingAgain", "jhump.protoreflect.desc.Bar", "testprotos.AnotherTestMessage"},
-		{"DoSomethingForever", "testprotos.TestRequest", "testprotos.TestResponse"},
+		{"DoSomethingForever", "testprotos.DummyRequest", "testprotos.DummyResponse"},
 	}
 
 	testutil.Eq(t, len(cases), len(sd.GetMethods()))