Add inline defaults to optional object attribute type constraints #31154
Conversation
alisdair
force-pushed
the
alisdair/module-variable-optional-default
branch
from
6c66d00
to
f0c82c4
Compare
In type constraints, object attributes may be marked as optional in order to allow them to be omitted from input values. Doing so results in filling the attribute value with a typed `null`. This commit adds a new type `typeexpr.Defaults` which mirrors the structure of a type constraint, storing default values for optional attributes. This will allow specification of non-`null` default values for attributes. The `Defaults` type is a tree structure, each node containing a sub-tree type, a map of children, and for object nodes, a map of defaults. The keys in the children map depend on the type of the node: - Object nodes have children for each attribute; - Tuple nodes have children for each index, with indices converted to string values; - Collection nodes have a single child at the empty string key. When traversing this tree we must take this structure into account, with special cases for map input values which may later be converted to objects. The traversal defined in this commit uses a pre-order transformer in order to pre-populate descendent nodes before their defaults are applied. This allows type nested type default values to be specified more compactly.
The optional modifier previously accepted a single argument: the attribute type. This commit adds an optional second argument, which specifies a default value for the attribute. To record the default values for a variable's type, we use a separate parallel structure of `typeexpr.Defaults`, rather than extending `cty.Type` to include a `cty.Value` of defaults (which may in turn include a `cty.Type` with defaults, and so on, and so forth). The new `typeexpr.TypeConstraintWithDefaults` returns a type constraint and defaults value. Defaults will be `nil` unless there are default values specified somewhere in the variable's type.
Now that variables parse and retain a set of default values for object attributes, we must apply the defaults during variable evaluation. We do so immediately before type conversion, preprocessing the given value so that conversion will receive the intended defaults as appropriate.
alisdair
force-pushed
the
alisdair/module-variable-optional-default
branch
from
f0c82c4
to
f92fcc4
Compare
| // the recieving scope. If so, it will return the given function verbatim. | ||
| // If not, it will return a placeholder function that just returns an | ||
| // error explaining that the function requires the experiment to be enabled. | ||
| func (s *Scope) experimentalFunction(experiment experiments.Experiment, fn function.Function) function.Function { |
There was a problem hiding this comment.
I guess this is probably now getting caught by our recently-added unused function lint... but is there some way we can tell it to let us keep this here? This is a non-trivial method that we'll probably need to reuse at some later point for another experiment, so I'd rather avoid digging around in the Git history to try to find it again next time we have another experimental function. 😬
There was a problem hiding this comment.
Yep, I only removed it because staticcheck yelled at me.
I've added a linter directive to disable the unused function error and re-added the function. We'll have to remove that directive if the function is used again but that's easier than Git spelunking.
Now that we are able to specify optional object attribute defaults inline in a type constraint, the separate `defaults` function is no longer needed.
Extend the documentation on type constraints to include the new default functionality, including a detailed example of a nested structure with multiple levels of defaults.
alisdair
force-pushed
the
alisdair/module-variable-optional-default
branch
from
f92fcc4
to
e2a3042
Compare
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This pull request responds to community feedback on the long-running optional object attributes experiment, adding a second argument to the
optional()call which describes a default value for the attribute. It's probably easiest to review one commit at a time.From a user perspective, the change here is in how default values are specified. The previous design used a
defaults()function, intended to be called in alocalscontext with the type-converted variable value. This design instead inlines the default value for a given attribute next to its type constraint, which is hoped to be easier to understand and enables several use cases which were not possible with the previous design.For example:
Assigning
{ a = "foo" }to this variable will result in the value{ a = "foo", b = null, c = 127 }.While the configuration language colocates the attribute type and default value, this is not so in the underlying data structure, as this is unsupported by
cty. Instead we maintain parallel data structures for the type constraint and any defaults present, which is the bulk of the new logic in this PR and is in the first commit.Note that the
typeexprpackage in Terraform is a fork of the HCL package of the same name, which was done as part of the first iteration of this experiment. Should this approach be approved, we should upstream the changes to HCL to make tool integration easier. This is why we have an awkward addition to the publictypeexprAPI instead of modifying theTypeConstraintfunction.If we move ahead with this design, I'd like to release it in an early 1.3 alpha to gather feedback from the community already using optional attributes. Should this approach seem viable, my goal would then be to conclude the experiment for the 1.3.0 release (i.e. before the first beta).