| Age | Commit message (Collapse) | Author |
|---|
|
We were calling .Range() on any unknown sourceVal, without first checking
whether it was marked. That method panics if called on a marked value,
so we need to strip that off first.
While testing this I found some return paths that weren't properly
transferring the source value's marks to the output, and so this also
addresses those so that all return paths preserve whatever markings are
present on the source value.
In particular, if a non-list/set/tuple value gets "upgraded" into a tuple
then we must transfer its marks onto the tuple, because the decision about
constructing that value was based on characteristics of the source value.
|
|
|
|
for namespaced functions
|
|
Now that we have namespaced functions, and implementations like
Terraform can add functions based on configuration, the reason for an
unknown function call name becomes a little less clear. Because
functions are populated outside of the hcl package scope, there isn't
enough context to provide a useful diagnostic to the user.
We can create a new Diagnostic.Extra value for
FunctionCallUnknownDiagExtra to indicate specifically when a diagnostic
is created due to an unknown function name. This will carry back the
namespace and function name for the caller to inspect, which will allow
refinement of the diagnostic based on information only known to the
caller.
|
|
This introduces a new syntax which allows function names to have namespace
prefixes, with the different name parts separated by a double-colon "::"
as is common in various other C-derived languages which need to
distinguish between scope resolution and attribute/field traversal.
Because HCL has separate namespaces for functions and variables, we need
to use different punctuation for each to avoid creating parsing ambiguity
that could be resolved only with infinite lookahead.
We cannot retroactively change the representation of function names to be
a slice of names without breaking the existing API, and so we instead
adopt a convention of packing the multi-part names into single strings
which the parser guarantees will always be a series of valid identifiers
separated by the literal "::" sequence. That means that applications will
make namespaced functions available in the EvalContext by naming them in
a way that matches this convention.
This is still a subtle compatibility break for any implementation of the
syntax-agnostic HCL API against another syntax, because it may now
encounter function names in the function table that are not entirely
valid identifiers. However, that's okay in practice because a calling
application is always in full control of both which syntaxes it supports
and which functions it places in the function table, and so an application
using some other syntax can simply avoid using namespaced functions until
that syntax is updated to understand the new convention.
This initial commit only includes the basic functionality and does not yet
update the specification or specification test suite. It also has only
minimal unit tests of the parser and evaluator. Before finalizing this
in a release we would need to complete that work to make sure everything
is consistent and that we have sufficient regression tests for this new
capability.
|
|
Correct mark handling for some conditional values.
Find correct refinement for overlapping ranges which could not have been
compared with `GreaterThan`. Also map inclusive flags for numeric
ranges.
Correct handling of DefinitelyNotNull collections.
Return a known null early when both conditional branches are null.
|
|
|
|
When attempting to determine the final length range for a conditional
expression with collections, the length values may still be unknown.
Always use `Range()` to get the lower and upper bounds.
|
|
Signed-off-by: Jakub Martin <kubam@spacelift.io>
|
|
We know that a splat expression can never produce a null result, and also
in many cases we can use length refinements from the source collection to
also refine the destination collection because we know that a splat
expression produces exactly one result for each input element.
This also allows us to be a little more precise in the case where the
splat operator is projecting a non-list/set value into a zero or one
element list and we know the source value isn't null. This refinement is
a bit more marginal since it would be weird to apply the splat operator
to a value already known to be non-null anyway, but the refinement might
come from far away from the splat expression and so could still have
useful downstream effects in some cases.
|
|
When ConditionalExpr has an unknown predicate it can still often infer
some refinement to the range of its result by noticing characteristics
that the two results have in common.
In all cases we can test if either result could be null and return a
definitely-not-null unknown value if not.
For two known numbers we can constrain the range to be between those two
numbers. This is primarily aimed at the common case where the two possible
results are zero and one, which significantly constrains the range.
For two known collections of the same kind we can constrain the length
to be between the two collection lengths.
In these last two cases we can also sometimes collapse the unknown into
a known value if the range gets reduced enough. For example, if choosing
between two collections of the same length we might return a known
collection of that length containing unknown elements, rather than an
unknown collection.
|
|
* [COMPLIANCE] Add Copyright and License Headers
* add copywrite file and revert headers in testdata
---------
Co-authored-by: hashicorp-copywrite[bot] <110428419+hashicorp-copywrite[bot]@users.noreply.github.com>
Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
|
|
The primary goal of the diagnostics design in HCL is to return
high-quality diagnostics messages primarily for human consumption, and so
their regular structure is only machine-processable in a general sense
where we treat all diagnostics as subject to the same processing.
A few times now we've ended up wanting to carry some additional optional
contextual information along with the diagnostic, for example so that a
more advanced diagnostics renderer might optionally annotate a diagnostic
with extra notes to help the reader debug.
We got pretty far with our previous extension of hcl.Diagnostic to include
the Expression and EvalContext fields, which allow an advanced diagnostic
renderer to offer hints about what values contributed to the expression
that failed, but some context is even more specific than that, or is
defined by the application itself and therefore not appropriate to model
directly here in HCL.
As a pragmatic compromise then, here we introduce one more field Extra
to hcl.Diagnostic, which comes with a documented convention of placing
into it situation-specific values that implement particular interfaces,
and therefore a diagnostics renderer or other consumer can potentially
"sniff" this field for particular interfaces it knows about and treat them
in a special way if present.
Since there is only one field here that might end up being asked to
capture multiple extra values as the call stack unwinds, there is also a
simple predefined protocol for "unwrapping" extra values in order to find
nested implementations within.
For callers that are prepared to require Go 1.18, the helper function
hcl.DiagnosticExtra provides a type-assertion-like mechanism for sniffing
for a particular interface type while automatically respecting the nesting
protocol. For the moment that function lives behind a build constraint
so that callers which are not yet ready to use Go 1.18 can continue to
use other parts of HCL, and can implement a non-generic equivalent of
this function within their own codebase if absolutely necessary.
As an initial example to demonstrate the idea I've also implemented some
extra information for error diagnostics returned from FunctionCallExpr,
which gives the name of the function being called and, if the diagnostic
is describing an error returned by the function itself, a direct reference
to the raw error value returned from the function call. I anticipate a
diagnostic renderer sniffing for hclsyntax.FunctionCallDiagExtra to see
if a particular diagnostic is related to a function call, and if so to
include additional context about the signature of that function in the
diagnostic messages (by correlating with the function in the EvalContext
functions table). For example:
While calling: join(separator, list)
An example application-specific "extra value" could be for Terraform to
annotate diagnostics that relate to situations where an unknown value is
invalid, or where a "sensitive" value (a Terraform-specific value mark) is
invalid, so that the diagnostic renderer can avoid distracting users with
"red herring" commentary about unknown or sensitive values unless they
seem likely to be relevant to the error being printed.
|
|
For a long time now we've had a very simplistic error message for the
case of conditional expression result arms not having the same type, which
only works for situations where the two types have differing "friendly
names" down in the cty layer.
Unfortunately due to the typical complexity of the structural type kinds
(object and tuple types) their friendly names are just "object" and
"tuple", which tends to lead us to seemingly-incorrect error messages
like:
The true and false result expressions must have consistent types.
The given expressions are object and object, respectively.
This then is an attempt to use some more specialized messaging in some
of the situations that led to that sort of weird message before. In
particular, this handles:
- both types are object types but their attributes don't match
- both types are tuple types but their elements don't match
- both types are the same kind of collection of either object or tuple
types which don't match
These are the three _shallow_ cases that the previous logic wasn't able to
properly describe. This still leaves unaddressed a hopefully-less-common
case of nested collections with differing structural types in their
depths, but still avoids generating a confusing error message by instead
generating a _very vague but still correct_ error message:
At least one deeply-nested attribute or element is not compatible
across both the 'true' and the 'false' value.
My intent here is to make HCL return something precise enough _most of the
time_, without letting perfect be the enemy of the good. This will
generate some quite obnoxious long messages for particularly complex
nested structures, but so far it appears that such values are relatively
rare inside conditional expressions and so we'll wait to see what arises
in practice before trying to handle those situations more concisely.
Ideally I would like to include some actionable feedback that in some
cases it can help to explicitly convert ambiguously-typed expressions
like "null" or tuples intended to be lists to the intended type, so that
the type unification step has more information to infer the author intent.
However, HCL itself doesn't have any builtins for such conversions and so
today any messaging about that would need to be generated up at the
application layer so the application can refer to whatever functions/etc
it provides for type conversion. It isn't clear how to do that with the
current design, so we'll leave that to be addressed another day.
|
|
When upgrading an unknown splat value, the resulting collection may have
0 elements if the value ends up being `null`. This means the type must
be dynamic.
|
|
When a splat expression is used to upgrade an unknown value to a list
with a single value, the result must be entirely unknown. For example,
if `ukstr` is an unknown string value, then `unkstr.*` could either
result in a list of 1 string value, or an empty list, depending on
whether the result is a string or null.
|
|
When a function returns a functions.ArgError it's supposed to set the
argument index to a valid index within the range of the given argument
slice.
However, we typically try to be robust in the face of
incorrectly-implemented functions, so rather than panicking in that case
as we would've before we'll now return a slightly-less-precise error
message.
The "if i > len(e.Args)-1" case in the previous code here was actually
trying to handle this situation before, but it didn't quite manage to do
it because it was incorrectly referring to e.Args rather than "args"
(so it couldn't take into account expanded arguments) and because it
incorrectly assumed that there would always be more declared parameters
than arguments, which is actually never true in any valid call.
Now we'll handle out of range a little differently for variadic vs.
non-variadic functions. For variadics we'll assume that the function was
trying to talk about the variadic parameter and name that in the error
message. For non-variadics we can't do anything special so we end up
just treating it the same as any other error type, simply stating that
the function call failed without naming a particular argument.
Functions that end up in this error-handling codepath should still be
fixed, because they'll likely end up returning a slightly confusing error
message which doesn't accurately reflect the source of the problem. The
goal here is just avoid a panic in that case.
|
|
An incompatibility between #440 and #438 was not caught until after
merging both to the main branch.
|
|
Mark objects with keys that are sensitive
|
|
hashicorp/alisdair/marked-for-expression-conditional
hclsyntax: Fix for expression marked conditional
|
|
This adjusts prior behavior that would error to now allow keys that
are marked; those marks are removed in evaluation in order to give
a valid string for the map key, but the resulting object or tuple
is marked as a whole with whatever mark the key contained, ensuring
marks are maintained.
|
|
Using marked values in a for expression conditional expression would
previously panic due to calling `.False()` on the result of the
expression.
This commit makes two changes:
- Unmark the conditional expression value before testing if it is false;
- Merge any marks from the conditional into the resulting marks for the
for expression.
|
|
|
|
Return an error for invalid for expressions with marks
|
|
When evaluating an HCL expression attempting
to use a marked value as an object key,
return an error rather than falling through
to the cty panic. The error style mimics similar
errors in the area.
|
|
|
|
Functions which accept multiple parameters can be called with the
expansion operator, `...`. When doing so, we must unmark the expanded
argument value before transforming it into a collection of function
arguments. To ensure that any marks applied to the collection are
preserved, we transfer the collection marks to the individual elements
as we build the argument list.
|
|
In conditional expressions that involve a marked value,
Unmark the value before inspecting its truthiness,
to avoid a mark panic in cty
|
|
So far the expression parentheses syntax has been handled entirely in the
parser and has been totally invisible in the AST. That's fine for typical
expression evaluation, but over the years it's led to a few quirky
behaviors in less common situations where we've assumed that all
expressions are covered by the AST itself or by the source ranges that the
AST captures.
In particular, hclwrite assumes that all expressions will have source
ranges that cover their tokens, and it generates an incorrect physical
syntax tree when the AST doesn't uphold that.
After resisting through a few other similar bugs, this commit finally
introduces an explicit AST node for parentheses, which makes the
parentheses explicit in the AST and captures the larger source range that
includes the TokenOParen and the TokenCParen.
This means that parentheses will now be visible as a distinct node when
walking the AST, as reflected in the updated tests here. That may cause
downstream applications that traverse the tree to exhibit different
behaviors but we're not considering that as a "breaking change" because
the Walk function doesn't make any guarantees about the specific AST
shape.
|
|
The rules for the splat operator call for it to return an empty tuple
when its operand is null, but this rule was previously being
overridden by another rule that a value whose type is unknown
causes the operator to return an unknown value of unknown
type.
This was initially reported and discussed in Terraform, under
hashicorp/terraform#26746.
|
|
A for expression over a marked collection should result in a new
collection with the same marks. Previously this would fail with a type
error.
|
|
|
|
vals
|
|
Previously functions such as concat() would result in a panic if there
was a null element and a sequence, as in the included test. This PR adds
a check if the error index is outside of the range of arguments and
crafts an error that references the entire function instead of the null
argument.
|
|
Most of the time, the standard expression decoding built in to HCL is
sufficient. Sometimes though, it's useful to be able to customize the
decoding of certain arguments where the application intends to use them
in a very specific way, such as in static analysis.
This extension is an approximate analog of gohcl's support for decoding
into an hcl.Expression, allowing hcldec-based applications and
applications with custom functions to similarly capture and manipulate
the physical expressions used in arguments, rather than their values.
This includes one example use-case: the typeexpr extension now includes
a cty.Function called ConvertFunc that takes a type expression as its
second argument. A type expression is not evaluatable in the usual sense,
but thanks to cty capsule types we _can_ produce a cty.Value from one
and then make use of it inside the function implementation, without
exposing this custom type to the broader language:
convert(["foo"], set(string))
This mechanism is intentionally restricted only to "argument-like"
locations where there is a specific type we are attempting to decode into.
For now, that's hcldec AttrSpec/BlockAttrsSpec -- analogous to gohcl
decoding into hcl.Expression -- and in arguments to functions.
|
|
Some HCL callers make the (reasonable) assumption that the overall source
range of an expression will be a superset of all of the ranges of its
child expressions, for purposes such as extraction of source code
snippets, parse tree annotation in hclwrite, text editor analysis
functions like "go to reference", etc.
The IndexExpr type was not previously honoring that assumption, since its
source range was placed around only the bracket portion. That is a good
region to use when reporting errors relating to the index operation, but
it is not a faithful representation of the full extent of the expression.
In order to meet both of these requirements at once, IndexExpr now has
both SrcRange covering the entire expression and BracketRange covering
the index part delimited by brackets. We can then use BracketRange in
our error messages but return SrcRange as the result of the general
Range method that is common to all expression types.
|
|
Our error message for the ambiguous situation recommends doing this, but
the parser didn't actually previously allow it. Now we'll accept the form
that the error message recommends.
As before, we also accept a template with an interpolation sequence as
a disambiguation, but the error message doesn't mention that because it's
no longer idiomatic to use an inline string template containing just a
single interpolation sequence.
|
|
The main HCL package is more visible this way, and so it's easier than
having to pick it out from dozens of other package directories.
|
