From f45c1cdace8b8ade7d2eb5e90c1eead6e600d1a8 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sat, 28 Jul 2018 14:42:53 -0700 Subject: [PATCH] hcl: Include variable values in rendered diagnostics messages If a diagnostic has an associated Expression and EvalContext then we can look up the values of any variables referenced in the expression and show them in the diagnostics message as additional context. This is particularly useful when dealing with situations where a given expression is evaluated multiple times with different variables, such as in a 'for' expression, since each evaluation may produce a different set of diagnostics. --- hcl/diagnostic_text.go | 132 ++++++++++++++++++++++++++++++++++++ hcl/diagnostic_text_test.go | 84 +++++++++++++++++++++++ 2 files changed, 216 insertions(+) diff --git a/hcl/diagnostic_text.go b/hcl/diagnostic_text.go index dfa473a..035292a 100644 --- a/hcl/diagnostic_text.go +++ b/hcl/diagnostic_text.go @@ -2,11 +2,14 @@ package hcl import ( "bufio" + "bytes" "errors" "fmt" "io" + "sort" wordwrap "github.com/mitchellh/go-wordwrap" + "github.com/zclconf/go-cty/cty" ) type diagnosticTextWriter struct { @@ -133,6 +136,57 @@ func (w *diagnosticTextWriter) WriteDiagnostic(diag *Diagnostic) error { w.wr.Write([]byte{'\n'}) } + + if diag.Expression != nil && diag.EvalContext != nil { + // We will attempt to render the values for any variables + // referenced in the given expression as additional context, for + // situations where the same expression is evaluated multiple + // times in different scopes. + expr := diag.Expression + ctx := diag.EvalContext + + vars := expr.Variables() + stmts := make([]string, 0, len(vars)) + for _, traversal := range vars { + val, diags := traversal.TraverseAbs(ctx) + if diags.HasErrors() { + // Skip anything that generates errors, since we probably + // already have the same error in our diagnostics set + // already. + continue + } + + traversalStr := w.traversalStr(traversal) + switch { + case !val.IsKnown(): + // Can't say anything about this yet, then. + continue + case val.IsNull(): + stmts = append(stmts, fmt.Sprintf("%s set to null", traversalStr)) + default: + stmts = append(stmts, fmt.Sprintf("%s as %s", traversalStr, w.valueStr(val))) + } + } + + sort.Strings(stmts) // FIXME: Should maybe use a traversal-aware sort that can sort numeric indexes properly? + last := len(stmts) - 1 + + for i, stmt := range stmts { + switch i { + case 0: + w.wr.Write([]byte{'w', 'i', 't', 'h', ' '}) + default: + w.wr.Write([]byte{' ', ' ', ' ', ' ', ' '}) + } + w.wr.Write([]byte(stmt)) + switch i { + case last: + w.wr.Write([]byte{'.', '\n', '\n'}) + default: + w.wr.Write([]byte{',', '\n'}) + } + } + } } if diag.Detail != "" { @@ -156,6 +210,84 @@ func (w *diagnosticTextWriter) WriteDiagnostics(diags Diagnostics) error { return nil } +func (w *diagnosticTextWriter) traversalStr(traversal Traversal) string { + // This is a specialized subset of traversal rendering tailored to + // producing helpful contextual messages in diagnostics. It is not + // comprehensive nor intended to be used for other purposes. + + var buf bytes.Buffer + for _, step := range traversal { + switch tStep := step.(type) { + case TraverseRoot: + buf.WriteString(tStep.Name) + case TraverseAttr: + buf.WriteByte('.') + buf.WriteString(tStep.Name) + case TraverseIndex: + buf.WriteByte('[') + buf.WriteString(w.valueStr(tStep.Key)) + buf.WriteByte(']') + } + } + return buf.String() +} + +func (w *diagnosticTextWriter) valueStr(val cty.Value) string { + // This is a specialized subset of value rendering tailored to producing + // helpful but concise messages in diagnostics. It is not comprehensive + // nor intended to be used for other purposes. + + ty := val.Type() + switch { + case val.IsNull(): + return "null" + case !val.IsKnown(): + // Should never happen here because we should filter before we get + // in here, but we'll do something reasonable rather than panic. + return "(not yet known)" + case ty == cty.Bool: + if val.True() { + return "true" + } + return "false" + case ty == cty.Number: + bf := val.AsBigFloat() + return bf.Text('g', 10) + case ty == cty.String: + // Go string syntax is not exactly the same as HCL native string syntax, + // but we'll accept the minor edge-cases where this is different here + // for now, just to get something reasonable here. + return fmt.Sprintf("%q", val.AsString()) + case ty.IsCollectionType() || ty.IsTupleType(): + l := val.LengthInt() + switch l { + case 0: + return "empty " + ty.FriendlyName() + case 1: + return ty.FriendlyName() + "with 1 element" + default: + return fmt.Sprintf("%s with %d elements", ty.FriendlyName(), l) + } + case ty.IsObjectType(): + atys := ty.AttributeTypes() + l := len(atys) + switch l { + case 0: + return "object with no attributes" + case 1: + var name string + for k := range atys { + name = k + } + return fmt.Sprintf("object with 1 attribute %q", name) + default: + return fmt.Sprintf("object with %d attributes", l) + } + default: + return ty.FriendlyName() + } +} + func contextString(file *File, offset int) string { type contextStringer interface { ContextString(offset int) string diff --git a/hcl/diagnostic_text_test.go b/hcl/diagnostic_text_test.go index 67c1e64..8757fbe 100644 --- a/hcl/diagnostic_text_test.go +++ b/hcl/diagnostic_text_test.go @@ -4,6 +4,8 @@ import ( "bytes" "fmt" "testing" + + "github.com/zclconf/go-cty/cty" ) func TestDiagnosticTextWriter(t *testing.T) { @@ -108,6 +110,79 @@ attribute. Did you mean "bam"? "pizza" is not a supported attribute. Did you mean "pizzetta"? +`, + }, + { + &Diagnostic{ + Severity: DiagError, + Summary: "Test of including relevant variable values", + Detail: `This diagnostic includes an expression and an evalcontext.`, + Subject: &Range{ + Start: Pos{ + Byte: 42, + Column: 3, + Line: 5, + }, + End: Pos{ + Byte: 47, + Column: 8, + Line: 5, + }, + }, + Expression: &diagnosticTestExpr{ + vars: []Traversal{ + { + TraverseRoot{ + Name: "foo", + }, + }, + { + TraverseRoot{ + Name: "bar", + }, + TraverseAttr{ + Name: "baz", + }, + }, + { + TraverseRoot{ + Name: "missing", + }, + }, + { + TraverseRoot{ + Name: "boz", + }, + }, + }, + }, + EvalContext: &EvalContext{ + parent: &EvalContext{ + Variables: map[string]cty.Value{ + "foo": cty.StringVal("foo value"), + }, + }, + Variables: map[string]cty.Value{ + "bar": cty.ObjectVal(map[string]cty.Value{ + "baz": cty.ListValEmpty(cty.String), + }), + "boz": cty.NumberIntVal(5), + "unused": cty.True, + }, + }, + }, + `Error: Test of including relevant variable values + + on line 5, in hardcoded-context: + 5: pizza = "cheese" + +with bar.baz as empty list of string, + boz as 5, + foo as "foo value". + +This diagnostic includes an expression +and an evalcontext. + `, }, } @@ -149,3 +224,12 @@ type diagnosticTestNav struct { func (tn *diagnosticTestNav) ContextString(offset int) string { return "hardcoded-context" } + +type diagnosticTestExpr struct { + vars []Traversal + staticExpr +} + +func (e *diagnosticTestExpr) Variables() []Traversal { + return e.vars +}