linters: Add modernise tool check and fix issues (#2012)

* linters: Add modernise tool check and fix issues

* engine: Simplify exch.SetDefaults call and remove localWG

* CI: Revert config versions lint workflow
This commit is contained in:
Adrian Gallagher
2025-08-26 12:45:13 +10:00
committed by GitHub
parent 85403fe801
commit d5b2cf1759
22 changed files with 103 additions and 159 deletions

View File

@@ -13,4 +13,4 @@ jobs:
with: with:
go-version: ${{ env.GO_VERSION }} go-version: ${{ env.GO_VERSION }}
- name: Check config versions are continuous - name: Check config versions are continuous
run: go test ./config/versions/ -tags config_versions -run Continuity run: go test ./config/versions/ -tags config_versions -run Continuity

View File

@@ -1,24 +0,0 @@
name: configs-json-lint
on: [push, pull_request]
jobs:
lint:
name: configs JSON lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5
- name: Check configs JSON format
run: |
files=("config_example.json" "testdata/configtest.json")
for file in "${files[@]}"; do
processed_file="${file%.*}_processed.${file##*.}"
jq '.exchanges |= sort_by(.name)' --indent 1 $file > $processed_file
if ! diff $file $processed_file; then
echo "jq differences found in $file! Please run 'make lint_configs'"
exit 1
else
rm $processed_file
echo "No differences found in $file 🌞"
fi
done

View File

@@ -6,6 +6,11 @@ jobs:
runs-on: ubuntu-latest runs-on: ubuntu-latest
steps: steps:
- uses: actions/checkout@v5 - uses: actions/checkout@v5
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: 1.25.x
- name: Check for currency.NewPair(BTC, USD) used instead of currency.NewBTCUSD - name: Check for currency.NewPair(BTC, USD) used instead of currency.NewBTCUSD
run: | run: |
grep -r -n --color=always -E "currency.NewPair\(currency.BTC, currency.USDT?\)" * || exit 0 grep -r -n --color=always -E "currency.NewPair\(currency.BTC, currency.USDT?\)" * || exit 0
@@ -62,4 +67,24 @@ jobs:
grep -r -n -I --color=always --exclude-dir=.git -P "$PATTERN" . || exit 0 grep -r -n -I --color=always --exclude-dir=.git -P "$PATTERN" . || exit 0
echo "::error::Remove zero-width/format, separator or combining-mark characters" echo "::error::Remove zero-width/format, separator or combining-mark characters"
exit 1 exit 1
- name: Check configs JSON format
run: |
files=("config_example.json" "testdata/configtest.json")
for file in "${files[@]}"; do
processed_file="${file%.*}_processed.${file##*.}"
jq '.exchanges |= sort_by(.name)' --indent 1 $file > $processed_file
if ! diff $file $processed_file; then
echo "jq differences found in $file! Please run 'make lint_configs'"
exit 1
else
rm $processed_file
echo "No differences found in $file 🌞"
fi
done
- name: Check Go modernise tool issues
run: |
go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -test ./...

View File

@@ -11,7 +11,7 @@ DRIVER ?= psql
RACE_FLAG := $(if $(NO_RACE_TEST),,-race) RACE_FLAG := $(if $(NO_RACE_TEST),,-race)
CONFIG_FLAG = $(if $(CONFIG),-config $(CONFIG),) CONFIG_FLAG = $(if $(CONFIG),-config $(CONFIG),)
.PHONY: all lint lint_docker check test build install fmt gofumpt update_deps .PHONY: all lint lint_docker check test build install fmt gofumpt update_deps modernise
all: check build all: check build
@@ -41,6 +41,10 @@ gofumpt:
@command -v gofumpt >/dev/null 2>&1 || go install mvdan.cc/gofumpt@latest @command -v gofumpt >/dev/null 2>&1 || go install mvdan.cc/gofumpt@latest
$(GOFUMPTBIN) -l -w $(GO_FILES_TO_FORMAT) $(GOFUMPTBIN) -l -w $(GO_FILES_TO_FORMAT)
modernise:
@command -v modernize >/dev/null 2>&1 || go install golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest
modernize -test ./...
update_deps: update_deps:
go mod verify go mod verify
go mod tidy go mod tidy

View File

@@ -81,25 +81,20 @@ func (bt *BackTest) RunLive() error {
if bt.LiveDataHandler == nil { if bt.LiveDataHandler == nil {
return errLiveOnly return errLiveOnly
} }
var err error
if bt.LiveDataHandler.IsRealOrders() { if bt.LiveDataHandler.IsRealOrders() {
err = bt.LiveDataHandler.UpdateFunding(false) if err := bt.LiveDataHandler.UpdateFunding(false); err != nil {
if err != nil {
return err return err
} }
} }
err = bt.LiveDataHandler.Start() if err := bt.LiveDataHandler.Start(); err != nil {
if err != nil {
return err return err
} }
bt.wg.Add(1)
go func() { bt.wg.Go(func() {
err = bt.liveCheck() if err := bt.liveCheck(); err != nil {
if err != nil {
log.Errorln(common.LiveStrategy, err) log.Errorln(common.LiveStrategy, err)
} }
bt.wg.Done() })
}()
return nil return nil
} }

View File

@@ -1321,37 +1321,27 @@ func TestLiveLoop(t *testing.T) {
// dataUpdated case // dataUpdated case
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Go(func() {
go func() { assert.NoError(t, bt.liveCheck())
err = bt.liveCheck() })
assert.NoError(t, err)
wg.Done()
}()
dc.dataUpdated <- true dc.dataUpdated <- true
dc.shutdown <- true dc.shutdown <- true
wg.Wait() wg.Wait()
// shutdown from error case // shutdown from error case
wg.Add(1)
dc.started = 0 dc.started = 0
go func() { wg.Go(func() {
defer wg.Done() assert.NoError(t, bt.liveCheck())
err = bt.liveCheck() })
assert.NoError(t, err)
}()
dc.shutdownErr <- true dc.shutdownErr <- true
wg.Wait() wg.Wait()
// shutdown case // shutdown case
dc.started = 1 dc.started = 1
bt.shutdown = make(chan struct{}) bt.shutdown = make(chan struct{})
wg.Add(1) wg.Go(func() {
go func() { assert.NoError(t, bt.liveCheck())
defer wg.Done() })
err = bt.liveCheck()
assert.NoError(t, err)
}()
dc.shutdown <- true dc.shutdown <- true
wg.Wait() wg.Wait()

View File

@@ -135,12 +135,9 @@ func TestLiveHandlerStopFromError(t *testing.T) {
dc.started = 1 dc.started = 1
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Go(func() {
go func() { assert.NoError(t, dc.SignalStopFromError(errNoCredsNoLive))
defer wg.Done() })
err = dc.SignalStopFromError(errNoCredsNoLive)
assert.NoError(t, err)
}()
wg.Wait() wg.Wait()
var dh *dataChecker var dh *dataChecker
@@ -177,19 +174,15 @@ func TestUpdated(t *testing.T) {
dataUpdated: make(chan bool, 10), dataUpdated: make(chan bool, 10),
} }
var wg sync.WaitGroup var wg sync.WaitGroup
wg.Add(1) wg.Go(func() {
go func() {
_ = dc.Updated() _ = dc.Updated()
wg.Done() })
}()
wg.Wait() wg.Wait()
dc = nil dc = nil
wg.Add(1) wg.Go(func() {
go func() {
_ = dc.Updated() _ = dc.Updated()
wg.Done() })
}()
wg.Wait() wg.Wait()
} }

View File

@@ -34,13 +34,11 @@ func main() {
var wg sync.WaitGroup var wg sync.WaitGroup
for i := range exchange.Exchanges { for i := range exchange.Exchanges {
name := exchange.Exchanges[i] name := exchange.Exchanges[i]
wg.Add(1) wg.Go(func() {
go func() {
defer wg.Done()
if err := engine.Bot.LoadExchange(name); err != nil { if err := engine.Bot.LoadExchange(name); err != nil {
log.Printf("Failed to load exchange %s. Err: %s", name, err) log.Printf("Failed to load exchange %s. Err: %s", name, err)
} }
}() })
} }
wg.Wait() wg.Wait()
log.Println("Done.") log.Println("Done.")

View File

@@ -74,13 +74,11 @@ func main() {
wrapperConfig.Exchanges[strings.ToLower(name)] = &config.APICredentialsConfig{} wrapperConfig.Exchanges[strings.ToLower(name)] = &config.APICredentialsConfig{}
} }
if shouldLoadExchange(name) { if shouldLoadExchange(name) {
wg.Add(1) wg.Go(func() {
go func() {
defer wg.Done()
if err = bot.LoadExchange(name); err != nil { if err = bot.LoadExchange(name); err != nil {
log.Printf("Failed to load exchange %s. Err: %s", name, err) log.Printf("Failed to load exchange %s. Err: %s", name, err)
} }
}() })
} }
} }
wg.Wait() wg.Wait()

View File

@@ -179,7 +179,7 @@ Run the following after completing changes:
This ensures proper formatting across the codebase. This ensures proper formatting across the codebase.
## Linters ## Linters and other miscellaneous checks
Run the following to check for linting issues: Run the following to check for linting issues:
@@ -187,6 +187,14 @@ Run the following to check for linting issues:
golangci-lint run ./... (or make lint) golangci-lint run ./... (or make lint)
``` ```
Run the following tool to check for Go modernise issues:
```console
make modernise
```
Several other miscellaneous checks will be ran via [GitHub actions](/.github/workflows/misc.yml).
- All lint warnings and errors must be resolved before merging. - All lint warnings and errors must be resolved before merging.
- Use `//nolint:linter-name` sparingly and always explain the reason in a comment next to the code. - Use `//nolint:linter-name` sparingly and always explain the reason in a comment next to the code.
- Examples of valid use: - Examples of valid use:

View File

@@ -180,9 +180,7 @@ func (m *apiServerManager) StartRESTServer() error {
ReadHeaderTimeout: time.Minute, ReadHeaderTimeout: time.Minute,
} }
} }
m.wgRest.Add(1) m.wgRest.Go(func() {
go func() {
defer m.wgRest.Done()
err := m.restHTTPServer.ListenAndServe() err := m.restHTTPServer.ListenAndServe()
if err != nil { if err != nil {
atomic.StoreInt32(&m.restStarted, 0) atomic.StoreInt32(&m.restStarted, 0)
@@ -190,7 +188,7 @@ func (m *apiServerManager) StartRESTServer() error {
log.Errorln(log.APIServerMgr, err) log.Errorln(log.APIServerMgr, err)
} }
} }
}() })
return nil return nil
} }
@@ -422,9 +420,7 @@ func (m *apiServerManager) StartWebsocketServer() error {
} }
} }
m.wgWebsocket.Add(1) m.wgWebsocket.Go(func() {
go func() {
defer m.wgWebsocket.Done()
err := m.websocketHTTPServer.ListenAndServe() err := m.websocketHTTPServer.ListenAndServe()
if err != nil { if err != nil {
atomic.StoreInt32(&m.websocketStarted, 0) atomic.StoreInt32(&m.websocketStarted, 0)
@@ -432,7 +428,7 @@ func (m *apiServerManager) StartWebsocketServer() error {
log.Errorln(log.APIServerMgr, err) log.Errorln(log.APIServerMgr, err)
} }
} }
}() })
return nil return nil
} }

View File

@@ -731,12 +731,8 @@ func (bot *Engine) LoadExchange(name string) error {
return ErrExchangeFailedToLoad return ErrExchangeFailedToLoad
} }
var localWG sync.WaitGroup exch.SetDefaults()
localWG.Add(1)
go func() {
exch.SetDefaults()
localWG.Done()
}()
exchCfg, err := bot.Config.GetExchangeConfig(name) exchCfg, err := bot.Config.GetExchangeConfig(name)
if err != nil { if err != nil {
return err return err
@@ -789,7 +785,6 @@ func (bot *Engine) LoadExchange(name string) error {
exchCfg.HTTPDebugging = bot.Settings.EnableExchangeHTTPDebugging exchCfg.HTTPDebugging = bot.Settings.EnableExchangeHTTPDebugging
} }
localWG.Wait()
if !bot.Settings.EnableExchangeHTTPRateLimiter { if !bot.Settings.EnableExchangeHTTPRateLimiter {
err = exch.DisableRateLimiter() err = exch.DisableRateLimiter()
if err != nil { if err != nil {

View File

@@ -134,19 +134,15 @@ func (m *WebsocketRoutineManager) websocketRoutine() {
continue continue
} }
wg.Add(1) wg.Go(func() {
go func() { if err := m.websocketDataReceiver(ws); err != nil {
defer wg.Done()
err = m.websocketDataReceiver(ws)
if err != nil {
log.Errorf(log.WebsocketMgr, "%v", err) log.Errorf(log.WebsocketMgr, "%v", err)
} }
err = ws.Connect() if err := ws.Connect(); err != nil {
if err != nil {
log.Errorf(log.WebsocketMgr, "%v", err) log.Errorf(log.WebsocketMgr, "%v", err)
} }
}() })
} }
wg.Wait() wg.Wait()
} }
@@ -166,9 +162,7 @@ func (m *WebsocketRoutineManager) websocketDataReceiver(ws *websocket.Manager) e
return errRoutineManagerNotStarted return errRoutineManagerNotStarted
} }
m.wg.Add(1) m.wg.Go(func() {
go func() {
defer m.wg.Done()
for { for {
select { select {
case <-m.shutdown: case <-m.shutdown:
@@ -187,7 +181,7 @@ func (m *WebsocketRoutineManager) websocketDataReceiver(ws *websocket.Manager) e
m.mu.RUnlock() m.mu.RUnlock()
} }
} }
}() })
return nil return nil
} }

View File

@@ -233,9 +233,7 @@ func (c *connection) SetupPingHandler(epl request.EndpointLimit, handler PingHan
}) })
return return
} }
c.Wg.Add(1) c.Wg.Go(func() {
go func() {
defer c.Wg.Done()
ticker := time.NewTicker(handler.Delay) ticker := time.NewTicker(handler.Delay)
for { for {
select { select {
@@ -250,7 +248,7 @@ func (c *connection) SetupPingHandler(epl request.EndpointLimit, handler PingHan
} }
} }
} }
}() })
} }
// setConnectedStatus sets connection status if changed it will return true. // setConnectedStatus sets connection status if changed it will return true.

View File

@@ -703,9 +703,7 @@ func (o *orderbookManager) setNeedsFetchingBook(pair currency.Pair) error {
// SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair // SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair
// asset // asset
func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) { func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
e.Websocket.Wg.Add(1) e.Websocket.Wg.Go(func() {
go func() {
defer e.Websocket.Wg.Done()
for { for {
select { select {
case <-e.Websocket.ShutdownC: case <-e.Websocket.ShutdownC:
@@ -717,15 +715,12 @@ func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
} }
} }
case j := <-e.obm.jobs: case j := <-e.obm.jobs:
err := e.processJob(ctx, j.Pair) if err := e.processJob(ctx, j.Pair); err != nil {
if err != nil { log.Errorf(log.WebsocketMgr, "%s processing websocket orderbook error: %v", e.Name, err)
log.Errorf(log.WebsocketMgr,
"%s processing websocket orderbook error %v",
e.Name, err)
} }
} }
} }
}() })
} }
// processJob fetches and processes orderbook updates // processJob fetches and processes orderbook updates

View File

@@ -632,9 +632,7 @@ func (e *Exchange) setupOrderbookManager(ctx context.Context) {
// SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair asset // SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair asset
func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) { func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
e.Websocket.Wg.Add(1) e.Websocket.Wg.Go(func() {
go func() {
defer e.Websocket.Wg.Done()
for { for {
select { select {
case <-e.Websocket.ShutdownC: case <-e.Websocket.ShutdownC:
@@ -646,15 +644,12 @@ func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
} }
} }
case j := <-e.obm.jobs: case j := <-e.obm.jobs:
err := e.processJob(ctx, j.Pair) if err := e.processJob(ctx, j.Pair); err != nil {
if err != nil { log.Errorf(log.WebsocketMgr, "%s processing websocket orderbook error: %v", e.Name, err)
log.Errorf(log.WebsocketMgr,
"%s processing websocket orderbook error %v",
e.Name, err)
} }
} }
} }
}() })
} }
// ProcessOrderbookUpdate processes the websocket orderbook update // ProcessOrderbookUpdate processes the websocket orderbook update

View File

@@ -105,9 +105,7 @@ func (e *Exchange) applyBufferUpdate(pair currency.Pair) error {
// SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair // SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair
// asset // asset
func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) { func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
e.Websocket.Wg.Add(1) e.Websocket.Wg.Go(func() {
go func() {
defer e.Websocket.Wg.Done()
for { for {
select { select {
case <-e.Websocket.ShutdownC: case <-e.Websocket.ShutdownC:
@@ -119,13 +117,12 @@ func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
} }
} }
case j := <-e.obm.jobs: case j := <-e.obm.jobs:
err := e.processJob(ctx, j.Pair) if err := e.processJob(ctx, j.Pair); err != nil {
if err != nil { log.Errorf(log.WebsocketMgr, "%s processing websocket orderbook error: %v", e.Name, err)
log.Errorf(log.WebsocketMgr, "%s processing websocket orderbook error %v", e.Name, err)
} }
} }
} }
}() })
} }
// processJob fetches and processes orderbook updates // processJob fetches and processes orderbook updates

View File

@@ -171,7 +171,7 @@ type WebsocketFuturesAmendOrder struct {
// WebsocketFutureOrdersList defines a websocket future orders list // WebsocketFutureOrdersList defines a websocket future orders list
type WebsocketFutureOrdersList struct { type WebsocketFutureOrdersList struct {
Contract currency.Pair `json:"contract,omitempty"` Contract currency.Pair `json:"contract,omitzero"`
Asset asset.Item `json:"-"` // Only used internally for routing Asset asset.Item `json:"-"` // Only used internally for routing
Status string `json:"status"` Status string `json:"status"`
Limit int64 `json:"limit,omitempty"` Limit int64 `json:"limit,omitempty"`

View File

@@ -72,10 +72,7 @@ func (e *Exchange) GetFuturesTickers(ctx context.Context) ([]*ticker.Price, erro
errC <- err errC <- err
break break
} }
wg.Add(1) wg.Go(func() {
go func() {
defer wg.Done()
if tick, err2 := e.GetFuturesTicker(ctx, p.String()); err2 != nil { if tick, err2 := e.GetFuturesTicker(ctx, p.String()); err2 != nil {
errC <- err2 errC <- err2
} else { } else {
@@ -92,7 +89,7 @@ func (e *Exchange) GetFuturesTickers(ctx context.Context) ([]*ticker.Price, erro
AssetType: asset.Futures, AssetType: asset.Futures,
} }
} }
}() })
} }
wg.Wait() wg.Wait()

View File

@@ -1248,9 +1248,7 @@ func (o *orderbookManager) setNeedsFetchingBook(pair currency.Pair, assetType as
// SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair // SynchroniseWebsocketOrderbook synchronises full orderbook for currency pair
// asset // asset
func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) { func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
e.Websocket.Wg.Add(1) e.Websocket.Wg.Go(func() {
go func() {
defer e.Websocket.Wg.Done()
for { for {
select { select {
case <-e.Websocket.ShutdownC: case <-e.Websocket.ShutdownC:
@@ -1262,15 +1260,12 @@ func (e *Exchange) SynchroniseWebsocketOrderbook(ctx context.Context) {
} }
} }
case j := <-e.obm.jobs: case j := <-e.obm.jobs:
err := e.processJob(ctx, j.Pair, j.AssetType) if err := e.processJob(ctx, j.Pair, j.AssetType); err != nil {
if err != nil { log.Errorf(log.WebsocketMgr, "%s processing websocket orderbook error: %v", e.Name, err)
log.Errorf(log.WebsocketMgr,
"%s processing websocket orderbook error %v",
e.Name, err)
} }
} }
} }
}() })
} }
// SeedLocalCache seeds depth data // SeedLocalCache seeds depth data

View File

@@ -329,8 +329,7 @@ func TestProcessOrderbook(t *testing.T) {
break break
} }
m.Unlock() m.Unlock()
wg.Add(1) wg.Go(func() {
go func() {
newName := "Exchange" + strconv.FormatInt(rand.Int63(), 10) //nolint:gosec // no need to import crypo/rand for testing newName := "Exchange" + strconv.FormatInt(rand.Int63(), 10) //nolint:gosec // no need to import crypo/rand for testing
newPairs := currency.NewPair(currency.NewCode("BTC"+strconv.FormatInt(rand.Int63(), 10)), newPairs := currency.NewPair(currency.NewCode("BTC"+strconv.FormatInt(rand.Int63(), 10)),
currency.NewCode("USD"+strconv.FormatInt(rand.Int63(), 10))) //nolint:gosec // no need to import crypo/rand for testing currency.NewCode("USD"+strconv.FormatInt(rand.Int63(), 10))) //nolint:gosec // no need to import crypo/rand for testing
@@ -351,13 +350,11 @@ func TestProcessOrderbook(t *testing.T) {
t.Error(err) t.Error(err)
catastrophicFailure = true catastrophicFailure = true
m.Unlock() m.Unlock()
wg.Done()
return return
} }
testArray = append(testArray, quick{Name: newName, P: newPairs, Bids: bids, Asks: asks}) testArray = append(testArray, quick{Name: newName, P: newPairs, Bids: bids, Asks: asks})
m.Unlock() m.Unlock()
wg.Done() })
}()
} }
wg.Wait() wg.Wait()

View File

@@ -362,8 +362,7 @@ func TestProcessTicker(t *testing.T) { // non-appending function to tickers
break break
} }
wg.Add(1) wg.Go(func() {
go func() {
//nolint:gosec // no need to import crypo/rand for testing //nolint:gosec // no need to import crypo/rand for testing
newName := "Exchange" + strconv.FormatInt(rand.Int63(), 10) newName := "Exchange" + strconv.FormatInt(rand.Int63(), 10)
newPairs, err := currency.NewPairFromStrings("BTC"+strconv.FormatInt(rand.Int63(), 10), //nolint:gosec // no need to import crypo/rand for testing newPairs, err := currency.NewPairFromStrings("BTC"+strconv.FormatInt(rand.Int63(), 10), //nolint:gosec // no need to import crypo/rand for testing
@@ -389,8 +388,7 @@ func TestProcessTicker(t *testing.T) { // non-appending function to tickers
testArray = append(testArray, quick{Name: newName, P: newPairs, TP: tp}) testArray = append(testArray, quick{Name: newName, P: newPairs, TP: tp})
sm.Unlock() sm.Unlock()
wg.Done() })
}()
} }
if catastrophicFailure { if catastrophicFailure {