Skip to content

Commit

Permalink
*: avoid direct error comparisons
Browse files Browse the repository at this point in the history
This adds a linter that checks that `error` object
are not compared directly with non-`nil` values using
`==` or `!=`.

Release note: None
  • Loading branch information
knz committed May 7, 2020
1 parent f80ec9c commit 15c7371
Show file tree
Hide file tree
Showing 110 changed files with 334 additions and 199 deletions.
2 changes: 1 addition & 1 deletion pkg/acceptance/cluster/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func (cli resilientDockerClient) ContainerStart(

// Keep going if ContainerStart timed out, but client's context is not
// expired.
if errors.Cause(err) == context.DeadlineExceeded && clientCtx.Err() == nil {
if errors.Is(err, context.DeadlineExceeded) && clientCtx.Err() == nil {
log.Warningf(clientCtx, "ContainerStart timed out, retrying")
continue
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func getDescriptorFromDB(
keys.RootNamespaceID,
dbName,
).Scan(&dbDescBytes); err != nil {
if err == gosql.ErrNoRows {
if errors.Is(err, gosql.ErrNoRows) {
continue
}
return nil, errors.Wrap(err, "fetch database descriptor")
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/importccl/read_import_mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func (m *mysqldumpReader) readFile(
if err == io.EOF {
break
}
if err == mysql.ErrEmpty {
if errors.Is(err, mysql.ErrEmpty) {
continue
}
if err != nil {
Expand Down Expand Up @@ -300,7 +300,7 @@ func readMysqlCreateTable(
if err == io.EOF {
break
}
if err == mysql.ErrEmpty {
if errors.Is(err, mysql.ErrEmpty) {
continue
}
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/read_import_pgcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (p *postgreStreamCopy) Next() (copyData, error) {
// Attempt to read an entire line.
scanned := p.s.Scan()
if err := p.s.Err(); err != nil {
if err == bufio.ErrTooLong {
if errors.Is(err, bufio.ErrTooLong) {
err = errors.New("line too long")
}
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions pkg/ccl/importccl/read_import_pgdump.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func splitSQLSemicolon(data []byte, atEOF bool) (advance int, token []byte, err
func (p *postgreStream) Next() (interface{}, error) {
if p.copy != nil {
row, err := p.copy.Next()
if err == errCopyDone {
if errors.Is(err, errCopyDone) {
p.copy = nil
return errCopyDone, nil
}
Expand Down Expand Up @@ -125,8 +125,8 @@ func (p *postgreStream) Next() (interface{}, error) {
}
}
if err := p.s.Err(); err != nil {
if err == bufio.ErrTooLong {
err = errors.New("line too long")
if errors.Is(err, bufio.ErrTooLong) {
err = errors.HandledWithMessage(err, "line too long")
}
return nil, err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/ccl/workloadccl/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func GetFixture(

fixtureFolder := generatorToGCSFolder(config, gen)
_, err := b.Objects(ctx, &storage.Query{Prefix: fixtureFolder, Delimiter: `/`}).Next()
if err == iterator.Done {
if errors.Is(err, iterator.Done) {
notFound = true
return errors.Errorf(`fixture not found: %s`, fixtureFolder)
} else if err != nil {
Expand All @@ -153,7 +153,7 @@ func GetFixture(
for _, table := range gen.Tables() {
tableFolder := filepath.Join(fixtureFolder, table.Name)
_, err := b.Objects(ctx, &storage.Query{Prefix: tableFolder, Delimiter: `/`}).Next()
if err == iterator.Done {
if errors.Is(err, iterator.Done) {
return errors.Errorf(`fixture table not found: %s`, tableFolder)
} else if err != nil {
return err
Expand Down Expand Up @@ -619,14 +619,14 @@ func ListFixtures(
gensPrefix := config.GCSPrefix + `/`
for genIter := b.Objects(ctx, &storage.Query{Prefix: gensPrefix, Delimiter: `/`}); ; {
gen, err := genIter.Next()
if err == iterator.Done {
if errors.Is(err, iterator.Done) {
break
} else if err != nil {
return nil, err
}
for genConfigIter := b.Objects(ctx, &storage.Query{Prefix: gen.Prefix, Delimiter: `/`}); ; {
genConfig, err := genConfigIter.Next()
if err == iterator.Done {
if errors.Is(err, iterator.Done) {
break
} else if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,7 @@ func (c *cliState) configurePreShellDefaults() (cleanupFn func(), err error) {
c.ins, c.exitErr = readline.InitFiles("cockroach",
true, /* wideChars */
stdin, os.Stdout, stderr)
if c.exitErr == readline.ErrWidecharNotSupported {
if errors.Is(c.exitErr, readline.ErrWidecharNotSupported) {
log.Warning(context.TODO(), "wide character support disabled")
c.ins, c.exitErr = readline.InitFiles("cockroach",
false, stdin, os.Stdout, stderr)
Expand Down
14 changes: 7 additions & 7 deletions pkg/cli/sql_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (c *sqlConn) getServerMetadata() (
) {
// Retrieve the node ID and server build info.
rows, err := c.Query("SELECT * FROM crdb_internal.node_build_info", nil)
if err == driver.ErrBadConn {
if errors.Is(err, driver.ErrBadConn) {
return 0, "", "", err
}
if err != nil {
Expand Down Expand Up @@ -241,7 +241,7 @@ func (c *sqlConn) checkServerMetadata() error {
}

_, newServerVersion, newClusterID, err := c.getServerMetadata()
if err == driver.ErrBadConn {
if errors.Is(err, driver.ErrBadConn) {
return err
}
if err != nil {
Expand Down Expand Up @@ -394,7 +394,7 @@ func (c *sqlConn) Exec(query string, args []driver.Value) error {
}
_, err := c.conn.Exec(query, args)
c.flushNotices()
if err == driver.ErrBadConn {
if errors.Is(err, driver.ErrBadConn) {
c.reconnecting = true
c.Close()
}
Expand All @@ -409,7 +409,7 @@ func (c *sqlConn) Query(query string, args []driver.Value) (*sqlRows, error) {
fmt.Fprintln(stderr, ">", query)
}
rows, err := c.conn.Query(query, args)
if err == driver.ErrBadConn {
if errors.Is(err, driver.ErrBadConn) {
c.reconnecting = true
c.Close()
}
Expand Down Expand Up @@ -447,7 +447,7 @@ func (c *sqlConn) Close() {
c.flushNotices()
if c.conn != nil {
err := c.conn.Close()
if err != nil && err != driver.ErrBadConn {
if err != nil && !errors.Is(err, driver.ErrBadConn) {
log.Infof(context.TODO(), "%v", err)
}
c.conn = nil
Expand Down Expand Up @@ -485,7 +485,7 @@ func (r *sqlRows) Tag() string {
func (r *sqlRows) Close() error {
r.conn.flushNotices()
err := r.rows.Close()
if err == driver.ErrBadConn {
if errors.Is(err, driver.ErrBadConn) {
r.conn.reconnecting = true
r.conn.Close()
}
Expand All @@ -498,7 +498,7 @@ func (r *sqlRows) Close() error {
// (since this is unobvious and unexpected behavior) outweigh.
func (r *sqlRows) Next(values []driver.Value) error {
err := r.rows.Next(values)
if err == driver.ErrBadConn {
if errors.Is(err, driver.ErrBadConn) {
r.conn.reconnecting = true
r.conn.Close()
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/sql_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/errors"
)

func TestConnRecover(t *testing.T) {
Expand Down Expand Up @@ -54,7 +55,7 @@ func TestConnRecover(t *testing.T) {
// and starts delivering ErrBadConn. We don't know the timing of
// this however.
testutils.SucceedsSoon(t, func() error {
if sqlRows, err := conn.Query(`SELECT 1`, nil); err != driver.ErrBadConn {
if sqlRows, err := conn.Query(`SELECT 1`, nil); !errors.Is(err, driver.ErrBadConn) {
return fmt.Errorf("expected ErrBadConn, got %v", err)
} else if err == nil {
if closeErr := sqlRows.Close(); closeErr != nil {
Expand All @@ -78,7 +79,7 @@ func TestConnRecover(t *testing.T) {

// Ditto from Query().
testutils.SucceedsSoon(t, func() error {
if err := conn.Exec(`SELECT 1`, nil); err != driver.ErrBadConn {
if err := conn.Exec(`SELECT 1`, nil); !errors.Is(err, driver.ErrBadConn) {
return fmt.Errorf("expected ErrBadConn, got %v", err)
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/cr2pg/sqlstream/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ func (s *Stream) Next() (tree.Statement, error) {
}
}
if err := s.scan.Err(); err != nil {
if err == bufio.ErrTooLong {
err = errors.New("line too long")
if errors.Is(err, bufio.ErrTooLong) {
err = errors.HandledWithMessage(err, "line too long")
}
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/reduce/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/testutils/reduce"
"github.com/cockroachdb/cockroach/pkg/testutils/reduce/reducesql"
"github.com/cockroachdb/errors"
)

var (
Expand Down Expand Up @@ -124,7 +125,7 @@ func reduceSQL(path, contains string, workers int, verbose bool) (string, error)
out, err := cmd.CombinedOutput()
switch err := err.(type) {
case *exec.Error:
if err.Err == exec.ErrNotFound {
if errors.Is(err.Err, exec.ErrNotFound) {
log.Fatal(err)
}
case *os.PathError:
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/roachprod/cloud/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/cmd/roachprod/config"
"github.com/cockroachdb/cockroach/pkg/cmd/roachprod/vm"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/nlopes/slack"
)

Expand Down Expand Up @@ -314,7 +315,7 @@ func GCClusters(cloud *Cloud, dryrun bool) error {
userChannel, err := findUserChannel(client, user+config.EmailDomain)
if err == nil {
postStatus(client, userChannel, dryrun, status, nil)
} else if err != errNoSlackClient {
} else if !errors.Is(err, errNoSlackClient) {
log.Printf("could not deliver Slack DM to %s: %v", user+config.EmailDomain, err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachprod/vm/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (p *Provider) List() (vm.List, error) {
if err != nil {
return nil, err
}
if err := p.fillNetworkDetails(ctx, &m, nicID); err == vm.ErrBadNetwork {
if err := p.fillNetworkDetails(ctx, &m, nicID); errors.Is(err, vm.ErrBadNetwork) {
m.Errors = append(m.Errors, err)
} else if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -2527,7 +2527,7 @@ func (m *monitor) wait(args ...string) error {
cmd.Stdout = io.MultiWriter(pipeW, monL.stdout)
cmd.Stderr = monL.stderr
if err := cmd.Run(); err != nil {
if err != context.Canceled && !strings.Contains(err.Error(), "killed") {
if !errors.Is(err, context.Canceled) && !strings.Contains(err.Error(), "killed") {
// The expected reason for an error is that the monitor was killed due
// to the context being canceled. Any other error is an actual error.
setMonitorCmdErr(err)
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/roachtest/rebalance_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"golang.org/x/sync/errgroup"
)

Expand Down Expand Up @@ -73,7 +74,7 @@ func registerRebalanceLoad(r *testRegistry) {
"./workload run kv --read-percent=95 --tolerate-errors --concurrency=%d "+
"--duration=%v {pgurl:1-%d}",
concurrency, maxDuration, len(roachNodes)))
if ctx.Err() == context.Canceled {
if errors.Is(ctx.Err(), context.Canceled) {
// We got canceled either because lease balance was achieved or the
// other worker hit an error. In either case, it's not this worker's
// fault.
Expand Down
2 changes: 2 additions & 0 deletions pkg/cmd/roachvet/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package main

import (
"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/descriptormarshal"
"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/errcmp"
"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/fmtsafe"
"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/hash"
"github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/nocopy"
Expand Down Expand Up @@ -57,6 +58,7 @@ func main() {
timer.Analyzer,
unconvert.Analyzer,
fmtsafe.Analyzer,
errcmp.Analyzer,

// Standard go vet analyzers:
asmdecl.Analyzer,
Expand Down
2 changes: 1 addition & 1 deletion pkg/gossip/infostore.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func (is *infoStore) combine(
// the data in *is is newer than in *delta.
if addErr := is.addInfo(key, &infoCopy); addErr == nil {
freshCount++
} else if addErr != errNotFresh {
} else if !errors.Is(addErr, errNotFresh) {
err = addErr
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/jobs/jobsprotectedts/jobs_protected_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/cockroachdb/errors"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -90,7 +91,7 @@ func TestJobsProtectedTimestamp(t *testing.T) {
_, recRemains := mkJobAndRecord()
ensureNotExists := func(ctx context.Context, txn *kv.Txn, ptsID uuid.UUID) (err error) {
_, err = ptp.GetRecord(ctx, txn, ptsID)
if err == protectedts.ErrNotExists {
if errors.Is(err, protectedts.ErrNotExists) {
return nil
}
return fmt.Errorf("waiting for %v, got %v", protectedts.ErrNotExists, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvclient/kvcoord/range_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ func TestRangeCacheContextCancellation(t *testing.T) {
}

expectContextCancellation := func(t *testing.T, c <-chan error) {
if err := <-c; errors.Cause(err) != context.Canceled {
if err := <-c; !errors.Is(err, context.Canceled) {
t.Errorf("expected context cancellation error, found %v", err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvclient/kvcoord/txn_coord_sender_savepoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (tc *TxnCoordSender) RollbackToSavepoint(ctx context.Context, s kv.Savepoin
sp := s.(*savepoint)
err := tc.checkSavepointLocked(sp)
if err != nil {
if err == errSavepointInvalidAfterTxnRestart {
if errors.Is(err, errSavepointInvalidAfterTxnRestart) {
err = roachpb.NewTransactionRetryWithProtoRefreshError(
"cannot rollback to savepoint after a transaction restart",
tc.mu.txn.ID,
Expand Down Expand Up @@ -169,7 +169,7 @@ func (tc *TxnCoordSender) ReleaseSavepoint(ctx context.Context, s kv.SavepointTo

sp := s.(*savepoint)
err := tc.checkSavepointLocked(sp)
if err == errSavepointInvalidAfterTxnRestart {
if errors.Is(err, errSavepointInvalidAfterTxnRestart) {
err = roachpb.NewTransactionRetryWithProtoRefreshError(
"cannot release savepoint after a transaction restart",
tc.mu.txn.ID,
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/client_raft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,7 @@ func TestFailedSnapshotFillsReservation(t *testing.T) {
// "snapshot accepted" message.
expectedErr := errors.Errorf("")
stream := fakeSnapshotStream{nil, expectedErr}
if err := mtc.stores[1].HandleSnapshot(&header, stream); err != expectedErr {
if err := mtc.stores[1].HandleSnapshot(&header, stream); !errors.Is(err, expectedErr) {
t.Fatalf("expected error %s, but found %v", expectedErr, err)
}
if n := mtc.stores[1].ReservationCount(); n != 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,7 @@ func (m *multiTestContext) heartbeatLiveness(ctx context.Context, store int) err
}

for r := retry.StartWithCtx(ctx, retry.Options{MaxRetries: 5}); r.Next(); {
if err = nl.Heartbeat(ctx, l); err != kvserver.ErrEpochIncremented {
if err = nl.Heartbeat(ctx, l); !errors.Is(err, kvserver.ErrEpochIncremented) {
break
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/kv/kvserver/concurrency/lock_table_waiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/stop"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
)

// LockTableLivenessPushDelay sets the delay before pushing in order to detect
Expand Down Expand Up @@ -278,7 +279,7 @@ func (w *lockTableWaiterImpl) WaitOn(
pushCtx, pushCancel := context.WithCancel(ctx)
go w.watchForNotifications(pushCtx, pushCancel, newStateC)
err = w.pushRequestTxn(pushCtx, req, timerWaitingState)
if pushCtx.Err() == context.Canceled {
if errors.Is(pushCtx.Err(), context.Canceled) {
// Ignore the context canceled error. If this was for the
// parent context then we'll notice on the next select.
err = nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/gc_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ func (gcq *gcQueue) process(ctx context.Context, repl *Replica, sysCfg *config.S
gcq.store.metrics.GCResolveSuccess.Inc(int64(len(intents)))
}
})
if errors.Cause(err) == stop.ErrThrottled {
if errors.Is(err, stop.ErrThrottled) {
log.Eventf(ctx, "processing txn %s: %s; skipping for future GC", txn.ID.Short(), err)
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/idalloc/id_alloc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func TestAllocateErrorAndRecovery(t *testing.T) {
defer cancel()
for i := 0; i < routines; i++ {
id, err := idAlloc.Allocate(ctx)
if id != 0 || err != context.DeadlineExceeded {
if id != 0 || !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("expected context cancellation, found id=%d, err=%v", id, err)
}
}
Expand Down
Loading

0 comments on commit 15c7371

Please sign in to comment.