hclsyntax: Fix for expression marked conditional#438
Conversation
apparentlymart
left a comment
apparentlymart
left a comment
There was a problem hiding this comment.
Given that the "marks" feature in cty is intended as a general thing rather than specifically for Terraform's sensitive feature I might argue that it's better to be conservative here and preserve the union of marks from all three operands in the result, but I can also see your argument to do the opposite here and I don't have a strong opposition to it.
One thing I would say though is that adding new marks in a place where there weren't before is likely to be a more disruptive change for a consuming application than the opposite action of making something no longer be marked where it was before, so if we're unsure I'd suggest erring on the side of preserving marks in order to better keep options open for later.
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.
alisdair
force-pushed
the
alisdair/marked-for-expression-conditional
branch
from
afd5ce8 to
fe026c3
Compare
|
Thanks, that makes sense! I've updated the commit to preserve the marks from the conditional in the final value. |
ghost
mentioned this pull request
2 participants
These test cases both panic without the associated changes, because
.False()on a marked value is invalid.If the conditional expression in a for expression evaluates to a marked value, we need to ignore the marks before testing it. Since this result will not interpolate into the result, dropping the mark here seems reasonable for now.