*: lint code with github actions (#1081) (#1112)

Co-authored-by: csuzhangxc <[email protected]>

.github/workflows/check-and-build.yml

@@ -0,0 +1,62 @@
+name: Build & Lint
+
+on:
+  push:
+    branches:
+    - master
+    - release-2.0
+  pull_request:
+    branches:
+    - master
+    - release-2.0
+
+jobs:
+
+  make_build:
+    name: Make Build
+    runs-on: ubuntu-18.04
+    steps:
+
+    - name: Set up Go 1.13
+      uses: actions/setup-go@v2
+      with:
+        go-version: 1.13
+
+    - name: Check out code
+      uses: actions/checkout@v2
+
+    - name: Cache go modules
+      uses: actions/cache@v2
+      with:
+        path: ~/go/pkg/mod
+        key: ${{ runner.os }}-dm-${{ hashFiles('**/go.sum') }}
+
+    - name: Cache retool
+      uses: actions/cache@v2
+      with:
+        path: |
+          **/_tools
+        key: ${{ runner.os }}-dm-retool-${{ hashFiles('**/_tools/manifest.json') }}
+
+    - name: Build
+      run: make build
+
+  lint:
+    name: Lint
+    runs-on: ubuntu-18.04
+    steps:
+
+      - name: Check out code
+        uses: actions/checkout@v2
+
+      - name: GolangCI Lint
+        uses: golangci/golangci-lint-action@v2
+        with:
+          version: v1.31
+          args: --timeout 10m0s --skip-dirs ^_tool/
+
+      - name: Revive Lint
+        uses: morphy2k/revive-action@v1
+        with:
+          config: .revive.toml
+          exclude: _tool/...

.revive.toml

@@ -0,0 +1,24 @@
+ignoreGeneratedHeader = false
+severity = "error"
+confidence = 0.8
+errorCode = -1
+warningCode = -1
+
+[rule.blank-imports]
+[rule.context-as-argument]
+[rule.dot-imports]
+[rule.error-return]
+[rule.error-strings]
+[rule.error-naming]
+[rule.exported]
+[rule.if-return]
+[rule.var-naming]
+[rule.package-comments]
+[rule.range]
+[rule.receiver-naming]
+[rule.indent-error-flow]
+[rule.superfluous-else]
+[rule.modifies-parameter]
+
+# This can be checked by other tools like megacheck
+[rule.unreachable-code]

chaos/cases/db.go

@@ -23,6 +23,7 @@ import (
 	"github.com/pingcap/errors"
 	"github.com/pingcap/tidb-tools/pkg/dbutil"
 	"github.com/pingcap/tidb/errno"
+	"go.uber.org/zap"
 
 	"github.com/pingcap/dm/pkg/conn"
 	tcontext "github.com/pingcap/dm/pkg/context"
@@ -48,7 +49,10 @@ func createDBConn(ctx context.Context, db *conn.BaseDB, currDB string) (*dbConn,
 		baseConn: c,
 		currDB:   currDB,
 		resetFunc: func(ctx context.Context, baseConn *conn.BaseConn) (*conn.BaseConn, error) {
-			db.CloseBaseConn(baseConn)
+			err2 := db.CloseBaseConn(baseConn)
+			if err2 != nil {
+				log.L().Warn("fail to close connection", zap.Error(err2))
+			}
 			return db.GetBaseConn(ctx)
 		},
 	}, nil
@@ -80,10 +84,7 @@ func (c *dbConn) execSQLs(ctx context.Context, queries ...string) error {
 			if retry.IsConnectionError(err) || forceIgnoreExecSQLError(err) {
 				// HACK: for some errors like `invalid connection`, `sql: connection is already closed`, we can ignore them just for testing.
 				err = c.resetConn(ctx)
-				if err != nil {
-					return false
-				}
-				return true
+				return err == nil
 			}
 			return false
 		},

chaos/cases/main.go

@@ -56,6 +56,7 @@ func main() {
 	}
 
 	go func() {
+		//nolint:errcheck
 		http.ListenAndServe("0.0.0.0:8899", nil) // for pprof
 	}()
 

chaos/cases/single.go

@@ -146,11 +146,7 @@ func (st *singleTask) run() error {
 		return err
 	}
 
-	if err := st.incrLoop(); err != nil {
-		return err
-	}
-
-	return nil
+	return st.incrLoop()
 }
 
 // stopPreviousTask stops the previous task with the same name if exists.
@@ -174,10 +170,7 @@ func (st *singleTask) clearPreviousData() error {
 	if err := dropDatabase(st.ctx, st.sourceConn, singleDB); err != nil {
 		return err
 	}
-	if err := dropDatabase(st.ctx, st.targetConn, singleDB); err != nil {
-		return err
-	}
-	return nil
+	return dropDatabase(st.ctx, st.targetConn, singleDB)
 }
 
 // genFullData generates data for the full stage.
@@ -204,6 +197,9 @@ func (st *singleTask) genFullData() error {
 			return err
 		}
 		err = st.sourceConn.execSQLs(st.ctx, query)
+		if err != nil {
+			return err
+		}
 		st.tables = append(st.tables, name)
 
 		col2, idx2, err := createTableToSmithSchema(singleDB, query)
@@ -287,7 +283,10 @@ func (st *singleTask) genIncrData(ctx context.Context) (err error) {
 			default:
 				if forceIgnoreExecSQLError(err) {
 					st.logger.Warn("ignore error when generating data for incremental stage", zap.Error(err))
-					st.sourceConn.resetConn(ctx) // reset connection for the next round.
+					err2 := st.sourceConn.resetConn(ctx) // reset connection for the next round.
+					if err2 != nil {
+						st.logger.Warn("fail to reset connection", zap.Error(err2))
+					}
 					err = nil
 				}
 			}

cmd/dm-ctl/main.go

@@ -54,16 +54,16 @@ func helpUsage(cfg *common.Config) {
 	fmt.Println()
 	fmt.Println("Special Commands:")
 	f := cfg.FlagSet.Lookup(common.EncryptCmdName)
-	fmt.Println(fmt.Sprintf("  --%s %s", f.Name, f.Usage))
+	fmt.Printf("  --%s %s\n", f.Name, f.Usage)
 	f = cfg.FlagSet.Lookup(common.DecryptCmdName)
-	fmt.Println(fmt.Sprintf("  --%s %s", f.Name, f.Usage))
+	fmt.Printf("  --%s %s\n", f.Name, f.Usage)
 	fmt.Println()
 	fmt.Println("Global Options:")
 	cfg.FlagSet.VisitAll(func(flag2 *flag.Flag) {
 		if flag2.Name == common.EncryptCmdName || flag2.Name == common.DecryptCmdName {
 			return
 		}
-		fmt.Println(fmt.Sprintf("  --%s %s", flag2.Name, flag2.Usage))
+		fmt.Printf("  --%s %s\n", flag2.Name, flag2.Usage)
 	})
 }
 
@@ -227,7 +227,10 @@ func loop() {
 		}
 
 		args := strings.Fields(line)
-		ctl.Start(args)
+		err = ctl.Start(args)
+		if err != nil {
+			fmt.Println("fail to run:", args)
+		}
 
 		syncErr := log.L().Sync()
 		if syncErr != nil {

dm/config/subtask.go

@@ -41,8 +41,7 @@ const (
 )
 
 var (
-	defaultMaxAllowedPacket = 64 * 1024 * 1024 // 64MiB, equal to TiDB's default
-	defaultMaxIdleConns     = 2
+	defaultMaxIdleConns = 2
 )
 
 // RawDBConfig contains some low level database config

dm/config/task.go

@@ -21,13 +21,14 @@ import (
 	"strings"
 	"time"
 
-	"github.com/pingcap/dm/pkg/log"
-	"github.com/pingcap/dm/pkg/terror"
 	bf "github.com/pingcap/tidb-tools/pkg/binlog-filter"
 	"github.com/pingcap/tidb-tools/pkg/column-mapping"
 	"github.com/pingcap/tidb-tools/pkg/filter"
 	router "github.com/pingcap/tidb-tools/pkg/table-router"
 
+	"github.com/pingcap/dm/pkg/log"
+	"github.com/pingcap/dm/pkg/terror"
+
 	"github.com/coreos/go-semver/semver"
 	"github.com/dustin/go-humanize"
 	"go.uber.org/zap"
@@ -363,6 +364,7 @@ func (c *TaskConfig) String() string {
 
 // JSON returns the config's json string
 func (c *TaskConfig) JSON() string {
+	//nolint:staticcheck
 	cfg, err := json.Marshal(c)
 	if err != nil {
 		log.L().Error("marshal task config to json", zap.String("task", c.Name), log.ShortError(err))
@@ -532,7 +534,7 @@ func (c *TaskConfig) adjust() error {
 		}
 
 		// for backward compatible, set global config `ansi-quotes: true` if any syncer is true
-		if inst.Syncer.EnableANSIQuotes == true {
+		if inst.Syncer.EnableANSIQuotes {
 			log.L().Warn("DM could discover proper ANSI_QUOTES, `enable-ansi-quotes` is no longer take effect")
 		}
 

dm/ctl/common/config.go

@@ -82,6 +82,7 @@ type Config struct {
 }
 
 func (c *Config) String() string {
+	//nolint:staticcheck
 	cfg, err := json.Marshal(c)
 	if err != nil {
 		fmt.Printf("marshal config to json error %v", err)

dm/ctl/common/util.go

@@ -53,6 +53,7 @@ func InitClient(addr string, securityCfg config.Security) error {
 		return terror.ErrCtlInvalidTLSCfg.Delegate(err)
 	}
 
+	//nolint:staticcheck
 	conn, err := grpc.Dial(addr, tls.ToGRPCDialOption(), grpc.WithBackoffMaxDelay(3*time.Second), grpc.WithBlock(), grpc.WithTimeout(3*time.Second))
 	if err != nil {
 		return terror.ErrCtlGRPCCreateConn.AnnotateDelegate(err, "can't connect to %s", addr)
@@ -225,3 +226,11 @@ func GetTaskNameFromArgOrFile(arg string) string {
 	}
 	return cfg.Name
 }
+
+// PrintCmdUsage prints the usage of the command.
+func PrintCmdUsage(cmd *cobra.Command) {
+	err := cmd.Usage()
+	if err != nil {
+		fmt.Println("can't output command's usage:", err)
+	}
+}

dm/ctl/ctl.go

@@ -117,11 +117,11 @@ func PrintHelp(args []string) {
 	if err != nil {
 		fmt.Println(err)
 		rootCmd.SetOut(os.Stdout)
-		rootCmd.Usage()
+		common.PrintCmdUsage(rootCmd)
 		return
 	}
 	cmd.SetOut(os.Stdout)
-	cmd.Usage()
+	common.PrintCmdUsage(cmd)
 }
 
 // Start starts running a command

dm/ctl/master/check_task.go

@@ -39,7 +39,7 @@ func NewCheckTaskCmd() *cobra.Command {
 func checkTaskFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) != 1 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/get_task_config.go

@@ -41,7 +41,7 @@ func NewGetTaskCfgCmd() *cobra.Command {
 func getTaskCfgFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) != 1 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/handle_error.go

@@ -53,7 +53,7 @@ func convertOp(t string) pb.ErrorOp {
 func handleErrorFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) < 2 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/list_member.go

@@ -67,7 +67,7 @@ func convertListMemberType(cmd *cobra.Command) (bool, bool, bool, error) {
 func listMemberFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) != 0 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/offline_member.go

@@ -59,7 +59,7 @@ func convertOfflineMemberType(cmd *cobra.Command) (string, error) {
 func offlineMemberFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) > 0 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/operate_leader.go

@@ -49,7 +49,7 @@ func convertOpType(op string) pb.LeaderOp {
 func operateLeaderFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) != 1 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/operate_schema.go

@@ -53,7 +53,7 @@ func convertSchemaOpType(t string) pb.SchemaOp {
 func operateSchemaCmd(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) < 2 || len(cmd.Flags().Args()) > 3 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/operate_source.go

@@ -79,7 +79,7 @@ func operateSourceFunc(cmd *cobra.Command, _ []string) (err error) {
 
 	if len(cmd.Flags().Args()) < 1 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/pause_relay.go

@@ -38,7 +38,7 @@ func NewPauseRelayCmd() *cobra.Command {
 func pauseRelayFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) > 0 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}

dm/ctl/master/pause_task.go

@@ -37,7 +37,7 @@ func NewPauseTaskCmd() *cobra.Command {
 func pauseTaskFunc(cmd *cobra.Command, _ []string) (err error) {
 	if len(cmd.Flags().Args()) != 1 {
 		cmd.SetOut(os.Stdout)
-		cmd.Usage()
+		common.PrintCmdUsage(cmd)
 		err = errors.New("please check output to see error")
 		return
 	}