From 59d355255f8cdf77b23c42a80f92298e3c0d29be Mon Sep 17 00:00:00 2001 From: "denis.bilenko" Date: Fri, 16 Oct 2026 08:28:10 +0000 Subject: [PATCH 1/2] Enable errorlint for error comparisons and type assertions Compare errors with errors.Is and unwrap them with errors.AsType, so the checks keep working if the error is wrapped later. errorf is disabled: formatting errors with %v is often deliberate. Co-authored-by: Isaac --- .golangci.yaml | 5 +++++ acceptance/acceptance_test.go | 4 ++-- acceptance/internal/prepare_server.go | 3 ++- bundle/artifacts/tarball_test.go | 3 ++- bundle/config/mutator/python/python_mutator.go | 2 +- cmd/air/logstream_test.go | 6 +++--- cmd/auth/env.go | 2 +- cmd/bundle/validate.go | 3 ++- experimental/ssh/internal/client/agentshim.go | 2 +- libs/appproxy/appproxy.go | 3 ++- libs/databrickscfg/loader.go | 4 ++-- libs/databrickscfg/ops.go | 2 +- libs/dyn/jsonloader/json.go | 2 +- libs/dyn/walk.go | 6 +++--- libs/dyn/walk_read_only.go | 4 +++- libs/dyn/yamlloader/yaml.go | 3 ++- libs/exec/exec.go | 3 ++- libs/filer/files_client_test.go | 2 +- libs/genie/agentstream/renderer.go | 6 +++--- libs/git/reference.go | 2 +- libs/tarpack/tarpack_test.go | 3 ++- libs/testserver/handlers.go | 5 +++-- tools/testrunner/main.go | 3 ++- 23 files changed, 47 insertions(+), 31 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 7bd0fcb8e32..7cc92082951 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -35,7 +35,12 @@ linters: - reassign - modernize - makezero + - errorlint settings: + errorlint: + # Formatting errors with %v instead of %w is often deliberate, to keep + # the cause out of the error chain. + errorf: false modernize: # omitzero changes JSON wire format for nested structs (unlike omitempty), # so each site needs per-site review rather than a bulk rewrite. diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index a1b2df2a7b9..d27cea9cbc2 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1648,7 +1648,7 @@ func formatOutput(w io.Writer, err error) { if err == nil { return } - if exiterr, ok := err.(*exec.ExitError); ok { + if exiterr, ok := errors.AsType[*exec.ExitError](err); ok { exitCode := exiterr.ExitCode() fmt.Fprintf(w, "\nExit code: %d\n", exitCode) } else { @@ -1859,7 +1859,7 @@ func runWithLog(t *testing.T, cmd *exec.Cmd, out *os.File, tail bool, timeout ti require.NoError(t, err) } } - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) diff --git a/acceptance/internal/prepare_server.go b/acceptance/internal/prepare_server.go index 468aa02dc9e..6d32556bbe0 100644 --- a/acceptance/internal/prepare_server.go +++ b/acceptance/internal/prepare_server.go @@ -3,6 +3,7 @@ package internal import ( "bytes" "encoding/json" + "errors" "fmt" "io" "mime" @@ -376,7 +377,7 @@ func normalizeMultipartBody(req *testserver.Request) (any, bool) { parts := map[string]any{} for { part, err := mr.NextPart() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { diff --git a/bundle/artifacts/tarball_test.go b/bundle/artifacts/tarball_test.go index 6da9a91d9e2..49764877ae6 100644 --- a/bundle/artifacts/tarball_test.go +++ b/bundle/artifacts/tarball_test.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "errors" "io" "os" "os/exec" @@ -26,7 +27,7 @@ func tarEntries(t *testing.T, b []byte) map[string]string { out := map[string]string{} for { hdr, err := tr.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) diff --git a/bundle/config/mutator/python/python_mutator.go b/bundle/config/mutator/python/python_mutator.go index 90d7f9ff287..5d6e8b0d801 100644 --- a/bundle/config/mutator/python/python_mutator.go +++ b/bundle/config/mutator/python/python_mutator.go @@ -293,7 +293,7 @@ func (m *pythonMutator) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno b.Metrics.PythonUpdatedResourcesCount += int64(result.UpdatedResources.Size()) b.Metrics.PythonAddedResourcesCount += int64(result.AddedResources.Size()) - if err == mutateDiagsHasError { + if errors.Is(err, mutateDiagsHasError) { if !mutateDiags.HasError() { panic("mutateDiags has no error, but error is expected") } diff --git a/cmd/air/logstream_test.go b/cmd/air/logstream_test.go index 242a0b51b38..fe0756d3862 100644 --- a/cmd/air/logstream_test.go +++ b/cmd/air/logstream_test.go @@ -62,10 +62,10 @@ func TestClassifyLogError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := classifyLogError(tt.err) - switch tt.want { - case errBricklensFeatureDisabled: + switch { + case errors.Is(tt.want, errBricklensFeatureDisabled): assert.ErrorIs(t, got, errBricklensFeatureDisabled) - case nil: + case tt.want == nil: assert.NoError(t, got) default: assert.ErrorIs(t, got, tt.want) diff --git a/cmd/auth/env.go b/cmd/auth/env.go index 9861d491e42..4f94beb67ce 100644 --- a/cmd/auth/env.go +++ b/cmd/auth/env.go @@ -79,7 +79,7 @@ func loadFromDatabricksCfg(ctx context.Context, cfg *config.Config) error { return err } profile, err := resolveSection(cfg, iniFile) - if err == ErrNoMatchingProfiles { + if errors.Is(err, ErrNoMatchingProfiles) { // it's also fine for Azure CLI or Databricks CLI, which // are resolved by unified auth handling in the Go SDK. return nil diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 9f8257731a5..d28f4ab8d3b 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -2,6 +2,7 @@ package bundle import ( "encoding/json" + "errors" "fmt" "github.com/databricks/cli/bundle" @@ -67,7 +68,7 @@ Please run this command before deploying to ensure configuration quality.`, }) ctx := cmd.Context() - if err != nil && err != root.ErrAlreadyPrinted { + if err != nil && !errors.Is(err, root.ErrAlreadyPrinted) { logdiag.LogError(ctx, err) err = root.ErrAlreadyPrinted } diff --git a/experimental/ssh/internal/client/agentshim.go b/experimental/ssh/internal/client/agentshim.go index d596a4e49d9..58663eb0d01 100644 --- a/experimental/ssh/internal/client/agentshim.go +++ b/experimental/ssh/internal/client/agentshim.go @@ -378,7 +378,7 @@ func probeModelServices(ctx context.Context, apiClient *client.DatabricksClient) it := catalog.NewAiGateway(apiClient).ListModelServices(ctx, catalog.ListModelServicesRequest{PageSize: modelServiceProbePageSize}) _, err := it.Next(ctx) if err != nil { - if err == listing.ErrNoMoreItems { + if errors.Is(err, listing.ErrNoMoreItems) { // endpoint is reachable but there are no results return gatewayProbe{reachable: true} } diff --git a/libs/appproxy/appproxy.go b/libs/appproxy/appproxy.go index bfe5150ff96..1358cf5d9b2 100644 --- a/libs/appproxy/appproxy.go +++ b/libs/appproxy/appproxy.go @@ -2,6 +2,7 @@ package appproxy import ( "context" + "errors" "io" "net" "net/http" @@ -113,7 +114,7 @@ func (p *Proxy) handleWebSocket(w http.ResponseWriter, r *http.Request) { err = <-errc if err != nil { // If the error is not EOF, then there was a problem - if err != io.EOF { + if !errors.Is(err, io.EOF) { // Log the error and perform cleanup log.Warnf(r.Context(), "error copying messages: %v", err) middlewareConn.Close() diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 732dcf9024f..536a35639fa 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -188,7 +188,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error { // Check if this section matches the normalized host return normalizeHost(key.Value()) == host }) - if err == errNoMatchingProfiles { + if errors.Is(err, errNoMatchingProfiles) { return nil } @@ -197,7 +197,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error { if names, ok := AsMultipleProfiles(err); ok && cfg.WorkspaceID != "" { originalErr := err match, err = l.disambiguateByWorkspaceID(ctx, configFile, host, cfg.WorkspaceID, names) - if err == errNoMatchingProfiles { + if errors.Is(err, errNoMatchingProfiles) { // workspace_id didn't match any of the host-matching profiles. // Fall back to the original ambiguity error. log.Debugf(ctx, "workspace_id=%s did not match any profiles for host %s: %v", cfg.WorkspaceID, host, names) diff --git a/libs/databrickscfg/ops.go b/libs/databrickscfg/ops.go index bb066a89edd..6358c5ba49d 100644 --- a/libs/databrickscfg/ops.go +++ b/libs/databrickscfg/ops.go @@ -353,7 +353,7 @@ func matchOrCreateSection(ctx context.Context, configFile *config.File, cfg *con // Check if this section matches the normalized host return normalizeHost(host) == normalizeHost(cfg.Host) }) - if err == errNoMatchingProfiles { + if errors.Is(err, errNoMatchingProfiles) { section, err = configFile.NewSection(cfg.Profile) if err != nil { return nil, fmt.Errorf("cannot create new profile: %w", err) diff --git a/libs/dyn/jsonloader/json.go b/libs/dyn/jsonloader/json.go index 8db526bd97d..2500e5624dc 100644 --- a/libs/dyn/jsonloader/json.go +++ b/libs/dyn/jsonloader/json.go @@ -24,7 +24,7 @@ func LoadJSON(data []byte, source string) (dyn.Value, error) { // Start decoding from the top-level value value, err := decodeValue(decoder, &offset) if err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { err = errors.New("unexpected end of JSON input") } // Get the current offset for error reporting diff --git a/libs/dyn/walk.go b/libs/dyn/walk.go index af03ef27743..08123cf85e9 100644 --- a/libs/dyn/walk.go +++ b/libs/dyn/walk.go @@ -25,7 +25,7 @@ func Walk(v Value, fn func(p Path, v Value) (Value, error)) (Value, error) { func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, error) { v, err := fn(p, v) if err != nil { - if err == ErrSkip { + if errors.Is(err, ErrSkip) { return v, nil } return InvalidValue, err @@ -39,7 +39,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro pk := pair.Key pv := pair.Value nv, err := walk(pv, append(p, Key(pk.MustString())), fn) - if err == ErrDrop { + if errors.Is(err, ErrDrop) { continue } if err != nil { @@ -53,7 +53,7 @@ func walk(v Value, p Path, fn func(p Path, v Value) (Value, error)) (Value, erro out := make([]Value, 0, len(s)) for i := range s { nv, err := walk(s[i], append(p, Index(i)), fn) - if err == ErrDrop { + if errors.Is(err, ErrDrop) { continue } if err != nil { diff --git a/libs/dyn/walk_read_only.go b/libs/dyn/walk_read_only.go index caead36289e..8f8d15e06ea 100644 --- a/libs/dyn/walk_read_only.go +++ b/libs/dyn/walk_read_only.go @@ -1,5 +1,7 @@ package dyn +import "errors" + // WalkReadOnly walks the configuration tree in readonly mode and calls the given function on each node. // The callback may return ErrSkip to skip traversal of a subtree. // If the callback returns another error, the walk is aborted, and the error is returned. @@ -12,7 +14,7 @@ func WalkReadOnly(v Value, fn func(p Path, v Value) error) error { // such that it can be passed to the callback function. func walkReadOnly(v Value, p Path, fn func(p Path, v Value) error) error { if err := fn(p, v); err != nil { - if err == ErrSkip { + if errors.Is(err, ErrSkip) { return nil } return err diff --git a/libs/dyn/yamlloader/yaml.go b/libs/dyn/yamlloader/yaml.go index 1225fcc7e2f..29570747ab1 100644 --- a/libs/dyn/yamlloader/yaml.go +++ b/libs/dyn/yamlloader/yaml.go @@ -1,6 +1,7 @@ package yamlloader import ( + "errors" "io" "github.com/databricks/cli/libs/dyn" @@ -12,7 +13,7 @@ func LoadYAML(path string, r io.Reader) (dyn.Value, error) { dec := yaml.NewDecoder(r) err := dec.Decode(&node) if err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { return dyn.NilValue, nil } return dyn.InvalidValue, err diff --git a/libs/exec/exec.go b/libs/exec/exec.go index b867b8631f6..feea8af0f92 100644 --- a/libs/exec/exec.go +++ b/libs/exec/exec.go @@ -3,6 +3,7 @@ package exec import ( "bytes" "context" + "errors" "fmt" "io" "os" @@ -190,7 +191,7 @@ func (e *Executor) ExecAndCapture(ctx context.Context, command string) (*ExecOut err = cmd.Wait() exitCode := 0 if err != nil { - if exitErr, ok := err.(*osexec.ExitError); ok { + if exitErr, ok := errors.AsType[*osexec.ExitError](err); ok { exitCode = exitErr.ExitCode() } else { return nil, err diff --git a/libs/filer/files_client_test.go b/libs/filer/files_client_test.go index c259d4a9f14..3579c5464c5 100644 --- a/libs/filer/files_client_test.go +++ b/libs/filer/files_client_test.go @@ -212,7 +212,7 @@ func TestMapUploadError(t *testing.T) { // Other errors pass through unchanged. other := errors.New("boom") - if got := mapUploadError(other, p); got != other { + if got := mapUploadError(other, p); got != other { //nolint:errorlint // must be the same error, not a wrapper t.Errorf("mapUploadError(other) = %v, want it unchanged", got) } } diff --git a/libs/genie/agentstream/renderer.go b/libs/genie/agentstream/renderer.go index ae63b1b2d1a..37aa3908d3d 100644 --- a/libs/genie/agentstream/renderer.go +++ b/libs/genie/agentstream/renderer.go @@ -37,7 +37,7 @@ func RenderDebug(r io.Reader, w io.Writer) error { reader := NewSSEReader(r) for { ev, err := reader.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { return nil } if err != nil { @@ -92,7 +92,7 @@ func RenderText(ctx context.Context, r io.Reader, stdout, stderr io.Writer, adap for { ev, err := reader.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { @@ -198,7 +198,7 @@ func RenderJSON(r io.Reader, stdout, stderr io.Writer, adapt AdapterFunc) (strin loop: for { ev, err := reader.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { diff --git a/libs/git/reference.go b/libs/git/reference.go index 6001d70de5c..35dc96bc59e 100644 --- a/libs/git/reference.go +++ b/libs/git/reference.go @@ -86,7 +86,7 @@ func (ref *Reference) ResolvePath() (string, error) { // `ref: refs/heads/my-branch` returns `my-branch` func (ref *Reference) CurrentBranch() (string, error) { branchRefPath, err := ref.ResolvePath() - if err == ErrNotAReferencePointer { + if errors.Is(err, ErrNotAReferencePointer) { return "", ErrNotABranch } if err != nil { diff --git a/libs/tarpack/tarpack_test.go b/libs/tarpack/tarpack_test.go index 6fd6d43b69c..3841e779ac0 100644 --- a/libs/tarpack/tarpack_test.go +++ b/libs/tarpack/tarpack_test.go @@ -3,6 +3,7 @@ package tarpack import ( "archive/tar" "bytes" + "errors" "io" "os" "path/filepath" @@ -26,7 +27,7 @@ func readTar(t *testing.T, b []byte) map[string]readEntry { out := map[string]readEntry{} for { hdr, err := tr.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index eeaae2bb56f..3a9a538313e 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/base64" "encoding/json" + "errors" "fmt" "io" "mime" @@ -141,7 +142,7 @@ func ParseImportForm(req Request) (ImportForm, error) { mr := multipart.NewReader(bytes.NewReader(req.Body), params["boundary"]) for { part, err := mr.NextPart() - if err == io.EOF { + if errors.Is(err, io.EOF) { return form, nil } if err != nil { @@ -907,7 +908,7 @@ func AddDefaultHandlers(server *Server) { var bundleID, snapshotID string for { p, err := mr.NextPart() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { diff --git a/tools/testrunner/main.go b/tools/testrunner/main.go index dcc8e9dd595..8415ff8ff9b 100644 --- a/tools/testrunner/main.go +++ b/tools/testrunner/main.go @@ -18,6 +18,7 @@ import ( "bufio" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -45,7 +46,7 @@ func getExitCode(err error) (int, error) { if err == nil { return 0, nil } - if exitError, ok := err.(*exec.ExitError); ok { + if exitError, ok := errors.AsType[*exec.ExitError](err); ok { if status, ok := exitError.Sys().(syscall.WaitStatus); ok { return status.ExitStatus(), nil } From 753f11daf96e4d5d013b86572192b61be0b1e57d Mon Sep 17 00:00:00 2001 From: "denis.bilenko" Date: Fri, 16 Oct 2026 08:45:58 +0000 Subject: [PATCH 2/2] Explain why errorlint errorf is disabled Co-authored-by: Isaac --- .golangci.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 7cc92082951..aa297e79221 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -38,8 +38,9 @@ linters: - errorlint settings: errorlint: - # Formatting errors with %v instead of %w is often deliberate, to keep - # the cause out of the error chain. + # Switching %v to %w changes which errors callers can match with + # errors.Is/As (e.g. fs.ErrNotExist, context.Canceled), so it needs + # per-site review rather than a bulk fix. errorf: false modernize: # omitzero changes JSON wire format for nested structs (unlike omitempty),