From fbb4f4dec86541dd243e99816ef84b3a4d4741c2 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Tue, 7 Feb 2023 11:34:17 +0000 Subject: [PATCH] bake: avoid nesting error diagnostics With changes to the lazy evaluation, the evaluation order is no longer fixed - this means that we can follow long and confusing paths to get to an error. Because of the co-recursive nature of the lazy evaluation, we need to take special care that the original HCL diagnostics are not discarded and are preserved so that the original source of the error can be detected. Preserving the full trace is not necessary, and probably not useful to the user - all of the file that is not lazily loaded will be eagerly loaded after all struct blocks are loaded - so the error would be found regardless. Signed-off-by: Justin Chadwell --- bake/hclparser/expr.go | 10 +--- bake/hclparser/hclparser.go | 100 +++++++++++------------------------- 2 files changed, 30 insertions(+), 80 deletions(-) diff --git a/bake/hclparser/expr.go b/bake/hclparser/expr.go index 23a99f54..80ecb020 100644 --- a/bake/hclparser/expr.go +++ b/bake/hclparser/expr.go @@ -14,15 +14,7 @@ func funcCalls(exp hcl.Expression) ([]string, hcl.Diagnostics) { if !ok { fns, err := jsonFuncCallsRecursive(exp) if err != nil { - return nil, hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: exp.Range().Ptr(), - Context: exp.Range().Ptr(), - }, - } + return nil, wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } return fns, nil } diff --git a/bake/hclparser/hclparser.go b/bake/hclparser/hclparser.go index 23477c43..ec378cf9 100644 --- a/bake/hclparser/hclparser.go +++ b/bake/hclparser/hclparser.go @@ -75,15 +75,7 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allow if allowMissing && errors.Is(err, errUndefined) { continue } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: exp.Range().Ptr(), - Context: exp.Range().Ptr(), - }, - } + return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } } @@ -136,30 +128,14 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allow if allowMissing && errors.Is(err, errUndefined) { continue } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: v.SourceRange().Ptr(), - Context: v.SourceRange().Ptr(), - }, - } + return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } } else { if err := p.resolveValue(v.RootName()); err != nil { if allowMissing && errors.Is(err, errUndefined) { continue } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid expression", - Detail: err.Error(), - Subject: v.SourceRange().Ptr(), - Context: v.SourceRange().Ptr(), - }, - } + return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr()) } } } @@ -325,14 +301,7 @@ func (p *parser) resolveValue(name string) (err error) { func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err error) { name := block.Labels[0] if err := p.opt.ValidateLabel(name); err != nil { - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid name", - Detail: err.Error(), - Subject: &block.LabelRanges[0], - }, - } + return wrapErrorDiagnostic("Invalid name", err, &block.LabelRanges[0], &block.LabelRanges[0]) } if _, ok := p.doneB[block]; !ok { @@ -584,15 +553,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { return diags } r := p.vars[k].Body.MissingItemRange() - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid value", - Detail: err.Error(), - Subject: &r, - Context: &r, - }, - } + return wrapErrorDiagnostic("Invalid value", err, &r, &r) } } @@ -615,15 +576,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { } } } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid function", - Detail: err.Error(), - Subject: subject, - Context: context, - }, - } + return wrapErrorDiagnostic("Invalid function", err, subject, context) } } @@ -684,15 +637,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { continue } } else { - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid attribute", - Detail: err.Error(), - Subject: &b.LabelRanges[0], - Context: &b.DefRange, - }, - } + return wrapErrorDiagnostic("Invalid block", err, &b.LabelRanges[0], &b.DefRange) } } @@ -737,21 +682,34 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics { if diags, ok := err.(hcl.Diagnostics); ok { return diags } - return hcl.Diagnostics{ - &hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid attribute", - Detail: err.Error(), - Subject: &p.attrs[k].Range, - Context: &p.attrs[k].Range, - }, - } + return wrapErrorDiagnostic("Invalid attribute", err, &p.attrs[k].Range, &p.attrs[k].Range) } } return nil } +// wrapErrorDiagnostic wraps an error into a hcl.Diagnostics object. +// If the error is already an hcl.Diagnostics object, it is returned as is. +func wrapErrorDiagnostic(message string, err error, subject *hcl.Range, context *hcl.Range) hcl.Diagnostics { + switch err := err.(type) { + case *hcl.Diagnostic: + return hcl.Diagnostics{err} + case hcl.Diagnostics: + return err + default: + return hcl.Diagnostics{ + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: message, + Detail: err.Error(), + Subject: subject, + Context: context, + }, + } + } +} + func setLabel(v reflect.Value, lbl string) int { // cache field index? numFields := v.Elem().Type().NumField()