From 503c413de290c440ba71fcadb9b8ea6d3c563f90 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 2 Apr 2021 16:49:25 -0400 Subject: [PATCH] add addresses to diagnostics Add an address argument to tfdiags.InConfigBody, and store the address string the diagnostics details. Since nearly every place where we want to annotate the diagnostics with the config context we also have some sort of address, we can use the same call to insert them both into the diagnostic. Perhaps we should rename InConfigBody and ElaborateFromConfigBody to reflect the additional address parameter, but for now we can verify this is a pattern that suits us. --- tfdiags/contextual.go | 27 ++++++++++++++++++++------- tfdiags/contextual_test.go | 16 +++++++++++++++- tfdiags/diagnostic.go | 1 + tfdiags/diagnostic_base.go | 2 ++ 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/tfdiags/contextual.go b/tfdiags/contextual.go index f66e5d913..c59280f34 100644 --- a/tfdiags/contextual.go +++ b/tfdiags/contextual.go @@ -19,17 +19,20 @@ import ( // contextualFromConfig is an interface type implemented by diagnostic types // that can elaborate themselves when given information about the configuration -// body they are embedded in. +// body they are embedded in, as well as the runtime address associated with +// that configuration. // // Usually this entails extracting source location information in order to // populate the "Subject" range. type contextualFromConfigBody interface { - ElaborateFromConfigBody(hcl.Body) Diagnostic + ElaborateFromConfigBody(hcl.Body, string) Diagnostic } // InConfigBody returns a copy of the receiver with any config-contextual -// diagnostics elaborated in the context of the given body. -func (diags Diagnostics) InConfigBody(body hcl.Body) Diagnostics { +// diagnostics elaborated in the context of the given body. An optional address +// argument may be added to indicate which instance of the configuration the +// error related to. +func (diags Diagnostics) InConfigBody(body hcl.Body, addr string) Diagnostics { if len(diags) == 0 { return nil } @@ -37,7 +40,7 @@ func (diags Diagnostics) InConfigBody(body hcl.Body) Diagnostics { ret := make(Diagnostics, len(diags)) for i, srcDiag := range diags { if cd, isCD := srcDiag.(contextualFromConfigBody); isCD { - ret[i] = cd.ElaborateFromConfigBody(body) + ret[i] = cd.ElaborateFromConfigBody(body, addr) } else { ret[i] = srcDiag } @@ -112,7 +115,12 @@ type attributeDiagnostic struct { // source location information is still available, for more accuracy. This // is not always possible due to system architecture, so this serves as a // "best effort" fallback behavior for such situations. -func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body) Diagnostic { +func (d *attributeDiagnostic) ElaborateFromConfigBody(body hcl.Body, addr string) Diagnostic { + // don't change an existing address + if d.address == "" { + d.address = addr + } + if len(d.attrPath) < 1 { // Should never happen, but we'll allow it rather than crashing. return d @@ -353,7 +361,12 @@ type wholeBodyDiagnostic struct { subject *SourceRange // populated only after ElaborateFromConfigBody } -func (d *wholeBodyDiagnostic) ElaborateFromConfigBody(body hcl.Body) Diagnostic { +func (d *wholeBodyDiagnostic) ElaborateFromConfigBody(body hcl.Body, addr string) Diagnostic { + // don't change an existing address + if d.address == "" { + d.address = addr + } + if d.subject != nil { // Don't modify an already-elaborated diagnostic. return d diff --git a/tfdiags/contextual_test.go b/tfdiags/contextual_test.go index 806d0c3a9..e29712a7b 100644 --- a/tfdiags/contextual_test.go +++ b/tfdiags/contextual_test.go @@ -185,6 +185,7 @@ simple_attr = "val" diagnosticBase: diagnosticBase{ summary: "preexisting", detail: "detail", + address: "original", }, subject: &SourceRange{ Filename: "somewhere_else.tf", @@ -535,9 +536,22 @@ simple_attr = "val" for i, tc := range testCases { t.Run(fmt.Sprintf("%d:%s", i, tc.Diag.Description()), func(t *testing.T) { var diags Diagnostics + + origAddr := tc.Diag.Description().Address diags = diags.Append(tc.Diag) - gotDiags := diags.InConfigBody(f.Body) + + gotDiags := diags.InConfigBody(f.Body, "test.addr") gotRange := gotDiags[0].Source().Subject + gotAddr := gotDiags[0].Description().Address + + switch { + case origAddr != "": + if gotAddr != origAddr { + t.Errorf("original diagnostic address modified from %s to %s", origAddr, gotAddr) + } + case gotAddr != "test.addr": + t.Error("missing detail address") + } for _, problem := range deep.Equal(gotRange, tc.ExpectedRange) { t.Error(problem) diff --git a/tfdiags/diagnostic.go b/tfdiags/diagnostic.go index a7699cf01..d84fa666f 100644 --- a/tfdiags/diagnostic.go +++ b/tfdiags/diagnostic.go @@ -25,6 +25,7 @@ const ( ) type Description struct { + Address string Summary string Detail string } diff --git a/tfdiags/diagnostic_base.go b/tfdiags/diagnostic_base.go index 50bf9d8eb..04e56773d 100644 --- a/tfdiags/diagnostic_base.go +++ b/tfdiags/diagnostic_base.go @@ -9,6 +9,7 @@ type diagnosticBase struct { severity Severity summary string detail string + address string } func (d diagnosticBase) Severity() Severity { @@ -19,6 +20,7 @@ func (d diagnosticBase) Description() Description { return Description{ Summary: d.summary, Detail: d.detail, + Address: d.address, } }