Skip to content

Commit

Permalink
*: Enable unused/deadcode/varcheck/errcheck (#140)
Browse files Browse the repository at this point in the history
  • Loading branch information
suzaku authored Nov 28, 2019
1 parent d4c7192 commit 05c5a6a
Show file tree
Hide file tree
Showing 22 changed files with 73 additions and 59 deletions.
6 changes: 1 addition & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 0 additions & 4 deletions cdc/capture.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
5 changes: 4 additions & 1 deletion cdc/capture_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions cdc/cdc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 1 addition & 9 deletions cdc/entry/kv_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion cdc/http_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion cdc/kv/etcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
7 changes: 5 additions & 2 deletions cdc/kv/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"fmt"
"strconv"
"strings"
"time"

"github.com/pingcap/kvproto/pkg/metapb"
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 2 additions & 1 deletion cdc/mock/mock_tidb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
5 changes: 4 additions & 1 deletion cdc/owner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 5 additions & 3 deletions cdc/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion cdc/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
8 changes: 6 additions & 2 deletions cdc/puller/puller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 0 additions & 8 deletions cdc/puller/span_frontier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion cdc/roles/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion cdc/roles/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion cdc/roles/storage/etcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion cdc/scheduler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
10 changes: 0 additions & 10 deletions cdc/schema/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 12 additions & 4 deletions cdc/sink/mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
}
Expand Down
5 changes: 4 additions & 1 deletion cdc/txn/txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ func CollectRawTxns(
Ts: resolvedTs,
Entries: nil,
}
outputFn(ctx, fakeTxn)
err := outputFn(ctx, fakeTxn)
if err != nil {
return err
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion tools/check/check-tidy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 05c5a6a

Please sign in to comment.