Return diag.Diagnostics from mutators (#1305)

## Changes

This diagnostics type allows us to capture multiple warnings as well as
errors in the return value. This is a preparation for returning
additional warnings from mutators in case we detect non-fatal problems.

* All return statements that previously returned an error now return
`diag.FromErr`
* All return statements that previously returned `fmt.Errorf` now return
`diag.Errorf`
* All `err != nil` checks now use `diags.HasError()` or `diags.Error()`

## Tests

* Existing tests pass.
* I confirmed no call site under `./bundle` or `./cmd/bundle` uses
`errors.Is` on the return value from mutators. This is relevant because
we cannot wrap errors with `%w` when calling `diag.Errorf` (like
`fmt.Errorf`; context in golang/go#47641).

bundle/artifacts/all.go

@@ -7,6 +7,7 @@ import (
 	"slices"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/diag"
 	"golang.org/x/exp/maps"
 )
 
@@ -21,7 +22,7 @@ func (m *all) Name() string {
 	return fmt.Sprintf("artifacts.%sAll", m.name)
 }
 
-func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *all) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	var out []bundle.Mutator
 
 	// Iterate with stable ordering.
@@ -31,7 +32,7 @@ func (m *all) Apply(ctx context.Context, b *bundle.Bundle) error {
 	for _, name := range keys {
 		m, err := m.fn(name)
 		if err != nil {
-			return err
+			return diag.FromErr(err)
 		}
 		if m != nil {
 			out = append(out, m)

bundle/artifacts/artifacts.go

@@ -14,6 +14,7 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/libraries"
 	"github.com/databricks/cli/libs/cmdio"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/cli/libs/log"
 )
@@ -57,17 +58,17 @@ func (m *basicBuild) Name() string {
 	return fmt.Sprintf("artifacts.Build(%s)", m.name)
 }
 
-func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *basicBuild) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact, ok := b.Config.Artifacts[m.name]
 	if !ok {
-		return fmt.Errorf("artifact doesn't exist: %s", m.name)
+		return diag.Errorf("artifact doesn't exist: %s", m.name)
 	}
 
 	cmdio.LogString(ctx, fmt.Sprintf("Building %s...", m.name))
 
 	out, err := artifact.Build(ctx)
 	if err != nil {
-		return fmt.Errorf("build for %s failed, error: %w, output: %s", m.name, err, out)
+		return diag.Errorf("build for %s failed, error: %v, output: %s", m.name, err, out)
 	}
 	log.Infof(ctx, "Build succeeded")
 
@@ -87,29 +88,29 @@ func (m *basicUpload) Name() string {
 	return fmt.Sprintf("artifacts.Upload(%s)", m.name)
 }
 
-func (m *basicUpload) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *basicUpload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact, ok := b.Config.Artifacts[m.name]
 	if !ok {
-		return fmt.Errorf("artifact doesn't exist: %s", m.name)
+		return diag.Errorf("artifact doesn't exist: %s", m.name)
 	}
 
 	if len(artifact.Files) == 0 {
-		return fmt.Errorf("artifact source is not configured: %s", m.name)
+		return diag.Errorf("artifact source is not configured: %s", m.name)
 	}
 
 	uploadPath, err := getUploadBasePath(b)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	client, err := filer.NewWorkspaceFilesClient(b.WorkspaceClient(), uploadPath)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	err = uploadArtifact(ctx, b, artifact, uploadPath, client)
 	if err != nil {
-		return fmt.Errorf("upload for %s failed, error: %w", m.name, err)
+		return diag.Errorf("upload for %s failed, error: %v", m.name, err)
 	}
 
 	return nil

bundle/artifacts/autodetect.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/artifacts/whl"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
 )
 
@@ -19,7 +20,7 @@ func (m *autodetect) Name() string {
 	return "artifacts.DetectPackages"
 }
 
-func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *autodetect) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	// If artifacts section explicitly defined, do not try to auto detect packages
 	if b.Config.Artifacts != nil {
 		log.Debugf(ctx, "artifacts block is defined, skipping auto-detecting")

bundle/artifacts/build.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/diag"
 )
 
 func BuildAll() bundle.Mutator {
@@ -27,18 +28,18 @@ func (m *build) Name() string {
 	return fmt.Sprintf("artifacts.Build(%s)", m.name)
 }
 
-func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact, ok := b.Config.Artifacts[m.name]
 	if !ok {
-		return fmt.Errorf("artifact doesn't exist: %s", m.name)
+		return diag.Errorf("artifact doesn't exist: %s", m.name)
 	}
 
 	// Skip building if build command is not specified or infered
 	if artifact.BuildCommand == "" {
 		// If no build command was specified or infered and there is no
 		// artifact output files specified, artifact is misconfigured
 		if len(artifact.Files) == 0 {
-			return fmt.Errorf("misconfigured artifact: please specify 'build' or 'files' property")
+			return diag.Errorf("misconfigured artifact: please specify 'build' or 'files' property")
 		}
 		return nil
 	}

bundle/artifacts/infer.go

@@ -7,6 +7,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/artifacts/whl"
 	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/libs/diag"
 )
 
 var inferMutators map[config.ArtifactType]mutatorFactory = map[config.ArtifactType]mutatorFactory{
@@ -41,10 +42,10 @@ func (m *infer) Name() string {
 	return fmt.Sprintf("artifacts.Infer(%s)", m.name)
 }
 
-func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact, ok := b.Config.Artifacts[m.name]
 	if !ok {
-		return fmt.Errorf("artifact doesn't exist: %s", m.name)
+		return diag.Errorf("artifact doesn't exist: %s", m.name)
 	}
 
 	// only try to infer command if it's not already defined

bundle/artifacts/upload.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/databricks-sdk-go/service/workspace"
 )
 
@@ -33,14 +34,14 @@ func (m *upload) Name() string {
 	return fmt.Sprintf("artifacts.Upload(%s)", m.name)
 }
 
-func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact, ok := b.Config.Artifacts[m.name]
 	if !ok {
-		return fmt.Errorf("artifact doesn't exist: %s", m.name)
+		return diag.Errorf("artifact doesn't exist: %s", m.name)
 	}
 
 	if len(artifact.Files) == 0 {
-		return fmt.Errorf("artifact source is not configured: %s", m.name)
+		return diag.Errorf("artifact source is not configured: %s", m.name)
 	}
 
 	// Check if source paths are absolute, if not, make them absolute
@@ -57,11 +58,11 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) error {
 	for _, f := range artifact.Files {
 		matches, err := filepath.Glob(f.Source)
 		if err != nil {
-			return fmt.Errorf("unable to find files for %s: %w", f.Source, err)
+			return diag.Errorf("unable to find files for %s: %v", f.Source, err)
 		}
 
 		if len(matches) == 0 {
-			return fmt.Errorf("no files found for %s", f.Source)
+			return diag.Errorf("no files found for %s", f.Source)
 		}
 
 		for _, match := range matches {
@@ -81,10 +82,10 @@ func (m *cleanUp) Name() string {
 	return "artifacts.CleanUp"
 }
 
-func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	uploadPath, err := getUploadBasePath(b)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	b.WorkspaceClient().Workspace.Delete(ctx, workspace.Delete{
@@ -94,7 +95,7 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) error {
 
 	err = b.WorkspaceClient().Workspace.MkdirsByPath(ctx, uploadPath)
 	if err != nil {
-		return fmt.Errorf("unable to create directory for %s: %w", uploadPath, err)
+		return diag.Errorf("unable to create directory for %s: %v", uploadPath, err)
 	}
 
 	return nil

bundle/artifacts/upload_test.go

@@ -9,13 +9,14 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/internal/bundletest"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/testfile"
 	"github.com/stretchr/testify/require"
 )
 
 type noop struct{}
 
-func (n *noop) Apply(context.Context, *bundle.Bundle) error {
+func (n *noop) Apply(context.Context, *bundle.Bundle) diag.Diagnostics {
 	return nil
 }
 
@@ -57,8 +58,8 @@ func TestExpandGlobFilesSource(t *testing.T) {
 		return &noop{}
 	}
 
-	err = bundle.Apply(context.Background(), b, u)
-	require.NoError(t, err)
+	diags := bundle.Apply(context.Background(), b, u)
+	require.NoError(t, diags.Error())
 
 	require.Equal(t, 2, len(b.Config.Artifacts["test"].Files))
 	require.Equal(t, filepath.Join(rootPath, "test", "myjar1.jar"), b.Config.Artifacts["test"].Files[0].Source)
@@ -93,6 +94,6 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) {
 		return &noop{}
 	}
 
-	err = bundle.Apply(context.Background(), b, u)
-	require.ErrorContains(t, err, "no files found for")
+	diags := bundle.Apply(context.Background(), b, u)
+	require.ErrorContains(t, diags.Error(), "no files found for")
 }

bundle/artifacts/whl/autodetect.go

@@ -11,6 +11,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/libraries"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
 )
 
@@ -25,7 +26,7 @@ func (m *detectPkg) Name() string {
 	return "artifacts.whl.AutoDetect"
 }
 
-func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	wheelTasks := libraries.FindAllWheelTasksWithLocalLibraries(b)
 	if len(wheelTasks) == 0 {
 		log.Infof(ctx, "No local wheel tasks in databricks.yml config, skipping auto detect")
@@ -50,7 +51,7 @@ func (m *detectPkg) Apply(ctx context.Context, b *bundle.Bundle) error {
 
 	pkgPath, err := filepath.Abs(b.Config.Path)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 	b.Config.Artifacts[module] = &config.Artifact{
 		Path: pkgPath,

bundle/artifacts/whl/build.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/cmdio"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
 	"github.com/databricks/cli/libs/python"
 )
@@ -27,10 +28,10 @@ func (m *build) Name() string {
 	return fmt.Sprintf("artifacts.whl.Build(%s)", m.name)
 }
 
-func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact, ok := b.Config.Artifacts[m.name]
 	if !ok {
-		return fmt.Errorf("artifact doesn't exist: %s", m.name)
+		return diag.Errorf("artifact doesn't exist: %s", m.name)
 	}
 
 	cmdio.LogString(ctx, fmt.Sprintf("Building %s...", m.name))
@@ -43,13 +44,13 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) error {
 
 	out, err := artifact.Build(ctx)
 	if err != nil {
-		return fmt.Errorf("build failed %s, error: %w, output: %s", m.name, err, out)
+		return diag.Errorf("build failed %s, error: %v, output: %s", m.name, err, out)
 	}
 	log.Infof(ctx, "Build succeeded")
 
 	wheels := python.FindFilesWithSuffixInPath(distPath, ".whl")
 	if len(wheels) == 0 {
-		return fmt.Errorf("cannot find built wheel in %s for package %s", dir, m.name)
+		return diag.Errorf("cannot find built wheel in %s for package %s", dir, m.name)
 	}
 	for _, wheel := range wheels {
 		artifact.Files = append(artifact.Files, config.ArtifactFile{

bundle/artifacts/whl/from_libraries.go

@@ -7,6 +7,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/libraries"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/log"
 )
 
@@ -20,7 +21,7 @@ func (m *fromLibraries) Name() string {
 	return "artifacts.whl.DefineArtifactsFromLibraries"
 }
 
-func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (*fromLibraries) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	if len(b.Config.Artifacts) != 0 {
 		log.Debugf(ctx, "Skipping defining artifacts from libraries because artifacts section is explicitly defined")
 		return nil

bundle/artifacts/whl/infer.go

@@ -5,18 +5,19 @@ import (
 	"fmt"
 
 	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/python"
 )
 
 type infer struct {
 	name string
 }
 
-func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) error {
+func (m *infer) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	artifact := b.Config.Artifacts[m.name]
 	py, err := python.DetectExecutable(ctx)
 	if err != nil {
-		return err
+		return diag.FromErr(err)
 	}
 
 	// Note: using --build-number (build tag) flag does not help with re-installing

bundle/config/mutator/default_target.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/libs/diag"
 )
 
 type defineDefaultTarget struct {
@@ -24,7 +25,7 @@ func (m *defineDefaultTarget) Name() string {
 	return fmt.Sprintf("DefineDefaultTarget(%s)", m.name)
 }
 
-func (m *defineDefaultTarget) Apply(_ context.Context, b *bundle.Bundle) error {
+func (m *defineDefaultTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
 	// Nothing to do if the configuration has at least 1 target.
 	if len(b.Config.Targets) > 0 {
 		return nil

bundle/config/mutator/default_target_test.go

@@ -13,8 +13,9 @@ import (
 
 func TestDefaultTarget(t *testing.T) {
 	b := &bundle.Bundle{}
-	err := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget())
-	require.NoError(t, err)
+	diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget())
+	require.NoError(t, diags.Error())
+
 	env, ok := b.Config.Targets["default"]
 	assert.True(t, ok)
 	assert.Equal(t, &config.Target{}, env)
@@ -28,8 +29,9 @@ func TestDefaultTargetAlreadySpecified(t *testing.T) {
 			},
 		},
 	}
-	err := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget())
-	require.NoError(t, err)
+	diags := bundle.Apply(context.Background(), b, mutator.DefineDefaultTarget())
+	require.NoError(t, diags.Error())
+
 	_, ok := b.Config.Targets["default"]
 	assert.False(t, ok)
 }