Skip to content

Commit 45ff522

Browse files
denikIsaac
andauthored
Enable errorlint for error comparisons and type assertions (#7084)
## Changes Enable `errorlint` with only its comparison and type-assertion checks, and fix its 29 findings: `err == ErrX` becomes `errors.Is(err, ErrX)` and `err.(*T)` becomes `errors.AsType[*T](err)`. The `errorf` check (`%v` → `%w` in `fmt.Errorf`) stays off: wrapping makes the cause visible to every `errors.Is`/`errors.As` up the stack (e.g. `fs.ErrNotExist`, `context.Canceled`), so converting its 36 sites is a behavior change that needs per-site review in a separate PR. ## Why These checks break silently as soon as the error is wrapped with `%w`. No current site is broken, since every sentinel is still returned unwrapped, but the codebase was inconsistent: for example `cmd/bundle/validate.go` compared `root.ErrAlreadyPrinted` with `!=` while `cmd/bundle/utils/process.go` uses `errors.Is`. ## Tests golangci-lint reports 0 issues on the root and tools modules. Unit tests pass for every changed package, and the `cmd/` and `bundle/validate/` acceptance tests pass. One test keeps its `!=` with a `//nolint:errorlint`, because it checks that `mapUploadError` returns the same error rather than a wrapper. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
1 parent c2f4faa commit 45ff522

23 files changed

Lines changed: 48 additions & 31 deletions

File tree

‎.golangci.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,13 @@ linters:
3535
- reassign
3636
- modernize
3737
- makezero
38+
- errorlint
3839
settings:
40+
errorlint:
41+
# Switching %v to %w changes which errors callers can match with
42+
# errors.Is/As (e.g. fs.ErrNotExist, context.Canceled), so it needs
43+
# per-site review rather than a bulk fix.
44+
errorf: false
3945
modernize:
4046
# omitzero changes JSON wire format for nested structs (unlike omitempty),
4147
# so each site needs per-site review rather than a bulk rewrite.

‎acceptance/acceptance_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1648,7 +1648,7 @@ func formatOutput(w io.Writer, err error) {
16481648
if err == nil {
16491649
return
16501650
}
1651-
if exiterr, ok := err.(*exec.ExitError); ok {
1651+
if exiterr, ok := errors.AsType[*exec.ExitError](err); ok {
16521652
exitCode := exiterr.ExitCode()
16531653
fmt.Fprintf(w, "\nExit code: %d\n", exitCode)
16541654
} else {
@@ -1859,7 +1859,7 @@ func runWithLog(t *testing.T, cmd *exec.Cmd, out *os.File, tail bool, timeout ti
18591859
require.NoError(t, err)
18601860
}
18611861
}
1862-
if err == io.EOF {
1862+
if errors.Is(err, io.EOF) {
18631863
break
18641864
}
18651865
require.NoError(t, err)

‎acceptance/internal/prepare_server.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package internal
33
import (
44
"bytes"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"io"
89
"mime"
@@ -376,7 +377,7 @@ func normalizeMultipartBody(req *testserver.Request) (any, bool) {
376377
parts := map[string]any{}
377378
for {
378379
part, err := mr.NextPart()
379-
if err == io.EOF {
380+
if errors.Is(err, io.EOF) {
380381
break
381382
}
382383
if err != nil {

‎bundle/artifacts/tarball_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"archive/tar"
55
"bytes"
66
"compress/gzip"
7+
"errors"
78
"io"
89
"os"
910
"os/exec"
@@ -26,7 +27,7 @@ func tarEntries(t *testing.T, b []byte) map[string]string {
2627
out := map[string]string{}
2728
for {
2829
hdr, err := tr.Next()
29-
if err == io.EOF {
30+
if errors.Is(err, io.EOF) {
3031
break
3132
}
3233
require.NoError(t, err)

‎bundle/config/mutator/python/python_mutator.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func (m *pythonMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno
293293
b.Metrics.PythonUpdatedResourcesCount += int64(result.UpdatedResources.Size())
294294
b.Metrics.PythonAddedResourcesCount += int64(result.AddedResources.Size())
295295

296-
if err == mutateDiagsHasError {
296+
if errors.Is(err, mutateDiagsHasError) {
297297
if !mutateDiags.HasError() {
298298
panic("mutateDiags has no error, but error is expected")
299299
}

‎cmd/air/logstream_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ func TestClassifyLogError(t *testing.T) {
6262
for _, tt := range tests {
6363
t.Run(tt.name, func(t *testing.T) {
6464
got := classifyLogError(tt.err)
65-
switch tt.want {
66-
case errBricklensFeatureDisabled:
65+
switch {
66+
case errors.Is(tt.want, errBricklensFeatureDisabled):
6767
assert.ErrorIs(t, got, errBricklensFeatureDisabled)
68-
case nil:
68+
case tt.want == nil:
6969
assert.NoError(t, got)
7070
default:
7171
assert.ErrorIs(t, got, tt.want)

‎cmd/auth/env.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func loadFromDatabricksCfg(ctx context.Context, cfg *config.Config) error {
7979
return err
8080
}
8181
profile, err := resolveSection(cfg, iniFile)
82-
if err == ErrNoMatchingProfiles {
82+
if errors.Is(err, ErrNoMatchingProfiles) {
8383
// it's also fine for Azure CLI or Databricks CLI, which
8484
// are resolved by unified auth handling in the Go SDK.
8585
return nil

‎cmd/bundle/validate.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package bundle
22

33
import (
44
"encoding/json"
5+
"errors"
56
"fmt"
67

78
"github.com/databricks/cli/bundle"
@@ -67,7 +68,7 @@ Please run this command before deploying to ensure configuration quality.`,
6768
})
6869
ctx := cmd.Context()
6970

70-
if err != nil && err != root.ErrAlreadyPrinted {
71+
if err != nil && !errors.Is(err, root.ErrAlreadyPrinted) {
7172
logdiag.LogError(ctx, err)
7273
err = root.ErrAlreadyPrinted
7374
}

‎experimental/ssh/internal/client/agentshim.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ func probeModelServices(ctx context.Context, apiClient *client.DatabricksClient)
378378
it := catalog.NewAiGateway(apiClient).ListModelServices(ctx, catalog.ListModelServicesRequest{PageSize: modelServiceProbePageSize})
379379
_, err := it.Next(ctx)
380380
if err != nil {
381-
if err == listing.ErrNoMoreItems {
381+
if errors.Is(err, listing.ErrNoMoreItems) {
382382
// endpoint is reachable but there are no results
383383
return gatewayProbe{reachable: true}
384384
}

‎libs/appproxy/appproxy.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package appproxy
22

33
import (
44
"context"
5+
"errors"
56
"io"
67
"net"
78
"net/http"
@@ -113,7 +114,7 @@ func (p *Proxy) handleWebSocket(w http.ResponseWriter, r *http.Request) {
113114
err = <-errc
114115
if err != nil {
115116
// If the error is not EOF, then there was a problem
116-
if err != io.EOF {
117+
if !errors.Is(err, io.EOF) {
117118
// Log the error and perform cleanup
118119
log.Warnf(r.Context(), "error copying messages: %v", err)
119120
middlewareConn.Close()

0 commit comments

Comments
 (0)