Skip to content

Conversation

@bbasata
Copy link
Collaborator

@bbasata bbasata commented Mar 25, 2025

Previously: #17

This PR updates this repo for parity with zclconf/go-cty v1.8.0. 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:

$ git ls-remote upstream refs/tags/v1.8.0^{}
fcc7075853dcb26bcb6c70f6a6eba35ddbae474c	refs/tags/v1.8.0^{}

$ # Check for non-trivial diffs between this v1.8.0 and upstream v1.8.0
$ GIT_PAGER= git diff -U0 --ignore-matching-lines 'github.com/(hashicorp|zclconf)/go-cty' fcc7075..HEAD -- . ':(exclude).github/CODEOWNERS'

[CHANGELOG.md go.mod go.sum]

$ go1.12 test ./...
ok  	github.com/hashicorp/go-cty/cty	1.063s
ok  	github.com/hashicorp/go-cty/cty/convert	0.926s
ok  	github.com/hashicorp/go-cty/cty/function	0.711s
ok  	github.com/hashicorp/go-cty/cty/function/stdlib	1.078s
ok  	github.com/hashicorp/go-cty/cty/gocty	0.889s
ok  	github.com/hashicorp/go-cty/cty/json	0.617s
ok  	github.com/hashicorp/go-cty/cty/msgpack	1.046s
ok  	github.com/hashicorp/go-cty/cty/set	(cached)

# Check for any un-replaced imports
$  ag zclconf -G '.go$'

apparentlymart and others added 30 commits March 25, 2025 08:58
Sets with the same type and same unknown values are equal enough for
RawEqual's purposes. This commit takes advantage of the setRules's
defined ordering to compare each element of the sets as lists.
This allows a caller to call a method like Unmark, but instead of getting a superset of marks, get an array of marks and their associated paths. This allows for a Value to be unmarked and then remarked later without losing so much detail about the marks.
The UnmarkDeep, UnmarkDeepWithPaths, and MarkWithPaths functions could
previously panic with nested complex values. This was due to the
Transform callback function only being called as part of a postorder
traversal, when it is not permitted to traverse a marked value.

This commit introduces a new TransformWithTransformer function,
requiring an implementation of a new Transformer interface. Using this
interface, callers can implement either preorder or postorder
traversals.

In turn, this allows us to write deep transformation for marks which
successfully cope with all nested structures.

The commit includes significantly more tests for these functions, which
should now cover all complex value types.
The immediately preceding code already unmarks the values and applies
the marking to the set, so this comment and panic are out of date.
When unmarking a complex value and retaining the marked paths, a
sufficiently deep structure would result in incorrect duplicate path
output. For example, this structure (in HCL syntax):

{
  environment = [
    {
      variables = {
        "x" = 1
        "y" = 2
      }
    }
  ]
}

If the 1 and 2 values are marked, the resulting path value marks from
UnmarkDeepWithPaths would have two entries, as expected. However, both
would have the same Path attribute, like so:

[
  { environment[0].variables["x"], "mark" },
  { environment[0].variables["x"], "mark" },
]

This is caused by calling `append` in the walk transform function with
the same path object repeatedly, which eventually does not result in
array reallocation, and therefore the path object is modified in-place.
For functions which do not explicitly support marks, we now deeply
unmark arguments before calling, and reapply those marks to the return
value. This ensures that these functions do not panic with arguments
which are collection values with marked descendants.

This does result in overly conservatively applying marks in some cases,
but that can be fixed with later work to add explicit support for marks
to those functions.
When working with big.Float values, it is important to ensure the
desired precision for the operations is maintained throughout. The
default precision for a Float is set initially by the max argument
precision when creating the non-zero value. If this is coming from a
Float which was set via SetInt, and that Int value was created by
SetInt64, then the precision is only 64 bits.
When determining which object attributes cause a type mismatch, we must
disregard equal attribute types. Previously we would attempt to find a
conversion between them, deciding that the attribute type was a mismatch
if no conversion exists. However, this is invalid if the two types are
equal, because GetConversion returns nil for equal primitive types.

Because of non-deterministic map iteration order, the test case added in
this commit failed approximately 2/3 of the time before the change. It
now passes consistently.
mildwonkey and others added 8 commits March 25, 2025 09:16
This includes both using the Unicode 13 normalization rules when we're
running on Go 1.16 or later, and also using the Unicode 13 character
segmentation rules for the stdlib string functions regardless of which
Go version we have selected.
@bbasata bbasata marked this pull request as ready for review March 25, 2025 13:20
@bbasata bbasata requested a review from a team as a code owner March 25, 2025 13:20
austinvalle
austinvalle previously approved these changes Mar 25, 2025
@bbasata bbasata requested a review from a team March 28, 2025 01:26
@bbasata bbasata merged commit 39789d2 into master Mar 31, 2025
1 check passed
@bbasata bbasata deleted the forward-to-1.8.0 branch March 31, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants