function/stdlib: ValuesFunc precise handling of marks

Previously ValuesFunc was just using the safe default behavior where if
any argument contains any marked values at all then the result is
automatically given the same marks.

That's overly conservative though, because it means that an unmarked
map with a single marked element value will become a wholly-marked list
result, rather than an unmarked list with one marked element value.

Now we'll opt in to custom mark handling within the function itself, and
handle shallowly the marking of the overall collection -- propagating it
to the result -- while letting the values inside retain their individual
markings as needed.

We were previously lacking unit tests for the values function, I think
because it's a pretty old one from when cty was new, so this also includes
additional test cases that are not directly related to marked values but
cover the other interesting cases that this function must deal with,
either directly (with its own code) or indirectly (by calling cty
operation functions that handle it automatically).

cty/function/stdlib/collection.go

@@ -1078,8 +1078,9 @@ func sliceIndexes(args []cty.Value) (int, int, bool, error) {
 var ValuesFunc = function.New(&function.Spec{
 	Params: []function.Parameter{
 		{
-			Name: "values",
-			Type: cty.DynamicPseudoType,
+			Name:        "values",
+			Type:        cty.DynamicPseudoType,
+			AllowMarked: true,
 		},
 	},
 	Type: func(args []cty.Value) (ret cty.Type, err error) {
@@ -1112,6 +1113,13 @@ var ValuesFunc = function.New(&function.Spec{
 	Impl: func(args []cty.Value, retType cty.Type) (ret cty.Value, err error) {
 		mapVar := args[0]
 
+		// We must unmark the value before we can use ElementIterator on it,
+		// and then re-apply the same marks (possibly none) when we return.
+		// (We leave the inner values just as they are, because we won't be
+		// doing anything with them aside from copying them verbatim into the
+		// result, marks and all.)
+		mapVar, marks := mapVar.Unmark()
+
 		// We can just iterate the map/object value here because cty guarantees
 		// that these types always iterate in key lexicographical order.
 		var values []cty.Value
@@ -1120,13 +1128,15 @@ var ValuesFunc = function.New(&function.Spec{
 			values = append(values, val)
 		}
 
+		// All of the return paths must include .WithMarks(marks) so that we
+		// will preserve the markings of the overall map/object we were given.
 		if retType.IsTupleType() {
-			return cty.TupleVal(values), nil
+			return cty.TupleVal(values).WithMarks(marks), nil
 		}
 		if len(values) == 0 {
-			return cty.ListValEmpty(retType.ElementType()), nil
+			return cty.ListValEmpty(retType.ElementType()).WithMarks(marks), nil
 		}
-		return cty.ListVal(values), nil
+		return cty.ListVal(values).WithMarks(marks), nil
 	},
 })
 

cty/function/stdlib/collection_test.go

@@ -983,3 +983,118 @@ func TestCoalesceList(t *testing.T) {
 		})
 	}
 }
+
+func TestValues(t *testing.T) {
+	tests := []struct {
+		Collection cty.Value
+		Want       cty.Value
+		Err        string
+	}{
+		{
+			cty.MapValEmpty(cty.String),
+			cty.ListValEmpty(cty.String),
+			``,
+		},
+		{
+			cty.MapValEmpty(cty.String).Mark("a"),
+			cty.ListValEmpty(cty.String).Mark("a"),
+			``,
+		},
+		{
+			cty.NullVal(cty.Map(cty.String)),
+			cty.NilVal,
+			`argument must not be null`,
+		},
+		{
+			cty.UnknownVal(cty.Map(cty.String)),
+			cty.UnknownVal(cty.List(cty.String)),
+			``,
+		},
+		{
+			cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world")}),
+			cty.ListVal([]cty.Value{cty.StringVal("world")}),
+			``,
+		},
+		{ // The map itself is not marked, just an inner element.
+			cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world").Mark("a")}),
+			cty.ListVal([]cty.Value{cty.StringVal("world").Mark("a")}),
+			``,
+		},
+		{ // The entire map is marked, so the resulting list is also marked.
+			cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world")}).Mark("a"),
+			cty.ListVal([]cty.Value{cty.StringVal("world")}).Mark("a"),
+			``,
+		},
+		{ // Marked both inside and outside.
+			cty.MapVal(map[string]cty.Value{"hello": cty.StringVal("world").Mark("a")}).Mark("a"),
+			cty.ListVal([]cty.Value{cty.StringVal("world").Mark("a")}).Mark("a"),
+			``,
+		},
+		{
+			cty.ObjectVal(map[string]cty.Value{"hello": cty.StringVal("world")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("world")}),
+			``,
+		},
+		{
+			cty.EmptyObjectVal,
+			cty.EmptyTupleVal,
+			``,
+		},
+		{
+			cty.EmptyObjectVal.Mark("a"),
+			cty.EmptyTupleVal.Mark("a"),
+			``,
+		},
+		{
+			cty.NullVal(cty.EmptyObject),
+			cty.NilVal,
+			`argument must not be null`,
+		},
+		{
+			cty.UnknownVal(cty.EmptyObject),
+			cty.UnknownVal(cty.EmptyTuple),
+			``,
+		},
+		{
+			cty.UnknownVal(cty.Object(map[string]cty.Type{"a": cty.String})),
+			cty.UnknownVal(cty.Tuple([]cty.Type{cty.String})),
+			``,
+		},
+		{ // The object itself is not marked, just an inner attribute value.
+			cty.ObjectVal(map[string]cty.Value{"hello": cty.StringVal("world").Mark("a")}),
+			cty.TupleVal([]cty.Value{cty.StringVal("world").Mark("a")}),
+			``,
+		},
+		{ // The entire object is marked, so the resulting tuple is also marked.
+			cty.ObjectVal(map[string]cty.Value{"hello": cty.StringVal("world")}).Mark("a"),
+			cty.TupleVal([]cty.Value{cty.StringVal("world")}).Mark("a"),
+			``,
+		},
+		{ // Marked both inside and outside.
+			cty.ObjectVal(map[string]cty.Value{"hello": cty.StringVal("world").Mark("a")}).Mark("a"),
+			cty.TupleVal([]cty.Value{cty.StringVal("world").Mark("a")}).Mark("a"),
+			``,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("Values(%#v)", test.Collection), func(t *testing.T) {
+			got, err := Values(test.Collection)
+			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)
+			}
+		})
+	}
+}