ci: enable more linters

- forbidigo
- gocritic
- lll
- reassign
- stylecheck
- unconvert

Author: oleg-jukovec

.github/workflows/check.yaml

@@ -40,24 +40,15 @@ jobs:
         uses: golangci/golangci-lint-action@v3
         continue-on-error: true
         with:
-          # The suppression of the rule `errcheck` may be removed after adding
-          # errors check in all methods calling EncodeXxx inside.
-          # For now those methods are not even able to return any error
-          # cause of internal implementation of writer interface (see smallbuf.go).
-          #
-          # The `//nolint` workaround was not the acceptable way of warnings suppression,
-          # cause those comments get rendered in documentation by godoc.
-          # See https://github.com/tarantool/go-tarantool/pull/160#discussion_r858608221
-          #
           # The first run is for GitHub Actions error format.
-          args: -E goimports -D errcheck
+          args: --config=.golangci.yaml
 
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
           # The second run is for human-readable error format with a file name
           # and a line number.
-          args: --out-${NO_FUTURE}format colored-line-number -E goimports -D errcheck
+          args: --out-${NO_FUTURE}format colored-line-number --config=.golangci.yaml
 
   codespell:
     runs-on: ubuntu-latest

.golangci.yaml

@@ -0,0 +1,30 @@
+run:
+  timeout: 3m
+
+linters:
+  disable:
+    - errcheck
+  enable:
+    - forbidigo
+    - gocritic
+    - goimports
+    - lll
+    - reassign
+    - stylecheck
+    - unconvert
+
+linters-settings:
+  gocritic:
+    disabled-checks:
+      - ifElseChain
+  lll:
+    line-length: 100
+    tab-width: 4
+  stylecheck:
+    checks: ["all", "-ST1003"]
+
+issues:
+  exclude-rules:
+    - linters:
+      - lll
+      source: "\t?// *(see )?https://"

CHANGELOG.md

@@ -13,6 +13,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.
 - Type() method to the Request interface (#158)
 - Enumeration types for RLimitAction/iterators (#158)
 - IsNullable flag for Field (#302)
+- More linters on CI (#310)
 
 ### Changed
 

Makefile

@@ -39,7 +39,7 @@ format:
 
 .PHONY: golangci-lint
 golangci-lint:
-	golangci-lint run -E goimports -D errcheck
+	golangci-lint run --config=.golangci.yaml
 
 .PHONY: test
 test:

box_error_test.go

@@ -129,12 +129,14 @@ var mpDecodeSamples = map[string]struct {
 	"InnerMapInvalidErrorType": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x00, 0xc1},
 		false,
-		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`),
+		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1` +
+			` decoding (?:string\/bytes|bytes) length`),
 	},
 	"InnerMapInvalidErrorFile": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x01, 0xc1},
 		false,
-		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`),
+		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1` +
+			` decoding (?:string\/bytes|bytes) length`),
 	},
 	"InnerMapInvalidErrorLine": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x02, 0xc1},
@@ -144,7 +146,8 @@ var mpDecodeSamples = map[string]struct {
 	"InnerMapInvalidErrorMessage": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x03, 0xc1},
 		false,
-		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`),
+		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding` +
+			` (?:string\/bytes|bytes) length`),
 	},
 	"InnerMapInvalidErrorErrno": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x04, 0xc1},
@@ -164,7 +167,8 @@ var mpDecodeSamples = map[string]struct {
 	"InnerMapInvalidErrorFieldsKey": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x06, 0x81, 0xc1},
 		false,
-		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`),
+		regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1` +
+			` decoding (?:string\/bytes|bytes) length`),
 	},
 	"InnerMapInvalidErrorFieldsValue": {
 		[]byte{0x81, 0x00, 0x91, 0x81, 0x06, 0x81, 0xa3, 0x6b, 0x65, 0x79, 0xc1},
@@ -435,7 +439,8 @@ func TestErrorTypeSelect(t *testing.T) {
 			var resp *Response
 			var offset uint32 = 0
 			var limit uint32 = 1
-			resp, err = conn.Select(space, index, offset, limit, IterEq, []interface{}{testcase.tuple.pk})
+			resp, err = conn.Select(space, index, offset, limit, IterEq,
+				[]interface{}{testcase.tuple.pk})
 			require.Nil(t, err)
 			require.NotNil(t, resp.Data)
 			require.Equalf(t, len(resp.Data), 1, "Exactly one tuple had been found")
@@ -479,7 +484,8 @@ func TestErrorTypeSelectTyped(t *testing.T) {
 			var offset uint32 = 0
 			var limit uint32 = 1
 			var resp []TupleBoxError
-			err = conn.SelectTyped(space, index, offset, limit, IterEq, []interface{}{testcase.tuple.pk}, &resp)
+			err = conn.SelectTyped(space, index, offset, limit, IterEq,
+				[]interface{}{testcase.tuple.pk}, &resp)
 			require.Nil(t, err)
 			require.NotNil(t, resp)
 			require.Equalf(t, len(resp), 1, "Exactly one tuple had been found")

connection.go

@@ -33,6 +33,11 @@ const shutdownEventKey = "box.shutdown"
 type ConnEventKind int
 type ConnLogKind int
 
+var (
+	errUnknownRequest = errors.New("the passed connected request doesn't belong " +
+		"to the current connection or connection pool")
+)
+
 const (
 	// Connected signals that connection is established or reestablished.
 	Connected ConnEventKind = iota + 1
@@ -84,13 +89,16 @@ func (d defaultLogger) Report(event ConnLogKind, conn *Connection, v ...interfac
 	case LogReconnectFailed:
 		reconnects := v[0].(uint)
 		err := v[1].(error)
-		log.Printf("tarantool: reconnect (%d/%d) to %s failed: %s", reconnects, conn.opts.MaxReconnects, conn.addr, err)
+		log.Printf("tarantool: reconnect (%d/%d) to %s failed: %s",
+			reconnects, conn.opts.MaxReconnects, conn.addr, err)
 	case LogLastReconnectFailed:
 		err := v[0].(error)
-		log.Printf("tarantool: last reconnect to %s failed: %s, giving it up", conn.addr, err)
+		log.Printf("tarantool: last reconnect to %s failed: %s, giving it up",
+			conn.addr, err)
 	case LogUnexpectedResultId:
 		resp := v[0].(*Response)
-		log.Printf("tarantool: connection %s got unexpected resultId (%d) in response", conn.addr, resp.RequestId)
+		log.Printf("tarantool: connection %s got unexpected resultId (%d) in response",
+			conn.addr, resp.RequestId)
 	case LogWatchEventReadFailed:
 		err := v[0].(error)
 		log.Printf("tarantool: unable to parse watch event: %s", err)
@@ -145,7 +153,8 @@ func (d defaultLogger) Report(event ConnLogKind, conn *Connection, v ...interfac
 // by timeout). Client reconnect will happen if connection options enable
 // reconnect. Beware that graceful shutdown event initialization is asynchronous.
 //
-// More on graceful shutdown: https://www.tarantool.io/en/doc/latest/dev_guide/internals/iproto/graceful_shutdown/
+// More on graceful shutdown:
+// https://www.tarantool.io/en/doc/latest/dev_guide/internals/iproto/graceful_shutdown/
 type Connection struct {
 	addr  string
 	c     Conn
@@ -579,7 +588,8 @@ func (conn *Connection) dial() (err error) {
 	go conn.reader(c, c)
 
 	// Subscribe shutdown event to process graceful shutdown.
-	if conn.shutdownWatcher == nil && isFeatureInSlice(WatchersFeature, conn.serverProtocolInfo.Features) {
+	if conn.shutdownWatcher == nil &&
+		isFeatureInSlice(WatchersFeature, conn.serverProtocolInfo.Features) {
 		watcher, werr := conn.newWatcherImpl(shutdownEventKey, shutdownEventCallback)
 		if werr != nil {
 			return werr
@@ -696,7 +706,10 @@ func (conn *Connection) closeConnection(neterr error, forever bool) (err error)
 	}
 	for i := range conn.shard {
 		conn.shard[i].buf.Reset()
-		requestsLists := []*[requestsMap]futureList{&conn.shard[i].requests, &conn.shard[i].requestsWithCtx}
+		requestsLists := []*[requestsMap]futureList{
+			&conn.shard[i].requests,
+			&conn.shard[i].requestsWithCtx,
+		}
 		for _, requests := range requestsLists {
 			for pos := range requests {
 				requests[pos].clear(neterr, conn)
@@ -1207,7 +1220,7 @@ func read(r io.Reader, lenbuf []byte) (response []byte, err error) {
 		return
 	}
 	if lenbuf[0] != 0xce {
-		err = errors.New("Wrong response header")
+		err = errors.New("wrong response header")
 		return
 	}
 	length = (int(lenbuf[1]) << 24) +
@@ -1216,7 +1229,7 @@ func read(r io.Reader, lenbuf []byte) (response []byte, err error) {
 		int(lenbuf[4])
 
 	if length == 0 {
-		err = errors.New("Response should not be 0 length")
+		err = errors.New("response should not be 0 length")
 		return
 	}
 	response = make([]byte, length)
@@ -1241,7 +1254,7 @@ func (conn *Connection) Do(req Request) *Future {
 	if connectedReq, ok := req.(ConnectedRequest); ok {
 		if connectedReq.Conn() != conn {
 			fut := NewFuture()
-			fut.SetError(fmt.Errorf("the passed connected request doesn't belong to the current connection or connection pool"))
+			fut.SetError(errUnknownRequest)
 			return fut
 		}
 	}

connector.go

@@ -13,78 +13,82 @@ type Connector interface {
 
 	// Deprecated: the method will be removed in the next major version,
 	// use a PingRequest object + Do() instead.
-	Ping() (resp *Response, err error)
+	Ping() (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a SelectRequest object + Do() instead.
-	Select(space, index interface{}, offset, limit uint32, iterator Iter, key interface{}) (resp *Response, err error)
+	Select(space, index interface{}, offset, limit uint32, iterator Iter,
+		key interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use an InsertRequest object + Do() instead.
-	Insert(space interface{}, tuple interface{}) (resp *Response, err error)
+	Insert(space interface{}, tuple interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a ReplicaRequest object + Do() instead.
-	Replace(space interface{}, tuple interface{}) (resp *Response, err error)
+	Replace(space interface{}, tuple interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a DeleteRequest object + Do() instead.
-	Delete(space, index interface{}, key interface{}) (resp *Response, err error)
+	Delete(space, index interface{}, key interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a UpdateRequest object + Do() instead.
-	Update(space, index interface{}, key, ops interface{}) (resp *Response, err error)
+	Update(space, index interface{}, key, ops interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a UpsertRequest object + Do() instead.
-	Upsert(space interface{}, tuple, ops interface{}) (resp *Response, err error)
+	Upsert(space interface{}, tuple, ops interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a CallRequest object + Do() instead.
-	Call(functionName string, args interface{}) (resp *Response, err error)
+	Call(functionName string, args interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a Call16Request object + Do() instead.
-	Call16(functionName string, args interface{}) (resp *Response, err error)
+	Call16(functionName string, args interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use a Call17Request object + Do() instead.
-	Call17(functionName string, args interface{}) (resp *Response, err error)
+	Call17(functionName string, args interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use an EvalRequest object + Do() instead.
-	Eval(expr string, args interface{}) (resp *Response, err error)
+	Eval(expr string, args interface{}) (*Response, error)
 	// Deprecated: the method will be removed in the next major version,
 	// use an ExecuteRequest object + Do() instead.
-	Execute(expr string, args interface{}) (resp *Response, err error)
+	Execute(expr string, args interface{}) (*Response, error)
 
 	// Deprecated: the method will be removed in the next major version,
 	// use a SelectRequest object + Do() instead.
-	GetTyped(space, index interface{}, key interface{}, result interface{}) (err error)
+	GetTyped(space, index interface{}, key interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a SelectRequest object + Do() instead.
-	SelectTyped(space, index interface{}, offset, limit uint32, iterator Iter, key interface{}, result interface{}) (err error)
+	SelectTyped(space, index interface{}, offset, limit uint32, iterator Iter, key interface{},
+		result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use an InsertRequest object + Do() instead.
-	InsertTyped(space interface{}, tuple interface{}, result interface{}) (err error)
+	InsertTyped(space interface{}, tuple interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a ReplaceRequest object + Do() instead.
-	ReplaceTyped(space interface{}, tuple interface{}, result interface{}) (err error)
+	ReplaceTyped(space interface{}, tuple interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a DeleteRequest object + Do() instead.
-	DeleteTyped(space, index interface{}, key interface{}, result interface{}) (err error)
+	DeleteTyped(space, index interface{}, key interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a UpdateRequest object + Do() instead.
-	UpdateTyped(space, index interface{}, key, ops interface{}, result interface{}) (err error)
+	UpdateTyped(space, index interface{}, key, ops interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a CallRequest object + Do() instead.
-	CallTyped(functionName string, args interface{}, result interface{}) (err error)
+	CallTyped(functionName string, args interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a Call16Request object + Do() instead.
-	Call16Typed(functionName string, args interface{}, result interface{}) (err error)
+	Call16Typed(functionName string, args interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use a Call17Request object + Do() instead.
-	Call17Typed(functionName string, args interface{}, result interface{}) (err error)
+	Call17Typed(functionName string, args interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use an EvalRequest object + Do() instead.
-	EvalTyped(expr string, args interface{}, result interface{}) (err error)
+	EvalTyped(expr string, args interface{}, result interface{}) error
 	// Deprecated: the method will be removed in the next major version,
 	// use an ExecuteRequest object + Do() instead.
-	ExecuteTyped(expr string, args interface{}, result interface{}) (SQLInfo, []ColumnMetaData, error)
+	ExecuteTyped(expr string, args interface{},
+		result interface{}) (SQLInfo, []ColumnMetaData, error)
 
 	// Deprecated: the method will be removed in the next major version,
 	// use a SelectRequest object + Do() instead.
-	SelectAsync(space, index interface{}, offset, limit uint32, iterator Iter, key interface{}) *Future
+	SelectAsync(space, index interface{}, offset, limit uint32, iterator Iter,
+		key interface{}) *Future
 	// Deprecated: the method will be removed in the next major version,
 	// use an InsertRequest object + Do() instead.
 	InsertAsync(space interface{}, tuple interface{}) *Future

crud/error.go

@@ -70,8 +70,8 @@ func (e *Error) DecodeMsgpack(d *msgpack.Decoder) error {
 }
 
 // Error converts an Error to a string.
-func (err Error) Error() string {
-	return err.Str
+func (e Error) Error() string {
+	return e.Str
 }
 
 // ErrorMany describes CRUD error object for `_many` methods.
@@ -104,9 +104,9 @@ func (e *ErrorMany) DecodeMsgpack(d *msgpack.Decoder) error {
 }
 
 // Error converts an Error to a string.
-func (errs ErrorMany) Error() string {
+func (e ErrorMany) Error() string {
 	var str []string
-	for _, err := range errs.Errors {
+	for _, err := range e.Errors {
 		str = append(str, err.Str)
 	}