Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hcl: Allow individual diagnostics to carry extra information #539

Merged
merged 1 commit into from Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 43 additions & 0 deletions diagnostic.go
Expand Up @@ -63,6 +63,28 @@ type Diagnostic struct {
// case of colliding names.
Expression Expression
EvalContext *EvalContext

// Extra is an extension point for additional machine-readable information
// about this problem.
//
// Recipients of diagnostic objects may type-assert this value with
// specific interface types they know about to discover if any additional
// information is available that is interesting for their use-case.
//
// Extra is always considered to be optional extra information and so a
// diagnostic message should still always be fully described (from the
// perspective of a human who understands the language the messages are
// written in) by the other fields in case a particular recipient.
//
// Functions that return diagnostics with Extra populated should typically
// document that they place values implementing a particular interface,
// rather than a concrete type, and define that interface such that its
// methods can dynamically indicate a lack of support at runtime even
// if the interface happens to be statically available. An Extra
// type that wraps other Extra values should additionally implement
// interface DiagnosticExtraUnwrapper to return the value they are wrapping
// so that callers can access inner values to type-assert against.
Extra interface{}
}

// Diagnostics is a list of Diagnostic instances.
Expand Down Expand Up @@ -141,3 +163,24 @@ type DiagnosticWriter interface {
WriteDiagnostic(*Diagnostic) error
WriteDiagnostics(Diagnostics) error
}

// DiagnosticExtraUnwrapper is an interface implemented by values in the
// Extra field of Diagnostic when they are wrapping another "Extra" value that
// was generated downstream.
//
// Diagnostic recipients which want to examine "Extra" values to sniff for
// particular types of extra data can either type-assert this interface
// directly and repeatedly unwrap until they recieve nil, or can use the
// helper function DiagnosticExtra.
type DiagnosticExtraUnwrapper interface {
// If the reciever is wrapping another "diagnostic extra" value, returns
// that value. Otherwise returns nil to indicate dynamically that nothing
// is wrapped.
//
// The "nothing is wrapped" condition can be signalled either by this
// method returning nil or by a type not implementing this interface at all.
//
// Implementers should never create unwrap "cycles" where a nested extra
// value returns a value that was also wrapping it.
UnwrapDiagnosticExtra() interface{}
}
39 changes: 39 additions & 0 deletions diagnostic_typeparams.go
@@ -0,0 +1,39 @@
//go:build go1.18
// +build go1.18

package hcl

// This file contains additional diagnostics-related symbols that use the
// Go 1.18 type parameters syntax and would therefore be incompatible with
// Go 1.17 and earlier.

// DiagnosticExtra attempts to retrieve an "extra value" of type T from the
// given diagnostic, if either the diag.Extra field directly contains a value
// of that type or the value implements DiagnosticExtraUnwrapper and directly
// or indirectly returns a value of that type.
//
// Type T should typically be an interface type, so that code which generates
// diagnostics can potentially return different implementations of the same
// interface dynamically as needed.
//
// If a value of type T is found, returns that value and true to indicate
// success. Otherwise, returns the zero value of T and false to indicate
// failure.
func DiagnosticExtra[T any](diag *Diagnostic) (T, bool) {
extra := diag.Extra
var zero T

for {
if ret, ok := extra.(T); ok {
return ret, true
}

if unwrap, ok := extra.(DiagnosticExtraUnwrapper); ok {
// If our "extra" implements DiagnosticExtraUnwrapper then we'll
// unwrap one level and try this again.
extra = unwrap.UnwrapDiagnosticExtra()
} else {
return zero, false
}
}
}
2 changes: 1 addition & 1 deletion go.mod
@@ -1,6 +1,6 @@
module github.com/hashicorp/hcl/v2

go 1.17
go 1.18

require (
github.com/agext/levenshtein v1.2.1
Expand Down
53 changes: 52 additions & 1 deletion hclsyntax/expression.go
Expand Up @@ -26,7 +26,7 @@ type Expression interface {
}

// Assert that Expression implements hcl.Expression
var assertExprImplExpr hcl.Expression = Expression(nil)
var _ hcl.Expression = Expression(nil)

// ParenthesesExpr represents an expression written in grouping
// parentheses.
Expand Down Expand Up @@ -270,6 +270,10 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
}
}

diagExtra := functionCallDiagExtra{
calledFunctionName: e.Name,
}

params := f.Params()
varParam := f.VarParam()

Expand Down Expand Up @@ -297,6 +301,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: expandExpr,
EvalContext: ctx,
Extra: &diagExtra,
})
return cty.DynamicVal, diags
}
Expand All @@ -311,6 +316,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: expandExpr,
EvalContext: ctx,
Extra: &diagExtra,
})
return cty.DynamicVal, diags
}
Expand Down Expand Up @@ -342,6 +348,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: expandExpr,
EvalContext: ctx,
Extra: &diagExtra,
})
return cty.DynamicVal, diags
}
Expand All @@ -365,6 +372,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
Extra: &diagExtra,
},
}
}
Expand All @@ -382,6 +390,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
Extra: &diagExtra,
},
}
}
Expand Down Expand Up @@ -426,6 +435,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: argExpr,
EvalContext: ctx,
Extra: &diagExtra,
})
}
}
Expand All @@ -442,6 +452,10 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti

resultVal, err := f.Call(argVals)
if err != nil {
// For errors in the underlying call itself we also return the raw
// call error via an extra method on our "diagnostic extra" value.
diagExtra.functionCallError = err

switch terr := err.(type) {
case function.ArgError:
i := terr.Index
Expand Down Expand Up @@ -479,6 +493,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Subject: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
Extra: &diagExtra,
})
default:
// This is the most degenerate case of all, where the
Expand All @@ -497,6 +512,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
Extra: &diagExtra,
})
}
} else {
Expand All @@ -515,6 +531,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: argExpr,
EvalContext: ctx,
Extra: &diagExtra,
})
}

Expand All @@ -530,6 +547,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
Extra: &diagExtra,
})
}

Expand Down Expand Up @@ -562,6 +580,39 @@ func (e *FunctionCallExpr) ExprCall() *hcl.StaticCall {
return ret
}

// FunctionCallDiagExtra is an interface implemented by the value in the "Extra"
// field of some diagnostics returned by FunctionCallExpr.Value, giving
// cooperating callers access to some machine-readable information about the
// call that a diagnostic relates to.
type FunctionCallDiagExtra interface {
// CalledFunctionName returns the name of the function being called at
// the time the diagnostic was generated, if any. Returns an empty string
// if there is no known called function.
CalledFunctionName() string

// FunctionCallError returns the error value returned by the implementation
// of the function being called, if any. Returns nil if the diagnostic was
// not returned in response to a call error.
//
// Some errors related to calling functions are generated by HCL itself
// rather than by the underlying function, in which case this method
// will return nil.
FunctionCallError() error
}

type functionCallDiagExtra struct {
calledFunctionName string
functionCallError error
}

func (e *functionCallDiagExtra) CalledFunctionName() string {
return e.calledFunctionName
}

func (e *functionCallDiagExtra) FunctionCallError() error {
return e.functionCallError
}

type ConditionalExpr struct {
Condition Expression
TrueResult Expression
Expand Down
102 changes: 102 additions & 0 deletions hclsyntax/expression_typeparams_test.go
@@ -0,0 +1,102 @@
//go:build go1.18
// +build go1.18

package hclsyntax

import (
"fmt"
"testing"

"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
)

// This file contains some additional tests that only make sense when using
// a Go compiler which supports type parameters (Go 1.18 or later).

func TestExpressionDiagnosticExtra(t *testing.T) {
tests := []struct {
input string
ctx *hcl.EvalContext
assert func(t *testing.T, diags hcl.Diagnostics)
}{
// Error messages describing inconsistent result types for conditional expressions.
{
"boop()",
&hcl.EvalContext{
Functions: map[string]function.Function{
"boop": function.New(&function.Spec{
Type: function.StaticReturnType(cty.String),
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
return cty.DynamicVal, fmt.Errorf("the expected error")
},
}),
},
},
func(t *testing.T, diags hcl.Diagnostics) {
try := func(diags hcl.Diagnostics) {
t.Helper()
for _, diag := range diags {
extra, ok := hcl.DiagnosticExtra[FunctionCallDiagExtra](diag)
if !ok {
continue
}

if got, want := extra.CalledFunctionName(), "boop"; got != want {
t.Errorf("wrong called function name %q; want %q", got, want)
}
err := extra.FunctionCallError()
if err == nil {
t.Fatal("FunctionCallError returned nil")
}
if got, want := err.Error(), "the expected error"; got != want {
t.Errorf("wrong error message\ngot: %q\nwant: %q", got, want)
}

return
}
t.Fatalf("None of the returned diagnostics implement FunctionCallDiagError\n%s", diags.Error())
}

t.Run("unwrapped", func(t *testing.T) {
try(diags)
})

// It should also work if we wrap up the "extras" in wrapper types.
for _, diag := range diags {
diag.Extra = diagnosticExtraWrapper{diag.Extra}
}
t.Run("wrapped", func(t *testing.T) {
try(diags)
})
},
},
}

for _, test := range tests {
t.Run(test.input, func(t *testing.T) {
var diags hcl.Diagnostics
expr, parseDiags := ParseExpression([]byte(test.input), "", hcl.Pos{Line: 1, Column: 1, Byte: 0})
diags = append(diags, parseDiags...)
_, valDiags := expr.Value(test.ctx)
diags = append(diags, valDiags...)

if !diags.HasErrors() {
t.Fatal("unexpected success")
}

test.assert(t, diags)
})
}
}

type diagnosticExtraWrapper struct {
wrapped interface{}
}

var _ hcl.DiagnosticExtraUnwrapper = diagnosticExtraWrapper{}

func (w diagnosticExtraWrapper) UnwrapDiagnosticExtra() interface{} {
return w.wrapped
}