Merge pull request #61 from Gsantomaggio/zerpet/staticcheck-warnings

Zerpet · web-flow · commit fc18bdfc9d07 · 2023-04-24T09:29:12.000+01:00
Fix staticcheck errors
diff --git a/.github/workflows/build_test_linux.yml b/.github/workflows/build_test_linux.yml
@@ -10,14 +10,13 @@ on:
     branches: [ "main" ]
 
 jobs:
-
   build:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
 
     - name: Set up Go
-      uses: actions/setup-go@v3
+      uses: actions/setup-go@v4
       with:
         go-version: oldstable
 
diff --git a/.github/workflows/static-check.yaml b/.github/workflows/static-check.yaml
@@ -0,0 +1,24 @@
+
+name: "Static Check Analysis"
+
+on: ["pull_request"]
+
+jobs:
+  static-check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 1
+
+      - name: Set up Go
+        uses: actions/setup-go@v4
+        with:
+          go-version: 'stable'
+
+      - name: Staticcheck
+        uses: dominikh/staticcheck-action@v1.3.0
+        with:
+          version: "2023.1.3"
+          install-go: false
+          build-tags: "rabbitmq.stream.test,rabbitmq.stream.e2e"
diff --git a/go.mod b/go.mod
@@ -1,6 +1,6 @@
 module github.com/gsantomaggio/rabbitmq-stream-go-client
 
-go 1.19
+go 1.20
 
 require (
 	github.com/golang/mock v1.6.0
@@ -13,7 +13,7 @@ require (
 	github.com/go-logr/logr v1.2.3 // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
 	golang.org/x/net v0.2.0 // indirect
-	golang.org/x/sys v0.2.0 // indirect
+	golang.org/x/sys v0.7.0 // indirect
 	golang.org/x/text v0.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/go.sum b/go.sum
@@ -27,8 +27,8 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
-golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU=
+golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
diff --git a/internal/exchange_command_versions.go b/internal/exchange_command_versions.go
@@ -117,9 +117,7 @@ type ExchangeCommandVersionsResponse struct {
 
 func NewExchangeCommandVersionsResponse(id uint32, responseCode uint16, commands ...commandInformer) *ExchangeCommandVersionsResponse {
 	e := &ExchangeCommandVersionsResponse{correlationId: id, responseCode: responseCode}
-	for _, c := range commands {
-		e.commands = append(e.commands, c)
-	}
+	e.commands = append(e.commands, commands...)
 	return e
 }
 
diff --git a/internal/open.go b/internal/open.go
@@ -71,9 +71,12 @@ func (o *OpenResponse) MarshalBinary() (data []byte, err error) {
 		return nil, fmt.Errorf("error in binary marshal: wrote %d expected %d", n, 6)
 	}
 
-	if o.connectionProperties != nil && len(o.connectionProperties) > 0 {
+	if len(o.connectionProperties) > 0 {
 		// FIXME: check how many bytes were written
 		_, err = writeMany(wr, o.connectionProperties)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	err = wr.Flush()
diff --git a/internal/sasl_mechanisms.go b/internal/sasl_mechanisms.go
@@ -66,6 +66,9 @@ func (s *SaslHandshakeResponse) MarshalBinary() (data []byte, err error) {
 
 	for _, mechanism := range s.Mechanisms {
 		n, err = writeString(wr, mechanism)
+		if err != nil {
+			return nil, err
+		}
 		if n != (len(mechanism) + 2) {
 			return nil, fmt.Errorf("error marshalling: wrote %d, expected %d", n, len(mechanism)+2)
 		}
diff --git a/internal/wire_formatting.go b/internal/wire_formatting.go
@@ -1,7 +1,6 @@
 package internal
 
 import (
-	"bufio"
 	"encoding/binary"
 	"io"
 )
@@ -18,14 +17,6 @@ func readUInt(readerStream io.Reader) (uint32, error) {
 	return res, err
 }
 
-func peekByte(readerStream *bufio.Reader) (uint8, error) {
-	res, err := readerStream.Peek(1)
-	if err != nil {
-		return 0, err
-	}
-	return res[0], nil
-}
-
 func readString(readerStream io.Reader) string {
 	// FIXME: handle the potential error from readUShort
 	lenString, _ := readUShort(readerStream)
@@ -67,23 +58,21 @@ func readMany(readerStream io.Reader, args ...interface{}) error {
 
 func readAny(readerStream io.Reader, arg interface{}) error {
 
-	switch arg.(type) {
+	switch arg := arg.(type) {
 	case *int:
 		uInt, err := readUInt(readerStream)
 		if err != nil {
 			return err
 		}
-		*arg.(*int) = int(uInt)
-		break
+		*arg = int(uInt)
 	case *string:
-		*arg.(*string) = readString(readerStream)
-		break
+		*arg = readString(readerStream)
 	case *[]byte:
 		byteSlice, err := readByteSlice(readerStream)
 		if err != nil {
 			return err
 		}
-		*arg.(*[]byte) = byteSlice
+		*arg = byteSlice
 	case *map[string]string:
 		mapLen, err := readUInt(readerStream)
 		if err != nil {
@@ -95,7 +84,7 @@ func readAny(readerStream io.Reader, arg interface{}) error {
 			v := readString(readerStream)
 			myMap[k] = v
 		}
-		*arg.(*map[string]string) = myMap
+		*arg = myMap
 	default:
 		err := binary.Read(readerStream, binary.BigEndian, arg)
 		if err != nil {
@@ -113,31 +102,31 @@ func writeMany(writer io.Writer, args ...any) (int, error) {
 	var written int
 
 	for _, arg := range args {
-		switch arg.(type) {
+		switch arg := arg.(type) {
 		case int:
-			err := binary.Write(writer, binary.BigEndian, int32(arg.(int)))
+			err := binary.Write(writer, binary.BigEndian, int32(arg))
 			if err != nil {
 				return written, err
 			}
-			written += binary.Size(int32(arg.(int)))
-			break
+			written += binary.Size(int32(arg))
 		case string:
-			n, err := writeString(writer, arg.(string))
+			n, err := writeString(writer, arg)
 			if err != nil {
 				return written, err
 			}
 			written += n
-			break
 		case map[string]string:
-			m := arg.(map[string]string)
-			n, err := writeMany(writer, len(m))
+			n, err := writeMany(writer, len(arg))
 			if err != nil {
 				return n, err
 			}
 			written += n
-			for key, value := range m {
+			for key, value := range arg {
 				n, err := writeString(writer, key)
 				written += n
+				if err != nil {
+					return n, err
+				}
 				n, err = writeString(writer, value)
 				written += n
 				if err != nil {
diff --git a/main.go b/main.go
@@ -61,7 +61,6 @@ func main() {
 				if (confirmed % totalMessages) == 0 {
 					log.Info("Confirmed", "messages", confirmed)
 				}
-				break
 			case 2:
 				confirmed += len(c.PublishingIds())
 				log.Info("Sub Entry Send Confirmed", "messages", confirmed)
@@ -112,6 +111,9 @@ func main() {
 	}()
 
 	err = streamClient.Subscribe(ctx, stream, constants.OffsetTypeFirst, 1, 10, map[string]string{"name": "my_consumer"}, 10)
+	if err != nil {
+		panic(err)
+	}
 
 	offset, err := streamClient.QueryOffset(ctx, "my_consumer", stream)
 	if err != nil {
diff --git a/pkg/e2e/end_to_end_test.go b/pkg/e2e/end_to_end_test.go
@@ -117,7 +117,7 @@ var _ = Describe("E2E", Serial, Label("e2e"), func() {
 })
 
 func wrap[T any](v T) []T {
-	r := make([]T, 1, 1)
+	r := make([]T, 1)
 	r[0] = v
 	return r
 }
diff --git a/pkg/raw/client.go b/pkg/raw/client.go
@@ -54,7 +54,6 @@ type Client struct {
 	confirmsCh           chan *PublishConfirm
 	chunkCh              chan *Chunk
 	notifyCh             chan *CreditError
-	metadataCh           chan *MetadataResponse
 }
 
 // IsOpen returns true if the connection is open, false otherwise
@@ -538,6 +537,9 @@ func (tc *Client) handleClose(ctx context.Context, req *internal.CloseRequest) e
 	if err != nil {
 		return err
 	}
+	if n != 14 { // simple response is always 10 bytes + 4 bytes for frame len
+		return errWriteShort
+	}
 	return tc.connection.GetWriter().Flush()
 }
 
diff --git a/pkg/raw/client_types.go b/pkg/raw/client_types.go
@@ -18,6 +18,7 @@ var (
 	errNilConfig           = errors.New("RabbitmqConfiguration cannot be nil")
 	errUnknownSubscription = errors.New("unknown subscription ID")
 	errNoMoreBrokersToTry  = errors.New("failed to dial RabbitMQ: no more brokers to try")
+	errWriteShort          = errors.New("wrote less bytes than expected")
 )
 
 var schemePorts = map[string]int{"rabbitmq-stream": 5552, "rabbitmq-stream+tls": 5551}
@@ -91,7 +92,7 @@ func NewClientConfiguration(rabbitmqUrls ...string) (*ClientConfiguration, error
 		clientMaxFrameSize: 1_048_576,
 	}
 
-	if rabbitmqUrls == nil || len(rabbitmqUrls) == 0 {
+	if len(rabbitmqUrls) == 0 {
 		builder.rabbitmqBrokers = append(builder.rabbitmqBrokers, defaultBroker)
 		return builder, nil
 	}
diff --git a/pkg/raw/stream_suite_test.go b/pkg/raw/stream_suite_test.go
@@ -523,6 +523,7 @@ func (rmq *fakeRabbitMQServer) fakeRabbitMQNewConsumer(
 	expectOffset1(err).ToNot(HaveOccurred())
 
 	data, err := bodyResp.MarshalBinary()
+	expectOffset1(err).ToNot(HaveOccurred())
 	_, err = serverWriter.Write(data)
 	expectOffset1(err).ToNot(HaveOccurred())
 	expectOffset1(serverWriter.Flush()).To(Succeed())
@@ -562,7 +563,7 @@ func (rmq *fakeRabbitMQServer) fakeRabbitMQPublisherConfirms(pubId uint8, numOfC
 	expectOffset1(rmq.connection.SetDeadline(time.Now().Add(rmq.deadlineDelta))).
 		To(Succeed())
 
-	fakeConfirms := make([]uint64, numOfConfirms, numOfConfirms)
+	fakeConfirms := make([]uint64, numOfConfirms)
 	for i := 0; i < len(fakeConfirms); i++ {
 		fakeConfirms[i] = uint64(i)
 	}
@@ -687,18 +688,26 @@ func (rmq *fakeRabbitMQServer) fakeRabbitMQCreditResponse(ctx context.Context, s
 	expectOffset1(serverWriter.Flush()).To(Succeed())
 }
 
+type ctxKey struct {
+	string
+}
+
 func newContextWithResponseCode(ctx context.Context, respCode uint16, suffix ...string) context.Context {
-	var key = "rabbitmq-stream.response-code"
+	var key ctxKey
 	if suffix != nil {
-		key += suffix[0]
+		key = ctxKey{"rabbitmq-stream.response-code" + suffix[0]}
+	} else {
+		key = ctxKey{"rabbitmq-stream.response-code"}
 	}
 	return context.WithValue(ctx, key, respCode)
 }
 
 func responseCodeFromContext(ctx context.Context, suffix ...string) (responseCode uint16) {
-	var key = "rabbitmq-stream.response-code"
+	var key ctxKey
 	if suffix != nil {
-		key += suffix[0]
+		key = ctxKey{"rabbitmq-stream.response-code" + suffix[0]}
+	} else {
+		key = ctxKey{"rabbitmq-stream.response-code"}
 	}
 	v := ctx.Value(key)
 	if v == nil {

Original file line number	Diff line number	Diff line change
`@@ -117,9 +117,7 @@ type ExchangeCommandVersionsResponse struct {`
`117`	`117`
`118`	`118`	`func NewExchangeCommandVersionsResponse(id uint32, responseCode uint16, commands ...commandInformer) *ExchangeCommandVersionsResponse {`
`119`	`119`	`e := &ExchangeCommandVersionsResponse{correlationId: id, responseCode: responseCode}`
`120`		`- for _, c := range commands {`
`121`		`- e.commands = append(e.commands, c)`
`122`		`- }`
	`120`	`+ e.commands = append(e.commands, commands...)`
`123`	`121`	`return e`
`124`	`122`	`}`
`125`	`123`
Original file line number	Diff line number	Diff line change
`@@ -71,9 +71,12 @@ func (o *OpenResponse) MarshalBinary() (data []byte, err error) {`
`71`	`71`	`return nil, fmt.Errorf("error in binary marshal: wrote %d expected %d", n, 6)`
`72`	`72`	`}`
`73`	`73`
`74`		`- if o.connectionProperties != nil && len(o.connectionProperties) > 0 {`
	`74`	`+ if len(o.connectionProperties) > 0 {`
`75`	`75`	`// FIXME: check how many bytes were written`
`76`	`76`	`_, err = writeMany(wr, o.connectionProperties)`
	`77`	`+ if err != nil {`
	`78`	`+ return nil, err`
	`79`	`+ }`
`77`	`80`	`}`
`78`	`81`
`79`	`82`	`err = wr.Flush()`
Original file line number	Diff line number	Diff line change
`@@ -66,6 +66,9 @@ func (s *SaslHandshakeResponse) MarshalBinary() (data []byte, err error) {`
`66`	`66`
`67`	`67`	`for _, mechanism := range s.Mechanisms {`
`68`	`68`	`n, err = writeString(wr, mechanism)`
	`69`	`+ if err != nil {`
	`70`	`+ return nil, err`
	`71`	`+ }`
`69`	`72`	`if n != (len(mechanism) + 2) {`
`70`	`73`	`return nil, fmt.Errorf("error marshalling: wrote %d, expected %d", n, len(mechanism)+2)`
`71`	`74`	`}`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ var _ = Describe("E2E", Serial, Label("e2e"), func() {`
`117`	`117`	`})`
`118`	`118`
`119`	`119`	`func wrap[T any](v T) []T {`
`120`		`- r := make([]T, 1, 1)`
	`120`	`+ r := make([]T, 1)`
`121`	`121`	`r[0] = v`
`122`	`122`	`return r`
`123`	`123`	`}`