command/format: Include function call information in diagnostics

When an error occurs in a function call, the error message text often
includes references to particular parameters in the function signature.

This commit improves that reporting by also including a summary of the
full function signature as part of the diagnostic context in that case,
so a reader can see which parameter is which given that function
arguments are always assigned positionally and so the parameter names
do not appear in the caller's source code.
pull/31321/head
Martin Atkins 4 years ago
parent 8405f46bc5
commit 31aee9650e

@ -278,7 +278,7 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color *
)
}
if len(snippet.Values) > 0 {
if len(snippet.Values) > 0 || (snippet.FunctionCall != nil && snippet.FunctionCall.Signature != nil) {
// The diagnostic may also have information about the dynamic
// values of relevant variables at the point of evaluation.
// This is particularly useful for expressions that get evaluated
@ -291,6 +291,24 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color *
})
fmt.Fprint(buf, color.Color(" [dark_gray]├────────────────[reset]\n"))
if callInfo := snippet.FunctionCall; callInfo != nil && callInfo.Signature != nil {
fmt.Fprintf(buf, color.Color(" [dark_gray]│[reset] while calling [bold]%s[reset]("), callInfo.CalledAs)
for i, param := range callInfo.Signature.Params {
if i > 0 {
buf.WriteString(", ")
}
buf.WriteString(param.Name)
}
if param := callInfo.Signature.VariadicParam; param != nil {
if len(callInfo.Signature.Params) > 0 {
buf.WriteString(", ")
}
buf.WriteString(param.Name)
buf.WriteString("...")
}
buf.WriteString(")\n")
}
for _, value := range values {
fmt.Fprintf(buf, color.Color(" [dark_gray]│[reset] [bold]%s[reset] %s\n"), value.Traversal, value.Statement)
}

@ -6,9 +6,11 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/hcl/v2/hcltest"
"github.com/mitchellh/colorstring"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
viewsjson "github.com/hashicorp/terraform/internal/command/views/json"
"github.com/hashicorp/terraform/internal/lang/marks"
@ -207,6 +209,54 @@ func TestDiagnostic(t *testing.T) {
[red][reset]
[red][reset] Whatever shall we do?
[red][reset]
`,
},
"error with source code subject and function call annotation": {
&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Bad bad bad",
Detail: "Whatever shall we do?",
Subject: &hcl.Range{
Filename: "test.tf",
Start: hcl.Pos{Line: 1, Column: 6, Byte: 5},
End: hcl.Pos{Line: 1, Column: 12, Byte: 11},
},
Expression: hcltest.MockExprLiteral(cty.True),
EvalContext: &hcl.EvalContext{
Functions: map[string]function.Function{
"beep": function.New(&function.Spec{
Params: []function.Parameter{
{
Name: "pos_param_0",
Type: cty.String,
},
{
Name: "pos_param_1",
Type: cty.Number,
},
},
VarParam: &function.Parameter{
Name: "var_param",
Type: cty.Bool,
},
}),
},
},
// This is simulating what the HCL function call expression
// type would generate on evaluation, by implementing the
// same interface it uses.
Extra: fakeDiagFunctionCallExtra("beep"),
},
`[red][reset]
[red][reset] [bold][red]Error: [reset][bold]Bad bad bad[reset]
[red][reset]
[red][reset] on test.tf line 1:
[red][reset] 1: test [underline]source[reset] code
[red][reset] [dark_gray][reset]
[red][reset] [dark_gray][reset] while calling [bold]beep[reset](pos_param_0, pos_param_1, var_param...)
[red][reset]
[red][reset] Whatever shall we do?
[red][reset]
`,
},
}
@ -755,3 +805,18 @@ func TestDiagnosticFromJSON_invalid(t *testing.T) {
})
}
}
// fakeDiagFunctionCallExtra is a fake implementation of the interface that
// HCL uses to provide "extra information" associated with diagnostics that
// describe errors during a function call.
type fakeDiagFunctionCallExtra string
var _ hclsyntax.FunctionCallDiagExtra = fakeDiagFunctionCallExtra("")
func (e fakeDiagFunctionCallExtra) CalledFunctionName() string {
return string(e)
}
func (e fakeDiagFunctionCallExtra) FunctionCallError() error {
return nil
}

@ -10,6 +10,7 @@ import (
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hcled"
"github.com/hashicorp/hcl/v2/hclparse"
"github.com/hashicorp/hcl/v2/hclsyntax"
"github.com/hashicorp/terraform/internal/lang/marks"
"github.com/hashicorp/terraform/internal/tfdiags"
"github.com/zclconf/go-cty/cty"
@ -95,6 +96,10 @@ type DiagnosticSnippet struct {
// Values is a sorted slice of expression values which may be useful in
// understanding the source of an error in a complex expression.
Values []DiagnosticExpressionValue `json:"values"`
// FunctionCall is information about a function call whose failure is
// being reported by this diagnostic, if any.
FunctionCall *DiagnosticFunctionCall `json:"function_call,omitempty"`
}
// DiagnosticExpressionValue represents an HCL traversal string (e.g.
@ -107,6 +112,20 @@ type DiagnosticExpressionValue struct {
Statement string `json:"statement"`
}
// DiagnosticFunctionCall represents a function call whose information is
// being included as part of a diagnostic snippet.
type DiagnosticFunctionCall struct {
// CalledAs is the full name that was used to call this function,
// potentially including namespace prefixes if the function does not belong
// to the default function namespace.
CalledAs string `json:"called_as"`
// Signature is a description of the signature of the function that was
// called, if any. Might be omitted if we're reporting that a call failed
// because the given function name isn't known, for example.
Signature *Function `json:"signature,omitempty"`
}
// NewDiagnostic takes a tfdiags.Diagnostic and a map of configuration sources,
// and returns a Diagnostic struct.
func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnostic {
@ -294,7 +313,24 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost
return values[i].Traversal < values[j].Traversal
})
diagnostic.Snippet.Values = values
if callInfo := tfdiags.ExtraInfo[hclsyntax.FunctionCallDiagExtra](diag); callInfo != nil && callInfo.CalledFunctionName() != "" {
calledAs := callInfo.CalledFunctionName()
baseName := calledAs
if idx := strings.LastIndex(baseName, "::"); idx >= 0 {
baseName = baseName[idx+2:]
}
callInfo := &DiagnosticFunctionCall{
CalledAs: calledAs,
}
if f, ok := ctx.Functions[calledAs]; ok {
callInfo.Signature = DescribeFunction(baseName, f)
}
diagnostic.Snippet.FunctionCall = callInfo
}
}
}
}

@ -0,0 +1,112 @@
package json
import (
"encoding/json"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
)
// Function is a description of the JSON representation of the signature of
// a function callable from the Terraform language.
type Function struct {
// Name is the leaf name of the function, without any namespace prefix.
Name string `json:"name"`
Params []FunctionParam `json:"params"`
VariadicParam *FunctionParam `json:"variadic_param,omitempty"`
// ReturnType is type constraint which is a static approximation of the
// possibly-dynamic return type of the function.
ReturnType json.RawMessage `json:"return_type"`
Description string `json:"description,omitempty"`
DescriptionKind string `json:"description_kind,omitempty"`
}
// FunctionParam represents a single parameter to a function, as represented
// by type Function.
type FunctionParam struct {
// Name is a name for the function which is used primarily for
// documentation purposes, because function arguments are positional
// and therefore don't appear directly in configuration source code.
Name string `json:"name"`
// Type is a type constraint which is a static approximation of the
// possibly-dynamic type of the parameter. Particular functions may
// have additional requirements that a type constraint alone cannot
// represent.
Type json.RawMessage `json:"type"`
// Maybe some of the other fields in function.Parameter would be
// interesting to describe here too, but we'll wait to see if there
// is a use-case first.
Description string `json:"description,omitempty"`
DescriptionKind string `json:"description_kind,omitempty"`
}
// DescribeFunction returns a description of the signature of the given cty
// function, as a pointer to this package's serializable type Function.
func DescribeFunction(name string, f function.Function) *Function {
ret := &Function{
Name: name,
}
params := f.Params()
ret.Params = make([]FunctionParam, len(params))
typeCheckArgs := make([]cty.Type, len(params), len(params)+1)
for i, param := range params {
ret.Params[i] = describeFunctionParam(&param)
typeCheckArgs[i] = param.Type
}
if varParam := f.VarParam(); varParam != nil {
descParam := describeFunctionParam(varParam)
ret.VariadicParam = &descParam
typeCheckArgs = append(typeCheckArgs, varParam.Type)
}
retType, err := f.ReturnType(typeCheckArgs)
if err != nil {
// Getting an error when type-checking with exactly the type constraints
// the function called for is weird, so we'll just treat it as if it
// has a dynamic return type instead, for our purposes here.
// One reason this can happen is for a function which has a variadic
// parameter but has logic inside it which considers it invalid to
// specify exactly one argument for that parameter (since that's what
// we did in typeCheckArgs as an approximation of a valid call above.)
retType = cty.DynamicPseudoType
}
if raw, err := retType.MarshalJSON(); err != nil {
// Again, we'll treat any errors as if the function is dynamically
// typed because it would be weird to get here.
ret.ReturnType = json.RawMessage(`"dynamic"`)
} else {
ret.ReturnType = json.RawMessage(raw)
}
// We don't currently have any sense of descriptions for functions and
// their parameters, so we'll just leave those fields unpopulated for now.
return ret
}
func describeFunctionParam(p *function.Parameter) FunctionParam {
ret := FunctionParam{
Name: p.Name,
}
if raw, err := p.Type.MarshalJSON(); err != nil {
// We'll treat any errors as if the function is dynamically
// typed because it would be weird to get here.
ret.Type = json.RawMessage(`"dynamic"`)
} else {
ret.Type = json.RawMessage(raw)
}
// We don't currently have any sense of descriptions for functions and
// their parameters, so we'll just leave those fields unpopulated for now.
return ret
}

@ -0,0 +1,92 @@
package json
import (
"encoding/json"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/zclconf/go-cty/cty/function"
"github.com/zclconf/go-cty/cty/function/stdlib"
)
func TestDescribeFunction(t *testing.T) {
// NOTE: This test case is referring to some real functions in other
// packages. and so if those functions change signature later it will
// probably make some cases here fail. If that is the cause of the failure,
// it's fine to update the test here to match rather than to revert the
// change to the function signature, as long as the change to the
// function signature is otherwise within the bounds of our compatibility
// promises.
tests := map[string]struct {
Function function.Function
Want *Function
}{
"upper": {
Function: stdlib.UpperFunc,
Want: &Function{
Name: "upper",
Params: []FunctionParam{
{
Name: "str",
Type: json.RawMessage(`"string"`),
},
},
ReturnType: json.RawMessage(`"string"`),
},
},
"coalesce": {
Function: stdlib.CoalesceFunc,
Want: &Function{
Name: "coalesce",
Params: []FunctionParam{},
VariadicParam: &FunctionParam{
Name: "vals",
Type: json.RawMessage(`"dynamic"`),
},
ReturnType: json.RawMessage(`"dynamic"`),
},
},
"join": {
Function: stdlib.JoinFunc,
Want: &Function{
Name: "join",
Params: []FunctionParam{
{
Name: "separator",
Type: json.RawMessage(`"string"`),
},
},
VariadicParam: &FunctionParam{
Name: "lists",
Type: json.RawMessage(`["list","string"]`),
},
ReturnType: json.RawMessage(`"string"`),
},
},
"jsonencode": {
Function: stdlib.JSONEncodeFunc,
Want: &Function{
Name: "jsonencode",
Params: []FunctionParam{
{
Name: "val",
Type: json.RawMessage(`"dynamic"`),
},
},
ReturnType: json.RawMessage(`"string"`),
},
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
got := DescribeFunction(name, test.Function)
want := test.Want
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("wrong result\n%s", diff)
}
})
}
}
Loading…
Cancel
Save