function/stdlib: ZipmapFunc precise mark handling

cty/function/stdlib/collection.go

@@ -1145,12 +1145,14 @@ var ValuesFunc = function.New(&function.Spec{
 var ZipmapFunc = function.New(&function.Spec{
 	Params: []function.Parameter{
 		{
-			Name: "keys",
-			Type: cty.List(cty.String),
+			Name:        "keys",
+			Type:        cty.List(cty.String),
+			AllowMarked: true,
 		},
 		{
-			Name: "values",
-			Type: cty.DynamicPseudoType,
+			Name:        "values",
+			Type:        cty.DynamicPseudoType,
+			AllowMarked: true,
 		},
 	},
 	Type: func(args []cty.Value) (ret cty.Type, err error) {
@@ -1168,13 +1170,21 @@ var ZipmapFunc = function.New(&function.Spec{
 				return cty.DynamicPseudoType, nil
 			}
 
+			// NOTE: Marking of the keys list can't be represented in the
+			// result type, so the tuple type here will disclose the keys.
+			// This is unfortunate but is a common compromise with dynamic
+			// return types; the result from Impl will still reflect the marks
+			// from the keys list, so a mark-using caller should look out for
+			// that if it's important for their use-case.
+			keys, _ := keys.Unmark()
 			keysRaw := keys.AsValueSlice()
 			valueTypesRaw := valuesTy.TupleElementTypes()
 			if len(keysRaw) != len(valueTypesRaw) {
 				return cty.NilType, fmt.Errorf("number of keys (%d) does not match number of values (%d)", len(keysRaw), len(valueTypesRaw))
 			}
 			atys := make(map[string]cty.Type, len(valueTypesRaw))
 			for i, keyVal := range keysRaw {
+				keyVal, _ = keyVal.Unmark()
 				if keyVal.IsNull() {
 					return cty.NilType, fmt.Errorf("keys list has null value at index %d", i)
 				}
@@ -1190,11 +1200,17 @@ var ZipmapFunc = function.New(&function.Spec{
 	Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) {
 		keys := args[0]
 		values := args[1]
+		keys, keysMarks := keys.Unmark()
+		values, valuesMarks := values.Unmark()
+
+		// All of our return paths must pass through the merged marks from
+		// both the keys and the values, if any, using .WithMarks(retMarks)
+		retMarks := cty.NewValueMarks(keysMarks, valuesMarks)
 
 		if !keys.IsWhollyKnown() {
 			// Unknown map keys and object attributes are not supported, so
 			// our entire result must be unknown in this case.
-			return cty.UnknownVal(retType), nil
+			return cty.UnknownVal(retType).WithMarks(retMarks), nil
 		}
 
 		// both keys and values are guaranteed to be shallowly-known here,
@@ -1208,19 +1224,25 @@ var ZipmapFunc = function.New(&function.Spec{
 		i := 0
 		for it := keys.ElementIterator(); it.Next(); {
 			_, v := it.Element()
+			v, vMarks := v.Unmark()
 			val := values.Index(cty.NumberIntVal(int64(i)))
 			output[v.AsString()] = val
+
+			// We also need to accumulate the individual key marks on the
+			// returned map, because keys can't carry marks on their own.
+			retMarks = cty.NewValueMarks(retMarks, vMarks)
+
 			i++
 		}
 
 		switch {
 		case retType.IsMapType():
 			if len(output) == 0 {
-				return cty.MapValEmpty(retType.ElementType()), nil
+				return cty.MapValEmpty(retType.ElementType()).WithMarks(retMarks), nil
 			}
-			return cty.MapVal(output), nil
+			return cty.MapVal(output).WithMarks(retMarks), nil
 		case retType.IsObjectType():
-			return cty.ObjectVal(output), nil
+			return cty.ObjectVal(output).WithMarks(retMarks), nil
 		default:
 			// Should never happen because the type-check function should've
 			// caught any other case.

cty/function/stdlib/collection_test.go

@@ -1098,3 +1098,232 @@ func TestValues(t *testing.T) {
 		})
 	}
 }
+
+func TestZipMap(t *testing.T) {
+	tests := []struct {
+		Keys   cty.Value
+		Values cty.Value
+		Want   cty.Value
+		Err    string
+	}{
+		// Lists of values (map result)
+		{
+			cty.ListValEmpty(cty.String),
+			cty.ListValEmpty(cty.String),
+			cty.MapValEmpty(cty.String),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop")}),
+			cty.MapVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep"), cty.StringVal("beep")}),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop"), cty.StringVal("boop")}),
+			cty.MapVal(map[string]cty.Value{
+				"beep":  cty.StringVal("boop"),
+				"bleep": cty.StringVal("bloop"),
+			}),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.List(cty.String)),
+			cty.UnknownVal(cty.List(cty.String)),
+			cty.UnknownVal(cty.Map(cty.String)),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.List(cty.String)),
+			cty.ListValEmpty(cty.String),
+			cty.UnknownVal(cty.Map(cty.String)),
+			``,
+		},
+		{
+			cty.ListValEmpty(cty.String),
+			cty.UnknownVal(cty.List(cty.String)),
+			cty.UnknownVal(cty.Map(cty.String)),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop")}),
+			cty.MapVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("a"),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
+			cty.MapVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("b"),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
+			cty.MapVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("a").Mark("b"),
+			``,
+		},
+		{
+			// cty map keys don't have individual marks, so marks on elements
+			// in the keys list aggregate with the resulting map as a whole.
+			cty.ListVal([]cty.Value{cty.StringVal("bleep").Mark("a")}),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop")}),
+			cty.MapVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("a"),
+			``,
+		},
+		{
+			// cty map _values_ can have individual marks, so individual
+			// elements in the values list should have their marks preserved.
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
+			cty.ListVal([]cty.Value{cty.StringVal("bloop").Mark("a")}),
+			cty.MapVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop").Mark("a"),
+			}),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("boop")}),
+			cty.ListValEmpty(cty.String),
+			cty.NilVal,
+			`number of keys (1) does not match number of values (0)`,
+		},
+		{
+			cty.ListValEmpty(cty.String),
+			cty.ListVal([]cty.Value{cty.StringVal("boop")}),
+			cty.NilVal,
+			`number of keys (0) does not match number of values (1)`,
+		},
+
+		// Tuple of values (object result)
+		{
+			cty.ListValEmpty(cty.String),
+			cty.EmptyTupleVal,
+			cty.EmptyObjectVal,
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop")}),
+			cty.ObjectVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep"), cty.StringVal("beep")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop"), cty.StringVal("boop")}),
+			cty.ObjectVal(map[string]cty.Value{
+				"beep":  cty.StringVal("boop"),
+				"bleep": cty.StringVal("bloop"),
+			}),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.List(cty.String)),
+			cty.UnknownVal(cty.EmptyTuple),
+			cty.DynamicVal,
+			``,
+		},
+		{
+			cty.UnknownVal(cty.List(cty.String)),
+			cty.EmptyTupleVal,
+			cty.DynamicVal,
+			``,
+		},
+		{
+			cty.ListValEmpty(cty.String),
+			cty.UnknownVal(cty.EmptyTuple),
+			cty.UnknownVal(cty.EmptyObject),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop")}),
+			cty.ObjectVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("a"),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
+			cty.ObjectVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("b"),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}).Mark("a"),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop")}).Mark("b"),
+			cty.ObjectVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("a").Mark("b"),
+			``,
+		},
+		{
+			// cty object attributes don't have individual marks, so marks on
+			// elements in the keys list aggregate with the resulting object as
+			// a whole.
+			cty.ListVal([]cty.Value{cty.StringVal("bleep").Mark("a")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop")}),
+			cty.ObjectVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop"),
+			}).Mark("a"),
+			``,
+		},
+		{
+			// cty attribute _values_ can have individual marks, so individual
+			// elements in the values list should have their marks preserved.
+			cty.ListVal([]cty.Value{cty.StringVal("bleep")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("bloop").Mark("a")}),
+			cty.ObjectVal(map[string]cty.Value{
+				"bleep": cty.StringVal("bloop").Mark("a"),
+			}),
+			``,
+		},
+		{
+			cty.ListVal([]cty.Value{cty.StringVal("boop")}),
+			cty.EmptyTupleVal,
+			cty.NilVal,
+			`number of keys (1) does not match number of values (0)`,
+		},
+		{
+			cty.ListValEmpty(cty.String),
+			cty.TupleVal([]cty.Value{cty.StringVal("boop")}),
+			cty.NilVal,
+			`number of keys (0) does not match number of values (1)`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("ZipMap(%#v, %#v)", test.Keys, test.Values), func(t *testing.T) {
+			got, err := Zipmap(test.Keys, test.Values)
+			if test.Err != "" {
+				if err == nil {
+					t.Fatal("succeeded; want error")
+				}
+				if got, want := err.Error(), test.Err; got != want {
+					t.Fatalf("wrong error\ngot:  %s\nwant: %s", got, want)
+				}
+				return
+			} else if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+
+			if !got.RawEquals(test.Want) {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.Want)
+			}
+		})
+	}
+}

cty/marks.go

@@ -27,14 +27,32 @@ type marker struct {
 type ValueMarks map[interface{}]struct{}
 
 // NewValueMarks constructs a new ValueMarks set with the given mark values.
+//
+// If any of the arguments are already ValueMarks values then they'll be merged
+// into the result, rather than used directly as individual marks.
 func NewValueMarks(marks ...interface{}) ValueMarks {
 	if len(marks) == 0 {
 		return nil
 	}
 	ret := make(ValueMarks, len(marks))
 	for _, v := range marks {
+		if vm, ok := v.(ValueMarks); ok {
+			// Constructing a new ValueMarks with an existing ValueMarks
+			// implements a merge operation. (This can cause our result to
+			// have a larger size than we expected, but that's okay.)
+			for v := range vm {
+				ret[v] = struct{}{}
+			}
+			continue
+		}
 		ret[v] = struct{}{}
 	}
+	if len(ret) == 0 {
+		// If we were merging ValueMarks values together and they were all
+		// empty then we'll avoid returning a zero-length map and return a
+		// nil instead, as is conventional.
+		return nil
+	}
 	return ret
 }
 
@@ -180,6 +198,9 @@ func (val Value) Mark(mark interface{}) Value {
 		for k, v := range mr.marks {
 			newMarker.marks[k] = v
 		}
+		// unwrap the inner marked value, so we don't get multiple layers
+		// of marking.
+		newMarker.realV = mr.realV
 	} else {
 		// It's not a marker yet, so we're creating the first mark.
 		newMarker.marks = make(ValueMarks, 1)

cty/marks_test.go

@@ -244,6 +244,15 @@ func TestMarks(t *testing.T) {
 		t.Fatalf("still marked after unmark: %#v", marks)
 	}
 	wantMarks(marks, "a", "b", "c")
+
+	// Multiple marks, applied separately
+	val = val.Mark("a").Mark("b")
+	wantMarks(val.Marks(), "a", "b")
+	val, marks = val.Unmark()
+	if val.IsMarked() {
+		t.Fatalf("still marked after unmark: %#v", marks)
+	}
+	wantMarks(marks, "a", "b")
 }
 
 func TestUnmarkDeep(t *testing.T) {