golangci-lint/CI: Bump versions and introduce new linters (#798)

* golangci-lint/CI: Bump versions

Fix remaining linter issues

* Specifically set AppVeyor version

* Fix the infamous typos 👀

* Add go env cmd to AppVeyor

* Add go version cmd to AppVeyor

* Specify AppVeyor image, adjust linters

* Update go get to go install due to deprecation

* Bump golangci-lint timeout time for AppVeyor

* Change NW contract to NQ

* Address nitters

* GetRandomPair -> Pair{}

* Address nits

* Address time nitterinos plus additional tweaks

* More time inception upgrades!

* Bending time and space

engine/apiserver_test.go

@@ -234,6 +234,7 @@ func TestGetAllActiveAccounts(t *testing.T) {
 }
 
 func makeHTTPGetRequest(t *testing.T, response interface{}) *http.Response {
+	t.Helper()
 	w := httptest.NewRecorder()
 
 	err := writeResponse(w, response)

engine/database_connection.go

@@ -51,8 +51,7 @@ func SetupDatabaseConnectionManager(cfg *database.Config) (*DatabaseConnectionMa
 		cfg:      *cfg,
 		dbConn:   database.DB,
 	}
-	err := m.dbConn.SetConfig(cfg)
-	if err != nil {
+	if err := m.dbConn.SetConfig(cfg); err != nil {
 		return nil, err
 	}
 
@@ -177,8 +176,7 @@ func (m *DatabaseConnectionManager) checkConnection() error {
 		return database.ErrNoDatabaseProvided
 	}
 
-	err := m.dbConn.Ping()
-	if err != nil {
+	if err := m.dbConn.Ping(); err != nil {
 		m.dbConn.SetConnected(false)
 		return err
 	}

engine/datahistory_manager.go

@@ -455,7 +455,7 @@ ranges:
 		lookup = append(lookup, *result)
 		job.Results[result.IntervalStartDate] = lookup
 	}
-	completed := true
+	completed := true // nolint:ifshort,nolintlint // false positive and triggers only on Windows
 	allResultsSuccessful := true
 	allResultsFailed := true
 completionCheck:
@@ -553,7 +553,7 @@ timesToFetch:
 		job.Results[result.IntervalStartDate] = lookup
 	}
 
-	completed := true
+	completed := true // nolint:ifshort,nolintlint // false positive and triggers only on Windows
 	allResultsSuccessful := true
 	allResultsFailed := true
 completionCheck:
@@ -709,7 +709,7 @@ func (m *DataHistoryManager) processCandleData(job *DataHistoryJob, exch exchang
 	if err != nil {
 		r.Result += "could not get candles: " + err.Error() + ". "
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	job.rangeHolder.SetHasDataFromCandles(candles.Candles)
 	for i := range job.rangeHolder.Ranges[intervalIndex].Intervals {
@@ -758,13 +758,13 @@ func (m *DataHistoryManager) processTradeData(job *DataHistoryJob, exch exchange
 	if err != nil {
 		r.Result += "could not get trades: " + err.Error() + ". "
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	candles, err := trade.ConvertTradesToCandles(job.Interval, trades...)
 	if err != nil {
 		r.Result += "could not convert candles to trades: " + err.Error() + ". "
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	job.rangeHolder.SetHasDataFromCandles(candles.Candles)
 	for i := range job.rangeHolder.Ranges[intervalIndex].Intervals {
@@ -825,13 +825,13 @@ func (m *DataHistoryManager) convertTradesToCandles(job *DataHistoryJob, startRa
 	if err != nil {
 		r.Result = "could not get trades in range: " + err.Error()
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	candles, err := trade.ConvertTradesToCandles(job.Interval, trades...)
 	if err != nil {
 		r.Result = "could not convert trades in range: " + err.Error()
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	candles.SourceJobID = job.ID
 	err = m.saveCandlesInBatches(job, &candles, r)
@@ -864,13 +864,13 @@ func (m *DataHistoryManager) convertCandleData(job *DataHistoryJob, startRange,
 	if err != nil {
 		r.Result = "could not get candles in range: " + err.Error()
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	newCandles, err := kline.ConvertToNewInterval(&candles, job.ConversionInterval)
 	if err != nil {
 		r.Result = "could not convert candles in range: " + err.Error()
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	newCandles.SourceJobID = job.ID
 	err = m.saveCandlesInBatches(job, &candles, r)
@@ -912,14 +912,14 @@ func (m *DataHistoryManager) validateCandles(job *DataHistoryJob, exch exchange.
 	if err != nil {
 		r.Result = "could not get API candles: " + err.Error()
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	apiCandles.ValidationJobID = job.ID
 	dbCandles, err := m.candleLoader(job.Exchange, job.Pair, job.Asset, job.Interval, startRange, endRange)
 	if err != nil {
 		r.Result = "could not get database candles: " + err.Error()
 		r.Status = dataHistoryStatusFailed
-		return r, nil
+		return r, nil // nolint:nilerr // error is returned in the job result
 	}
 	if len(dbCandles.Candles) == 0 {
 		r.Result = fmt.Sprintf("missing database candles for period %v-%v", startRange, endRange)

engine/datahistory_manager_test.go

@@ -655,6 +655,7 @@ func TestCompareJobsToData(t *testing.T) {
 }
 
 func TestRunJob(t *testing.T) {
+	t.Parallel()
 	testCases := []*DataHistoryJob{
 		{
 			Nickname:  "TestRunJobDataHistoryCandleDataType",
@@ -923,6 +924,7 @@ func TestConverters(t *testing.T) {
 
 // test helper functions
 func createDHM(t *testing.T) (*DataHistoryManager, *datahistoryjob.DataHistoryJob) {
+	t.Helper()
 	em := SetupExchangeManager()
 	exch, err := em.NewExchangeByName(testExchange)
 	if !errors.Is(err, nil) {

engine/engine.go

@@ -708,8 +708,7 @@ func (bot *Engine) Stop() {
 
 	// Wait for services to gracefully shutdown
 	bot.ServicesWG.Wait()
-	err := gctlog.CloseLogger()
-	if err != nil {
+	if err := gctlog.CloseLogger(); err != nil {
 		log.Printf("Failed to close logger. Error: %v\n", err)
 	}
 }

engine/helpers_test.go

@@ -36,6 +36,7 @@ import (
 var testExchange = "Bitstamp"
 
 func CreateTestBot(t *testing.T) *Engine {
+	t.Helper()
 	cFormat := &currency.PairFormat{Uppercase: true}
 	cp1 := currency.NewPair(currency.BTC, currency.USD)
 	cp2 := currency.NewPair(currency.BTC, currency.USDT)
@@ -86,11 +87,9 @@ func CreateTestBot(t *testing.T) *Engine {
 				},
 			},
 		}}}
-	err := bot.LoadExchange(testExchange, nil)
-	if err != nil {
+	if err := bot.LoadExchange(testExchange, nil); err != nil {
 		t.Fatalf("SetupTest: Failed to load exchange: %s", err)
 	}
-
 	return bot
 }
 
@@ -117,6 +116,7 @@ func TestGetRPCEndpoints(t *testing.T) {
 }
 
 func TestSetSubsystem(t *testing.T) {
+	t.Parallel()
 	testCases := []struct {
 		Subsystem    string
 		Engine       *Engine

engine/order_manager_test.go

@@ -182,6 +182,7 @@ func TestOrderManagerStop(t *testing.T) {
 }
 
 func OrdersSetup(t *testing.T) *OrderManager {
+	t.Helper()
 	var wg sync.WaitGroup
 	em := SetupExchangeManager()
 	exch, err := em.NewExchangeByName(testExchange)
@@ -377,8 +378,7 @@ func TestExists(t *testing.T) {
 		Exchange: testExchange,
 		ID:       "TestExists",
 	}
-	err := m.orderStore.add(o)
-	if err != nil {
+	if err := m.orderStore.add(o); err != nil {
 		t.Error(err)
 	}
 	b := m.orderStore.exists(o)
@@ -551,8 +551,7 @@ func TestCancelAllOrders(t *testing.T) {
 		ID:       "TestCancelAllOrders",
 		Status:   order.New,
 	}
-	err := m.orderStore.add(o)
-	if err != nil {
+	if err := m.orderStore.add(o); err != nil {
 		t.Error(err)
 	}
 

engine/rpcserver.go

@@ -109,8 +109,7 @@ func (s *RPCServer) authenticateClient(ctx context.Context) (context.Context, er
 // StartRPCServer starts a gRPC server with TLS auth
 func StartRPCServer(engine *Engine) {
 	targetDir := utils.GetTLSDir(engine.Settings.DataDir)
-	err := checkCerts(targetDir)
-	if err != nil {
+	if err := checkCerts(targetDir); err != nil {
 		log.Errorf(log.GRPCSys, "gRPC checkCerts failed. err: %s\n", err)
 		return
 	}
@@ -678,7 +677,15 @@ func (s *RPCServer) GetAccountInfoStream(r *gctrpc.GetAccountInfoRequest, stream
 			return errDispatchSystem
 		}
 
-		acc := (*data.(*interface{})).(account.Holdings)
+		d := *data.(*interface{})
+		if d == nil {
+			return errors.New("unable to type assert data")
+		}
+
+		acc, ok := d.(account.Holdings)
+		if !ok {
+			return errors.New("unable to type assert account holdings data")
+		}
 
 		var accounts []*gctrpc.Account
 		for x := range acc.Accounts {
@@ -1977,7 +1984,16 @@ func (s *RPCServer) GetExchangeOrderbookStream(r *gctrpc.GetExchangeOrderbookStr
 			return errDispatchSystem
 		}
 
-		ob := (*data.(*interface{})).(orderbook.Base)
+		d := *data.(*interface{})
+		if d == nil {
+			return errors.New("unable to type assert data")
+		}
+
+		ob, ok := d.(orderbook.Base)
+		if !ok {
+			return errors.New("unable to type assert orderbook data")
+		}
+
 		bids := make([]*gctrpc.OrderbookItem, len(ob.Bids))
 		for i := range ob.Bids {
 			bids[i] = &gctrpc.OrderbookItem{
@@ -2050,7 +2066,16 @@ func (s *RPCServer) GetTickerStream(r *gctrpc.GetTickerStreamRequest, stream gct
 		if !ok {
 			return errDispatchSystem
 		}
-		t := (*data.(*interface{})).(ticker.Price)
+
+		d := *data.(*interface{})
+		if d == nil {
+			return errors.New("unable to type assert data")
+		}
+
+		t, ok := d.(ticker.Price)
+		if !ok {
+			return errors.New("unable to type assert ticker data")
+		}
 
 		err := stream.Send(&gctrpc.TickerResponse{
 			Pair: &gctrpc.CurrencyPair{
@@ -2099,7 +2124,16 @@ func (s *RPCServer) GetExchangeTickerStream(r *gctrpc.GetExchangeTickerStreamReq
 		if !ok {
 			return errDispatchSystem
 		}
-		t := (*data.(*interface{})).(ticker.Price)
+
+		d := *data.(*interface{})
+		if d == nil {
+			return errors.New("unable to type assert data")
+		}
+
+		t, ok := d.(ticker.Price)
+		if !ok {
+			return errors.New("unable to type assert ticker data")
+		}
 
 		err := stream.Send(&gctrpc.TickerResponse{
 			Pair: &gctrpc.CurrencyPair{
@@ -2355,7 +2389,11 @@ func (s *RPCServer) GCTScriptStatus(_ context.Context, _ *gctrpc.GCTScriptStatus
 	}
 
 	gctscript.AllVMSync.Range(func(k, v interface{}) bool {
-		vm := v.(*gctscript.VM)
+		vm, ok := v.(*gctscript.VM)
+		if !ok {
+			log.Errorf(log.GRPCSys, "Unable to type assert gctscript.VM")
+			return false
+		}
 		resp.Scripts = append(resp.Scripts, &gctrpc.GCTScript{
 			UUID:    vm.ID.String(),
 			Name:    vm.ShortName(),
@@ -2376,6 +2414,7 @@ func (s *RPCServer) GCTScriptQuery(_ context.Context, r *gctrpc.GCTScriptQueryRe
 
 	UUID, err := uuid.FromString(r.Script.UUID)
 	if err != nil {
+		// nolint:nilerr // error is returned in the GCTScriptQueryResponse
 		return &gctrpc.GCTScriptQueryResponse{Status: MsgStatusError, Data: err.Error()}, nil
 	}
 
@@ -2415,9 +2454,8 @@ func (s *RPCServer) GCTScriptExecute(_ context.Context, r *gctrpc.GCTScriptExecu
 	}
 
 	script := filepath.Join(r.Script.Path, r.Script.Name)
-	err := gctVM.Load(script)
-	if err != nil {
-		return &gctrpc.GenericResponse{
+	if err := gctVM.Load(script); err != nil {
+		return &gctrpc.GenericResponse{ // nolint:nilerr // error is returned in the generic response
 			Status: MsgStatusError,
 			Data:   err.Error(),
 		}, nil
@@ -2439,7 +2477,7 @@ func (s *RPCServer) GCTScriptStop(_ context.Context, r *gctrpc.GCTScriptStopRequ
 
 	UUID, err := uuid.FromString(r.Script.UUID)
 	if err != nil {
-		return &gctrpc.GenericResponse{Status: MsgStatusError, Data: err.Error()}, nil
+		return &gctrpc.GenericResponse{Status: MsgStatusError, Data: err.Error()}, nil // nolint:nilerr // error is returned in the generic response
 	}
 
 	if v, f := gctscript.AllVMSync.Load(UUID); f {
@@ -2603,7 +2641,7 @@ func (s *RPCServer) GCTScriptStopAll(context.Context, *gctrpc.GCTScriptStopAllRe
 
 	err := s.gctScriptManager.ShutdownAll()
 	if err != nil {
-		return &gctrpc.GenericResponse{Status: "error", Data: err.Error()}, nil
+		return &gctrpc.GenericResponse{Status: "error", Data: err.Error()}, nil // nolint:nilerr // error is returned in the generic response
 	}
 
 	return &gctrpc.GenericResponse{
@@ -2621,6 +2659,7 @@ func (s *RPCServer) GCTScriptAutoLoadToggle(_ context.Context, r *gctrpc.GCTScri
 	if r.Status {
 		err := s.gctScriptManager.Autoload(r.Script, true)
 		if err != nil {
+			// nolint:nilerr // error is returned in the generic response
 			return &gctrpc.GenericResponse{Status: "error", Data: err.Error()}, nil
 		}
 		return &gctrpc.GenericResponse{Status: "success", Data: "script " + r.Script + " removed from autoload list"}, nil
@@ -2628,7 +2667,7 @@ func (s *RPCServer) GCTScriptAutoLoadToggle(_ context.Context, r *gctrpc.GCTScri
 
 	err := s.gctScriptManager.Autoload(r.Script, false)
 	if err != nil {
-		return &gctrpc.GenericResponse{Status: "error", Data: err.Error()}, nil
+		return &gctrpc.GenericResponse{Status: "error", Data: err.Error()}, nil // nolint:nilerr // error is returned in the generic response
 	}
 	return &gctrpc.GenericResponse{Status: "success", Data: "script " + r.Script + " added to autoload list"}, nil
 }
@@ -2726,7 +2765,7 @@ func (s *RPCServer) UpdateExchangeSupportedPairs(ctx context.Context, r *gctrpc.
 		return nil, err
 	}
 
-	base := exch.GetBase()
+	base := exch.GetBase() // nolint:ifshort,nolintlint // false positive and triggers only on Windows
 	if base == nil {
 		return nil, errExchangeBaseNotFound
 	}

engine/rpcserver_test.go

@@ -1557,8 +1557,7 @@ func TestGetActiveDataHistoryJobs(t *testing.T) {
 		Interval:  kline.OneMin,
 	}
 
-	err := m.UpsertJob(dhj, false)
-	if !errors.Is(err, nil) {
+	if err := m.UpsertJob(dhj, false); !errors.Is(err, nil) {
 		t.Fatalf("received %v, expected %v", err, nil)
 	}
 

engine/withdraw_manager.go

@@ -98,8 +98,7 @@ func (m *WithdrawManager) WithdrawalEventByID(id string) (*withdraw.Response, er
 	if m == nil {
 		return nil, ErrNilSubsystem
 	}
-	v := withdraw.Cache.Get(id)
-	if v != nil {
+	if v := withdraw.Cache.Get(id); v != nil {
 		return v.(*withdraw.Response), nil
 	}