Skip to content

Conversation

@bbasata
Copy link
Collaborator

@bbasata bbasata commented Mar 25, 2025

Previously: #16

This PR updates this repo for parity with zclconf/go-cty v1.7.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.7.0^{}
a7cdc18f42cbb42e745bc372946ea049af573762	refs/tags/v1.7.0^{}

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

[CHANGELOG.md go.mod go.sum]

$ go1.12 test ./...
ok  	github.com/hashicorp/go-cty/cty	1.099s
ok  	github.com/hashicorp/go-cty/cty/convert	0.827s
ok  	github.com/hashicorp/go-cty/cty/function	0.625s
ok  	github.com/hashicorp/go-cty/cty/function/stdlib	1.101s
ok  	github.com/hashicorp/go-cty/cty/gocty	0.929s
ok  	github.com/hashicorp/go-cty/cty/json	0.681s
ok  	github.com/hashicorp/go-cty/cty/msgpack	0.838s
ok  	github.com/hashicorp/go-cty/cty/set	(cached)

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

apparentlymart and others added 17 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.
@bbasata bbasata marked this pull request as ready for review March 25, 2025 13:12
@bbasata bbasata requested a review from a team as a code owner March 25, 2025 13:12
@bbasata bbasata mentioned this pull request Mar 25, 2025
austinvalle
austinvalle previously approved these changes Mar 25, 2025
@bbasata bbasata dismissed austinvalle’s stale review March 25, 2025 23:31

The merge-base changed after approval.

@bbasata bbasata requested a review from a team March 25, 2025 23:31
@bbasata bbasata merged commit ca64fed into master Mar 26, 2025
1 check passed
@bbasata bbasata deleted the forward-to-1.7.0 branch March 26, 2025 12:45
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.

7 participants