diff --git a/.github/workflows/golangci-linter.yml b/.github/workflows/golangci-linter.yml index 4cc7f1c025a..084ee49235e 100644 --- a/.github/workflows/golangci-linter.yml +++ b/.github/workflows/golangci-linter.yml @@ -25,7 +25,7 @@ jobs: uses: actions/checkout@v2 - name: Install golangci-lint - run: curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.31.0 + run: curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.39.0 - name: Clean Env run: $(go env GOPATH)/bin/golangci-lint cache clean diff --git a/.golangci.yml b/.golangci.yml index ca7b6e25c3d..4eb3326090b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,6 +4,8 @@ run: - go/vt/topo/k8stopo/client linters-settings: + errcheck: + exclude: ./misc/errcheck_excludes.txt goimports: local-prefixes: vitess.io/vitess @@ -12,7 +14,7 @@ linters: enable: # Defaults - deadcode - # - errcheck + - errcheck - gosimple - govet - ineffassign @@ -29,8 +31,113 @@ issues: exclude-rules: - path: '^go/vt/proto/' linters: + - errcheck - goimports + ### BEGIN: errcheck exclusion rules. Each rule should be considered + # a TODO for removal after adding error checks to that package/file/etc, + # except where otherwise noted. + - path: '^go/cmd/(vtcombo|vtgateclienttest|vtorc)/' + linters: + - errcheck + - path: '^go/mysql/' + linters: + - errcheck + - path: '^go/pools/.*_test.go' + linters: + - errcheck + - path: '^go/sqltypes/' + linters: + - errcheck + - path: '^go/stats/statsd/' + linters: + - errcheck + - path: '^go/test/' + linters: + - errcheck + - path: '^go/vt/automation/' + linters: + - errcheck + - path: '^go/vt/mysqlctl/' + linters: + - errcheck + # (NB: @ajm188) Technically, the below `exclude-rules` for `go/vt/orchestrator/external` is + # redundant with this line, but we actually want to permanently ignore orchestrator's + # vendored code, so we are keeping the exclusion rules separate because this one + # should be temporary while the latter should be permanent. + - path: '^go/vt/orchestrator/' + linters: + - errcheck + # This subtree should be permanently excluded, as it's vendored code. + - path: '^go/vt/orchestrator/external/' + linters: + - errcheck + - path: '^go/vt/schemamanager/' + linters: + - errcheck + - path: '^go/vt/servenv/' + linters: + - errcheck + # This code is autogenerated and should be permanently excluded. + - path: '^go/vt/sqlparser/goyacc' + linters: + - errcheck + - path: '^go/vt/throttler/.*_test.go' + linters: + - errcheck + - path: '^go/vt/topo/.*/*._test.go' + linters: + - errcheck + - path: '^go/vt/vtcombo/' + linters: + - errcheck + - path: '^go/vt/vtctl/[^/]*.go' + linters: + - errcheck + - path: '^go/vt/vtctl/grpcvtctlclient/' + linters: + - errcheck + - path: '^go/vt/vtctl/grpcvtctlserver/' + linters: + - errcheck + - path: '^go/vt/vtctld/(schema|.*_test).go' + linters: + - errcheck + - path: '^go/vt/vtexplain/' + linters: + - errcheck + - path: '^go/vt/vtgate/.*_test.go' + linters: + - errcheck + - path: '^go/vt/vtgr/' + linters: + - errcheck + - path: '^go/vt/vttablet/(customrule|filelogger|grpctmserver|onlineddl|sandboxconn|tabletserver)/' + linters: + - errcheck + - path: '^go/vt/vttablet/tabletmanager/vreplication' + linters: + - errcheck + - path: '^go/vt/vttablet/(.*endtoend.*|.*_test.go)' + linters: + - errcheck + - path: '^go/vt/vttest' + linters: + - errcheck + - path: '^go/vt/worker' + linters: + - errcheck + - path: '^go/vt/workflow' + linters: + - errcheck + - path: '^go/vt/wrangler' + linters: + - errcheck + - path: '^go/vt/zkctl' + linters: + - errcheck + ### END: errcheck exclusion rules + # https://github.com/golangci/golangci/wiki/Configuration service: - golangci-lint-version: 1.31.0 # use the fixed version to not introduce new linters unexpectedly + golangci-lint-version: 1.39.0 # use the fixed version to not introduce new linters unexpectedly diff --git a/go/cache/ristretto/bloom/bbloom_test.go b/go/cache/ristretto/bloom/bbloom_test.go index 960fb034e63..c0f9a916d10 100644 --- a/go/cache/ristretto/bloom/bbloom_test.go +++ b/go/cache/ristretto/bloom/bbloom_test.go @@ -19,7 +19,7 @@ func TestMain(m *testing.M) { wordlist1 = make([][]byte, n) for i := range wordlist1 { b := make([]byte, 32) - rand.Read(b) + _, _ = rand.Read(b) wordlist1[i] = b } fmt.Println("\n###############\nbbloom_test.go") diff --git a/go/cmd/mysqlctld/mysqlctld.go b/go/cmd/mysqlctld/mysqlctld.go index 2b9a7f84f3b..aabca15b796 100644 --- a/go/cmd/mysqlctld/mysqlctld.go +++ b/go/cmd/mysqlctld/mysqlctld.go @@ -124,7 +124,9 @@ func main() { servenv.OnTermSync(func() { log.Infof("mysqlctl received SIGTERM, shutting down mysqld first") ctx := context.Background() - mysqld.Shutdown(ctx, cnf, true) + if err := mysqld.Shutdown(ctx, cnf, true); err != nil { + log.Errorf("failed to shutdown mysqld: %v", err) + } }) // Start RPC server and wait for SIGTERM. diff --git a/go/cmd/rulesctl/cmd/list.go b/go/cmd/rulesctl/cmd/list.go index 30a7456c477..1f6e4615ee4 100644 --- a/go/cmd/rulesctl/cmd/list.go +++ b/go/cmd/rulesctl/cmd/list.go @@ -40,11 +40,13 @@ func List() *cobra.Command { out = rules } } else { - out = rules.Find(listOptName) - if listOptNamesOnly && out != nil { + rule := rules.Find(listOptName) + if listOptNamesOnly && rule != nil { out = listOptName } else if listOptNamesOnly { out = "" + } else { + out = rule } } diff --git a/go/cmd/vtbackup/vtbackup.go b/go/cmd/vtbackup/vtbackup.go index 6e1e0f44b03..42e6c1b7e4a 100644 --- a/go/cmd/vtbackup/vtbackup.go +++ b/go/cmd/vtbackup/vtbackup.go @@ -235,7 +235,9 @@ func takeBackup(ctx context.Context, topoServer *topo.Server, backupStorage back // skip shutdown just because we timed out waiting for other things. ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() - mysqld.Shutdown(ctx, mycnf, false) + if err := mysqld.Shutdown(ctx, mycnf, false); err != nil { + log.Errorf("failed to shutdown mysqld: %v", err) + } }() extraEnv := map[string]string{ diff --git a/go/cmd/vtclient/vtclient_test.go b/go/cmd/vtclient/vtclient_test.go index 4c688925489..4dbe9010ef8 100644 --- a/go/cmd/vtclient/vtclient_test.go +++ b/go/cmd/vtclient/vtclient_test.go @@ -77,7 +77,7 @@ func TestVtclient(t *testing.T) { if err := cluster.Setup(); err != nil { t.Fatalf("InitSchemas failed: %v", err) } - defer cluster.TearDown() + defer cluster.TearDown() // nolint:errcheck vtgateAddr := fmt.Sprintf("localhost:%v", cluster.Env.PortForProtocol("vtcombo", "grpc")) queries := []struct { diff --git a/go/cmd/vtctl/vtctl.go b/go/cmd/vtctl/vtctl.go index 4444d23a375..844e8f8ec09 100644 --- a/go/cmd/vtctl/vtctl.go +++ b/go/cmd/vtctl/vtctl.go @@ -85,7 +85,7 @@ func main() { startMsg := fmt.Sprintf("USER=%v SUDO_USER=%v %v", os.Getenv("USER"), os.Getenv("SUDO_USER"), strings.Join(os.Args, " ")) if syslogger, err := syslog.New(syslog.LOG_INFO, "vtctl "); err == nil { - syslogger.Info(startMsg) + syslogger.Info(startMsg) // nolint:errcheck } else { log.Warningf("cannot connect to syslog: %v", err) } diff --git a/go/cmd/vttestserver/vttestserver_test.go b/go/cmd/vttestserver/vttestserver_test.go index 47a22ee7b38..45c81724076 100644 --- a/go/cmd/vttestserver/vttestserver_test.go +++ b/go/cmd/vttestserver/vttestserver_test.go @@ -135,7 +135,7 @@ func TestForeignKeysAndDDLModes(t *testing.T) { assert.NoError(t, err) defer cluster.TearDown() - execOnCluster(cluster, "test_keyspace", func(conn *mysql.Conn) error { + err = execOnCluster(cluster, "test_keyspace", func(conn *mysql.Conn) error { _, err := conn.ExecuteFetch(`CREATE TABLE test_table_2 ( id BIGINT, test_table_id BIGINT, @@ -154,13 +154,14 @@ func TestForeignKeysAndDDLModes(t *testing.T) { assert.NoError(t, err) return nil }) + assert.NoError(t, err) cluster.TearDown() cluster, err = startCluster("-foreign_key_mode=disallow", "-enable_online_ddl=false", "-enable_direct_ddl=false") assert.NoError(t, err) defer cluster.TearDown() - execOnCluster(cluster, "test_keyspace", func(conn *mysql.Conn) error { + err = execOnCluster(cluster, "test_keyspace", func(conn *mysql.Conn) error { _, err := conn.ExecuteFetch(`CREATE TABLE test_table_2 ( id BIGINT, test_table_id BIGINT, @@ -177,6 +178,7 @@ func TestForeignKeysAndDDLModes(t *testing.T) { assert.Error(t, err) return nil }) + assert.NoError(t, err) } func TestCanVtGateExecute(t *testing.T) { diff --git a/go/streamlog/streamlog.go b/go/streamlog/streamlog.go index 32fdb6be241..53636264d98 100644 --- a/go/streamlog/streamlog.go +++ b/go/streamlog/streamlog.go @@ -179,7 +179,7 @@ func (logger *StreamLogger) LogToFile(path string, logf LogFormatter) (chan inte for { select { case record := <-logChan: - logf(f, formatParams, record) + logf(f, formatParams, record) // nolint:errcheck case <-rotateChan: f.Close() f, _ = os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0644) diff --git a/go/streamlog/streamlog_flaky_test.go b/go/streamlog/streamlog_flaky_test.go index 1dd987da5a6..8b6e40b11bb 100644 --- a/go/streamlog/streamlog_flaky_test.go +++ b/go/streamlog/streamlog_flaky_test.go @@ -235,7 +235,9 @@ func TestFile(t *testing.T) { // Send the rotate signal which should reopen the original file path // for new logs to go to - syscall.Kill(syscall.Getpid(), syscall.SIGUSR2) + if err := syscall.Kill(syscall.Getpid(), syscall.SIGUSR2); err != nil { + t.Logf("failed to send streamlog rotate signal: %v", err) + } time.Sleep(10 * time.Millisecond) logger.Send(&logMessage{"test 4"}) diff --git a/go/vt/binlog/binlogplayer/binlog_player.go b/go/vt/binlog/binlogplayer/binlog_player.go index ead415289e7..238ed71a0dc 100644 --- a/go/vt/binlog/binlogplayer/binlog_player.go +++ b/go/vt/binlog/binlogplayer/binlog_player.go @@ -741,7 +741,9 @@ func MysqlUncompress(input string) []byte { return nil } var outputBytes bytes.Buffer - io.Copy(&outputBytes, reader) + if _, err := io.Copy(&outputBytes, reader); err != nil { + return nil + } if outputBytes.Len() == 0 { return nil } diff --git a/go/vt/binlog/binlogplayertest/player.go b/go/vt/binlog/binlogplayertest/player.go index bfbd234f939..e3468f92913 100644 --- a/go/vt/binlog/binlogplayertest/player.go +++ b/go/vt/binlog/binlogplayertest/player.go @@ -112,7 +112,9 @@ func (fake *FakeBinlogStreamer) StreamKeyRange(ctx context.Context, position str !proto.Equal(charset, testKeyRangeRequest.Charset) { fake.t.Errorf("wrong StreamKeyRange parameter, got %+v want %+v", req, testKeyRangeRequest) } - callback(testBinlogTransaction) + if err := callback(testBinlogTransaction); err != nil { + fake.t.Logf("StreamKeyRange callback failed: %v", err) + } return nil } @@ -178,7 +180,9 @@ func (fake *FakeBinlogStreamer) StreamTables(ctx context.Context, position strin !proto.Equal(charset, testTablesRequest.Charset) { fake.t.Errorf("wrong StreamTables parameter, got %+v want %+v", req, testTablesRequest) } - callback(testBinlogTransaction) + if err := callback(testBinlogTransaction); err != nil { + fake.t.Logf("StreamTables callback failed: %v", err) + } return nil } diff --git a/go/vt/binlog/tables_filter_test.go b/go/vt/binlog/tables_filter_test.go index aa4fa36b77e..abdf8a44b67 100644 --- a/go/vt/binlog/tables_filter_test.go +++ b/go/vt/binlog/tables_filter_test.go @@ -58,7 +58,7 @@ func TestTablesFilterPass(t *testing.T) { got = bltToString(reply) return nil }) - f(eventToken, statements) + _ = f(eventToken, statements) want := `statement: <6, "set1"> statement: <7, "dml1 /* _stream included1 (id ) (500 ); */"> statement: <7, "dml2 /* _stream included2 (id ) (500 ); */"> position: "MariaDB/0-41983-1" ` if want != got { t.Errorf("want\n%s, got\n%s", want, got) @@ -88,7 +88,7 @@ func TestTablesFilterSkip(t *testing.T) { got = bltToString(reply) return nil }) - f(eventToken, statements) + _ = f(eventToken, statements) want := `position: "MariaDB/0-41983-1" ` if want != got { t.Errorf("want %s, got %s", want, got) @@ -118,7 +118,7 @@ func TestTablesFilterDDL(t *testing.T) { got = bltToString(reply) return nil }) - f(eventToken, statements) + _ = f(eventToken, statements) want := `position: "MariaDB/0-41983-1" ` if want != got { t.Errorf("want %s, got %s", want, got) @@ -154,7 +154,7 @@ func TestTablesFilterMalformed(t *testing.T) { got = bltToString(reply) return nil }) - f(eventToken, statements) + _ = f(eventToken, statements) want := `position: "MariaDB/0-41983-1" ` if want != got { t.Errorf("want %s, got %s", want, got) diff --git a/go/vt/dbconfigs/dbconfigs_test.go b/go/vt/dbconfigs/dbconfigs_test.go index 7ddc131564d..2223990d6c4 100644 --- a/go/vt/dbconfigs/dbconfigs_test.go +++ b/go/vt/dbconfigs/dbconfigs_test.go @@ -283,7 +283,7 @@ func hupTest(t *testing.T, tmpFile *os.File, oldStr, newStr string) { if pass != oldStr { t.Fatalf("%s's Password should still be '%s'", oldStr, oldStr) } - syscall.Kill(syscall.Getpid(), syscall.SIGHUP) + _ = syscall.Kill(syscall.Getpid(), syscall.SIGHUP) time.Sleep(100 * time.Millisecond) // wait for signal handler _, _, err := cs.GetUserAndPassword(oldStr) if err != ErrUnknownUser { diff --git a/go/vt/discovery/healthcheck_test.go b/go/vt/discovery/healthcheck_test.go index d1052eb6b3f..d2f6d25eaf9 100644 --- a/go/vt/discovery/healthcheck_test.go +++ b/go/vt/discovery/healthcheck_test.go @@ -1062,7 +1062,7 @@ func TestCellAliases(t *testing.T) { Cells: []string{"cell1", "cell2"}, } assert.Nil(t, ts.CreateCellsAlias(context.Background(), "region1", cellsAlias), "failed to create cell alias") - defer ts.DeleteCellsAlias(context.Background(), "region1") + defer deleteCellsAlias(t, ts, "region1") // add a tablet as replica in diff cell, same region tablet := createTestTablet(1, "cell2", "host2") @@ -1311,3 +1311,9 @@ func createTestTablet(uid uint32, cell, host string) *topodatapb.Tablet { } var mustMatch = utils.MustMatchFn(".Conn" /* ignored fields*/) + +func deleteCellsAlias(t *testing.T, ts *topo.Server, alias string) { + if err := ts.DeleteCellsAlias(context.Background(), alias); err != nil { + t.Logf("DeleteCellsAlias(%s) failed: %v", alias, err) + } +} diff --git a/go/vt/discovery/legacy_tablet_stats_cache_test.go b/go/vt/discovery/legacy_tablet_stats_cache_test.go index c88b6c0bd8c..eabbb38ffa5 100644 --- a/go/vt/discovery/legacy_tablet_stats_cache_test.go +++ b/go/vt/discovery/legacy_tablet_stats_cache_test.go @@ -41,7 +41,7 @@ func TestLegacyTabletStatsCache(t *testing.T) { log.Errorf("creating cellsAlias \"region1\" failed: %v", err) } - defer ts.DeleteCellsAlias(context.Background(), "region1") + defer deleteCellsAlias(t, ts, "region1") cellsAlias = &topodatapb.CellsAlias{ Cells: []string{"cell2"}, @@ -51,7 +51,7 @@ func TestLegacyTabletStatsCache(t *testing.T) { log.Errorf("creating cellsAlias \"region2\" failed: %v", err) } - defer ts.DeleteCellsAlias(context.Background(), "region2") + defer deleteCellsAlias(t, ts, "region2") // We want to unit test LegacyTabletStatsCache without a full-blown // LegacyHealthCheck object, so we can't call NewLegacyTabletStatsCache. diff --git a/go/vt/discovery/tablet_health.go b/go/vt/discovery/tablet_health.go index 75e44480d27..37501d58ca2 100644 --- a/go/vt/discovery/tablet_health.go +++ b/go/vt/discovery/tablet_health.go @@ -89,6 +89,6 @@ func (th *TabletHealth) GetHostNameLevel(level int) string { // https://{{.GetHostNameLevel 0}}.bastion.corp -> https://host.bastion.corp func (th *TabletHealth) getTabletDebugURL() string { var buffer bytes.Buffer - tabletURLTemplate.Execute(&buffer, th) + _ = tabletURLTemplate.Execute(&buffer, th) return buffer.String() } diff --git a/go/vt/key/key_test.go b/go/vt/key/key_test.go index d8d98e92f2b..49babeff3f5 100644 --- a/go/vt/key/key_test.go +++ b/go/vt/key/key_test.go @@ -697,7 +697,9 @@ func BenchmarkKeyRangesOverlap(b *testing.B) { } for i := 0; i < b.N; i++ { - KeyRangesOverlap(kr1, kr2) + if _, err := KeyRangesOverlap(kr1, kr2); err != nil { + b.Fatal(err) + } } } diff --git a/go/vt/orchestrator/collection/collection.go b/go/vt/orchestrator/collection/collection.go index 044f63f405e..cb27c697d7f 100644 --- a/go/vt/orchestrator/collection/collection.go +++ b/go/vt/orchestrator/collection/collection.go @@ -280,11 +280,11 @@ func (c *Collection) Append(m Metric) error { if c == nil { return errors.New("Collection.Append: c == nil") } - c.Lock() //nolint SA5011: possible nil pointer dereference - defer c.Unlock() //nolint SA5011: possible nil pointer dereference + c.Lock() + defer c.Unlock() // we don't want to add nil metrics - if c == nil { - return errors.New("Collection.Append: c == nil") + if m == nil { + return errors.New("Collection.Append: m == nil") } c.collection = append(c.collection, m) diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go index 474487cc56d..facffd39a97 100644 --- a/go/vt/sqlparser/parse_test.go +++ b/go/vt/sqlparser/parse_test.go @@ -2055,7 +2055,7 @@ func TestValid(t *testing.T) { // There's no way automated way to verify that a node calls // all its children. But we can examine code coverage and // ensure that all walkSubtree functions were called. - Walk(func(node SQLNode) (bool, error) { + _ = Walk(func(node SQLNode) (bool, error) { return true, nil }, tree) }) diff --git a/go/vt/srvtopo/resilient_server_test.go b/go/vt/srvtopo/resilient_server_test.go index caffc75a52a..796dcffc329 100644 --- a/go/vt/srvtopo/resilient_server_test.go +++ b/go/vt/srvtopo/resilient_server_test.go @@ -66,7 +66,8 @@ func TestGetSrvKeyspace(t *testing.T) { ShardingColumnName: "id", ShardingColumnType: topodatapb.KeyspaceIdType_UINT64, } - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want) // wait until we get the right value var got *topodatapb.SrvKeyspace @@ -126,7 +127,8 @@ func TestGetSrvKeyspace(t *testing.T) { ShardingColumnType: topodatapb.KeyspaceIdType_UINT64, } - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want) expiry = time.Now().Add(5 * time.Second) updateTime := time.Now() for { @@ -250,7 +252,8 @@ func TestGetSrvKeyspace(t *testing.T) { ShardingColumnName: "id3", ShardingColumnType: topodatapb.KeyspaceIdType_UINT64, } - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want) expiry = time.Now().Add(5 * time.Second) for { got, err = rs.GetSrvKeyspace(context.Background(), "test_cell", "test_ks") @@ -382,7 +385,8 @@ func TestGetSrvKeyspaceCreated(t *testing.T) { ShardingColumnName: "id", ShardingColumnType: topodatapb.KeyspaceIdType_UINT64, } - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + err := ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want) // Wait until we get the right value. expiry := time.Now().Add(5 * time.Second) @@ -506,8 +510,11 @@ func TestGetSrvKeyspaceNames(t *testing.T) { ShardingColumnName: "id", ShardingColumnType: topodatapb.KeyspaceIdType_UINT64, } - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) - ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks2", want) + err := ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks", want) + require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks, %s) failed", want) + + err = ts.UpdateSrvKeyspace(context.Background(), "test_cell", "test_ks2", want) + require.NoError(t, err, "UpdateSrvKeyspace(test_cell, test_ks2, %s) failed", want) ctx := context.Background() names, err := rs.GetSrvKeyspaceNames(ctx, "test_cell", false) diff --git a/go/vt/tlstest/tlstest_test.go b/go/vt/tlstest/tlstest_test.go index 02e6d839153..de34b077afb 100644 --- a/go/vt/tlstest/tlstest_test.go +++ b/go/vt/tlstest/tlstest_test.go @@ -104,7 +104,7 @@ func testClientServer(t *testing.T, combineCerts bool) { defer wg.Done() clientConn, clientErr := tls.DialWithDialer(dialer, "tcp", addr, clientConfig) if clientErr == nil { - clientConn.Write([]byte{42}) + _, _ = clientConn.Write([]byte{42}) clientConn.Close() } }() diff --git a/go/vt/vtadmin/cluster/cluster_test.go b/go/vt/vtadmin/cluster/cluster_test.go index 7a77598a756..0c88ea69ce0 100644 --- a/go/vt/vtadmin/cluster/cluster_test.go +++ b/go/vt/vtadmin/cluster/cluster_test.go @@ -791,7 +791,7 @@ func TestGetSchema(t *testing.T) { err := c.Vtctld.Dial(ctx) require.NoError(t, err, "could not dial test vtctld") - c.GetSchema(ctx, "testkeyspace", cluster.GetSchemaOptions{ + _, _ = c.GetSchema(ctx, "testkeyspace", cluster.GetSchemaOptions{ BaseRequest: req, Tablets: []*vtadminpb.Tablet{tablet}, }) diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go index f15c506c503..cdf13da57d0 100644 --- a/go/vt/vtgate/engine/set.go +++ b/go/vt/vtgate/engine/set.go @@ -382,7 +382,7 @@ func (svss *SysVarSetAware) Execute(vcursor VCursor, env evalengine.ExpressionEn if err != nil { return err } - vcursor.Session().SetSQLSelectLimit(intValue) + vcursor.Session().SetSQLSelectLimit(intValue) // nolint:errcheck case sysvars.TransactionMode.Name: str, err := svss.evalAsString(env) if err != nil { diff --git a/go/vt/vtgate/executor.go b/go/vt/vtgate/executor.go index 1203443e429..463e1605503 100644 --- a/go/vt/vtgate/executor.go +++ b/go/vt/vtgate/executor.go @@ -1518,7 +1518,7 @@ func (e *Executor) startVStream(ctx context.Context, rss []*srvtopo.ResolvedShar send: callback, rss: rss, } - vs.stream(ctx) + _ = vs.stream(ctx) return nil } diff --git a/go/vt/vtgate/plan_execute.go b/go/vt/vtgate/plan_execute.go index 3865ffabad3..e24d9537184 100644 --- a/go/vt/vtgate/plan_execute.go +++ b/go/vt/vtgate/plan_execute.go @@ -141,7 +141,7 @@ func (e *Executor) insideTransaction(ctx context.Context, safeSession *SafeSessi } // The defer acts as a failsafe. If commit was successful, // the rollback will be a no-op. - defer e.txConn.Rollback(ctx, safeSession) + defer e.txConn.Rollback(ctx, safeSession) // nolint:errcheck } // The SetAutocommitable flag should be same as mustCommit. diff --git a/go/vt/vtgate/planbuilder/gen4_planner.go b/go/vt/vtgate/planbuilder/gen4_planner.go index 0f3d3fb04d6..09966c8e218 100644 --- a/go/vt/vtgate/planbuilder/gen4_planner.go +++ b/go/vt/vtgate/planbuilder/gen4_planner.go @@ -109,7 +109,7 @@ func newBuildSelectPlan(sel *sqlparser.Select, reservedVars *sqlparser.ReservedV directives := sqlparser.ExtractCommentDirectives(sel.Comments) if directives.IsSet(sqlparser.DirectiveScatterErrorsAsWarnings) { - visit(plan, func(logicalPlan logicalPlan) (bool, logicalPlan, error) { + _, _ = visit(plan, func(logicalPlan logicalPlan) (bool, logicalPlan, error) { switch plan := logicalPlan.(type) { case *route: plan.eroute.ScatterErrorsAsWarnings = true diff --git a/go/vt/vtgate/plugin_mysql_server.go b/go/vt/vtgate/plugin_mysql_server.go index 117cebc8029..56a4642959b 100644 --- a/go/vt/vtgate/plugin_mysql_server.go +++ b/go/vt/vtgate/plugin_mysql_server.go @@ -437,7 +437,7 @@ func initMySQLProtocol() { log.Exitf("mysql.NewListener failed: %v", err) } - initTLSConfig(mysqlListener, *mysqlSslCert, *mysqlSslKey, *mysqlSslCa, *mysqlSslServerCA, *mysqlServerRequireSecureTransport, tlsVersion) + _ = initTLSConfig(mysqlListener, *mysqlSslCert, *mysqlSslKey, *mysqlSslCa, *mysqlSslServerCA, *mysqlServerRequireSecureTransport, tlsVersion) } mysqlListener.AllowClearTextWithoutTLS.Set(*mysqlAllowClearTextWithoutTLS) // Check for the connection threshold diff --git a/go/vt/vtgate/scatter_conn.go b/go/vt/vtgate/scatter_conn.go index fd2c4bb5044..0c68192c41e 100644 --- a/go/vt/vtgate/scatter_conn.go +++ b/go/vt/vtgate/scatter_conn.go @@ -600,7 +600,7 @@ func (stc *ScatterConn) multiGoTransaction( } if session.MustRollback() { - stc.txConn.Rollback(ctx, session) + _ = stc.txConn.Rollback(ctx, session) } return allErrors } diff --git a/go/vt/vtgate/vstream_manager.go b/go/vt/vtgate/vstream_manager.go index c87b72b9037..01c06b83cb0 100644 --- a/go/vt/vtgate/vstream_manager.go +++ b/go/vt/vtgate/vstream_manager.go @@ -451,7 +451,7 @@ func (vs *vstream) streamFromTablet(ctx context.Context, sgtid *binlogdatapb.Sha errCh := make(chan error, 1) go func() { - tabletConn.StreamHealth(ctx, func(shr *querypb.StreamHealthResponse) error { + _ = tabletConn.StreamHealth(ctx, func(shr *querypb.StreamHealthResponse) error { var err error if ctx.Err() != nil { err = fmt.Errorf("context has ended") diff --git a/go/vt/vttablet/queryservice/fakes/stream_health_query_service.go b/go/vt/vttablet/queryservice/fakes/stream_health_query_service.go index d3426e96df4..24882d84d4a 100644 --- a/go/vt/vttablet/queryservice/fakes/stream_health_query_service.go +++ b/go/vt/vttablet/queryservice/fakes/stream_health_query_service.go @@ -72,7 +72,7 @@ func (q *StreamHealthQueryService) Execute(ctx context.Context, target *querypb. // the healthcheck module. func (q *StreamHealthQueryService) StreamHealth(ctx context.Context, callback func(*querypb.StreamHealthResponse) error) error { for shr := range q.healthResponses { - callback(shr) + callback(shr) // nolint:errcheck } return nil } diff --git a/go/vt/vttablet/tabletconntest/fakequeryservice.go b/go/vt/vttablet/tabletconntest/fakequeryservice.go index bb63a6fcdef..847d7b3e0a0 100644 --- a/go/vt/vttablet/tabletconntest/fakequeryservice.go +++ b/go/vt/vttablet/tabletconntest/fakequeryservice.go @@ -644,7 +644,9 @@ func (f *FakeQueryService) MessageStream(ctx context.Context, target *querypb.Ta if name != MessageName { f.t.Errorf("name: %s, want %s", name, MessageName) } - callback(MessageStreamResult) + if err := callback(MessageStreamResult); err != nil { + f.t.Logf("MessageStream callback failed: %v", err) + } return nil } @@ -700,7 +702,9 @@ func (f *FakeQueryService) StreamHealth(ctx context.Context, callback func(*quer if shr == nil { shr = TestStreamHealthStreamHealthResponse } - callback(shr) + if err := callback(shr); err != nil { + f.t.Logf("StreamHealth callback failed: %v", err) + } return nil } diff --git a/go/vt/vttablet/tabletmanager/rpc_query.go b/go/vt/vttablet/tabletmanager/rpc_query.go index 1372052643c..419124e7b44 100644 --- a/go/vt/vttablet/tabletmanager/rpc_query.go +++ b/go/vt/vttablet/tabletmanager/rpc_query.go @@ -46,7 +46,7 @@ func (tm *TabletManager) ExecuteFetchAsDba(ctx context.Context, query []byte, db if dbName != "" { // This execute might fail if db does not exist. // Error is ignored because given query might create this database. - conn.ExecuteFetch("USE "+sqlescape.EscapeID(dbName), 1, false) + _, _ = conn.ExecuteFetch("USE "+sqlescape.EscapeID(dbName), 1, false) } // run the query @@ -83,7 +83,7 @@ func (tm *TabletManager) ExecuteFetchAsAllPrivs(ctx context.Context, query []byt if dbName != "" { // This execute might fail if db does not exist. // Error is ignored because given query might create this database. - conn.ExecuteFetch("USE "+sqlescape.EscapeID(dbName), 1, false) + _, _ = conn.ExecuteFetch("USE "+sqlescape.EscapeID(dbName), 1, false) } // run the query diff --git a/go/vt/vttablet/tabletmanager/rpc_schema.go b/go/vt/vttablet/tabletmanager/rpc_schema.go index 5a5b1425d69..879dc630013 100644 --- a/go/vt/vttablet/tabletmanager/rpc_schema.go +++ b/go/vt/vttablet/tabletmanager/rpc_schema.go @@ -89,6 +89,6 @@ func (tm *TabletManager) ApplySchema(ctx context.Context, change *tmutils.Schema } // and if it worked, reload the schema - tm.ReloadSchema(ctx, "") + tm.ReloadSchema(ctx, "") // nolint:errcheck return scr, nil } diff --git a/go/vt/withddl/withddl_test.go b/go/vt/withddl/withddl_test.go index dd44a325d75..5d624721760 100644 --- a/go/vt/withddl/withddl_test.go +++ b/go/vt/withddl/withddl_test.go @@ -42,7 +42,7 @@ func TestExec(t *testing.T) { defer conn.Close() _, err = conn.ExecuteFetch("create database t", 10000, true) require.NoError(t, err) - defer conn.ExecuteFetch("drop database t", 10000, true) + defer conn.ExecuteFetch("drop database t", 10000, true) // nolint:errcheck testcases := []struct { name string @@ -207,7 +207,7 @@ func TestExecIgnore(t *testing.T) { defer conn.Close() _, err = conn.ExecuteFetch("create database t", 10000, true) require.NoError(t, err) - defer conn.ExecuteFetch("drop database t", 10000, true) + defer conn.ExecuteFetch("drop database t", 10000, true) // nolint:errcheck withdb := connParams withdb.DbName = "t" @@ -225,7 +225,7 @@ func TestExecIgnore(t *testing.T) { assert.Error(t, err) _, _ = execconn.ExecuteFetch("create table a(id int, primary key(id))", 10000, false) - defer execconn.ExecuteFetch("drop table a", 10000, false) + defer execconn.ExecuteFetch("drop table a", 10000, false) // nolint:errcheck _, _ = execconn.ExecuteFetch("insert into a values(1)", 10000, false) qr, err = wd.ExecIgnore(ctx, "select * from a", execconn.ExecuteFetch) require.NoError(t, err) diff --git a/misc/errcheck_excludes.txt b/misc/errcheck_excludes.txt new file mode 100644 index 00000000000..eae0e2e3243 --- /dev/null +++ b/misc/errcheck_excludes.txt @@ -0,0 +1,47 @@ +// This file contains one function signature per line, which errcheck should not warn about. +// +// The format for function signatures is `package.FunctionName`. +// The format for method signatures is `(package.Receiver).MethodName` for value receivers, +// and the format for pointer receivers is `(*package.Receiver).MethodName`. +// +// See https://github.com/kisielk/errcheck#excluding-functions for more details. + +flag.Set +(*flag.FlagSet).Parse +(flag.Value).Set + +fmt.Fprint +fmt.Fprintf + +io.WriteString(fmt.State) +io.WriteString(net/http.ResponseWriter) + +(net.Listener).Close +(net/http.ResponseWriter).Write +net/http.Serve + +(*os.File).Close +os.Remove +os.RemoveAll +os.Rename + +(*github.com/spf13/cobra.Command).Help +(*github.com/spf13/cobra.Command).MarkFlagRequired +(*github.com/spf13/cobra.Command).MarkPersistentFlagFilename + +(*google.golang.org/grpc.ClientConn).Close +(*google.golang.org/grpc.Server).Serve + +(*vitess.io/vitess/go/bytes2.Buffer).Write +(*vitess.io/vitess/go/bytes2.Buffer).WriteByte +(*vitess.io/vitess/go/bytes2.Buffer).WriteString + +(vitess.io/vitess/go/sqltypes.BinWriter).Write + +vitess.io/vitess/go/vt/orchestrator/external/golib/log.Errore +vitess.io/vitess/go/vt/orchestrator/external/golib/log.Errorf +vitess.io/vitess/go/vt/orchestrator/external/golib/log.Fatal +vitess.io/vitess/go/vt/orchestrator/external/golib/log.Fatale +vitess.io/vitess/go/vt/orchestrator/external/golib/log.Fatalf + +(*vitess.io/vitess/go/vt/vttest.LocalCluster).TearDown diff --git a/misc/git/hooks/golangci-lint b/misc/git/hooks/golangci-lint index bf80fe4f4cd..d54258d2ca8 100755 --- a/misc/git/hooks/golangci-lint +++ b/misc/git/hooks/golangci-lint @@ -25,7 +25,7 @@ gopackages=$(echo $gofiles | xargs -n1 dirname | sort -u) GOLANGCI_LINT=$(command -v golangci-lint >/dev/null 2>&1) if [ $? -eq 1 ]; then echo "Downloading golangci-lint..." - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.34.1 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.39.0 fi for gopackage in $gopackages