From 05c5a6adc3f0c5d1ceeb130953f8693945815f10 Mon Sep 17 00:00:00 2001
From: satoru <satorulogic@gmail.com>
Date: Thu, 28 Nov 2019 17:04:33 +0800
Subject: [PATCH] *: Enable unused/deadcode/varcheck/errcheck (#140)

---
 Makefile                       |  6 +-----
 cdc/capture.go                 |  4 ----
 cdc/capture_info_test.go       |  5 ++++-
 cdc/cdc_test.go                |  4 ++++
 cdc/entry/kv_entry.go          | 10 +---------
 cdc/http_status_test.go        |  3 ++-
 cdc/kv/etcd_test.go            |  5 ++++-
 cdc/kv/testing.go              |  7 +++++--
 cdc/mock/mock_tidb.go          |  3 ++-
 cdc/owner_test.go              |  5 ++++-
 cdc/processor.go               |  8 +++++---
 cdc/processor_test.go          |  3 ++-
 cdc/puller/puller.go           |  8 ++++++--
 cdc/puller/span_frontier.go    |  8 --------
 cdc/roles/manager.go           |  1 -
 cdc/roles/manager_test.go      |  5 ++++-
 cdc/roles/storage/etcd_test.go |  5 ++++-
 cdc/scheduler_test.go          |  5 ++++-
 cdc/schema/storage.go          | 10 ----------
 cdc/sink/mysql.go              | 16 ++++++++++++----
 cdc/txn/txn.go                 |  5 ++++-
 tools/check/check-tidy.sh      |  6 +++++-
 22 files changed, 73 insertions(+), 59 deletions(-)

diff --git a/Makefile b/Makefile
index dda12abe178..3bbf2d63f6d 100644
--- a/Makefile
+++ b/Makefile
@@ -98,13 +98,9 @@ else
 	grep -F '<option' "$(TEST_DIR)/all_cov.html"
 endif
 
-# TODO: deadcode unused varcheck reports too many "** is unused now"
 check-static: tools/bin/golangci-lint
 	$(GO) mod vendor
-	tools/bin/golangci-lint --disable errcheck \
-		--disable deadcode \
-		--disable unused \
-		--disable varcheck \
+	tools/bin/golangci-lint \
 		run ./... # $$($(PACKAGE_DIRECTORIES))
 
 clean:
diff --git a/cdc/capture.go b/cdc/capture.go
index 7094a3eb052..c141993141d 100644
--- a/cdc/capture.go
+++ b/cdc/capture.go
@@ -134,10 +134,6 @@ func (c *Capture) Close(ctx context.Context) error {
 	return errors.Trace(DeleteCaptureInfo(ctx, c.info.ID, c.etcdClient))
 }
 
-func (c *Capture) infoKey() string {
-	return infoKey(c.info.ID)
-}
-
 // register registers the capture information in etcd
 func (c *Capture) register(ctx context.Context) error {
 	return errors.Trace(PutCaptureInfo(ctx, c.info, c.etcdClient))
diff --git a/cdc/capture_info_test.go b/cdc/capture_info_test.go
index 68caa4288e2..fa6ab18ab6c 100644
--- a/cdc/capture_info_test.go
+++ b/cdc/capture_info_test.go
@@ -42,7 +42,10 @@ func (ci *captureInfoSuite) SetUpTest(c *check.C) {
 func (ci *captureInfoSuite) TearDownTest(c *check.C) {
 	ci.etcd.Close()
 	ci.cancel()
-	ci.errg.Wait()
+	err := ci.errg.Wait()
+	if err != nil {
+		c.Errorf("Error group error: %s", err)
+	}
 }
 
 func (ci *captureInfoSuite) TestPutDeleteGet(c *check.C) {
diff --git a/cdc/cdc_test.go b/cdc/cdc_test.go
index bac45e32c04..fc950ece7b5 100644
--- a/cdc/cdc_test.go
+++ b/cdc/cdc_test.go
@@ -63,6 +63,10 @@ func NewCDCSuite() *CDCSuite {
 	return cdcSuite
 }
 
+func (s *CDCSuite) TearDownSuite(c *C) {
+	s.puller.TearDown()
+}
+
 func (s *CDCSuite) Forward(span util.Span, ts uint64) bool {
 	return true
 }
diff --git a/cdc/entry/kv_entry.go b/cdc/entry/kv_entry.go
index d6090b5c416..eb393a96ab7 100644
--- a/cdc/entry/kv_entry.go
+++ b/cdc/entry/kv_entry.go
@@ -179,20 +179,12 @@ func unmarshalTableKVEntry(raw *model.RawKVEntry) (kvEntry, error) {
 }
 
 const (
-	ddlJobListKey    = "DDLJobList"
-	ddlJobAddIDxList = "DDLJobAddIDxList"
-	ddlJobHistoryKey = "DDLJobHistory"
-	ddlJobReorgKey   = "DDLJobReorg"
+	ddlJobListKey = "DDLJobList"
 
 	dbMetaPrefix    = "DB:"
 	tableMetaPrefix = "Table:"
 )
 
-var (
-	dbMetaPrefixLen    = len(dbMetaPrefix)
-	tableMetaPrefixLen = len(tableMetaPrefix)
-)
-
 func unmarshalMetaKVEntry(raw *model.RawKVEntry) (kvEntry, error) {
 	meta, err := decodeMetaKey(raw.Key)
 	if err != nil {
diff --git a/cdc/http_status_test.go b/cdc/http_status_test.go
index 2086422486a..be1c97df520 100644
--- a/cdc/http_status_test.go
+++ b/cdc/http_status_test.go
@@ -33,7 +33,8 @@ func (s *httpStatusSuite) waitUntilServerOnline(c *check.C) {
 	for i := 0; i < retryTime; i++ {
 		resp, err := http.Get(statusURL)
 		if err == nil {
-			ioutil.ReadAll(resp.Body)
+			_, err := ioutil.ReadAll(resp.Body)
+			c.Assert(err, check.IsNil)
 			resp.Body.Close()
 			return
 		}
diff --git a/cdc/kv/etcd_test.go b/cdc/kv/etcd_test.go
index 2614092cfb9..b0b10f21460 100644
--- a/cdc/kv/etcd_test.go
+++ b/cdc/kv/etcd_test.go
@@ -55,7 +55,10 @@ func (s *etcdSuite) SetUpTest(c *check.C) {
 func (s *etcdSuite) TearDownTest(c *check.C) {
 	s.e.Close()
 	s.cancel()
-	s.errg.Wait()
+	err := s.errg.Wait()
+	if err != nil {
+		c.Errorf("Error group error: %s", err)
+	}
 }
 
 func (s *etcdSuite) TestGetChangeFeeds(c *check.C) {
diff --git a/cdc/kv/testing.go b/cdc/kv/testing.go
index 673f916cd9f..b7eee85b651 100644
--- a/cdc/kv/testing.go
+++ b/cdc/kv/testing.go
@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/pingcap/kvproto/pkg/metapb"
@@ -90,8 +91,10 @@ func (ec *eventChecker) stop() {
 // CreateStorage creates a tikv Storage instance.
 func CreateStorage(pdAddr string) (storage kv.Storage, err error) {
 	tiPath := fmt.Sprintf("tikv://%s?disableGC=true", pdAddr)
-	store.Register("tikv", tikv.Driver{})
-
+	err = store.Register("tikv", tikv.Driver{})
+	if err != nil && !strings.Contains(err.Error(), "already registered") {
+		return
+	}
 	storage, err = store.New(tiPath)
 	return
 }
diff --git a/cdc/mock/mock_tidb.go b/cdc/mock/mock_tidb.go
index d10c0103c85..84d6501dabf 100644
--- a/cdc/mock/mock_tidb.go
+++ b/cdc/mock/mock_tidb.go
@@ -71,7 +71,8 @@ func (p *TiDB) setUp() (err error) {
 	return nil
 }
 
-func (p *TiDB) tearDown() {
+// TearDown releases resources used by the mock tidb
+func (p *TiDB) TearDown() {
 	p.domain.Close()
 	p.store.Close()
 }
diff --git a/cdc/owner_test.go b/cdc/owner_test.go
index 097bc7dd378..33e6ae59bd9 100644
--- a/cdc/owner_test.go
+++ b/cdc/owner_test.go
@@ -51,7 +51,10 @@ func (s *ownerSuite) SetUpTest(c *check.C) {
 func (s *ownerSuite) TearDownTest(c *check.C) {
 	s.e.Close()
 	s.cancel()
-	s.errg.Wait()
+	err := s.errg.Wait()
+	if err != nil {
+		c.Errorf("Error group error: %s", err)
+	}
 }
 
 type handlerForPrueDMLTest struct {
diff --git a/cdc/processor.go b/cdc/processor.go
index 86bbafcdfb2..c241369c303 100644
--- a/cdc/processor.go
+++ b/cdc/processor.go
@@ -138,8 +138,7 @@ func newTxnChannel(inputTxn <-chan model.RawTxn, chanSize int, handleResolvedTs
 type processorEntryType int
 
 const (
-	processorEntryUnknown processorEntryType = iota
-	processorEntryDMLS
+	processorEntryDMLS processorEntryType = iota
 	processorEntryResolved
 )
 
@@ -499,7 +498,10 @@ func (p *processor) globalResolvedWorker(ctx context.Context) error {
 			})
 		}
 		p.inputChansLock.RUnlock()
-		wg.Wait()
+		err = wg.Wait()
+		if err != nil && errors.Cause(err) == context.Canceled {
+			return err
+		}
 		select {
 		case <-ctx.Done():
 			err := ctx.Err()
diff --git a/cdc/processor_test.go b/cdc/processor_test.go
index 701af84887d..ecd0926bdf0 100644
--- a/cdc/processor_test.go
+++ b/cdc/processor_test.go
@@ -163,7 +163,8 @@ func runCase(c *check.C, cases *processorTestCase) {
 
 	for i, rawTxnTs := range cases.rawTxnTs {
 		input := make(chan model.RawTxn)
-		p.setInputChan(int64(i), input)
+		err := p.setInputChan(int64(i), input)
+		c.Assert(err, check.IsNil)
 		go func(rawTxnTs []uint64) {
 			for _, txnTs := range rawTxnTs {
 				input <- model.RawTxn{Ts: txnTs}
diff --git a/cdc/puller/puller.go b/cdc/puller/puller.go
index a6640e5d040..7ecc1009eaa 100644
--- a/cdc/puller/puller.go
+++ b/cdc/puller/puller.go
@@ -120,10 +120,14 @@ func (p *pullerImpl) Run(ctx context.Context) error {
 						Ts:     val.Ts,
 					}
 
-					p.buf.AddKVEntry(ctx, kv)
+					if err := p.buf.AddKVEntry(ctx, kv); err != nil {
+						return err
+					}
 				} else if e.Checkpoint != nil {
 					cp := e.Checkpoint
-					p.buf.AddResolved(ctx, cp.Span, cp.ResolvedTs)
+					if err := p.buf.AddResolved(ctx, cp.Span, cp.ResolvedTs); err != nil {
+						return err
+					}
 				}
 			case <-ctx.Done():
 				return ctx.Err()
diff --git a/cdc/puller/span_frontier.go b/cdc/puller/span_frontier.go
index a642b65220e..82f36e1e33a 100644
--- a/cdc/puller/span_frontier.go
+++ b/cdc/puller/span_frontier.go
@@ -197,14 +197,6 @@ func (s *spanFrontier) Frontier() uint64 {
 	return s.minHeap[0].ts
 }
 
-func (s *spanFrontier) peekFrontierSpan() util.Span {
-	if s.minHeap.Len() == 0 {
-		return util.Span{}
-	}
-
-	return s.minHeap[0].span
-}
-
 // Forward advances the timestamp for a span.
 // True is returned if the frontier advanced.
 func (s *spanFrontier) Forward(span util.Span, ts uint64) bool {
diff --git a/cdc/roles/manager.go b/cdc/roles/manager.go
index be8d9b8c4bc..abbc2a7ae7d 100644
--- a/cdc/roles/manager.go
+++ b/cdc/roles/manager.go
@@ -60,7 +60,6 @@ const (
 	NewSessionDefaultRetryCnt = 3
 	// NewSessionRetryUnlimited is the unlimited retry times when create new session.
 	NewSessionRetryUnlimited = math.MaxInt64
-	keyOpDefaultTimeout      = 5 * time.Second
 )
 
 // ownerManager represents the structure which is used for electing owner.
diff --git a/cdc/roles/manager_test.go b/cdc/roles/manager_test.go
index bfa22c8d47b..a01d273288d 100644
--- a/cdc/roles/manager_test.go
+++ b/cdc/roles/manager_test.go
@@ -39,7 +39,10 @@ func (s *managerSuite) SetUpTest(c *check.C) {
 func (s *managerSuite) TearDownTest(c *check.C) {
 	s.etcd.Close()
 	s.cancel()
-	s.errg.Wait()
+	err := s.errg.Wait()
+	if err != nil {
+		c.Errorf("Error group error: %s", err)
+	}
 }
 
 func (s *managerSuite) TestManager(c *check.C) {
diff --git a/cdc/roles/storage/etcd_test.go b/cdc/roles/storage/etcd_test.go
index e201a916449..99982477527 100644
--- a/cdc/roles/storage/etcd_test.go
+++ b/cdc/roles/storage/etcd_test.go
@@ -60,7 +60,10 @@ func (s *etcdSuite) SetUpTest(c *check.C) {
 func (s *etcdSuite) TearDownTest(c *check.C) {
 	s.e.Close()
 	s.cancel()
-	s.errg.Wait()
+	err := s.errg.Wait()
+	if err != nil {
+		c.Errorf("Error group error: %s", err)
+	}
 }
 
 func (s *etcdSuite) TestInfoReader(c *check.C) {
diff --git a/cdc/scheduler_test.go b/cdc/scheduler_test.go
index c893b836bfc..03be5bfe1d6 100644
--- a/cdc/scheduler_test.go
+++ b/cdc/scheduler_test.go
@@ -59,7 +59,10 @@ func (s *schedulerSuite) SetUpTest(c *check.C) {
 func (s *schedulerSuite) TearDownTest(c *check.C) {
 	s.etcd.Close()
 	s.cancel()
-	s.errg.Wait()
+	err := s.errg.Wait()
+	if err != nil {
+		c.Errorf("Error group error: %s", err)
+	}
 }
 
 func mockRunProcessor(
diff --git a/cdc/schema/storage.go b/cdc/schema/storage.go
index 19f101eb35d..de8aed04011 100644
--- a/cdc/schema/storage.go
+++ b/cdc/schema/storage.go
@@ -483,16 +483,6 @@ func (s *Storage) IsTruncateTableID(id int64) bool {
 	return ok
 }
 
-func (s *Storage) getSchemaTableAndDelete(version int64) (string, string, error) {
-	schemaTable, ok := s.version2SchemaTable[version]
-	if !ok {
-		return "", "", errors.NotFoundf("version: %d", version)
-	}
-	delete(s.version2SchemaTable, version)
-
-	return schemaTable.Schema, schemaTable.Table, nil
-}
-
 func addImplicitColumn(table *model.TableInfo) {
 	newColumn := &model.ColumnInfo{
 		ID:   implicitColID,
diff --git a/cdc/sink/mysql.go b/cdc/sink/mysql.go
index 438068daa8f..dc39e663dfc 100644
--- a/cdc/sink/mysql.go
+++ b/cdc/sink/mysql.go
@@ -180,13 +180,17 @@ func (s *mysqlSink) execDDL(ctx context.Context, ddl *model.DDL) error {
 	if shouldSwitchDB {
 		_, err = tx.ExecContext(ctx, "USE "+util.QuoteName(ddl.Database)+";")
 		if err != nil {
-			tx.Rollback()
+			if rbErr := tx.Rollback(); rbErr != nil {
+				log.Error("Failed to rollback", zap.Error(err))
+			}
 			return err
 		}
 	}
 
 	if _, err = tx.ExecContext(ctx, ddl.Job.Query); err != nil {
-		tx.Rollback()
+		if rbErr := tx.Rollback(); rbErr != nil {
+			log.Error("Failed to rollback", zap.String("sql", ddl.Job.Query), zap.Error(err))
+		}
 		return err
 	}
 
@@ -216,11 +220,15 @@ func (s *mysqlSink) execDMLs(ctx context.Context, dmls []*model.DML) error {
 		}
 		query, args, err := fPrepare(dml)
 		if err != nil {
-			tx.Rollback()
+			if rbErr := tx.Rollback(); rbErr != nil {
+				log.Error("Failed to rollback", zap.Error(err))
+			}
 			return err
 		}
 		if _, err := tx.ExecContext(ctx, query, args...); err != nil {
-			tx.Rollback()
+			if rbErr := tx.Rollback(); rbErr != nil {
+				log.Error("Failed to rollback", zap.String("sql", query), zap.Error(err))
+			}
 			return err
 		}
 	}
diff --git a/cdc/txn/txn.go b/cdc/txn/txn.go
index 71216d7dc82..3621585001c 100644
--- a/cdc/txn/txn.go
+++ b/cdc/txn/txn.go
@@ -77,7 +77,10 @@ func CollectRawTxns(
 					Ts:      resolvedTs,
 					Entries: nil,
 				}
-				outputFn(ctx, fakeTxn)
+				err := outputFn(ctx, fakeTxn)
+				if err != nil {
+					return err
+				}
 			}
 		}
 	}
diff --git a/tools/check/check-tidy.sh b/tools/check/check-tidy.sh
index b0f35462356..f7ff4f9d48e 100755
--- a/tools/check/check-tidy.sh
+++ b/tools/check/check-tidy.sh
@@ -2,4 +2,8 @@
 set -euo pipefail
 
 GO111MODULE=on go mod tidy
-git diff --quiet
+
+if ! git diff-index --quiet HEAD --; then
+  echo "Please run \`go mod tidy\` to clean up"
+  exit 1
+fi