Add golangci-lint CI action, fix gosimple, govet + unused lint errors (#1127)

timvaillancourt · web-flow · commit ed46138c0643 · 2022-05-31T21:23:39.000+02:00
* Add `golangci-lint`, fix `gosimple`, `govet` and `unused` linter complaints

* Go 1.16

* Update copyright dates
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -0,0 +1,21 @@
+name: golangci-lint
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  # pull-requests: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.16
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
diff --git a/.golangci.yml b/.golangci.yml
@@ -0,0 +1,12 @@
+run:
+  timeout: 5m
+  modules-download-mode: readonly
+
+linters:
+  disable:
+    - errcheck
+    - staticcheck
+  enable:
+    - gosimple
+    - govet
+    - unused
diff --git a/go/base/context.go b/go/base/context.go
@@ -1,5 +1,5 @@
 /*
-   Copyright 2016 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -15,7 +15,7 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/satori/go.uuid"
+	uuid "github.com/satori/go.uuid"
 
 	"github.com/github/gh-ost/go/mysql"
 	"github.com/github/gh-ost/go/sql"
diff --git a/go/base/context_test.go b/go/base/context_test.go
@@ -1,12 +1,11 @@
 /*
-   Copyright 2021 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
 package base
 
 import (
-	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -89,7 +88,7 @@ func TestReadConfigFile(t *testing.T) {
 		}
 		defer os.Remove(f.Name())
 
-		f.Write([]byte(fmt.Sprintf("[client]\nuser=test\npassword=123456")))
+		f.Write([]byte("[client]\nuser=test\npassword=123456"))
 		context := NewMigrationContext()
 		context.ConfigFile = f.Name()
 		if err := context.ReadConfigFile(); err != nil {
@@ -109,7 +108,7 @@ func TestReadConfigFile(t *testing.T) {
 		}
 		defer os.Remove(f.Name())
 
-		f.Write([]byte(fmt.Sprintf("[osc]\nmax_load=10")))
+		f.Write([]byte("[osc]\nmax_load=10"))
 		context := NewMigrationContext()
 		context.ConfigFile = f.Name()
 		if err := context.ReadConfigFile(); err != nil {
diff --git a/go/base/default_logger.go b/go/base/default_logger.go
@@ -1,3 +1,8 @@
+/*
+   Copyright 2022 GitHub Inc.
+         See https://github.com/github/gh-ost/blob/master/LICENSE
+*/
+
 package base
 
 import (
@@ -12,22 +17,18 @@ func NewDefaultLogger() *simpleLogger {
 
 func (*simpleLogger) Debug(args ...interface{}) {
 	log.Debug(args[0].(string), args[1:])
-	return
 }
 
 func (*simpleLogger) Debugf(format string, args ...interface{}) {
 	log.Debugf(format, args...)
-	return
 }
 
 func (*simpleLogger) Info(args ...interface{}) {
 	log.Info(args[0].(string), args[1:])
-	return
 }
 
 func (*simpleLogger) Infof(format string, args ...interface{}) {
 	log.Infof(format, args...)
-	return
 }
 
 func (*simpleLogger) Warning(args ...interface{}) error {
@@ -64,10 +65,8 @@ func (*simpleLogger) Fatale(err error) error {
 
 func (*simpleLogger) SetLevel(level log.LogLevel) {
 	log.SetLevel(level)
-	return
 }
 
 func (*simpleLogger) SetPrintStackTrace(printStackTraceFlag bool) {
 	log.SetPrintStackTrace(printStackTraceFlag)
-	return
 }
diff --git a/go/base/utils.go b/go/base/utils.go
@@ -1,5 +1,5 @@
 /*
-   Copyright 2016 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -25,9 +25,7 @@ func PrettifyDurationOutput(d time.Duration) string {
 	if d < time.Second {
 		return "0s"
 	}
-	result := fmt.Sprintf("%s", d)
-	result = prettifyDurationRegexp.ReplaceAllString(result, "")
-	return result
+	return prettifyDurationRegexp.ReplaceAllString(d.String(), "")
 }
 
 func FileExists(fileName string) bool {
diff --git a/go/binlog/gomysql_reader.go b/go/binlog/gomysql_reader.go
@@ -1,5 +1,5 @@
 /*
-   Copyright 2021 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -64,7 +64,10 @@ func (this *GoMySQLReader) ConnectBinlogStreamer(coordinates mysql.BinlogCoordin
 	this.currentCoordinates = coordinates
 	this.migrationContext.Log.Infof("Connecting binlog streamer at %+v", this.currentCoordinates)
 	// Start sync with specified binlog file and position
-	this.binlogStreamer, err = this.binlogSyncer.StartSync(gomysql.Position{this.currentCoordinates.LogFile, uint32(this.currentCoordinates.LogPos)})
+	this.binlogStreamer, err = this.binlogSyncer.StartSync(gomysql.Position{
+		Name: this.currentCoordinates.LogFile,
+		Pos:  uint32(this.currentCoordinates.LogPos),
+	})
 
 	return err
 }
diff --git a/go/logic/hooks.go b/go/logic/hooks.go
@@ -1,6 +1,5 @@
 /*
-/*
-   Copyright 2016 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -72,9 +71,7 @@ func (this *HooksExecutor) applyEnvironmentVariables(extraVariables ...string) [
 	env = append(env, fmt.Sprintf("GH_OST_HOOKS_HINT_TOKEN=%s", this.migrationContext.HooksHintToken))
 	env = append(env, fmt.Sprintf("GH_OST_DRY_RUN=%t", this.migrationContext.Noop))
 
-	for _, variable := range extraVariables {
-		env = append(env, variable)
-	}
+	env = append(env, extraVariables...)
 	return env
 }
 
diff --git a/go/logic/inspect.go b/go/logic/inspect.go
@@ -804,5 +804,4 @@ func (this *Inspector) getReplicationLag() (replicationLag time.Duration, err er
 func (this *Inspector) Teardown() {
 	this.db.Close()
 	this.informationSchemaDb.Close()
-	return
 }
diff --git a/go/logic/migrator.go b/go/logic/migrator.go
@@ -1,5 +1,5 @@
 /*
-   Copyright 2016 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -176,16 +176,6 @@ func (this *Migrator) retryOperationWithExponentialBackoff(operation func() erro
 	return err
 }
 
-// executeAndThrottleOnError executes a given function. If it errors, it
-// throttles.
-func (this *Migrator) executeAndThrottleOnError(operation func() error) (err error) {
-	if err := operation(); err != nil {
-		this.throttler.throttle(nil)
-		return err
-	}
-	return nil
-}
-
 // consumeRowCopyComplete blocks on the rowCopyComplete channel once, and then
 // consumes and drops any further incoming events that may be left hanging.
 func (this *Migrator) consumeRowCopyComplete() {
@@ -823,78 +813,78 @@ func (this *Migrator) initiateStatus() error {
 // migration, and as response to the "status" interactive command.
 func (this *Migrator) printMigrationStatusHint(writers ...io.Writer) {
 	w := io.MultiWriter(writers...)
-	fmt.Fprintln(w, fmt.Sprintf("# Migrating %s.%s; Ghost table is %s.%s",
+	fmt.Fprintf(w, "# Migrating %s.%s; Ghost table is %s.%s\n",
 		sql.EscapeName(this.migrationContext.DatabaseName),
 		sql.EscapeName(this.migrationContext.OriginalTableName),
 		sql.EscapeName(this.migrationContext.DatabaseName),
 		sql.EscapeName(this.migrationContext.GetGhostTableName()),
-	))
-	fmt.Fprintln(w, fmt.Sprintf("# Migrating %+v; inspecting %+v; executing on %+v",
+	)
+	fmt.Fprintf(w, "# Migrating %+v; inspecting %+v; executing on %+v\n",
 		*this.applier.connectionConfig.ImpliedKey,
 		*this.inspector.connectionConfig.ImpliedKey,
 		this.migrationContext.Hostname,
-	))
-	fmt.Fprintln(w, fmt.Sprintf("# Migration started at %+v",
+	)
+	fmt.Fprintf(w, "# Migration started at %+v\n",
 		this.migrationContext.StartTime.Format(time.RubyDate),
-	))
+	)
 	maxLoad := this.migrationContext.GetMaxLoad()
 	criticalLoad := this.migrationContext.GetCriticalLoad()
-	fmt.Fprintln(w, fmt.Sprintf("# chunk-size: %+v; max-lag-millis: %+vms; dml-batch-size: %+v; max-load: %s; critical-load: %s; nice-ratio: %f",
+	fmt.Fprintf(w, "# chunk-size: %+v; max-lag-millis: %+vms; dml-batch-size: %+v; max-load: %s; critical-load: %s; nice-ratio: %f\n",
 		atomic.LoadInt64(&this.migrationContext.ChunkSize),
 		atomic.LoadInt64(&this.migrationContext.MaxLagMillisecondsThrottleThreshold),
 		atomic.LoadInt64(&this.migrationContext.DMLBatchSize),
 		maxLoad.String(),
 		criticalLoad.String(),
 		this.migrationContext.GetNiceRatio(),
-	))
+	)
 	if this.migrationContext.ThrottleFlagFile != "" {
 		setIndicator := ""
 		if base.FileExists(this.migrationContext.ThrottleFlagFile) {
 			setIndicator = "[set]"
 		}
-		fmt.Fprintln(w, fmt.Sprintf("# throttle-flag-file: %+v %+v",
+		fmt.Fprintf(w, "# throttle-flag-file: %+v %+v\n",
 			this.migrationContext.ThrottleFlagFile, setIndicator,
-		))
+		)
 	}
 	if this.migrationContext.ThrottleAdditionalFlagFile != "" {
 		setIndicator := ""
 		if base.FileExists(this.migrationContext.ThrottleAdditionalFlagFile) {
 			setIndicator = "[set]"
 		}
-		fmt.Fprintln(w, fmt.Sprintf("# throttle-additional-flag-file: %+v %+v",
+		fmt.Fprintf(w, "# throttle-additional-flag-file: %+v %+v\n",
 			this.migrationContext.ThrottleAdditionalFlagFile, setIndicator,
-		))
+		)
 	}
 	if throttleQuery := this.migrationContext.GetThrottleQuery(); throttleQuery != "" {
-		fmt.Fprintln(w, fmt.Sprintf("# throttle-query: %+v",
+		fmt.Fprintf(w, "# throttle-query: %+v\n",
 			throttleQuery,
-		))
+		)
 	}
 	if throttleControlReplicaKeys := this.migrationContext.GetThrottleControlReplicaKeys(); throttleControlReplicaKeys.Len() > 0 {
-		fmt.Fprintln(w, fmt.Sprintf("# throttle-control-replicas count: %+v",
+		fmt.Fprintf(w, "# throttle-control-replicas count: %+v\n",
 			throttleControlReplicaKeys.Len(),
-		))
+		)
 	}
 
 	if this.migrationContext.PostponeCutOverFlagFile != "" {
 		setIndicator := ""
 		if base.FileExists(this.migrationContext.PostponeCutOverFlagFile) {
 			setIndicator = "[set]"
 		}
-		fmt.Fprintln(w, fmt.Sprintf("# postpone-cut-over-flag-file: %+v %+v",
+		fmt.Fprintf(w, "# postpone-cut-over-flag-file: %+v %+v\n",
 			this.migrationContext.PostponeCutOverFlagFile, setIndicator,
-		))
+		)
 	}
 	if this.migrationContext.PanicFlagFile != "" {
-		fmt.Fprintln(w, fmt.Sprintf("# panic-flag-file: %+v",
+		fmt.Fprintf(w, "# panic-flag-file: %+v\n",
 			this.migrationContext.PanicFlagFile,
-		))
+		)
 	}
-	fmt.Fprintln(w, fmt.Sprintf("# Serving on unix socket: %+v",
+	fmt.Fprintf(w, "# Serving on unix socket: %+v\n",
 		this.migrationContext.ServeSocketFile,
-	))
+	)
 	if this.migrationContext.ServeTCPPort != 0 {
-		fmt.Fprintln(w, fmt.Sprintf("# Serving on TCP port: %+v", this.migrationContext.ServeTCPPort))
+		fmt.Fprintf(w, "# Serving on TCP port: %+v\n", this.migrationContext.ServeTCPPort)
 	}
 }
 
@@ -1195,7 +1185,6 @@ func (this *Migrator) iterateChunks() error {
 		// Enqueue copy operation; to be executed by executeWriteFuncs()
 		this.copyRowsQueue <- copyRowsFunc
 	}
-	return nil
 }
 
 func (this *Migrator) onApplyEventStruct(eventStruct *applyEventStruct) error {
@@ -1301,7 +1290,6 @@ func (this *Migrator) executeWriteFuncs() error {
 			}
 		}
 	}
-	return nil
 }
 
 // finalCleanup takes actions at very end of migration, dropping tables etc.
diff --git a/go/logic/streamer.go b/go/logic/streamer.go
@@ -1,5 +1,5 @@
 /*
-   Copyright 2016 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -220,5 +220,4 @@ func (this *EventsStreamer) Close() (err error) {
 
 func (this *EventsStreamer) Teardown() {
 	this.db.Close()
-	return
 }
diff --git a/go/logic/throttler.go b/go/logic/throttler.go
@@ -1,5 +1,5 @@
 /*
-   Copyright 2016 GitHub Inc.
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
diff --git a/go/mysql/instance_key.go b/go/mysql/instance_key.go
@@ -1,5 +1,6 @@
 /*
    Copyright 2015 Shlomi Noach, courtesy Booking.com
+   Copyright 2022 GitHub Inc.
 	 See https://github.com/github/gh-ost/blob/master/LICENSE
 */
 
@@ -12,15 +13,16 @@ import (
 	"strings"
 )
 
-const (
-	DefaultInstancePort = 3306
-)
+const DefaultInstancePort = 3306
 
 var (
 	ipv4HostPortRegexp = regexp.MustCompile("^([^:]+):([0-9]+)$")
 	ipv4HostRegexp     = regexp.MustCompile("^([^:]+)$")
-	ipv6HostPortRegexp = regexp.MustCompile("^\\[([:0-9a-fA-F]+)\\]:([0-9]+)$") // e.g. [2001:db8:1f70::999:de8:7648:6e8]:3308
-	ipv6HostRegexp     = regexp.MustCompile("^([:0-9a-fA-F]+)$")                // e.g. 2001:db8:1f70::999:de8:7648:6e8
+
+	// e.g. [2001:db8:1f70::999:de8:7648:6e8]:3308
+	ipv6HostPortRegexp = regexp.MustCompile("^\\[([:0-9a-fA-F]+)\\]:([0-9]+)$") //nolint:gosimple
+	// e.g. 2001:db8:1f70::999:de8:7648:6e8
+	ipv6HostRegexp = regexp.MustCompile("^([:0-9a-fA-F]+)$")
 )
 
 // InstanceKey is an instance indicator, identified by hostname and port
diff --git a/go/sql/builder.go b/go/sql/builder.go

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,8 @@`
	`1`	`+/*`
	`2`	`+ Copyright 2022 GitHub Inc.`
	`3`	`+ See https://github.com/github/gh-ost/blob/master/LICENSE`
	`4`	`+*/`
	`5`	`+`
`1`	`6`	`package base`
`2`	`7`
`3`	`8`	`import (`
`@@ -12,22 +17,18 @@ func NewDefaultLogger() *simpleLogger {`
`12`	`17`
`13`	`18`	`func (*simpleLogger) Debug(args ...interface{}) {`
`14`	`19`	`log.Debug(args[0].(string), args[1:])`
`15`		`- return`
`16`	`20`	`}`
`17`	`21`
`18`	`22`	`func (*simpleLogger) Debugf(format string, args ...interface{}) {`
`19`	`23`	`log.Debugf(format, args...)`
`20`		`- return`
`21`	`24`	`}`
`22`	`25`
`23`	`26`	`func (*simpleLogger) Info(args ...interface{}) {`
`24`	`27`	`log.Info(args[0].(string), args[1:])`
`25`		`- return`
`26`	`28`	`}`
`27`	`29`
`28`	`30`	`func (*simpleLogger) Infof(format string, args ...interface{}) {`
`29`	`31`	`log.Infof(format, args...)`
`30`		`- return`
`31`	`32`	`}`
`32`	`33`
`33`	`34`	`func (*simpleLogger) Warning(args ...interface{}) error {`
`@@ -64,10 +65,8 @@ func (*simpleLogger) Fatale(err error) error {`
`64`	`65`
`65`	`66`	`func (*simpleLogger) SetLevel(level log.LogLevel) {`
`66`	`67`	`log.SetLevel(level)`
`67`		`- return`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`func (*simpleLogger) SetPrintStackTrace(printStackTraceFlag bool) {`
`71`	`71`	`log.SetPrintStackTrace(printStackTraceFlag)`
`72`		`- return`
`73`	`72`	`}`
Original file line number	Diff line number	Diff line change
`@@ -804,5 +804,4 @@ func (this *Inspector) getReplicationLag() (replicationLag time.Duration, err er`
`804`	`804`	`func (this *Inspector) Teardown() {`
`805`	`805`	`this.db.Close()`
`806`	`806`	`this.informationSchemaDb.Close()`
`807`		`- return`
`808`	`807`	`}`