Support multiple locations for diagnostics

bundle/config/mutator/python/python_diagnostics.go

@@ -55,12 +55,18 @@ func parsePythonDiagnostics(input io.Reader) (diag.Diagnostics, error) {
  return nil, fmt.Errorf("failed to parse path: %s", err)
  }
 
+ var locations []dyn.Location
+ location := convertPythonLocation(parsedLine.Location)
+ if location != (dyn.Location{}) {
+ locations = append(locations, location)
+ }
+
  diag := diag.Diagnostic{
- Severity: severity,
- Summary: parsedLine.Summary,
- Detail: parsedLine.Detail,
-Location: convertPythonLocation(parsedLine.Location),
- Path: path,
+ Severity: severity,
+ Summary: parsedLine.Summary,
+ Detail: parsedLine.Detail,
+Locations: locations,
+ Path: path,
  }
 
  diags = diags.Append(diag)

bundle/config/mutator/python/python_diagnostics_test.go

@@ -39,10 +39,12 @@ func TestParsePythonDiagnostics(t *testing.T) {
  {
  Severity: diag.Error,
  Summary: "error summary",
- Location: dyn.Location{
- File: "src/examples/file.py",
- Line: 1,
- Column: 2,
+ Locations: []dyn.Location{
+ {
+ File: "src/examples/file.py",
+ Line: 1,
+ Column: 2,
+ },
  },
  },
  },

bundle/config/mutator/python/python_mutator_test.go

@@ -97,11 +97,14 @@ func TestPythonMutator_load(t *testing.T) {
 
 	assert.Equal(t, 1, len(diags))
 	assert.Equal(t, "job doesn't have any tasks", diags[0].Summary)
-	assert.Equal(t, dyn.Location{
- File: "src/examples/file.py",
- Line: 10,
- Column: 5,
-	}, diags[0].Location)
+	assert.Equal(t, []dyn.Location{
+ {
+ File: "src/examples/file.py",
+ Line: 10,
+ Column: 5,
+ },
+	}, diags[0].Locations)
+
 }
 
 func TestPythonMutator_load_disallowed(t *testing.T) {

bundle/config/mutator/run_as.go

@@ -178,10 +178,10 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
  setRunAsForJobs(b)
  return diag.Diagnostics{
  {
- Severity: diag.Warning,
- Summary: "You are using the legacy mode of run_as. The support for this mode is experimental and might be removed in a future release of the CLI. In order to run the DLT pipelines in your DAB as the run_as user this mode changes the owners of the pipelines to the run_as identity, which requires the user deploying the bundle to be a workspace admin, and also a Metastore admin if the pipeline target is in UC.",
- Path: dyn.MustPathFromString("experimental.use_legacy_run_as"),
-Location: b.Config.GetLocation("experimental.use_legacy_run_as"),
+ Severity: diag.Warning,
+ Summary: "You are using the legacy mode of run_as. The support for this mode is experimental and might be removed in a future release of the CLI. In order to run the DLT pipelines in your DAB as the run_as user this mode changes the owners of the pipelines to the run_as identity, which requires the user deploying the bundle to be a workspace admin, and also a Metastore admin if the pipeline target is in UC.",
+ Path: dyn.MustPathFromString("experimental.use_legacy_run_as"),
+Locations: b.Config.GetLocations("experimental.use_legacy_run_as"),
  },
  }
 	}

bundle/config/root.go

@@ -524,6 +524,17 @@ func (r Root) GetLocation(path string) dyn.Location {
 	return v.Location()
 }
 
+// Get all locations of the configuration value at the specified path. We need both
+// this function and it's singular version (GetLocation) because some diagnostics just need
+// the primary location and some need all locations associated with a configuration value.
+func (r Root) GetLocations(path string) []dyn.Location {
+	v, err := dyn.Get(r.value, path)
+	if err != nil {
+ return []dyn.Location{}
+	}
+	return v.Locations()
+}
+
 // Value returns the dynamic configuration value of the root object. This value
 // is the source of truth and is kept in sync with values in the typed configuration.
 func (r Root) Value() dyn.Value {

bundle/config/validate/files_to_sync.go

@@ -45,8 +45,10 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.
  diags = diags.Append(diag.Diagnostic{
  Severity: diag.Warning,
  Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration",
- Location: loc.Location(),
- Path: loc.Path(),
+ // Show all locations where sync.exclude is defined, since merging
+ // sync.exclude is additive.
+ Locations: loc.Locations(),
+ Path: loc.Path(),
  })
 	}
 

bundle/config/validate/job_cluster_key_defined.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/dyn"
 )
 
 func JobClusterKeyDefined() bundle.ReadOnlyMutator {
@@ -41,8 +42,11 @@ func (v *jobClusterKeyDefined) Apply(ctx context.Context, rb bundle.ReadOnlyBund
  diags = diags.Append(diag.Diagnostic{
  Severity: diag.Warning,
  Summary: fmt.Sprintf("job_cluster_key %s is not defined", task.JobClusterKey),
- Location: loc.Location(),
- Path: loc.Path(),
+ // Show only the location where the job_cluster_key is defined.
+ // Other associated locations are not relevant since they are
+ // overridden during merging.
+ Locations: []dyn.Location{loc.Location()},
+ Path: loc.Path(),
  })
  }
  }

bundle/config/validate/validate.go

@@ -20,6 +20,10 @@ func (l location) Location() dyn.Location {
 	return l.rb.Config().GetLocation(l.path)
 }
 
+func (l location) Locations() []dyn.Location {
+	return l.rb.Config().GetLocations(l.path)
+}
+
 func (l location) Path() dyn.Path {
 	return dyn.MustPathFromString(l.path)
 }

bundle/config/validate/validate_sync_patterns.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/fileset"
 	"golang.org/x/sync/errgroup"
 )
@@ -64,10 +65,10 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di
  loc := location{path: fmt.Sprintf("%s[%d]", path, index), rb: rb}
  mu.Lock()
  diags = diags.Append(diag.Diagnostic{
- Severity: diag.Warning,
- Summary: fmt.Sprintf("Pattern %s does not match any files", p),
-Location: loc.Location(),
- Path: loc.Path(),
+ Severity: diag.Warning,
+ Summary: fmt.Sprintf("Pattern %s does not match any files", p),
+Locations: []dyn.Location{loc.Location()},
+ Path: loc.Path(),
  })
  mu.Unlock()
  }

bundle/render/render_text_output.go

@@ -32,8 +32,8 @@ const errorTemplate = `{{ "Error" | red }}: {{ .Summary }}
 {{- if .Path.String }}
  {{ "at " }}{{ .Path.String | green }}
 {{- end }}
-{{- if .Location.File }}
- {{ "in " }}{{ .Location.String | cyan }}
+{{- range $index, $element := .Locations }}
+ {{ if eq $index 0 }}in {{else}} {{ end}}{{ $element.String | cyan }}
 {{- end }}
 {{- if .Detail }}
 
@@ -46,8 +46,8 @@ const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }}
 {{- if .Path.String }}
  {{ "at " }}{{ .Path.String | green }}
 {{- end }}
-{{- if .Location.File }}
- {{ "in " }}{{ .Location.String | cyan }}
+{{- range $index, $element := .Locations }}
+ {{ if eq $index 0 }}in {{else}} {{ end}}{{ $element.String | cyan }}
 {{- end }}
 {{- if .Detail }}
 
@@ -141,12 +141,18 @@ func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics)
  t = warningT
  }
 
- // Make file relative to bundle root
- if d.Location.File != "" && b != nil {
- out, err := filepath.Rel(b.RootPath, d.Location.File)
- // if we can't relativize the path, just use path as-is
- if err == nil {
- d.Location.File = out
+ for i := range d.Locations {
+ if b == nil {
+ break
+ }
+
+ // Make location relative to bundle root
+ if d.Locations[i].File != "" {
+ out, err := filepath.Rel(b.RootPath, d.Locations[i].File)
+ // if we can't relativize the path, just use path as-is
+ if err == nil {
+ d.Locations[i].File = out
+ }
  }
  }
 

bundle/render/render_text_output_test.go

@@ -88,34 +88,22 @@ func TestRenderTextOutput(t *testing.T) {
  bundle: loadingBundle,
  diags: diag.Diagnostics{
  diag.Diagnostic{
- Severity: diag.Error,
- Summary: "error (1)",
- Detail: "detail (1)",
- Location: dyn.Location{
- File: "foo.py",
- Line: 1,
- Column: 1,
- },
+ Severity: diag.Error,
+ Summary: "error (1)",
+ Detail: "detail (1)",
+ Locations: []dyn.Location{{File: "foo.py", Line: 1, Column: 1}},
  },
  diag.Diagnostic{
- Severity: diag.Error,
- Summary: "error (2)",
- Detail: "detail (2)",
- Location: dyn.Location{
- File: "foo.py",
- Line: 2,
- Column: 1,
- },
+ Severity: diag.Error,
+ Summary: "error (2)",
+ Detail: "detail (2)",
+ Locations: []dyn.Location{{File: "foo.py", Line: 2, Column: 1}},
  },
  diag.Diagnostic{
- Severity: diag.Warning,
- Summary: "warning (3)",
- Detail: "detail (3)",
- Location: dyn.Location{
- File: "foo.py",
- Line: 3,
- Column: 1,
- },
+ Severity: diag.Warning,
+ Summary: "warning (3)",
+ Detail: "detail (3)",
+ Locations: []dyn.Location{{File: "foo.py", Line: 3, Column: 1}},
  },
  },
  opts: RenderOptions{RenderSummaryTable: true},
@@ -174,24 +162,16 @@ func TestRenderTextOutput(t *testing.T) {
  bundle: nil,
  diags: diag.Diagnostics{
  diag.Diagnostic{
- Severity: diag.Error,
- Summary: "error (1)",
- Detail: "detail (1)",
- Location: dyn.Location{
- File: "foo.py",
- Line: 1,
- Column: 1,
- },
+ Severity: diag.Error,
+ Summary: "error (1)",
+ Detail: "detail (1)",
+ Locations: []dyn.Location{{File: "foo.py", Line: 1, Column: 1}},
  },
  diag.Diagnostic{
- Severity: diag.Warning,
- Summary: "warning (2)",
- Detail: "detail (2)",
- Location: dyn.Location{
- File: "foo.py",
- Line: 3,
- Column: 1,
- },
+ Severity: diag.Warning,
+ Summary: "warning (2)",
+ Detail: "detail (2)",
+ Locations: []dyn.Location{{File: "foo.py", Line: 3, Column: 1}},
  },
  },
  opts: RenderOptions{RenderSummaryTable: false},
@@ -252,17 +232,42 @@ func TestRenderDiagnostics(t *testing.T) {
  Severity: diag.Error,
  Summary: "failed to load xxx",
  Detail: "'name' is required",
-Location: dyn.Location{
+Locations: []dyn.Location{{
  File: "foo.yaml",
  Line: 1,
- Column: 2,
- },
+ Column: 2}},
  },
  },
  expected: "Error: failed to load xxx\n" +
  " in foo.yaml:1:2\n\n" +
  "'name' is required\n\n",
  },
+ {
+ name: "error with multiple source locations",
+ diags: diag.Diagnostics{
+ {
+ Severity: diag.Error,
+ Summary: "failed to load xxx",
+ Detail: "'name' is required",
+ Locations: []dyn.Location{
+ {
+ File: "foo.yaml",
+ Line: 1,
+ Column: 2,
+ },
+ {
+ File: "bar.yaml",
+ Line: 3,
+ Column: 4,
+ },
+ },
+ },
+ },
+ expected: "Error: failed to load xxx\n" +
+ " in foo.yaml:1:2\n" +
+ " bar.yaml:3:4\n\n" +
+ "'name' is required\n\n",
+ },
  {
  name: "error with path",
  diags: diag.Diagnostics{

bundle/tests/sync_include_exclude_no_matches_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/validate"
 	"github.com/databricks/cli/libs/diag"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -21,11 +22,13 @@ func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) {
 
 	require.Equal(t, diags[0].Severity, diag.Warning)
 	require.Equal(t, diags[0].Summary, "Pattern dist does not match any files")
-	require.Equal(t, diags[0].Location.File, filepath.Join("sync", "override", "databricks.yml"))
-	require.Equal(t, diags[0].Location.Line, 17)
-	require.Equal(t, diags[0].Location.Column, 11)
 	require.Equal(t, diags[0].Path.String(), "sync.exclude[0]")
 
+	assert.Len(t, diags[0].Locations, 1)
+	require.Equal(t, diags[0].Locations[0].File, filepath.Join("sync", "override", "databricks.yml"))
+	require.Equal(t, diags[0].Locations[0].Line, 17)
+	require.Equal(t, diags[0].Locations[0].Column, 11)
+
 	summaries := []string{
  fmt.Sprintf("Pattern %s does not match any files", filepath.Join("src", "*")),
  fmt.Sprintf("Pattern %s does not match any files", filepath.Join("tests", "*")),

libs/diag/diagnostic.go

@@ -17,9 +17,9 @@ type Diagnostic struct {
 	// This may be multiple lines and may be nil.
 	Detail string
 
-	// Location is a source code location associated with the diagnostic message.
-	// It may be zero if there is no associated location.
-Location dyn.Location
+	// Locations are the source code locations associated with the diagnostic message.
+	// It may be empty if there are no associated locations.
+Locations []dyn.Location
 
 	// Path is a path to the value in a configuration tree that the diagnostic is associated with.
 	// It may be nil if there is no associated path.