Skip to content

Commit

Permalink
hcl: Annotate diagnostics with expression EvalContext
Browse files Browse the repository at this point in the history
When we're evaluating expressions, we may end up evaluating the same
source-level expression a number of times in different contexts, such as
in a 'for' expression, where each one may produce a different set of
diagnostic messages.

Now we'll attach the EvalContext to each expression diagnostic so that
a diagnostic renderer can potentially show additional information to help
distinguish the different iterations in rendered diagnostics.
  • Loading branch information
apparentlymart committed Jul 28, 2018
1 parent 41cff85 commit 93562f8
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 171 deletions.
45 changes: 25 additions & 20 deletions ext/dynblock/expand_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,21 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc

if !eachVal.CanIterateElements() {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid dynamic for_each value",
Detail: fmt.Sprintf("Cannot use a value of type %s in for_each. An iterable collection is required.", eachVal.Type()),
Subject: eachAttr.Expr.Range().Ptr(),
Severity: hcl.DiagError,
Summary: "Invalid dynamic for_each value",
Detail: fmt.Sprintf("Cannot use a value of type %s in for_each. An iterable collection is required.", eachVal.Type()),
Subject: eachAttr.Expr.Range().Ptr(),
EvalContext: b.forEachCtx,
})
return nil, diags
}
if eachVal.IsNull() {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid dynamic for_each value",
Detail: "Cannot use a null value in for_each.",
Subject: eachAttr.Expr.Range().Ptr(),
Severity: hcl.DiagError,
Summary: "Invalid dynamic for_each value",
Detail: "Cannot use a null value in for_each.",
Subject: eachAttr.Expr.Range().Ptr(),
EvalContext: b.forEachCtx,
})
return nil, diags
}
Expand Down Expand Up @@ -159,28 +161,31 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h
labelVal, convErr = convert.Convert(labelVal, cty.String)
if convErr != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid dynamic block label",
Detail: fmt.Sprintf("Cannot use this value as a dynamic block label: %s.", convErr),
Subject: labelExpr.Range().Ptr(),
Severity: hcl.DiagError,
Summary: "Invalid dynamic block label",
Detail: fmt.Sprintf("Cannot use this value as a dynamic block label: %s.", convErr),
Subject: labelExpr.Range().Ptr(),
EvalContext: lCtx,
})
return nil, diags
}
if labelVal.IsNull() {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid dynamic block label",
Detail: "Cannot use a null value as a dynamic block label.",
Subject: labelExpr.Range().Ptr(),
Severity: hcl.DiagError,
Summary: "Invalid dynamic block label",
Detail: "Cannot use a null value as a dynamic block label.",
Subject: labelExpr.Range().Ptr(),
EvalContext: lCtx,
})
return nil, diags
}
if !labelVal.IsKnown() {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid dynamic block label",
Detail: "This value is not yet known. Dynamic block labels must be immediately-known values.",
Subject: labelExpr.Range().Ptr(),
Severity: hcl.DiagError,
Summary: "Invalid dynamic block label",
Detail: "This value is not yet known. Dynamic block labels must be immediately-known values.",
Subject: labelExpr.Range().Ptr(),
EvalContext: lCtx,
})
return nil, diags
}
Expand Down
31 changes: 30 additions & 1 deletion hcl/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,43 @@ const (
type Diagnostic struct {
Severity DiagnosticSeverity

// Summary and detail contain the English-language description of the
// Summary and Detail contain the English-language description of the
// problem. Summary is a terse description of the general problem and
// detail is a more elaborate, often-multi-sentence description of
// the probem and what might be done to solve it.
Summary string
Detail string

// Subject and Context are both source ranges relating to the diagnostic.
//
// Subject is a tight range referring to exactly the construct that
// is problematic, while Context is an optional broader range (which should
// fully contain Subject) that ought to be shown around Subject when
// generating isolated source-code snippets in diagnostic messages.
// If Context is nil, the Subject is also the Context.
//
// Some diagnostics have no source ranges at all. If Context is set then
// Subject should always also be set.
Subject *Range
Context *Range

// For diagnostics that occur when evaluating an expression, EvalContext
// may point to the EvalContext that was active when evaluating that
// expression, which may allow for the inclusion of additional useful
// information when rendering a diagnostic message to the user.
//
// It is not always possible to select a single EvalContext for a
// diagnostic, and so in some cases this field may be nil even when an
// expression causes a problem. Therefore it is not valid to use the
// nil-ness of this field to definitively decide whether a diagnostic
// relates to an expression.
//
// EvalContexts form a tree, so the given EvalContext may refer to a parent
// which in turn refers to another parent, etc. For a full picture of all
// of the active variables and functions the caller must walk up this
// chain, preferring definitions that are "closer" to the expression in
// case of colliding names.
EvalContext *EvalContext
}

// Diagnostics is a list of Diagnostic instances.
Expand Down
22 changes: 22 additions & 0 deletions hcl/hclsyntax/diagnostics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package hclsyntax

import (
"github.com/hashicorp/hcl2/hcl"
)

// setDiagEvalContext is an internal helper that will impose a particular
// EvalContext on a set of diagnostics in-place, for any diagnostic that
// does not already have an EvalContext set.
//
// We generally expect diagnostics to be immutable, but this is safe to use
// on any Diagnostics where none of the contained Diagnostic objects have yet
// been seen by a caller. Its purpose is to apply additional context to a
// set of diagnostics produced by a "deeper" component as the stack unwinds
// during expression evaluation.
func setDiagEvalContext(diags hcl.Diagnostics, ctx *hcl.EvalContext) {
for _, diag := range diags {
if diag.EvalContext == nil {
diag.EvalContext = ctx
}
}
}
Loading

0 comments on commit 93562f8

Please sign in to comment.