From 4bedb54d8f19f7d00127ce37532fb31c32340305 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 18:01:45 +0200 Subject: [PATCH 01/18] rename Location -> Locations. Also add bundle.config.GetLocations function and modify all diagnostics paths to be relative to the bundle root path --- .../mutator/python/python_diagnostics.go | 10 +- .../mutator/python/python_diagnostics_test.go | 2 +- .../mutator/python/python_mutator_test.go | 2 +- bundle/config/mutator/run_as.go | 8 +- bundle/config/root.go | 8 + bundle/config/validate/files_to_sync.go | 8 +- .../validate/job_cluster_key_defined.go | 8 +- .../config/validate/validate_sync_patterns.go | 8 +- bundle/render/render_text_output.go | 19 +- bundle/render/render_text_output_test.go | 12 +- .../sync_include_exclude_no_matches_test.go | 6 +- libs/diag/diagnostic.go | 4 +- libs/dyn/convert/normalize.go | 56 ++--- libs/dyn/convert/normalize_test.go | 208 +++++++++--------- 14 files changed, 187 insertions(+), 172 deletions(-) diff --git a/bundle/config/mutator/python/python_diagnostics.go b/bundle/config/mutator/python/python_diagnostics.go index b8efc9ef73..ce50e38cdc 100644 --- a/bundle/config/mutator/python/python_diagnostics.go +++ b/bundle/config/mutator/python/python_diagnostics.go @@ -56,11 +56,11 @@ func parsePythonDiagnostics(input io.Reader) (diag.Diagnostics, error) { } 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: convertPythonLocation(parsedLine.Location), + Path: path, } diags = diags.Append(diag) diff --git a/bundle/config/mutator/python/python_diagnostics_test.go b/bundle/config/mutator/python/python_diagnostics_test.go index 7b66e2537b..3c895ad99c 100644 --- a/bundle/config/mutator/python/python_diagnostics_test.go +++ b/bundle/config/mutator/python/python_diagnostics_test.go @@ -39,7 +39,7 @@ func TestParsePythonDiagnostics(t *testing.T) { { Severity: diag.Error, Summary: "error summary", - Location: dyn.Location{ + Locations: dyn.Location{ File: "src/examples/file.py", Line: 1, Column: 2, diff --git a/bundle/config/mutator/python/python_mutator_test.go b/bundle/config/mutator/python/python_mutator_test.go index 588589831b..e13234447d 100644 --- a/bundle/config/mutator/python/python_mutator_test.go +++ b/bundle/config/mutator/python/python_mutator_test.go @@ -101,7 +101,7 @@ func TestPythonMutator_load(t *testing.T) { File: "src/examples/file.py", Line: 10, Column: 5, - }, diags[0].Location) + }, diags[0].Locations) } func TestPythonMutator_load_disallowed(t *testing.T) { diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index d344a988ae..e757964b05 100644 --- a/bundle/config/mutator/run_as.go +++ b/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.GetLocation("experimental.use_legacy_run_as"), }, } } diff --git a/bundle/config/root.go b/bundle/config/root.go index 594a9105f6..0c1f710b4e 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -524,6 +524,14 @@ func (r Root) GetLocation(path string) dyn.Location { return v.Location() } +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 { diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go index d53e382432..4ad58d2613 100644 --- a/bundle/config/validate/files_to_sync.go +++ b/bundle/config/validate/files_to_sync.go @@ -43,10 +43,10 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag. } else { loc := location{path: "sync.exclude", rb: rb} 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(), + Severity: diag.Warning, + Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration", + Locations: loc.Location(), + Path: loc.Path(), }) } diff --git a/bundle/config/validate/job_cluster_key_defined.go b/bundle/config/validate/job_cluster_key_defined.go index 37ed3f417e..d4f2ba782c 100644 --- a/bundle/config/validate/job_cluster_key_defined.go +++ b/bundle/config/validate/job_cluster_key_defined.go @@ -39,10 +39,10 @@ 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(), + Severity: diag.Warning, + Summary: fmt.Sprintf("job_cluster_key %s is not defined", task.JobClusterKey), + Locations: loc.Location(), + Path: loc.Path(), }) } } diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go index a04c10776c..758ccec0e2 100644 --- a/bundle/config/validate/validate_sync_patterns.go +++ b/bundle/config/validate/validate_sync_patterns.go @@ -64,10 +64,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: loc.Location(), + Path: loc.Path(), }) mu.Unlock() } diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index 439ae61323..68876dc783 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -127,6 +127,7 @@ func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnosti return err } +// TODO: Write tests when multiple locations are rendered. func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { errorT := template.Must(template.New("error").Funcs(renderFuncMap).Parse(errorTemplate)) warningT := template.Must(template.New("warning").Funcs(renderFuncMap).Parse(warningTemplate)) @@ -141,12 +142,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 + } } } diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index b7aec88648..470d007390 100644 --- a/bundle/render/render_text_output_test.go +++ b/bundle/render/render_text_output_test.go @@ -91,7 +91,7 @@ func TestRenderTextOutput(t *testing.T) { Severity: diag.Error, Summary: "error (1)", Detail: "detail (1)", - Location: dyn.Location{ + Locations: dyn.Location{ File: "foo.py", Line: 1, Column: 1, @@ -101,7 +101,7 @@ func TestRenderTextOutput(t *testing.T) { Severity: diag.Error, Summary: "error (2)", Detail: "detail (2)", - Location: dyn.Location{ + Locations: dyn.Location{ File: "foo.py", Line: 2, Column: 1, @@ -111,7 +111,7 @@ func TestRenderTextOutput(t *testing.T) { Severity: diag.Warning, Summary: "warning (3)", Detail: "detail (3)", - Location: dyn.Location{ + Locations: dyn.Location{ File: "foo.py", Line: 3, Column: 1, @@ -177,7 +177,7 @@ func TestRenderTextOutput(t *testing.T) { Severity: diag.Error, Summary: "error (1)", Detail: "detail (1)", - Location: dyn.Location{ + Locations: dyn.Location{ File: "foo.py", Line: 1, Column: 1, @@ -187,7 +187,7 @@ func TestRenderTextOutput(t *testing.T) { Severity: diag.Warning, Summary: "warning (2)", Detail: "detail (2)", - Location: dyn.Location{ + Locations: dyn.Location{ File: "foo.py", Line: 3, Column: 1, @@ -252,7 +252,7 @@ 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, diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go index 94cedbaa62..d01de84c06 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/bundle/tests/sync_include_exclude_no_matches_test.go @@ -21,9 +21,9 @@ 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].Locations.File, filepath.Join("sync", "override", "databricks.yml")) + require.Equal(t, diags[0].Locations.Line, 17) + require.Equal(t, diags[0].Locations.Column, 11) require.Equal(t, diags[0].Path.String(), "sync.exclude[0]") summaries := []string{ diff --git a/libs/diag/diagnostic.go b/libs/diag/diagnostic.go index 6215275512..062ca7e552 100644 --- a/libs/diag/diagnostic.go +++ b/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. + // Locations is a source code location associated with the diagnostic message. // It may be zero if there is no associated location. - Location dyn.Location + 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. diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index 246c97eaf9..fea0e57d6a 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -65,19 +65,19 @@ func (n normalizeOptions) normalizeType(typ reflect.Type, src dyn.Value, seen [] func nullWarning(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnostic { return diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("expected a %s value, found null", expected), - Location: src.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf("expected a %s value, found null", expected), + Locations: src.Location(), + Path: path, } } func typeMismatch(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnostic { return diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("expected %s, found %s", expected, src.Kind()), - Location: src.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf("expected %s, found %s", expected, src.Kind()), + Locations: src.Location(), + Path: path, } } @@ -96,10 +96,10 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen if !ok { if !pv.IsAnchor() { diags = diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("unknown field: %s", pk.MustString()), - Location: pk.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf("unknown field: %s", pk.MustString()), + Locations: pk.Location(), + Path: path, }) } continue @@ -320,10 +320,10 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn out = int64(src.MustFloat()) if src.MustFloat() != float64(out) { return dyn.InvalidValue, diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf(`cannot accurately represent "%g" as integer due to precision loss`, src.MustFloat()), - Location: src.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf(`cannot accurately represent "%g" as integer due to precision loss`, src.MustFloat()), + Locations: src.Location(), + Path: path, }) } case dyn.KindString: @@ -336,10 +336,10 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn } return dyn.InvalidValue, diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("cannot parse %q as an integer", src.MustString()), - Location: src.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf("cannot parse %q as an integer", src.MustString()), + Locations: src.Location(), + Path: path, }) } case dyn.KindNil: @@ -363,10 +363,10 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d out = float64(src.MustInt()) if src.MustInt() != int64(out) { return dyn.InvalidValue, diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf(`cannot accurately represent "%d" as floating point number due to precision loss`, src.MustInt()), - Location: src.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf(`cannot accurately represent "%d" as floating point number due to precision loss`, src.MustInt()), + Locations: src.Location(), + Path: path, }) } case dyn.KindString: @@ -379,10 +379,10 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d } return dyn.InvalidValue, diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()), - Location: src.Location(), - Path: path, + Severity: diag.Warning, + Summary: fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()), + Locations: src.Location(), + Path: path, }) } case dyn.KindNil: diff --git a/libs/dyn/convert/normalize_test.go b/libs/dyn/convert/normalize_test.go index 452ed4eb1d..cf3a1935a1 100644 --- a/libs/dyn/convert/normalize_test.go +++ b/libs/dyn/convert/normalize_test.go @@ -40,10 +40,10 @@ func TestNormalizeStructElementDiagnostic(t *testing.T) { vout, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected string, found map`, - Location: dyn.Location{}, - Path: dyn.NewPath(dyn.Key("bar")), + Severity: diag.Warning, + Summary: `expected string, found map`, + Locations: dyn.Location{}, + Path: dyn.NewPath(dyn.Key("bar")), }, err[0]) // Elements that encounter an error during normalization are dropped. @@ -66,10 +66,10 @@ func TestNormalizeStructUnknownField(t *testing.T) { vout, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `unknown field: bar`, - Location: vin.Get("foo").Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `unknown field: bar`, + Locations: vin.Get("foo").Location(), + Path: dyn.EmptyPath, }, err[0]) // The field that can be mapped to the struct field is retained. @@ -100,10 +100,10 @@ func TestNormalizeStructError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected map, found string`, - Location: vin.Get("foo").Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected map, found string`, + Locations: vin.Get("foo").Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -245,10 +245,10 @@ func TestNormalizeStructRandomStringError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected map, found string`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected map, found string`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -262,10 +262,10 @@ func TestNormalizeStructIntError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected map, found int`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected map, found int`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -291,10 +291,10 @@ func TestNormalizeMapElementDiagnostic(t *testing.T) { vout, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected string, found map`, - Location: dyn.Location{}, - Path: dyn.NewPath(dyn.Key("bar")), + Severity: diag.Warning, + Summary: `expected string, found map`, + Locations: dyn.Location{}, + Path: dyn.NewPath(dyn.Key("bar")), }, err[0]) // Elements that encounter an error during normalization are dropped. @@ -317,10 +317,10 @@ func TestNormalizeMapError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected map, found string`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected map, found string`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -372,10 +372,10 @@ func TestNormalizeMapRandomStringError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected map, found string`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected map, found string`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -385,10 +385,10 @@ func TestNormalizeMapIntError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected map, found int`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected map, found int`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -415,10 +415,10 @@ func TestNormalizeSliceElementDiagnostic(t *testing.T) { vout, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected string, found map`, - Location: dyn.Location{}, - Path: dyn.NewPath(dyn.Index(2)), + Severity: diag.Warning, + Summary: `expected string, found map`, + Locations: dyn.Location{}, + Path: dyn.NewPath(dyn.Index(2)), }, err[0]) // Elements that encounter an error during normalization are dropped. @@ -439,10 +439,10 @@ func TestNormalizeSliceError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected sequence, found string`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected sequence, found string`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -494,10 +494,10 @@ func TestNormalizeSliceRandomStringError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected sequence, found string`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected sequence, found string`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -507,10 +507,10 @@ func TestNormalizeSliceIntError(t *testing.T) { _, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected sequence, found int`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected sequence, found int`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -528,10 +528,10 @@ func TestNormalizeStringNil(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected a string value, found null`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected a string value, found null`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -565,10 +565,10 @@ func TestNormalizeStringError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected string, found map`, - Location: dyn.Location{}, - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected string, found map`, + Locations: dyn.Location{}, + Path: dyn.EmptyPath, }, err[0]) } @@ -586,10 +586,10 @@ func TestNormalizeBoolNil(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected a bool value, found null`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected a bool value, found null`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -628,10 +628,10 @@ func TestNormalizeBoolFromStringError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected bool, found string`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected bool, found string`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -641,10 +641,10 @@ func TestNormalizeBoolError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected bool, found map`, - Location: dyn.Location{}, - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected bool, found map`, + Locations: dyn.Location{}, + Path: dyn.EmptyPath, }, err[0]) } @@ -662,10 +662,10 @@ func TestNormalizeIntNil(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected a int value, found null`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected a int value, found null`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -683,10 +683,10 @@ func TestNormalizeIntFromFloatError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `cannot accurately represent "1.5" as integer due to precision loss`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `cannot accurately represent "1.5" as integer due to precision loss`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -712,10 +712,10 @@ func TestNormalizeIntFromStringError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `cannot parse "abc" as an integer`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `cannot parse "abc" as an integer`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -725,10 +725,10 @@ func TestNormalizeIntError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected int, found map`, - Location: dyn.Location{}, - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected int, found map`, + Locations: dyn.Location{}, + Path: dyn.EmptyPath, }, err[0]) } @@ -746,10 +746,10 @@ func TestNormalizeFloatNil(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected a float value, found null`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected a float value, found null`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -771,10 +771,10 @@ func TestNormalizeFloatFromIntError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `cannot accurately represent "9007199254740993" as floating point number due to precision loss`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `cannot accurately represent "9007199254740993" as floating point number due to precision loss`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -800,10 +800,10 @@ func TestNormalizeFloatFromStringError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `cannot parse "abc" as a floating point number`, - Location: vin.Location(), - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `cannot parse "abc" as a floating point number`, + Locations: vin.Location(), + Path: dyn.EmptyPath, }, err[0]) } @@ -813,10 +813,10 @@ func TestNormalizeFloatError(t *testing.T) { _, err := Normalize(&typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `expected float, found map`, - Location: dyn.Location{}, - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `expected float, found map`, + Locations: dyn.Location{}, + Path: dyn.EmptyPath, }, err[0]) } From 7ab131fe7013aa68745a861033e2723e0184b7fc Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 18:37:08 +0200 Subject: [PATCH 02/18] validate.Location() -> validate.Locations() --- bundle/config/validate/validate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundle/config/validate/validate.go b/bundle/config/validate/validate.go index af7e984a11..ef6488862a 100644 --- a/bundle/config/validate/validate.go +++ b/bundle/config/validate/validate.go @@ -16,8 +16,8 @@ type location struct { rb bundle.ReadOnlyBundle } -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 { From 39b3eeab1ddeb5ddc99d7fa90622ae636ad4351d Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 18:45:36 +0200 Subject: [PATCH 03/18] fix TestSyncIncludeExcludeNoMatchesTest --- bundle/tests/sync_include_exclude_no_matches_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go index d01de84c06..5f4fa47ce4 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/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].Locations.File, filepath.Join("sync", "override", "databricks.yml")) - require.Equal(t, diags[0].Locations.Line, 17) - require.Equal(t, diags[0].Locations.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", "*")), From 14a200a39d9acc169121876ba3d209904481958f Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 18:47:59 +0200 Subject: [PATCH 04/18] patch to transform location -> locations --- .../mutator/python/python_diagnostics.go | 2 +- .../mutator/python/python_diagnostics_test.go | 5 +- .../mutator/python/python_mutator_test.go | 6 +- bundle/config/mutator/run_as.go | 2 +- bundle/config/validate/files_to_sync.go | 2 +- .../validate/job_cluster_key_defined.go | 2 +- .../config/validate/validate_sync_patterns.go | 2 +- bundle/render/render_text_output_test.go | 65 +++++++------------ libs/dyn/convert/normalize.go | 14 ++-- libs/dyn/convert/normalize_test.go | 52 +++++++-------- 10 files changed, 65 insertions(+), 87 deletions(-) diff --git a/bundle/config/mutator/python/python_diagnostics.go b/bundle/config/mutator/python/python_diagnostics.go index ce50e38cdc..e9cb0c5668 100644 --- a/bundle/config/mutator/python/python_diagnostics.go +++ b/bundle/config/mutator/python/python_diagnostics.go @@ -59,7 +59,7 @@ func parsePythonDiagnostics(input io.Reader) (diag.Diagnostics, error) { Severity: severity, Summary: parsedLine.Summary, Detail: parsedLine.Detail, - Locations: convertPythonLocation(parsedLine.Location), + Locations: []dyn.Location{convertPythonLocation(parsedLine.Location)}, Path: path, } diff --git a/bundle/config/mutator/python/python_diagnostics_test.go b/bundle/config/mutator/python/python_diagnostics_test.go index 3c895ad99c..eee29bee7d 100644 --- a/bundle/config/mutator/python/python_diagnostics_test.go +++ b/bundle/config/mutator/python/python_diagnostics_test.go @@ -39,11 +39,10 @@ func TestParsePythonDiagnostics(t *testing.T) { { Severity: diag.Error, Summary: "error summary", - Locations: dyn.Location{ + Locations: []dyn.Location{{ File: "src/examples/file.py", Line: 1, - Column: 2, - }, + Column: 2}}, }, }, }, diff --git a/bundle/config/mutator/python/python_mutator_test.go b/bundle/config/mutator/python/python_mutator_test.go index e13234447d..dc4ed9322e 100644 --- a/bundle/config/mutator/python/python_mutator_test.go +++ b/bundle/config/mutator/python/python_mutator_test.go @@ -97,11 +97,11 @@ 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{ + assert.Equal(t, []dyn.Location{{ File: "src/examples/file.py", Line: 10, - Column: 5, - }, diags[0].Locations) + Column: 5}}, diags[0].Locations) + } func TestPythonMutator_load_disallowed(t *testing.T) { diff --git a/bundle/config/mutator/run_as.go b/bundle/config/mutator/run_as.go index e757964b05..168918d0db 100644 --- a/bundle/config/mutator/run_as.go +++ b/bundle/config/mutator/run_as.go @@ -181,7 +181,7 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) 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"), - Locations: b.Config.GetLocation("experimental.use_legacy_run_as"), + Locations: b.Config.GetLocations("experimental.use_legacy_run_as"), }, } } diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go index 4ad58d2613..67b94090a3 100644 --- a/bundle/config/validate/files_to_sync.go +++ b/bundle/config/validate/files_to_sync.go @@ -45,7 +45,7 @@ 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", - Locations: loc.Location(), + Locations: loc.Locations(), Path: loc.Path(), }) } diff --git a/bundle/config/validate/job_cluster_key_defined.go b/bundle/config/validate/job_cluster_key_defined.go index d4f2ba782c..392259ade6 100644 --- a/bundle/config/validate/job_cluster_key_defined.go +++ b/bundle/config/validate/job_cluster_key_defined.go @@ -41,7 +41,7 @@ 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), - Locations: loc.Location(), + Locations: loc.Locations(), Path: loc.Path(), }) } diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go index 758ccec0e2..abcbce16a2 100644 --- a/bundle/config/validate/validate_sync_patterns.go +++ b/bundle/config/validate/validate_sync_patterns.go @@ -66,7 +66,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("Pattern %s does not match any files", p), - Locations: loc.Location(), + Locations: loc.Locations(), Path: loc.Path(), }) mu.Unlock() diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index 470d007390..8adf54161e 100644 --- a/bundle/render/render_text_output_test.go +++ b/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)", - Locations: 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)", - Locations: 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)", - Locations: 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)", - Locations: 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)", - Locations: 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,11 +232,10 @@ func TestRenderDiagnostics(t *testing.T) { Severity: diag.Error, Summary: "failed to load xxx", Detail: "'name' is required", - Locations: dyn.Location{ + Locations: []dyn.Location{{ File: "foo.yaml", Line: 1, - Column: 2, - }, + Column: 2}}, }, }, expected: "Error: failed to load xxx\n" + diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index fea0e57d6a..df3c858b06 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -67,7 +67,7 @@ func nullWarning(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnosti return diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("expected a %s value, found null", expected), - Locations: src.Location(), + Locations: []dyn.Location{src.Location()}, Path: path, } } @@ -76,7 +76,7 @@ func typeMismatch(expected dyn.Kind, src dyn.Value, path dyn.Path) diag.Diagnost return diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("expected %s, found %s", expected, src.Kind()), - Locations: src.Location(), + Locations: []dyn.Location{src.Location()}, Path: path, } } @@ -98,7 +98,7 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("unknown field: %s", pk.MustString()), - Locations: pk.Location(), + Locations: []dyn.Location{pk.Location()}, Path: path, }) } @@ -322,7 +322,7 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn return dyn.InvalidValue, diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf(`cannot accurately represent "%g" as integer due to precision loss`, src.MustFloat()), - Locations: src.Location(), + Locations: []dyn.Location{src.Location()}, Path: path, }) } @@ -338,7 +338,7 @@ func (n normalizeOptions) normalizeInt(typ reflect.Type, src dyn.Value, path dyn return dyn.InvalidValue, diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("cannot parse %q as an integer", src.MustString()), - Locations: src.Location(), + Locations: []dyn.Location{src.Location()}, Path: path, }) } @@ -365,7 +365,7 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d return dyn.InvalidValue, diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf(`cannot accurately represent "%d" as floating point number due to precision loss`, src.MustInt()), - Locations: src.Location(), + Locations: []dyn.Location{src.Location()}, Path: path, }) } @@ -381,7 +381,7 @@ func (n normalizeOptions) normalizeFloat(typ reflect.Type, src dyn.Value, path d return dyn.InvalidValue, diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("cannot parse %q as a floating point number", src.MustString()), - Locations: src.Location(), + Locations: []dyn.Location{src.Location()}, Path: path, }) } diff --git a/libs/dyn/convert/normalize_test.go b/libs/dyn/convert/normalize_test.go index cf3a1935a1..536bfa6d0a 100644 --- a/libs/dyn/convert/normalize_test.go +++ b/libs/dyn/convert/normalize_test.go @@ -42,7 +42,7 @@ func TestNormalizeStructElementDiagnostic(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected string, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.NewPath(dyn.Key("bar")), }, err[0]) @@ -68,7 +68,7 @@ func TestNormalizeStructUnknownField(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `unknown field: bar`, - Locations: vin.Get("foo").Location(), + Locations: []dyn.Location{vin.Get("foo").Location()}, Path: dyn.EmptyPath, }, err[0]) @@ -102,7 +102,7 @@ func TestNormalizeStructError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected map, found string`, - Locations: vin.Get("foo").Location(), + Locations: []dyn.Location{vin.Get("foo").Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -247,7 +247,7 @@ func TestNormalizeStructRandomStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected map, found string`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -264,7 +264,7 @@ func TestNormalizeStructIntError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected map, found int`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -293,7 +293,7 @@ func TestNormalizeMapElementDiagnostic(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected string, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.NewPath(dyn.Key("bar")), }, err[0]) @@ -319,7 +319,7 @@ func TestNormalizeMapError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected map, found string`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -374,7 +374,7 @@ func TestNormalizeMapRandomStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected map, found string`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -387,7 +387,7 @@ func TestNormalizeMapIntError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected map, found int`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -417,7 +417,7 @@ func TestNormalizeSliceElementDiagnostic(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected string, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.NewPath(dyn.Index(2)), }, err[0]) @@ -441,7 +441,7 @@ func TestNormalizeSliceError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected sequence, found string`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -496,7 +496,7 @@ func TestNormalizeSliceRandomStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected sequence, found string`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -509,7 +509,7 @@ func TestNormalizeSliceIntError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected sequence, found int`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -530,7 +530,7 @@ func TestNormalizeStringNil(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected a string value, found null`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -567,7 +567,7 @@ func TestNormalizeStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected string, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.EmptyPath, }, err[0]) } @@ -588,7 +588,7 @@ func TestNormalizeBoolNil(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected a bool value, found null`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -630,7 +630,7 @@ func TestNormalizeBoolFromStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected bool, found string`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -643,7 +643,7 @@ func TestNormalizeBoolError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected bool, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.EmptyPath, }, err[0]) } @@ -664,7 +664,7 @@ func TestNormalizeIntNil(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected a int value, found null`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -685,7 +685,7 @@ func TestNormalizeIntFromFloatError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `cannot accurately represent "1.5" as integer due to precision loss`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -714,7 +714,7 @@ func TestNormalizeIntFromStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `cannot parse "abc" as an integer`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -727,7 +727,7 @@ func TestNormalizeIntError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected int, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.EmptyPath, }, err[0]) } @@ -748,7 +748,7 @@ func TestNormalizeFloatNil(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected a float value, found null`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -773,7 +773,7 @@ func TestNormalizeFloatFromIntError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `cannot accurately represent "9007199254740993" as floating point number due to precision loss`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -802,7 +802,7 @@ func TestNormalizeFloatFromStringError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `cannot parse "abc" as a floating point number`, - Locations: vin.Location(), + Locations: []dyn.Location{vin.Location()}, Path: dyn.EmptyPath, }, err[0]) } @@ -815,7 +815,7 @@ func TestNormalizeFloatError(t *testing.T) { assert.Equal(t, diag.Diagnostic{ Severity: diag.Warning, Summary: `expected float, found map`, - Locations: dyn.Location{}, + Locations: []dyn.Location{{}}, Path: dyn.EmptyPath, }, err[0]) } From b380344608e814da3ab758c21017af75f6e397d5 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 19:45:27 +0200 Subject: [PATCH 05/18] add support for printing multiple locations in validate errors and warnings --- bundle/config/validate/files_to_sync.go | 2 ++ bundle/render/render_text_output.go | 32 +++++++++++++++++++----- bundle/render/render_text_output_test.go | 26 +++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go index 67b94090a3..af34ac1f54 100644 --- a/bundle/config/validate/files_to_sync.go +++ b/bundle/config/validate/files_to_sync.go @@ -46,6 +46,8 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag. Severity: diag.Warning, Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration", Locations: loc.Locations(), + // TODO: Highlight in the PR that the semantics have been changed. Also that + // for array values, .Location or .Locations does not make a difference. Path: loc.Path(), }) } diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index 68876dc783..3ba221dafb 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -9,10 +9,33 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/fatih/color" ) +func printLocations(locations []dyn.Location) string { + res := strings.Builder{} + + first := true + for _, loc := range locations { + if loc.File == "" { + continue + } + + res.WriteString("\n") + if first { + res.WriteString(" in ") + first = false + } else { + res.WriteString(" ") + } + + res.WriteString(loc.String()) + } + return res.String() +} + var renderFuncMap = template.FuncMap{ "red": color.RedString, "green": color.GreenString, @@ -26,15 +49,14 @@ var renderFuncMap = template.FuncMap{ "italic": func(format string, a ...interface{}) string { return color.New(color.Italic).Sprintf(format, a...) }, + "printLocations": printLocations, } const errorTemplate = `{{ "Error" | red }}: {{ .Summary }} {{- if .Path.String }} {{ "at " }}{{ .Path.String | green }} {{- end }} -{{- if .Location.File }} - {{ "in " }}{{ .Location.String | cyan }} -{{- end }} +{{- printLocations .Locations -}} {{- if .Detail }} {{ .Detail }} @@ -46,9 +68,7 @@ const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }} {{- if .Path.String }} {{ "at " }}{{ .Path.String | green }} {{- end }} -{{- if .Location.File }} - {{ "in " }}{{ .Location.String | cyan }} -{{- end }} +{{- printLocations .Locations -}} {{- if .Detail }} {{ .Detail }} diff --git a/bundle/render/render_text_output_test.go b/bundle/render/render_text_output_test.go index 8adf54161e..81e2761995 100644 --- a/bundle/render/render_text_output_test.go +++ b/bundle/render/render_text_output_test.go @@ -242,6 +242,32 @@ func TestRenderDiagnostics(t *testing.T) { " 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{ From 50b8fa57de0ba76b1c458330df9a62cccd988ab3 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 19:47:08 +0200 Subject: [PATCH 06/18] newlines in python_diagnostics_test.go --- .../config/mutator/python/python_diagnostics_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bundle/config/mutator/python/python_diagnostics_test.go b/bundle/config/mutator/python/python_diagnostics_test.go index eee29bee7d..09d9f93bd2 100644 --- a/bundle/config/mutator/python/python_diagnostics_test.go +++ b/bundle/config/mutator/python/python_diagnostics_test.go @@ -39,10 +39,13 @@ func TestParsePythonDiagnostics(t *testing.T) { { Severity: diag.Error, Summary: "error summary", - Locations: []dyn.Location{{ - File: "src/examples/file.py", - Line: 1, - Column: 2}}, + Locations: []dyn.Location{ + { + File: "src/examples/file.py", + Line: 1, + Column: 2, + }, + }, }, }, }, From 4d1e2a04f6817b5f4e7de80c4b9547137583ec03 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 19:48:17 +0200 Subject: [PATCH 07/18] newlines in python_mutator_test.go --- bundle/config/mutator/python/python_mutator_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bundle/config/mutator/python/python_mutator_test.go b/bundle/config/mutator/python/python_mutator_test.go index dc4ed9322e..fbe835f928 100644 --- a/bundle/config/mutator/python/python_mutator_test.go +++ b/bundle/config/mutator/python/python_mutator_test.go @@ -97,10 +97,13 @@ 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].Locations) + assert.Equal(t, []dyn.Location{ + { + File: "src/examples/file.py", + Line: 10, + Column: 5, + }, + }, diags[0].Locations) } From 3b8fc61f99ebe87402c737c61f72d82aef1b6013 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 19:50:10 +0200 Subject: [PATCH 08/18] clean up --- bundle/config/validate/files_to_sync.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go index af34ac1f54..67b94090a3 100644 --- a/bundle/config/validate/files_to_sync.go +++ b/bundle/config/validate/files_to_sync.go @@ -46,8 +46,6 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag. Severity: diag.Warning, Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration", Locations: loc.Locations(), - // TODO: Highlight in the PR that the semantics have been changed. Also that - // for array values, .Location or .Locations does not make a difference. Path: loc.Path(), }) } From 83558a658aadb153637df6cc9dcacf4e4de536e9 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 19:57:02 +0200 Subject: [PATCH 09/18] only select single location for undefined job_cluster_key --- bundle/config/validate/files_to_sync.go | 6 ++++-- bundle/config/validate/job_cluster_key_defined.go | 10 +++++++--- bundle/config/validate/validate.go | 4 ++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go index 67b94090a3..ae6bfef1a9 100644 --- a/bundle/config/validate/files_to_sync.go +++ b/bundle/config/validate/files_to_sync.go @@ -43,8 +43,10 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag. } else { loc := location{path: "sync.exclude", rb: rb} diags = diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration", + Severity: diag.Warning, + Summary: "There are no files to sync, please check your .gitignore and sync.exclude configuration", + // Show all locations where sync.exclude is defined, since merging + // sync.exclude is additive. Locations: loc.Locations(), Path: loc.Path(), }) diff --git a/bundle/config/validate/job_cluster_key_defined.go b/bundle/config/validate/job_cluster_key_defined.go index 392259ade6..168303d83b 100644 --- a/bundle/config/validate/job_cluster_key_defined.go +++ b/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 { @@ -39,9 +40,12 @@ 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), - Locations: loc.Locations(), + Severity: diag.Warning, + Summary: fmt.Sprintf("job_cluster_key %s is not defined", task.JobClusterKey), + // 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(), }) } diff --git a/bundle/config/validate/validate.go b/bundle/config/validate/validate.go index ef6488862a..b4da0bc053 100644 --- a/bundle/config/validate/validate.go +++ b/bundle/config/validate/validate.go @@ -16,6 +16,10 @@ type location struct { rb bundle.ReadOnlyBundle } +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) } From d9b8902bce09d532326d50a5582c59f13a340a49 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 18 Jul 2024 20:00:56 +0200 Subject: [PATCH 10/18] single location for validate_sync_patterns.go --- bundle/config/validate/validate_sync_patterns.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go index abcbce16a2..f3655ca949 100644 --- a/bundle/config/validate/validate_sync_patterns.go +++ b/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" ) @@ -66,7 +67,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("Pattern %s does not match any files", p), - Locations: loc.Locations(), + Locations: []dyn.Location{loc.Location()}, Path: loc.Path(), }) mu.Unlock() From 5b94edd1a683e1eb4c6b397ad85777351f742195 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 19 Jul 2024 14:05:26 +0200 Subject: [PATCH 11/18] add cyan color to location --- bundle/render/render_text_output.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index 3ba221dafb..9389ecead2 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -31,7 +31,7 @@ func printLocations(locations []dyn.Location) string { res.WriteString(" ") } - res.WriteString(loc.String()) + res.WriteString(color.CyanString(loc.String())) } return res.String() } @@ -147,7 +147,6 @@ func renderSummaryTemplate(out io.Writer, b *bundle.Bundle, diags diag.Diagnosti return err } -// TODO: Write tests when multiple locations are rendered. func renderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics) error { errorT := template.Must(template.New("error").Funcs(renderFuncMap).Parse(errorTemplate)) warningT := template.Must(template.New("warning").Funcs(renderFuncMap).Parse(warningTemplate)) From 52c2d2e321f21ea8dc22fa06d45b6605316f5743 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 19 Jul 2024 14:05:46 +0200 Subject: [PATCH 12/18] Fix test in python_diagnostics_test.go --- .../mutator/python/python_diagnostics_test.go | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/bundle/config/mutator/python/python_diagnostics_test.go b/bundle/config/mutator/python/python_diagnostics_test.go index 09d9f93bd2..51f3a5fc7f 100644 --- a/bundle/config/mutator/python/python_diagnostics_test.go +++ b/bundle/config/mutator/python/python_diagnostics_test.go @@ -54,9 +54,10 @@ func TestParsePythonDiagnostics(t *testing.T) { input: `{"severity": "error", "summary": "error summary", "path": "resources.jobs.job0.name"}`, expected: diag.Diagnostics{ { - Severity: diag.Error, - Summary: "error summary", - Path: dyn.MustPathFromString("resources.jobs.job0.name"), + Severity: diag.Error, + Summary: "error summary", + Path: dyn.MustPathFromString("resources.jobs.job0.name"), + Locations: []dyn.Location{{}}, }, }, }, @@ -75,9 +76,10 @@ func TestParsePythonDiagnostics(t *testing.T) { input: `{"severity": "warning", "summary": "warning summary", "detail": "warning detail"}`, expected: diag.Diagnostics{ { - Severity: diag.Warning, - Summary: "warning summary", - Detail: "warning detail", + Severity: diag.Warning, + Summary: "warning summary", + Detail: "warning detail", + Locations: []dyn.Location{{}}, }, }, }, @@ -87,12 +89,14 @@ func TestParsePythonDiagnostics(t *testing.T) { `{"severity": "error", "summary": "error summary (2)"}`, expected: diag.Diagnostics{ { - Severity: diag.Error, - Summary: "error summary (1)", + Severity: diag.Error, + Summary: "error summary (1)", + Locations: []dyn.Location{{}}, }, { - Severity: diag.Error, - Summary: "error summary (2)", + Severity: diag.Error, + Summary: "error summary (2)", + Locations: []dyn.Location{{}}, }, }, }, From 6ed74d044ede9d696d802f6807bae55e7e7a3232 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 19 Jul 2024 14:21:20 +0200 Subject: [PATCH 13/18] fix comment for Diagnostic.Locations --- libs/diag/diagnostic.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/diag/diagnostic.go b/libs/diag/diagnostic.go index 062ca7e552..305089d228 100644 --- a/libs/diag/diagnostic.go +++ b/libs/diag/diagnostic.go @@ -17,8 +17,8 @@ type Diagnostic struct { // This may be multiple lines and may be nil. Detail string - // Locations is a source code location associated with the diagnostic message. - // It may be zero if there is no associated 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. From 1202bcc6fc8b558173619b6d0b636bac187a1202 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 19 Jul 2024 14:41:34 +0200 Subject: [PATCH 14/18] include multiple locations for unknown field warning --- libs/dyn/convert/normalize.go | 3 ++- libs/dyn/convert/normalize_test.go | 28 +++++++++++++++++++--------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index df3c858b06..b62c106c15 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -98,7 +98,8 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, Summary: fmt.Sprintf("unknown field: %s", pk.MustString()), - Locations: []dyn.Location{pk.Location()}, + // Show all locations the unknown field is defined at. + Locations: pk.Locations(), Path: path, }) } diff --git a/libs/dyn/convert/normalize_test.go b/libs/dyn/convert/normalize_test.go index 536bfa6d0a..df9a1a9a53 100644 --- a/libs/dyn/convert/normalize_test.go +++ b/libs/dyn/convert/normalize_test.go @@ -58,23 +58,33 @@ func TestNormalizeStructUnknownField(t *testing.T) { } var typ Tmp - vin := dyn.V(map[string]dyn.Value{ - "foo": dyn.V("bar"), - "bar": dyn.V("baz"), - }) + + m := dyn.NewMapping() + m.Set(dyn.V("foo"), dyn.V("val-foo")) + // Set the unknown field, with location information. + m.Set(dyn.NewValue("bar", []dyn.Location{ + {File: "hello.yaml", Line: 1, Column: 1}, + {File: "world.yaml", Line: 2, Column: 2}, + }), dyn.V("var-bar")) + + vin := dyn.V(m) vout, err := Normalize(typ, vin) assert.Len(t, err, 1) assert.Equal(t, diag.Diagnostic{ - Severity: diag.Warning, - Summary: `unknown field: bar`, - Locations: []dyn.Location{vin.Get("foo").Location()}, - Path: dyn.EmptyPath, + Severity: diag.Warning, + Summary: `unknown field: bar`, + // Assert location of the unknown field is included in the diagnostic. + Locations: []dyn.Location{ + {File: "hello.yaml", Line: 1, Column: 1}, + {File: "world.yaml", Line: 2, Column: 2}, + }, + Path: dyn.EmptyPath, }, err[0]) // The field that can be mapped to the struct field is retained. assert.Equal(t, map[string]any{ - "foo": "bar", + "foo": "val-foo", }, vout.AsAny()) } From 16072f6c2d11451e5dce9b50c670703e3e81bb64 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 19 Jul 2024 14:47:28 +0200 Subject: [PATCH 15/18] fmt --- libs/dyn/convert/normalize.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index b62c106c15..bf5756e7f1 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -96,8 +96,8 @@ func (n normalizeOptions) normalizeStruct(typ reflect.Type, src dyn.Value, seen if !ok { if !pv.IsAnchor() { diags = diags.Append(diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("unknown field: %s", pk.MustString()), + Severity: diag.Warning, + Summary: fmt.Sprintf("unknown field: %s", pk.MustString()), // Show all locations the unknown field is defined at. Locations: pk.Locations(), Path: path, From 9aedd3d6c23e4bb104cdf58163061ccbe4c84568 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 23 Jul 2024 18:47:16 +0200 Subject: [PATCH 16/18] nil locations in python_diagnostics --- .../mutator/python/python_diagnostics.go | 8 ++++++- .../mutator/python/python_diagnostics_test.go | 24 ++++++++----------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/bundle/config/mutator/python/python_diagnostics.go b/bundle/config/mutator/python/python_diagnostics.go index e9cb0c5668..96baa50937 100644 --- a/bundle/config/mutator/python/python_diagnostics.go +++ b/bundle/config/mutator/python/python_diagnostics.go @@ -55,11 +55,17 @@ 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, - Locations: []dyn.Location{convertPythonLocation(parsedLine.Location)}, + Locations: locations, Path: path, } diff --git a/bundle/config/mutator/python/python_diagnostics_test.go b/bundle/config/mutator/python/python_diagnostics_test.go index 51f3a5fc7f..09d9f93bd2 100644 --- a/bundle/config/mutator/python/python_diagnostics_test.go +++ b/bundle/config/mutator/python/python_diagnostics_test.go @@ -54,10 +54,9 @@ func TestParsePythonDiagnostics(t *testing.T) { input: `{"severity": "error", "summary": "error summary", "path": "resources.jobs.job0.name"}`, expected: diag.Diagnostics{ { - Severity: diag.Error, - Summary: "error summary", - Path: dyn.MustPathFromString("resources.jobs.job0.name"), - Locations: []dyn.Location{{}}, + Severity: diag.Error, + Summary: "error summary", + Path: dyn.MustPathFromString("resources.jobs.job0.name"), }, }, }, @@ -76,10 +75,9 @@ func TestParsePythonDiagnostics(t *testing.T) { input: `{"severity": "warning", "summary": "warning summary", "detail": "warning detail"}`, expected: diag.Diagnostics{ { - Severity: diag.Warning, - Summary: "warning summary", - Detail: "warning detail", - Locations: []dyn.Location{{}}, + Severity: diag.Warning, + Summary: "warning summary", + Detail: "warning detail", }, }, }, @@ -89,14 +87,12 @@ func TestParsePythonDiagnostics(t *testing.T) { `{"severity": "error", "summary": "error summary (2)"}`, expected: diag.Diagnostics{ { - Severity: diag.Error, - Summary: "error summary (1)", - Locations: []dyn.Location{{}}, + Severity: diag.Error, + Summary: "error summary (1)", }, { - Severity: diag.Error, - Summary: "error summary (2)", - Locations: []dyn.Location{{}}, + Severity: diag.Error, + Summary: "error summary (2)", }, }, }, From 2ca95a06e336473b3ca2bf8b2d61cdf5c04740d3 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 23 Jul 2024 18:50:45 +0200 Subject: [PATCH 17/18] comment why plural is needed: --- bundle/config/root.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bundle/config/root.go b/bundle/config/root.go index 0c1f710b4e..4239a34d0c 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -524,6 +524,9 @@ 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 { From 4beb8f9847978a23e7d3b35ca6aa4403f9e7cec7 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 23 Jul 2024 18:57:58 +0200 Subject: [PATCH 18/18] inline printing locations --- bundle/render/render_text_output.go | 32 ++++++----------------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/bundle/render/render_text_output.go b/bundle/render/render_text_output.go index 9389ecead2..2ef6b26569 100644 --- a/bundle/render/render_text_output.go +++ b/bundle/render/render_text_output.go @@ -9,33 +9,10 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/fatih/color" ) -func printLocations(locations []dyn.Location) string { - res := strings.Builder{} - - first := true - for _, loc := range locations { - if loc.File == "" { - continue - } - - res.WriteString("\n") - if first { - res.WriteString(" in ") - first = false - } else { - res.WriteString(" ") - } - - res.WriteString(color.CyanString(loc.String())) - } - return res.String() -} - var renderFuncMap = template.FuncMap{ "red": color.RedString, "green": color.GreenString, @@ -49,14 +26,15 @@ var renderFuncMap = template.FuncMap{ "italic": func(format string, a ...interface{}) string { return color.New(color.Italic).Sprintf(format, a...) }, - "printLocations": printLocations, } const errorTemplate = `{{ "Error" | red }}: {{ .Summary }} {{- if .Path.String }} {{ "at " }}{{ .Path.String | green }} {{- end }} -{{- printLocations .Locations -}} +{{- range $index, $element := .Locations }} + {{ if eq $index 0 }}in {{else}} {{ end}}{{ $element.String | cyan }} +{{- end }} {{- if .Detail }} {{ .Detail }} @@ -68,7 +46,9 @@ const warningTemplate = `{{ "Warning" | yellow }}: {{ .Summary }} {{- if .Path.String }} {{ "at " }}{{ .Path.String | green }} {{- end }} -{{- printLocations .Locations -}} +{{- range $index, $element := .Locations }} + {{ if eq $index 0 }}in {{else}} {{ end}}{{ $element.String | cyan }} +{{- end }} {{- if .Detail }} {{ .Detail }}