forked from zclconf/go-cty
-
Notifications
You must be signed in to change notification settings - Fork 3
Forward to v1.6.0 #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously a negative index would lead to a panic.
Previously we required that when converting to an object type the input must have at least the attributes from the target type, and optionally additional attributes that would then be discarded. We'll now allow creating an object type with one or more optional attributes. These are optional only for conversion purposes: converting to an object type with optional arguments will substitute a null value for any attribute that doesn't exist in the source object type. This being handled only under conversion is consistent with some other similar cases: an object type is not conforming to a type constraint if it has attributes that are not in the type constraint, even though a conversion to that type constraint would be accepted. Likewise, a number doesn't conform to the string type even though a conversion is available. This distinction is so that cty applications can potentially bring their own separate conversion rules if they wish, ignoring the convert package in this repository. For now this new capability is explicitly experimental and so not subject to our typical backward-compatibility promises. The behavior of any function producing or working with object types that have optional attributes is subject to change even in future minor versions of this package.
This is just in the interests of keeping things up-to-date and doesn't really confer any specific benefit. The underlying library now defaults to always using the full-length encoding for integers provided as int64, so this change also includes an explicit opt-in to the old behavior of selecting the most compact integer representation possible, because for cty the different numeric encodings are just a size optimization and have no semantic meaning.
Early in the implementation of cty I made an oversight which has, unfortunately, played out as a variety of incorrect behaviors throughout the cty functions: when a set contains unknown values, those unknown values can potentially be standing in for values that are equal to others in the set, and thus the effective length of the set can't be predicted. Previously the Length function would return, in effect, the maximum possible size of the set, which would result if all of the values represented by the unknowns are non-equal. The caller might then get a different known result from a call with unknowns than from a subsequent call with those unknowns replaced with known values, which violates the guarantees that cty intends to make when handling unknown values. This changeset therefore starts by addressing that root bug: Length will now return an unknown number if called on a set containing unknown values, correctly representing that the final length is unpredictable. This in turn had some downstream consequences for other functionality. In particular, it should not have been possible to convert a set whose length is unknown to a list, because it's not possible for a list to have an unknown length. Therefore this changeset also includes a fix to the convert package so that an unknown-length set converts to an unknown list, which also addresses the related problem that a conversion from a set to list can't predict where the unknown values will appear in the sort order and thus can't predict the list indices even for the known elements. The rest of the changes here are similar adjustments to account for the possibility of an unknown set length, in most cases similarly returning an unknown result. This changeset causes a difference in observable behavior from the previous version, but it's not considered to be a breaking change because the previous behavior was defective. With that said, callers that were inadvertently depending on the defective behavior will find that their calling application now behaves slightly differently, and so may wish to make adjustments if the defective prior behavior was actually desirable for some unusual situation. As a pragmatic accommodation for existing callers, the Value.LengthInt function is now defined as returning the maximum possible length in situations where the length is unknown. This aligns with the number of iterations a caller would encounter using an ElementIterator on such a value, and also reflects a suitable capacity to use when allocating a Go slice to append iterated elements into.
austinvalle
approved these changes
Mar 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously: #14
This PR updates this repo for parity with zclconf/go-cty v1.6.0. Highlights:
ctywould often return incorrect results when working with sets containing unknown values."The changes remain "Unreleased" with no new release tag for the moment. This prevents a flood of Dependabot PRs like hashicorp/terraform-plugin-testing#444 for all affected providers.
Verification: