Skip to content

Commit

Permalink
hcl: Include Expression reference in diagnostics
Browse files Browse the repository at this point in the history
If a diagnostic occurs while we're evaluating an expression, we'll now
include a reference to that expression in the diagnostic object. We
previously added the corresponding EvalContext here too, and so with these
together it is now possible for a diagnostic renderer to see not only
what was in scope when the problem occurred but also what parts of that
scope the expression was relying on (via method Expression.Variables).
  • Loading branch information
apparentlymart committed Jul 28, 2018
1 parent 956c336 commit 6356254
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 17 deletions.
5 changes: 5 additions & 0 deletions ext/dynblock/expand_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc
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(),
Expression: eachAttr.Expr,
EvalContext: b.forEachCtx,
})
return nil, diags
Expand All @@ -57,6 +58,7 @@ func (b *expandBody) decodeSpec(blockS *hcl.BlockHeaderSchema, rawSpec *hcl.Bloc
Summary: "Invalid dynamic for_each value",
Detail: "Cannot use a null value in for_each.",
Subject: eachAttr.Expr.Range().Ptr(),
Expression: eachAttr.Expr,
EvalContext: b.forEachCtx,
})
return nil, diags
Expand Down Expand Up @@ -165,6 +167,7 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h
Summary: "Invalid dynamic block label",
Detail: fmt.Sprintf("Cannot use this value as a dynamic block label: %s.", convErr),
Subject: labelExpr.Range().Ptr(),
Expression: labelExpr,
EvalContext: lCtx,
})
return nil, diags
Expand All @@ -175,6 +178,7 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h
Summary: "Invalid dynamic block label",
Detail: "Cannot use a null value as a dynamic block label.",
Subject: labelExpr.Range().Ptr(),
Expression: labelExpr,
EvalContext: lCtx,
})
return nil, diags
Expand All @@ -185,6 +189,7 @@ func (s *expandSpec) newBlock(i *iteration, ctx *hcl.EvalContext) (*hcl.Block, h
Summary: "Invalid dynamic block label",
Detail: "This value is not yet known. Dynamic block labels must be immediately-known values.",
Subject: labelExpr.Range().Ptr(),
Expression: labelExpr,
EvalContext: lCtx,
})
return nil, diags
Expand Down
14 changes: 7 additions & 7 deletions hcl/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,22 @@ type Diagnostic struct {
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.
// For diagnostics that occur when evaluating an expression, Expression
// may refer to that expression and EvalContext may point to the
// EvalContext that was active when evaluating it. This 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.
// expression causes a problem.
//
// 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.
Expression Expression
EvalContext *EvalContext
}

Expand Down
5 changes: 3 additions & 2 deletions hcl/hclsyntax/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import (
// 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) {
func setDiagEvalContext(diags hcl.Diagnostics, expr hcl.Expression, ctx *hcl.EvalContext) {
for _, diag := range diags {
if diag.EvalContext == nil {
if diag.Expression == nil {
diag.Expression = expr
diag.EvalContext = ctx
}
}
Expand Down
30 changes: 23 additions & 7 deletions hcl/hclsyntax/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (e *ScopeTraversalExpr) walkChildNodes(w internalWalkFunc) {

func (e *ScopeTraversalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
val, diags := e.Traversal.TraverseAbs(ctx)
setDiagEvalContext(diags, ctx)
setDiagEvalContext(diags, e, ctx)
return val, diags
}

Expand Down Expand Up @@ -138,7 +138,7 @@ func (e *RelativeTraversalExpr) walkChildNodes(w internalWalkFunc) {
func (e *RelativeTraversalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
src, diags := e.Source.Value(ctx)
ret, travDiags := e.Traversal.TraverseRel(src)
setDiagEvalContext(travDiags, ctx)
setDiagEvalContext(travDiags, e, ctx)
diags = append(diags, travDiags...)
return ret, diags
}
Expand Down Expand Up @@ -214,6 +214,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Summary: "Function calls not allowed",
Detail: "Functions may not be called here.",
Subject: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
},
}
Expand All @@ -235,6 +236,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Detail: fmt.Sprintf("There is no function named %q.%s", e.Name, suggestion),
Subject: &e.NameRange,
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
},
}
Expand Down Expand Up @@ -263,8 +265,9 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Severity: hcl.DiagError,
Summary: "Invalid expanding argument value",
Detail: "The expanding argument (indicated by ...) must not be null.",
Context: expandExpr.Range().Ptr(),
Subject: e.Range().Ptr(),
Subject: expandExpr.Range().Ptr(),
Context: e.Range().Ptr(),
Expression: expandExpr,
EvalContext: ctx,
})
return cty.DynamicVal, diags
Expand All @@ -289,8 +292,9 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
Severity: hcl.DiagError,
Summary: "Invalid expanding argument value",
Detail: "The expanding argument (indicated by ...) must be of a tuple, list, or set type.",
Context: expandExpr.Range().Ptr(),
Subject: e.Range().Ptr(),
Subject: expandExpr.Range().Ptr(),
Context: e.Range().Ptr(),
Expression: expandExpr,
EvalContext: ctx,
})
return cty.DynamicVal, diags
Expand All @@ -313,6 +317,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
),
Subject: &e.CloseParenRange,
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
},
}
Expand All @@ -329,6 +334,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
),
Subject: args[len(params)].StartRange().Ptr(),
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
},
}
Expand Down Expand Up @@ -361,6 +367,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
),
Subject: argExpr.StartRange().Ptr(),
Context: e.Range().Ptr(),
Expression: argExpr,
EvalContext: ctx,
})
}
Expand Down Expand Up @@ -399,6 +406,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
),
Subject: argExpr.StartRange().Ptr(),
Context: e.Range().Ptr(),
Expression: argExpr,
EvalContext: ctx,
})

Expand All @@ -412,6 +420,7 @@ func (e *FunctionCallExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
),
Subject: e.StartRange().Ptr(),
Context: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
})
}
Expand Down Expand Up @@ -481,6 +490,7 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
),
Subject: hcl.RangeBetween(e.TrueResult.Range(), e.FalseResult.Range()).Ptr(),
Context: &e.SrcRange,
Expression: e,
EvalContext: ctx,
},
}
Expand All @@ -495,6 +505,7 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
Detail: "The condition value is null. Conditions must either be true or false.",
Subject: e.Condition.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.Condition,
EvalContext: ctx,
})
return cty.UnknownVal(resultType), diags
Expand All @@ -510,6 +521,7 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
Detail: fmt.Sprintf("The condition expression must be of type bool."),
Subject: e.Condition.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.Condition,
EvalContext: ctx,
})
return cty.UnknownVal(resultType), diags
Expand All @@ -531,6 +543,7 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
),
Subject: e.TrueResult.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.TrueResult,
EvalContext: ctx,
})
trueResult = cty.UnknownVal(resultType)
Expand All @@ -551,8 +564,9 @@ func (e *ConditionalExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostic
"The false result value has the wrong type: %s.",
err.Error(),
),
Subject: e.TrueResult.Range().Ptr(),
Subject: e.FalseResult.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.FalseResult,
EvalContext: ctx,
})
falseResult = cty.UnknownVal(resultType)
Expand Down Expand Up @@ -697,6 +711,7 @@ func (e *ObjectConsExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics
Summary: "Null value as key",
Detail: "Can't use a null value as a key.",
Subject: item.ValueExpr.Range().Ptr(),
Expression: item.KeyExpr,
EvalContext: ctx,
})
known = false
Expand All @@ -711,6 +726,7 @@ func (e *ObjectConsExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics
Summary: "Incorrect key type",
Detail: fmt.Sprintf("Can't use this value as a key: %s.", err.Error()),
Subject: item.ValueExpr.Range().Ptr(),
Expression: item.ValueExpr,
EvalContext: ctx,
})
known = false
Expand Down
5 changes: 5 additions & 0 deletions hcl/hclsyntax/expression_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func (e *BinaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics)
Detail: fmt.Sprintf("Unsuitable value for left operand: %s.", err),
Subject: e.LHS.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.LHS,
EvalContext: ctx,
})
}
Expand All @@ -165,6 +166,7 @@ func (e *BinaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics)
Detail: fmt.Sprintf("Unsuitable value for right operand: %s.", err),
Subject: e.RHS.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.RHS,
EvalContext: ctx,
})
}
Expand All @@ -184,6 +186,7 @@ func (e *BinaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics)
Summary: "Operation failed",
Detail: fmt.Sprintf("Error during operation: %s.", err),
Subject: &e.SrcRange,
Expression: e,
EvalContext: ctx,
})
return cty.UnknownVal(e.Op.Type), diags
Expand Down Expand Up @@ -227,6 +230,7 @@ func (e *UnaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
Detail: fmt.Sprintf("Unsuitable value for unary operand: %s.", err),
Subject: e.Val.Range().Ptr(),
Context: &e.SrcRange,
Expression: e.Val,
EvalContext: ctx,
})
}
Expand All @@ -246,6 +250,7 @@ func (e *UnaryOpExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
Summary: "Operation failed",
Detail: fmt.Sprintf("Error during operation: %s.", err),
Subject: &e.SrcRange,
Expression: e,
EvalContext: ctx,
})
return cty.UnknownVal(e.Op.Type), diags
Expand Down
4 changes: 4 additions & 0 deletions hcl/hclsyntax/expression_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (e *TemplateExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics)
),
Subject: part.Range().Ptr(),
Context: &e.SrcRange,
Expression: part,
EvalContext: ctx,
})
continue
Expand All @@ -64,6 +65,7 @@ func (e *TemplateExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics)
),
Subject: part.Range().Ptr(),
Context: &e.SrcRange,
Expression: part,
EvalContext: ctx,
})
continue
Expand Down Expand Up @@ -130,6 +132,7 @@ func (e *TemplateJoinExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
"An iteration result is null. Cannot include a null value in a string template.",
),
Subject: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
})
continue
Expand All @@ -147,6 +150,7 @@ func (e *TemplateJoinExpr) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnosti
err.Error(),
),
Subject: e.Range().Ptr(),
Expression: e,
EvalContext: ctx,
})
continue
Expand Down
6 changes: 5 additions & 1 deletion hcl/json/structure.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,8 @@ func (e *expression) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
Value: jsonAttr.Name,
SrcRange: jsonAttr.NameRange,
}}).Value(ctx)
val, valDiags := (&expression{src: jsonAttr.Value}).Value(ctx)
valExpr := &expression{src: jsonAttr.Value}
val, valDiags := valExpr.Value(ctx)
diags = append(diags, nameDiags...)
diags = append(diags, valDiags...)

Expand All @@ -444,6 +445,7 @@ func (e *expression) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
Summary: "Invalid object key expression",
Detail: fmt.Sprintf("Cannot use this expression as an object key: %s.", err),
Subject: &jsonAttr.NameRange,
Expression: valExpr,
EvalContext: ctx,
})
continue
Expand All @@ -454,6 +456,7 @@ func (e *expression) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
Summary: "Invalid object key expression",
Detail: "Cannot use null value as an object key.",
Subject: &jsonAttr.NameRange,
Expression: valExpr,
EvalContext: ctx,
})
continue
Expand All @@ -477,6 +480,7 @@ func (e *expression) Value(ctx *hcl.EvalContext) (cty.Value, hcl.Diagnostics) {
Summary: "Duplicate object attribute",
Detail: fmt.Sprintf("An attribute named %q was already defined at %s.", nameStr, attrRanges[nameStr]),
Subject: &jsonAttr.NameRange,
Expression: e,
EvalContext: ctx,
})
continue
Expand Down

0 comments on commit 6356254

Please sign in to comment.