Merge pull request #699 from hashicorp/jbardin/marked-traversals

Preserve marks when traversing unknown values
hashicorp · Sep 24, 2024 · 78fe993 · 78fe993
2 parents 2442aca + e2f43f4
commit 78fe993
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 9 deletions.
diff --git a/ops.go b/ops.go
@@ -49,7 +49,7 @@ func Index(collection, key cty.Value, srcRange *Range) (cty.Value, Diagnostics)
 	ty := collection.Type()
 	kty := key.Type()
 	if kty == cty.DynamicPseudoType || ty == cty.DynamicPseudoType {
-		return cty.DynamicVal, nil
+		return cty.DynamicVal.WithSameMarks(collection), nil
 	}
 
 	switch {
@@ -87,9 +87,9 @@ func Index(collection, key cty.Value, srcRange *Range) (cty.Value, Diagnostics)
 		has, _ := collection.HasIndex(key).Unmark()
 		if !has.IsKnown() {
 			if ty.IsTupleType() {
-				return cty.DynamicVal, nil
+				return cty.DynamicVal.WithSameMarks(collection), nil
 			} else {
-				return cty.UnknownVal(ty.ElementType()), nil
+				return cty.UnknownVal(ty.ElementType()).WithSameMarks(collection), nil
 			}
 		}
 		if has.False() {
@@ -196,10 +196,10 @@ func Index(collection, key cty.Value, srcRange *Range) (cty.Value, Diagnostics)
 			}
 		}
 		if !collection.IsKnown() {
-			return cty.DynamicVal, nil
+			return cty.DynamicVal.WithSameMarks(collection), nil
 		}
 		if !key.IsKnown() {
-			return cty.DynamicVal, nil
+			return cty.DynamicVal.WithSameMarks(collection), nil
 		}
 
 		key, _ = key.Unmark()
@@ -291,13 +291,13 @@ func GetAttr(obj cty.Value, attrName string, srcRange *Range) (cty.Value, Diagno
 		}
 
 		if !obj.IsKnown() {
-			return cty.UnknownVal(ty.AttributeType(attrName)), nil
+			return cty.UnknownVal(ty.AttributeType(attrName)).WithSameMarks(obj), nil
 		}
 
 		return obj.GetAttr(attrName), nil
 	case ty.IsMapType():
 		if !obj.IsKnown() {
-			return cty.UnknownVal(ty.ElementType()), nil
+			return cty.UnknownVal(ty.ElementType()).WithSameMarks(obj), nil
 		}
 
 		idx := cty.StringVal(attrName)
@@ -319,7 +319,7 @@ func GetAttr(obj cty.Value, attrName string, srcRange *Range) (cty.Value, Diagno
 
 		return obj.Index(idx), nil
 	case ty == cty.DynamicPseudoType:
-		return cty.DynamicVal, nil
+		return cty.DynamicVal.WithSameMarks(obj), nil
 	case ty.IsListType() && ty.ElementType().IsObjectType():
 		// It seems a common mistake to try to access attributes on a whole
 		// list of objects rather than on a specific individual element, so

diff --git a/ops_test.go b/ops_test.go
@@ -249,6 +249,113 @@ func TestApplyPath(t *testing.T) {
 			cty.NilVal,
 			`Attempt to get attribute from null value: This value is null, so it does not have any attributes.`,
 		},
+
+		// Marks should be retained during index and getattr ops, even when
+		// types and values are unknown. This reflects the same behavior when
+		// using cty to directly call GetAttr and Index methods.
+		{
+			cty.DynamicVal.Mark("marked"),
+			(cty.Path)(nil).GetAttr("foo"),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("should be marked"),
+			}).Mark("marked"),
+			(cty.Path)(nil).GetAttr("foo"),
+			cty.StringVal("should be marked").Mark("marked"),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.Object(map[string]cty.Type{
+				"foo": cty.DynamicPseudoType,
+			})).Mark("marked"),
+			(cty.Path)(nil).GetAttr("foo"),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.DynamicVal.Mark("marked"),
+			(cty.Path)(nil).Index(cty.StringVal("foo")),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("should be marked"),
+			}).Mark("marked"),
+			(cty.Path)(nil).Index(cty.StringVal("foo")),
+			cty.StringVal("should be marked").Mark("marked"),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.Object(map[string]cty.Type{
+				"foo": cty.DynamicPseudoType,
+			})).Mark("marked"),
+			(cty.Path)(nil).Index(cty.StringVal("foo")),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.DynamicVal.Mark("marked"),
+			(cty.Path)(nil).Index(cty.NumberIntVal(0)),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("should be marked")}).Mark("marked"),
+			(cty.Path)(nil).Index(cty.NumberIntVal(0)),
+			cty.StringVal("should be marked").Mark("marked"),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.List(cty.String)).Mark("marked"),
+			(cty.Path)(nil).Index(cty.NumberIntVal(0)),
+			cty.UnknownVal(cty.String).Mark("marked"),
+			``,
+		},
+
+		{
+			cty.DynamicVal.Mark("marked"),
+			(cty.Path)(nil).Index(cty.UnknownVal(cty.String)),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("should be marked"),
+			}).Mark("marked"),
+			(cty.Path)(nil).Index(cty.UnknownVal(cty.String)),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.Object(map[string]cty.Type{
+				"foo": cty.DynamicPseudoType,
+			})).Mark("marked"),
+			(cty.Path)(nil).Index(cty.UnknownVal(cty.String)),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.DynamicVal.Mark("marked"),
+			(cty.Path)(nil).Index(cty.UnknownVal(cty.Number)),
+			cty.DynamicVal.Mark("marked"),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("should be marked")}).Mark("marked"),
+			(cty.Path)(nil).Index(cty.UnknownVal(cty.Number)),
+			cty.UnknownVal(cty.String).Mark("marked"),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.List(cty.String)).Mark("marked"),
+			(cty.Path)(nil).Index(cty.UnknownVal(cty.Number)),
+			cty.UnknownVal(cty.String).Mark("marked"),
+			``,
+		},
 	}
 
 	for _, test := range tests {
@@ -257,7 +364,7 @@ func TestApplyPath(t *testing.T) {
 			t.Logf("testing ApplyPath\nstart: %#v\npath:  %#v", test.Start, test.Path)
 
 			for _, diag := range diags {
-				t.Logf(diag.Error())
+				t.Log(diag.Error())
 			}
 
 			if test.WantErr != "" {