tfdiags: new package for normalizing error and warning messages

Currently we lean heavily on the Go error type as our primary means of
describing errors, and along with that use several more specialized
implementations of it in different spots for additional capabilities such
as multiple errors in one object, source code range references, etc.

We also have a rather ad-hoc approach of returning an array of warnings
from certain functions along with one or multiple errors.

This rather-disorganized approach makes it hard for us to present
user-facing error messages consistently. As a step towards mitigating
this, package tfdiags provides a model for user-facing error and warning
messages and helper functions for creating them from various other
error and warning types used elsewhere in Terraform.

This mechanism is intended to be used to report errors and warnings where
the audience is the Terraform user, and so it may go a few layers deep
down the call stack into codepaths like config parsing, interpolation, etc
but is primarily a UX concern. The deepest reaches of Terraform core will
continue using "error" as normal, with higher layers preparing error
messages for presentation to the user.

To avoid needing to change the interface of every function that might
generate error diagnostics, the Diagnostics type can be "smuggled" via
an error value through other APIs and then unwrapped at the other end,
though it will lose any naked warnings (without at least one error) along
the way, and so codepaths that are expected to generate warnings
(validation, primarily) should use the concrete Diagnostics type
throughout the call chain.

tfdiags/diagnostic.go

@@ -0,0 +1,26 @@
+package tfdiags
+
+type Diagnostic interface {
+	Severity() Severity
+	Description() Description
+	Source() Source
+}
+
+type Severity rune
+
+//go:generate stringer -type=Severity
+
+const (
+	Error   Severity = 'E'
+	Warning Severity = 'W'
+)
+
+type Description struct {
+	Summary string
+	Detail  string
+}
+
+type Source struct {
+	Subject *SourceRange
+	Context *SourceRange
+}

tfdiags/diagnostics.go

@@ -0,0 +1,154 @@
+package tfdiags
+
+import (
+	"bytes"
+	"fmt"
+
+	"github.com/hashicorp/errwrap"
+	multierror "github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/hcl2/hcl"
+)
+
+// Diagnostics is a list of diagnostics. Diagnostics is intended to be used
+// where a Go "error" might normally be used, allowing richer information
+// to be conveyed (more context, support for warnings).
+//
+// A nil Diagnostics is a valid, empty diagnostics list, thus allowing
+// heap allocation to be avoided in the common case where there are no
+// diagnostics to report at all.
+type Diagnostics []Diagnostic
+
+// Append is the main interface for constructing Diagnostics lists, taking
+// an existing list (which may be nil) and appending the new objects to it
+// after normalizing them to be implementations of Diagnostic.
+//
+// The usual pattern for a function that natively "speaks" diagnostics is:
+//
+//     // Create a nil Diagnostics at the start of the function
+//     var diags diag.Diagnostics
+//
+//     // At later points, build on it if errors / warnings occur:
+//     foo, err := DoSomethingRisky()
+//     if err != nil {
+//         diags = diags.Append(err)
+//     }
+//
+//     // Eventually return the result and diagnostics in place of error
+//     return result, diags
+//
+// Append accepts a variety of different diagnostic-like types, including
+// native Go errors and HCL diagnostics. It also knows how to unwrap
+// a multierror.Error into separate error diagnostics. It can be passed
+// another Diagnostics to concatenate the two lists. If given something
+// it cannot handle, this function will panic.
+func (diags Diagnostics) Append(new ...interface{}) Diagnostics {
+	for _, item := range new {
+		if item == nil {
+			continue
+		}
+
+		switch ti := item.(type) {
+		case Diagnostic:
+			diags = append(diags, ti)
+		case Diagnostics:
+			diags = append(diags, ti...) // flatten
+		case diagnosticsAsError:
+			diags = diags.Append(ti.Diagnostics) // unwrap
+		case hcl.Diagnostics:
+			for _, hclDiag := range ti {
+				diags = append(diags, hclDiagnostic{hclDiag})
+			}
+		case *hcl.Diagnostic:
+			diags = append(diags, hclDiagnostic{ti})
+		case *multierror.Error:
+			for _, err := range ti.Errors {
+				diags = append(diags, nativeError{err})
+			}
+		case error:
+			switch {
+			case errwrap.ContainsType(ti, Diagnostics(nil)):
+				// If we have an errwrap wrapper with a Diagnostics hiding
+				// inside then we'll unpick it here to get access to the
+				// individual diagnostics.
+				diags = diags.Append(errwrap.GetType(ti, Diagnostics(nil)))
+			case errwrap.ContainsType(ti, hcl.Diagnostics(nil)):
+				// Likewise, if we have HCL diagnostics we'll unpick that too.
+				diags = diags.Append(errwrap.GetType(ti, hcl.Diagnostics(nil)))
+			default:
+				diags = append(diags, nativeError{ti})
+			}
+		default:
+			panic(fmt.Errorf("can't construct diagnostic(s) from %T", item))
+		}
+	}
+
+	// Given the above, we should never end up with a non-nil empty slice
+	// here, but we'll make sure of that so callers can rely on empty == nil
+	if len(diags) == 0 {
+		return nil
+	}
+
+	return diags
+}
+
+// HasErrors returns true if any of the diagnostics in the list have
+// a severity of Error.
+func (diags Diagnostics) HasErrors() bool {
+	for _, diag := range diags {
+		if diag.Severity() == Error {
+			return true
+		}
+	}
+	return false
+}
+
+// Err flattens a diagnostics list into a single Go error, or to nil
+// if the diagnostics list does not include any error-level diagnostics.
+//
+// This can be used to smuggle diagnostics through an API that deals in
+// native errors, but unfortunately it will lose naked warnings (warnings
+// that aren't accompanied by at least one error) since such APIs have no
+// mechanism through which to report these.
+//
+//     return result, diags.Error()
+func (diags Diagnostics) Err() error {
+	if !diags.HasErrors() {
+		return nil
+	}
+	return diagnosticsAsError{diags}
+}
+
+type diagnosticsAsError struct {
+	Diagnostics
+}
+
+func (dae diagnosticsAsError) Error() string {
+	diags := dae.Diagnostics
+	switch {
+	case len(diags) == 0:
+		// should never happen, since we don't create this wrapper if
+		// there are no diagnostics in the list.
+		return "no errors"
+	case len(diags) == 1:
+		return diags[0].Description().Summary
+	default:
+		var ret bytes.Buffer
+		fmt.Fprintf(&ret, "%d problems:\n", len(diags))
+		for _, diag := range dae.Diagnostics {
+			fmt.Fprintf(&ret, "\n- %s", diag.Description().Summary)
+		}
+		return ret.String()
+	}
+}
+
+// WrappedErrors is an implementation of errwrap.Wrapper so that an error-wrapped
+// diagnostics object can be picked apart by errwrap-aware code.
+func (dae diagnosticsAsError) WrappedErrors() []error {
+	var errs []error
+	for _, diag := range dae.Diagnostics {
+		if wrapper, isErr := diag.(nativeError); isErr {
+			errs = append(errs, wrapper.err)
+		}
+	}
+	return errs
+}

tfdiags/diagnostics_test.go

@@ -0,0 +1,282 @@
+package tfdiags
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/go-multierror"
+
+	"github.com/davecgh/go-spew/spew"
+	"github.com/hashicorp/hcl2/hcl"
+)
+
+func TestBuild(t *testing.T) {
+	type diagFlat struct {
+		Severity Severity
+		Summary  string
+		Detail   string
+		Subject  *SourceRange
+		Context  *SourceRange
+	}
+
+	tests := map[string]struct {
+		Cons func(Diagnostics) Diagnostics
+		Want []diagFlat
+	}{
+		"nil": {
+			func(diags Diagnostics) Diagnostics {
+				return diags
+			},
+			nil,
+		},
+		"fmt.Errorf": {
+			func(diags Diagnostics) Diagnostics {
+				diags = diags.Append(fmt.Errorf("oh no bad"))
+				return diags
+			},
+			[]diagFlat{
+				{
+					Severity: Error,
+					Summary:  "oh no bad",
+				},
+			},
+		},
+		"errors.New": {
+			func(diags Diagnostics) Diagnostics {
+				diags = diags.Append(errors.New("oh no bad"))
+				return diags
+			},
+			[]diagFlat{
+				{
+					Severity: Error,
+					Summary:  "oh no bad",
+				},
+			},
+		},
+		"hcl.Diagnostic": {
+			func(diags Diagnostics) Diagnostics {
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Something bad happened",
+					Detail:   "It was really, really bad.",
+					Subject: &hcl.Range{
+						Filename: "foo.tf",
+						Start:    hcl.Pos{Line: 1, Column: 10, Byte: 9},
+						End:      hcl.Pos{Line: 2, Column: 3, Byte: 25},
+					},
+					Context: &hcl.Range{
+						Filename: "foo.tf",
+						Start:    hcl.Pos{Line: 1, Column: 1, Byte: 0},
+						End:      hcl.Pos{Line: 3, Column: 1, Byte: 30},
+					},
+				})
+				return diags
+			},
+			[]diagFlat{
+				{
+					Severity: Error,
+					Summary:  "Something bad happened",
+					Detail:   "It was really, really bad.",
+					Subject: &SourceRange{
+						Filename: "foo.tf",
+						Start:    SourcePos{Line: 1, Column: 10, Byte: 9},
+						End:      SourcePos{Line: 2, Column: 3, Byte: 25},
+					},
+					Context: &SourceRange{
+						Filename: "foo.tf",
+						Start:    SourcePos{Line: 1, Column: 1, Byte: 0},
+						End:      SourcePos{Line: 3, Column: 1, Byte: 30},
+					},
+				},
+			},
+		},
+		"hcl.Diagnostics": {
+			func(diags Diagnostics) Diagnostics {
+				diags = diags.Append(hcl.Diagnostics{
+					{
+						Severity: hcl.DiagError,
+						Summary:  "Something bad happened",
+						Detail:   "It was really, really bad.",
+					},
+					{
+						Severity: hcl.DiagWarning,
+						Summary:  "Also, somebody sneezed",
+						Detail:   "How rude!",
+					},
+				})
+				return diags
+			},
+			[]diagFlat{
+				{
+					Severity: Error,
+					Summary:  "Something bad happened",
+					Detail:   "It was really, really bad.",
+				},
+				{
+					Severity: Warning,
+					Summary:  "Also, somebody sneezed",
+					Detail:   "How rude!",
+				},
+			},
+		},
+		"multierror.Error": {
+			func(diags Diagnostics) Diagnostics {
+				err := multierror.Append(nil, errors.New("bad thing A"))
+				err = multierror.Append(err, errors.New("bad thing B"))
+				diags = diags.Append(err)
+				return diags
+			},
+			[]diagFlat{
+				{
+					Severity: Error,
+					Summary:  "bad thing A",
+				},
+				{
+					Severity: Error,
+					Summary:  "bad thing B",
+				},
+			},
+		},
+		"concat Diagnostics": {
+			func(diags Diagnostics) Diagnostics {
+				var moreDiags Diagnostics
+				moreDiags = moreDiags.Append(errors.New("bad thing A"))
+				moreDiags = moreDiags.Append(errors.New("bad thing B"))
+				return diags.Append(moreDiags)
+			},
+			[]diagFlat{
+				{
+					Severity: Error,
+					Summary:  "bad thing A",
+				},
+				{
+					Severity: Error,
+					Summary:  "bad thing B",
+				},
+			},
+		},
+		"single Diagnostic": {
+			func(diags Diagnostics) Diagnostics {
+				return diags.Append(SimpleWarning("Don't forget your toothbrush!"))
+			},
+			[]diagFlat{
+				{
+					Severity: Warning,
+					Summary:  "Don't forget your toothbrush!",
+				},
+			},
+		},
+		"multiple appends": {
+			func(diags Diagnostics) Diagnostics {
+				diags = diags.Append(SimpleWarning("Don't forget your toothbrush!"))
+				diags = diags.Append(fmt.Errorf("exploded"))
+				return diags
+			},
+			[]diagFlat{
+				{
+					Severity: Warning,
+					Summary:  "Don't forget your toothbrush!",
+				},
+				{
+					Severity: Error,
+					Summary:  "exploded",
+				},
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			gotDiags := test.Cons(nil)
+			var got []diagFlat
+			for _, item := range gotDiags {
+				desc := item.Description()
+				source := item.Source()
+				got = append(got, diagFlat{
+					Severity: item.Severity(),
+					Summary:  desc.Summary,
+					Detail:   desc.Detail,
+					Subject:  source.Subject,
+					Context:  source.Context,
+				})
+			}
+
+			if !reflect.DeepEqual(got, test.Want) {
+				t.Errorf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(test.Want))
+			}
+		})
+	}
+}
+
+func TestDiagnosticsErr(t *testing.T) {
+	t.Run("empty", func(t *testing.T) {
+		var diags Diagnostics
+		err := diags.Err()
+		if err != nil {
+			t.Errorf("got non-nil error %#v; want nil", err)
+		}
+	})
+	t.Run("warning only", func(t *testing.T) {
+		var diags Diagnostics
+		diags = diags.Append(SimpleWarning("bad"))
+		err := diags.Err()
+		if err != nil {
+			t.Errorf("got non-nil error %#v; want nil", err)
+		}
+	})
+	t.Run("one error", func(t *testing.T) {
+		var diags Diagnostics
+		diags = diags.Append(errors.New("didn't work"))
+		err := diags.Err()
+		if err == nil {
+			t.Fatalf("got nil error %#v; want non-nil", err)
+		}
+		if got, want := err.Error(), "didn't work"; got != want {
+			t.Errorf("wrong error message\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+	t.Run("two errors", func(t *testing.T) {
+		var diags Diagnostics
+		diags = diags.Append(errors.New("didn't work"))
+		diags = diags.Append(errors.New("didn't work either"))
+		err := diags.Err()
+		if err == nil {
+			t.Fatalf("got nil error %#v; want non-nil", err)
+		}
+		want := strings.TrimSpace(`
+2 problems:
+
+- didn't work
+- didn't work either
+`)
+		if got := err.Error(); got != want {
+			t.Errorf("wrong error message\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+	t.Run("error and warning", func(t *testing.T) {
+		var diags Diagnostics
+		diags = diags.Append(errors.New("didn't work"))
+		diags = diags.Append(SimpleWarning("didn't work either"))
+		err := diags.Err()
+		if err == nil {
+			t.Fatalf("got nil error %#v; want non-nil", err)
+		}
+		// Since this "as error" mode is just a fallback for
+		// non-diagnostics-aware situations like tests, we don't actually
+		// distinguish warnings and errors here since the point is to just
+		// get the messages rendered. User-facing code should be printing
+		// each diagnostic separately, so won't enter this codepath,
+		want := strings.TrimSpace(`
+2 problems:
+
+- didn't work
+- didn't work either
+`)
+		if got := err.Error(); got != want {
+			t.Errorf("wrong error message\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+}

tfdiags/doc.go

@@ -0,0 +1,16 @@
+// Package tfdiags is a utility package for representing errors and
+// warnings in a manner that allows us to produce good messages for the
+// user.
+//
+// "diag" is short for "diagnostics", and is meant as a general word for
+// feedback to a user about potential or actual problems.
+//
+// A design goal for this package is for it to be able to provide rich
+// messaging where possible but to also be pragmatic about dealing with
+// generic errors produced by system components that _can't_ provide
+// such rich messaging. As a consequence, the main types in this package --
+// Diagnostics and Diagnostic -- are designed so that they can be "smuggled"
+// over an error channel and then be unpacked at the other end, so that
+// error diagnostics (at least) can transit through APIs that are not
+// aware of this package.
+package tfdiags

tfdiags/error.go

@@ -0,0 +1,23 @@
+package tfdiags
+
+// nativeError is a Diagnostic implementation that wraps a normal Go error
+type nativeError struct {
+	err error
+}
+
+var _ Diagnostic = nativeError{}
+
+func (e nativeError) Severity() Severity {
+	return Error
+}
+
+func (e nativeError) Description() Description {
+	return Description{
+		Summary: e.err.Error(),
+	}
+}
+
+func (e nativeError) Source() Source {
+	// No source information available for a native error
+	return Source{}
+}

tfdiags/hcl.go

@@ -0,0 +1,63 @@
+package tfdiags
+
+import (
+	"github.com/hashicorp/hcl2/hcl"
+)
+
+// hclDiagnostic is a Diagnostic implementation that wraps a HCL Diagnostic
+type hclDiagnostic struct {
+	diag *hcl.Diagnostic
+}
+
+var _ Diagnostic = hclDiagnostic{}
+
+func (d hclDiagnostic) Severity() Severity {
+	switch d.diag.Severity {
+	case hcl.DiagWarning:
+		return Warning
+	default:
+		return Error
+	}
+}
+
+func (d hclDiagnostic) Description() Description {
+	return Description{
+		Summary: d.diag.Summary,
+		Detail:  d.diag.Detail,
+	}
+}
+
+func (d hclDiagnostic) Source() Source {
+	var ret Source
+	if d.diag.Subject != nil {
+		ret.Subject = &SourceRange{
+			Filename: d.diag.Subject.Filename,
+			Start: SourcePos{
+				Line:   d.diag.Subject.Start.Line,
+				Column: d.diag.Subject.Start.Column,
+				Byte:   d.diag.Subject.Start.Byte,
+			},
+			End: SourcePos{
+				Line:   d.diag.Subject.End.Line,
+				Column: d.diag.Subject.End.Column,
+				Byte:   d.diag.Subject.End.Byte,
+			},
+		}
+	}
+	if d.diag.Context != nil {
+		ret.Context = &SourceRange{
+			Filename: d.diag.Context.Filename,
+			Start: SourcePos{
+				Line:   d.diag.Context.Start.Line,
+				Column: d.diag.Context.Start.Column,
+				Byte:   d.diag.Context.Start.Byte,
+			},
+			End: SourcePos{
+				Line:   d.diag.Context.End.Line,
+				Column: d.diag.Context.End.Column,
+				Byte:   d.diag.Context.End.Byte,
+			},
+		}
+	}
+	return ret
+}

tfdiags/severity_string.go

@@ -0,0 +1,26 @@
+// Code generated by "stringer -type=Severity"; DO NOT EDIT.
+
+package tfdiags
+
+import "fmt"
+
+const (
+	_Severity_name_0 = "Error"
+	_Severity_name_1 = "Warning"
+)
+
+var (
+	_Severity_index_0 = [...]uint8{0, 5}
+	_Severity_index_1 = [...]uint8{0, 7}
+)
+
+func (i Severity) String() string {
+	switch {
+	case i == 69:
+		return _Severity_name_0
+	case i == 87:
+		return _Severity_name_1
+	default:
+		return fmt.Sprintf("Severity(%d)", i)
+	}
+}

tfdiags/simple_warning.go

@@ -0,0 +1,25 @@
+package tfdiags
+
+type simpleWarning string
+
+var _ Diagnostic = simpleWarning("")
+
+// SimpleWarning constructs a simple (summary-only) warning diagnostic.
+func SimpleWarning(msg string) Diagnostic {
+	return simpleWarning(msg)
+}
+
+func (e simpleWarning) Severity() Severity {
+	return Warning
+}
+
+func (e simpleWarning) Description() Description {
+	return Description{
+		Summary: string(e),
+	}
+}
+
+func (e simpleWarning) Source() Source {
+	// No source information available for a native error
+	return Source{}
+}

tfdiags/source_range.go

@@ -0,0 +1,10 @@
+package tfdiags
+
+type SourceRange struct {
+	Filename   string
+	Start, End SourcePos
+}
+
+type SourcePos struct {
+	Line, Column, Byte int
+}