Inline locations with paths in diagnostics

bundle/artifacts/expand_globs.go

@@ -30,10 +30,14 @@ func createGlobError(v dyn.Value, p dyn.Path, message string) diag.Diagnostic {
 	}
 
 	return diag.Diagnostic{
-		Severity:  diag.Error,
-		Summary:   fmt.Sprintf("%s: %s", source, message),
-		Locations: []dyn.Location{v.Location()},
-		Paths:     []dyn.Path{p},
+		Severity: diag.Error,
+		Summary:  fmt.Sprintf("%s: %s", source, message),
+		LocationPathPairs: []diag.LocationPathPair{
+			{
+				L: v.Location(),
+				P: p,
+			},
+		},
 	}
 }
 

bundle/config/loader/process_include.go

@@ -107,27 +107,24 @@ func validateSingleResourceDefined(configRoot dyn.Value, ext, typ string) diag.D
 		detail.WriteString(l)
 	}
 
-	locations := []dyn.Location{}
-	paths := []dyn.Path{}
+	// TODO: test this.
+	locationPathPairs := []diag.LocationPathPair{}
 	for _, rr := range resources {
-		locations = append(locations, rr.value.Locations()...)
-		paths = append(paths, rr.path)
+		for _, l := range rr.value.Locations() {
+			locationPathPairs = append(locationPathPairs, diag.LocationPathPair{L: l, P: rr.path})
+		}
 	}
-	// Sort the locations and paths to make the output deterministic.
-	sort.Slice(locations, func(i, j int) bool {
-		return locations[i].String() < locations[j].String()
-	})
-	sort.Slice(paths, func(i, j int) bool {
-		return paths[i].String() < paths[j].String()
+	// Sort the location-path pairs to make the output deterministic.
+	sort.Slice(locationPathPairs, func(i, j int) bool {
+		return locationPathPairs[i].L.String() < locationPathPairs[j].L.String()
 	})
 
 	return diag.Diagnostics{
 		{
-			Severity:  diag.Recommendation,
-			Summary:   fmt.Sprintf("define a single %s in a file with the %s extension.", strings.ReplaceAll(typ, "_", " "), ext),
-			Detail:    detail.String(),
-			Locations: locations,
-			Paths:     paths,
+			Severity:          diag.Recommendation,
+			Summary:           fmt.Sprintf("define a single %s in a file with the %s extension.", strings.ReplaceAll(typ, "_", " "), ext),
+			Detail:            detail.String(),
+			LocationPathPairs: locationPathPairs,
 		},
 	}
 }

bundle/config/mutator/apply_presets.go

@@ -220,10 +220,16 @@ func validatePauseStatus(b *bundle.Bundle) diag.Diagnostics {
 	if p == "" || p == config.Paused || p == config.Unpaused {
 		return nil
 	}
+	// TODO: test this.
 	return diag.Diagnostics{{
-		Summary:   "Invalid value for trigger_pause_status, should be PAUSED or UNPAUSED",
-		Severity:  diag.Error,
-		Locations: []dyn.Location{b.Config.GetLocation("presets.trigger_pause_status")},
+		Summary:  "Invalid value for trigger_pause_status, should be PAUSED or UNPAUSED",
+		Severity: diag.Error,
+		LocationPathPairs: []diag.LocationPathPair{
+			{
+				L: b.Config.GetLocation("presets.trigger_pause_status"),
+				P: dyn.MustPathFromString("presets.trigger_pause_status"),
+			},
+		},
 	}}
 }
 

bundle/config/mutator/compute_id_compat.go

@@ -52,11 +52,19 @@ func rewriteComputeIdToClusterId(v dyn.Value, p dyn.Path) (dyn.Value, diag.Diagn
 		return v, nil
 	}
 
+	// TODO: test this.
+	locationPathPaths := []diag.LocationPathPair{}
+	for _, l := range computeId.Locations() {
+		locationPathPaths = append(locationPathPaths, diag.LocationPathPair{
+			L: l,
+			P: computeIdPath,
+		})
+	}
+
 	diags = diags.Append(diag.Diagnostic{
-		Severity:  diag.Warning,
-		Summary:   "compute_id is deprecated, please use cluster_id instead",
-		Locations: computeId.Locations(),
-		Paths:     []dyn.Path{computeIdPath},
+		Severity:          diag.Warning,
+		Summary:           "compute_id is deprecated, please use cluster_id instead",
+		LocationPathPairs: locationPathPaths,
 	})
 
 	clusterIdPath := p.Append(dyn.Key("cluster_id"))

bundle/config/mutator/process_target_mode.go

@@ -76,9 +76,15 @@ func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
 	// historically allowed.)
 	if p.TriggerPauseStatus == config.Unpaused {
 		diags = diags.Append(diag.Diagnostic{
-			Severity:  diag.Error,
-			Summary:   "target with 'mode: development' cannot set trigger pause status to UNPAUSED by default",
-			Locations: []dyn.Location{b.Config.GetLocation("presets.trigger_pause_status")},
+			Severity: diag.Error,
+			Summary:  "target with 'mode: development' cannot set trigger pause status to UNPAUSED by default",
+			// TODO: test this.
+			LocationPathPairs: []diag.LocationPathPair{
+				{
+					L: b.Config.GetLocation("presets.trigger_pause_status"),
+					P: dyn.MustPathFromString("presets.trigger_pause_status"),
+				},
+			},
 		})
 	}
 
@@ -98,9 +104,15 @@ func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
 		// it's a pitfall for users if they don't include it and later find out that
 		// only a single user can do development deployments.
 		diags = diags.Append(diag.Diagnostic{
-			Severity:  diag.Error,
-			Summary:   "prefix should contain the current username or ${workspace.current_user.short_name} to ensure uniqueness when using 'mode: development'",
-			Locations: []dyn.Location{b.Config.GetLocation("presets.name_prefix")},
+			Severity: diag.Error,
+			Summary:  "prefix should contain the current username or ${workspace.current_user.short_name} to ensure uniqueness when using 'mode: development'",
+			// TODO: test this.
+			LocationPathPairs: []diag.LocationPathPair{
+				{
+					L: b.Config.GetLocation("presets.name_prefix"),
+					P: dyn.MustPathFromString("presets.name_prefix"),
+				},
+			},
 		})
 	}
 	return diags

bundle/config/mutator/python/python_diagnostics.go

@@ -54,23 +54,23 @@ func parsePythonDiagnostics(input io.Reader) (diag.Diagnostics, error) {
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse path: %s", err)
 		}
-		var paths []dyn.Path
-		if path != nil {
-			paths = []dyn.Path{path}
-		}
 
-		var locations []dyn.Location
 		location := convertPythonLocation(parsedLine.Location)
-		if location != (dyn.Location{}) {
-			locations = append(locations, location)
+
+		// TODO: test this.
+		locationPathPairs := []diag.LocationPathPair{}
+		if path != nil || location != (dyn.Location{}) {
+			locationPathPairs = append(locationPathPairs, diag.LocationPathPair{
+				L: location,
+				P: path,
+			})
 		}
 
 		diag := diag.Diagnostic{
-			Severity:  severity,
-			Summary:   parsedLine.Summary,
-			Detail:    parsedLine.Detail,
-			Locations: locations,
-			Paths:     paths,
+			Severity:          severity,
+			Summary:           parsedLine.Summary,
+			Detail:            parsedLine.Detail,
+			LocationPathPairs: locationPathPairs,
 		}
 
 		diags = diags.Append(diag)

bundle/config/mutator/rewrite_workspace_prefix.go

@@ -47,12 +47,21 @@ func (m *rewriteWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di
 			for path, replacePath := range paths {
 				if strings.Contains(vv, path) {
 					newPath := strings.Replace(vv, path, replacePath, 1)
+
+					locationPathPairs := []diag.LocationPathPair{}
+					for _, l := range v.Locations() {
+						locationPathPairs = append(locationPathPairs, diag.LocationPathPair{
+							L: l,
+							P: p,
+						})
+					}
+
+					// TODO: test this.
 					diags = append(diags, diag.Diagnostic{
-						Severity:  diag.Warning,
-						Summary:   fmt.Sprintf("substring %q found in %q. Please update this to %q.", path, vv, newPath),
-						Detail:    "For more information, please refer to: https://docs.databricks.com/en/release-notes/dev-tools/bundles.html#workspace-paths",
-						Locations: v.Locations(),
-						Paths:     []dyn.Path{p},
+						Severity:          diag.Warning,
+						Summary:           fmt.Sprintf("substring %q found in %q. Please update this to %q.", path, vv, newPath),
+						Detail:            "For more information, please refer to: https://docs.databricks.com/en/release-notes/dev-tools/bundles.html#workspace-paths",
+						LocationPathPairs: locationPathPairs,
 					})
 
 					// Remove the workspace prefix from the string.

bundle/config/mutator/run_as.go

@@ -30,11 +30,19 @@
 	return "SetRunAs"
 }
 
-func reportRunAsNotSupported(resourceType string, location dyn.Location, currentUser string, runAsUser string) diag.Diagnostics {
+
+// TODO: CONTINUE with transformation to pair and test this.
+func reportRunAsNotSupported(resourceType string, location dyn.Location, path dyn.Path, currentUser string, runAsUser string) diag.Diagnostics {
 	return diag.Diagnostics{{
 		Summary: fmt.Sprintf("%s do not support a setting a run_as user that is different from the owner.\n"+
 			"Current identity: %s. Run as identity: %s.\n"+
 			"See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", resourceType, currentUser, runAsUser),
+		LocationPathPairs: []diag.LocationPathPair{
+			{
+				L: location,
+
+			},
+		},
 		Locations: []dyn.Location{location},
 		Severity:  diag.Error,
 	}}
@@ -86,7 +94,7 @@
 			"pipelines",
 			b.Config.GetLocation("resources.pipelines"),
 			b.Config.Workspace.CurrentUser.UserName,
 			identity,
 		))
 	}
 
@@ -96,7 +104,7 @@
 			"model_serving_endpoints",
 			b.Config.GetLocation("resources.model_serving_endpoints"),
 			b.Config.Workspace.CurrentUser.UserName,
 			identity,
 		))
 	}
 
@@ -106,7 +114,7 @@
 			"quality_monitors",
 			b.Config.GetLocation("resources.quality_monitors"),
 			b.Config.Workspace.CurrentUser.UserName,
 			identity,
 		))
 	}
 

bundle/render/render_text_output.go

@@ -112,18 +112,19 @@ func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnosti
 }
 
 func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error {
+	// TODO: test this.
 	for _, d := range diags {
-		for i := range d.Locations {
+		for i := range d.LocationPathPairs {
 			if b == nil {
 				break
 			}
 
 			// Make location relative to bundle root
-			if d.Locations[i].File != "" {
-				out, err := filepath.Rel(b.BundleRootPath, d.Locations[i].File)
+			if d.LocationPathPairs[i].L.File != "" {
+				out, err := filepath.Rel(b.BundleRootPath, d.LocationPathPairs[i].L.File)
 				// if we can't relativize the path, just use path as-is
 				if err == nil {
-					d.Locations[i].File = out
+					d.LocationPathPairs[i].L.File = out
 				}
 			}
 		}

libs/cmdio/render.go

@@ -394,10 +394,7 @@ func fancyJSON(v any) ([]byte, error) {
 
 const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
 {{- range $index, $element := .Paths }}
-  {{ if eq $index 0 }}at {{else}}   {{ end}}{{ $element.String | green }}
-{{- end }}
-{{- range $index, $element := .Locations }}
-  {{ if eq $index 0 }}in {{else}}   {{ end}}{{ $element.String | cyan }}
+  {{ if eq $index 0 }}at {{else}}   {{ end}}{{ $element.P.String | green }} in {{ $element.L.String | cyan }}
 {{- end }}
 {{- if .Detail }}
 
@@ -406,12 +403,11 @@ const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
 
 `
 
+// TODO: Only print "at" and "in" if both are not nil.
+// TODO: also add tests for this.
 const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }}
 {{- range $index, $element := .Paths }}
-  {{ if eq $index 0 }}at {{else}}   {{ end}}{{ $element.String | green }}
-{{- end }}
-{{- range $index, $element := .Locations }}
-  {{ if eq $index 0 }}in {{else}}   {{ end}}{{ $element.String | cyan }}
+  {{ if eq $index 0 }}at {{else}}   {{ end}}{{ $element.P.String | green }} in {{ $element.L.String | cyan }}
 {{- end }}
 {{- if .Detail }}
 
@@ -421,11 +417,8 @@ const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }}
 `
 
 const recommendationTemplate = `{{ "Recommendation" | blue }}: {{ .Summary }}
-{{- range $index, $element := .Paths }}
-  {{ if eq $index 0 }}at {{else}}   {{ end}}{{ $element.String | green }}
-{{- end }}
-{{- range $index, $element := .Locations }}
-  {{ if eq $index 0 }}in {{else}}   {{ end}}{{ $element.String | cyan }}
+{{- range $index, $element := .LocationPathPairs}}
+  {{ if eq $index 0 }}at {{else}}   {{ end}}{{ $element.P.String | green }} in {{ $element.L.String | cyan }}
 {{- end }}
 {{- if .Detail }}
 

libs/diag/diagnostic.go

@@ -7,6 +7,15 @@ import (
 	"github.com/databricks/cli/libs/dyn"
 )
 
+// Convenience struct for retaining a 1:1 mapping between locations and paths.
+type LocationPathPair struct {
+	// Locations are the source code locations associated with the diagnostic message.
+	L dyn.Location
+
+	// Paths are paths to the values in the configuration tree that the diagnostic is associated with.
+	P dyn.Path
+}
+
 type Diagnostic struct {
 	Severity Severity
 
@@ -18,11 +27,13 @@ type Diagnostic struct {
 	// This may be multiple lines and may be nil.
 	Detail string
 
-	// Locations are the source code locations associated with the diagnostic message.
+	// LocationPathPairs are the source code locations and paths associated with the diagnostic message.
+	// It may be empty if there are no associated locations and paths.
+	LocationPathPairs []LocationPathPair
+
 	// It may be empty if there are no associated locations.
 	Locations []dyn.Location
 
-	// Paths are paths to the values in the configuration tree that the diagnostic is associated with.
 	// It may be nil if there are no associated paths.
 	Paths []dyn.Path