diff --git a/.travis.yml b/.travis.yml index 8de8c12..5b64798 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,5 +9,4 @@ matrix: - go: tip script: - # TODO: change to "make" when golint, errcheck, staticcheck pass - - make deps checkgofmt vet unused test + - make diff --git a/Makefile b/Makefile index 48f25fc..1fdc13e 100644 --- a/Makefile +++ b/Makefile @@ -1,64 +1,65 @@ -SRCS := $(shell find . -name '*.go') -PKGS := $(shell go list ./...) - -.PHONY: all -all: deps lint test +# TODO: run golint and errcheck, but only to catch *new* violations and +# decide whether to change code or not (e.g. we need to be able to whitelist +# violations already in the code). They can be useful to catch errors, but +# they are just too noisy to be a requirement for a CI -- we don't even *want* +# to fix some of the things they consider to be violations. +.PHONY: ci +ci: deps checkgofmt vet staticcheck unused ineffassign predeclared test .PHONY: deps deps: - go get -d -v -t $(PKGS) + go get -d -v -t ./... .PHONY: updatedeps updatedeps: - go get -d -v -t -u -f $(PKGS) + go get -d -v -t -u -f ./... .PHONY: install install: - go install $(PKGS) - -.PHONY: golint -golint: - @go get github.com/golang/lint/golint - for file in $(SRCS); do \ - golint $${file}; \ - if [ -n "$$(golint $${file})" ]; then \ - exit 1; \ - fi; \ - done + go install ./... .PHONY: checkgofmt checkgofmt: - gofmt -s -l $(SRCS) - if [ -n "$$(gofmt -s -l $(SRCS))" ]; then \ + gofmt -s -l . + @if [ -n "$$(gofmt -s -l .)" ]; then \ exit 1; \ fi .PHONY: vet vet: - go vet $(PKGS) - -.PHONY: -errcheck: - @go get github.com/kisielk/errcheck - errcheck $(PKGS) + go vet ./... .PHONY: staticcheck staticcheck: @go get honnef.co/go/tools/cmd/staticcheck - staticcheck $(PKGS) + staticcheck ./... .PHONY: unused unused: @go get honnef.co/go/tools/cmd/unused - unused $(PKGS) + unused ./... -.PHONY: lint -lint: golint checkgofmt vet errcheck staticcheck unused +.PHONY: ineffassign +ineffassign: + @go get github.com/gordonklaus/ineffassign + ineffassign . + +.PHONY: predeclared + @go get github.com/nishanths/predeclared + predeclared . + +# Intentionally omitted from CI, but target here for ad-hoc reports. +.PHONY: golint +golint: + @go get github.com/golang/lint/golint + golint -min_confidence 0.9 -set_exit_status ./... + +# Intentionally omitted from CI, but target here for ad-hoc reports. +.PHONY: +errcheck: + @go get github.com/kisielk/errcheck + errcheck ./... .PHONY: test test: - go test -race $(PKGS) - -.PHONY: clean -clean: - go clean -i $(PKGS) + go test -race ./... diff --git a/cmd/grpcurl/grpcurl.go b/cmd/grpcurl/grpcurl.go index 7500655..b317c11 100644 --- a/cmd/grpcurl/grpcurl.go +++ b/cmd/grpcurl/grpcurl.go @@ -235,7 +235,9 @@ func main() { fail(err, "Failed to configure transport credentials") } if *serverName != "" { - creds.OverrideServerName(*serverName) + if err := creds.OverrideServerName(*serverName); err != nil { + fail(err, "Failed to override server name as %q", *serverName) + } } } cc, err := grpcurl.BlockingDial(ctx, target, creds, opts...) @@ -360,7 +362,10 @@ func main() { tmpl := makeTemplate(dynamic.NewMessage(dsc)) fmt.Println("\nMessage template:") jsm := jsonpb.Marshaler{Indent: " ", EmitDefaults: true} - jsm.Marshal(os.Stdout, tmpl) + err := jsm.Marshal(os.Stdout, tmpl) + if err != nil { + fail(err, "Failed to print template for message %s", s) + } fmt.Println() } } @@ -476,10 +481,9 @@ func (h *handler) getRequestData() ([]byte, error) { var msg json.RawMessage if err := h.dec.Decode(&msg); err != nil { return nil, err - } else { - h.reqCount++ - return msg, nil } + h.reqCount++ + return msg, nil } func (*handler) OnReceiveHeaders(md metadata.MD) { diff --git a/grpcurl.go b/grpcurl.go index 095ac88..550708a 100644 --- a/grpcurl.go +++ b/grpcurl.go @@ -304,9 +304,8 @@ func InvokeRpc(ctx context.Context, source DescriptorSource, cc *grpc.ClientConn if err != nil { if isNotFoundError(err) { return fmt.Errorf("target server does not expose service %q", svc) - } else { - return fmt.Errorf("failed to query for service descriptor %q: %v", svc, err) } + return fmt.Errorf("failed to query for service descriptor %q: %v", svc, err) } sd, ok := dsc.(*desc.ServiceDescriptor) if !ok { @@ -714,8 +713,11 @@ func fetchAllExtensions(source DescriptorSource, ext *dynamic.ExtensionRegistry, alreadyFetched[msgTypeName] = true if len(md.GetExtensionRanges()) > 0 { fds, err := source.AllExtensionsForType(msgTypeName) + if err != nil { + return fmt.Errorf("failed to query for extensions of type %s: %v", msgTypeName, err) + } for _, fd := range fds { - if err = ext.AddExtension(fd); err != nil { + if err := ext.AddExtension(fd); err != nil { return fmt.Errorf("could not register extension %s of type %s: %v", fd.GetFullyQualifiedName(), msgTypeName, err) } } @@ -765,9 +767,8 @@ func fullyConvertToDynamic(msgFact *dynamic.MessageFactory, msg proto.Message) ( newVal, err := fullyConvertToDynamic(msgFact, v.(proto.Message)) if err != nil { return nil, err - } else { - dm.PutMapField(fd, k, newVal) } + dm.PutMapField(fd, k, newVal) } } } else if fd.IsRepeated() { @@ -777,9 +778,8 @@ func fullyConvertToDynamic(msgFact *dynamic.MessageFactory, msg proto.Message) ( newVal, err := fullyConvertToDynamic(msgFact, e.(proto.Message)) if err != nil { return nil, err - } else { - dm.SetRepeatedField(fd, i, newVal) } + dm.SetRepeatedField(fd, i, newVal) } } } else { @@ -788,9 +788,8 @@ func fullyConvertToDynamic(msgFact *dynamic.MessageFactory, msg proto.Message) ( newVal, err := fullyConvertToDynamic(msgFact, v.(proto.Message)) if err != nil { return nil, err - } else { - dm.SetField(fd, newVal) } + dm.SetField(fd, newVal) } } } @@ -936,9 +935,8 @@ func BlockingDial(ctx context.Context, address string, creds credentials.Transpo case res := <-result: if conn, ok := res.(*grpc.ClientConn); ok { return conn, nil - } else { - return nil, res.(error) } + return nil, res.(error) case <-ctx.Done(): return nil, ctx.Err() } diff --git a/testing/test_server.go b/testing/test_server.go index 37a04d1..9c04c13 100644 --- a/testing/test_server.go +++ b/testing/test_server.go @@ -15,9 +15,10 @@ import ( "github.com/fullstorydev/grpcurl" ) +// TestServer implements the TestService interface defined in example.proto. type TestServer struct{} -// One empty request followed by one empty response. +// EmptyCall accepts one empty request and issues one empty response. func (TestServer) EmptyCall(ctx context.Context, req *grpc_testing.Empty) (*grpc_testing.Empty, error) { headers, trailers, failEarly, failLate := processMetadata(ctx) grpc.SetHeader(ctx, headers) @@ -32,8 +33,8 @@ func (TestServer) EmptyCall(ctx context.Context, req *grpc_testing.Empty) (*grpc return req, nil } -// One request followed by one response. -// The server returns the client payload as-is. +// UnaryCall accepts one request and issues one response. The response includes +// the client's payload as-is. func (TestServer) UnaryCall(ctx context.Context, req *grpc_testing.SimpleRequest) (*grpc_testing.SimpleResponse, error) { headers, trailers, failEarly, failLate := processMetadata(ctx) grpc.SetHeader(ctx, headers) @@ -50,8 +51,9 @@ func (TestServer) UnaryCall(ctx context.Context, req *grpc_testing.SimpleRequest }, nil } -// One request followed by a sequence of responses (streamed download). -// The server returns the payload with client desired type and sizes. +// StreamingOutputCall accepts one request and issues a sequence of responses +// (streamed download). The server returns the payload with client desired type +// and sizes as specified in the request's ResponseParameters. func (TestServer) StreamingOutputCall(req *grpc_testing.StreamingOutputCallRequest, str grpc_testing.TestService_StreamingOutputCallServer) error { headers, trailers, failEarly, failLate := processMetadata(str.Context()) str.SetHeader(headers) @@ -87,8 +89,9 @@ func (TestServer) StreamingOutputCall(req *grpc_testing.StreamingOutputCallReque return nil } -// A sequence of requests followed by one response (streamed upload). -// The server returns the aggregated size of client payload as the result. +// StreamingInputCall accepts a sequence of requests and issues one response +// (streamed upload). The server returns the aggregated size of client payloads +// as the result. func (TestServer) StreamingInputCall(str grpc_testing.TestService_StreamingInputCallServer) error { headers, trailers, failEarly, failLate := processMetadata(str.Context()) str.SetHeader(headers) @@ -121,9 +124,9 @@ func (TestServer) StreamingInputCall(str grpc_testing.TestService_StreamingInput return nil } -// A sequence of requests with each request served by the server immediately. -// As one request could lead to multiple responses, this interface -// demonstrates the idea of full duplexing. +// FullDuplexCall accepts a sequence of requests with each request served by the +// server immediately. As one request could lead to multiple responses, this +// interface demonstrates the idea of full duplexing. func (TestServer) FullDuplexCall(str grpc_testing.TestService_FullDuplexCallServer) error { headers, trailers, failEarly, failLate := processMetadata(str.Context()) str.SetHeader(headers) @@ -163,10 +166,10 @@ func (TestServer) FullDuplexCall(str grpc_testing.TestService_FullDuplexCallServ return nil } -// A sequence of requests followed by a sequence of responses. -// The server buffers all the client requests and then serves them in order. A -// stream of responses are returned to the client when the server starts with -// first request. +// HalfDuplexCall accepts a sequence of requests and issues a sequence of +// responses. The server buffers all the client requests and then serves them +// in order. A stream of responses is returned to the client once the client +// half-closes the stream. func (TestServer) HalfDuplexCall(str grpc_testing.TestService_HalfDuplexCallServer) error { headers, trailers, failEarly, failLate := processMetadata(str.Context()) str.SetHeader(headers) @@ -204,10 +207,26 @@ func (TestServer) HalfDuplexCall(str grpc_testing.TestService_HalfDuplexCallServ } const ( - MetadataReplyHeaders = "reply-with-headers" + // MetadataReplyHeaders is a request header that contains values that will + // be echoed back to the client as response headers. The format of the value + // is "key: val". To have the server reply with more than one response + // header, supply multiple values in request metadata. + MetadataReplyHeaders = "reply-with-headers" + // MetadataReplyTrailers is a request header that contains values that will + // be echoed back to the client as response trailers. Its format its the + // same as MetadataReplyHeaders. MetadataReplyTrailers = "reply-with-trailers" - MetadataFailEarly = "fail-early" - MetadataFailLate = "fail-late" + // MetadataFailEarly is a request header that, if present and not zero, + // indicates that the RPC should fail immediately with that code. + MetadataFailEarly = "fail-early" + // MetadataFailLate is a request header that, if present and not zero, + // indicates that the RPC should fail at the end with that code. This is + // different from MetadataFailEarly only for streaming calls. An early + // failure means the call to fail before any request stream is read or any + // response stream is generated. A late failure means the entire request and + // response streams will be consumed/processed and only then will the error + // code be sent. + MetadataFailLate = "fail-late" ) func processMetadata(ctx context.Context) (metadata.MD, metadata.MD, codes.Code, codes.Code) {