Merge pull request #30476 from hashicorp/alisdair/fix-type-marks

cli: Prevent complex uses of the console-only `type` function

internal/lang/funcs/conversion.go

@@ -1,12 +1,10 @@
 package funcs
 
 import (
-	"fmt"
-	"sort"
 	"strconv"
-	"strings"
 
 	"github.com/hashicorp/terraform/internal/lang/marks"
+	"github.com/hashicorp/terraform/internal/lang/types"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
 	"github.com/zclconf/go-cty/cty/function"
@@ -97,6 +95,9 @@ func MakeToFunc(wantTy cty.Type) function.Function {
 	})
 }
 
+// TypeFunc returns an encapsulated value containing its argument's type. This
+// value is marked to allow us to limit the use of this function at the moment
+// to only a few supported use cases.
 var TypeFunc = function.New(&function.Spec{
 	Params: []function.Parameter{
 		{
@@ -107,117 +108,13 @@ var TypeFunc = function.New(&function.Spec{
 			AllowNull:        true,
 		},
 	},
-	Type: function.StaticReturnType(cty.String),
+	Type: function.StaticReturnType(types.TypeType),
 	Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
-		return cty.StringVal(TypeString(args[0].Type())).Mark(marks.Raw), nil
+		givenType := args[0].Type()
+		return cty.CapsuleVal(types.TypeType, &givenType).Mark(marks.TypeType), nil
 	},
 })
 
-// Modified copy of TypeString from go-cty:
-// https://github.com/zclconf/go-cty-debug/blob/master/ctydebug/type_string.go
-//
-// TypeString returns a string representation of a given type that is
-// reminiscent of Go syntax calling into the cty package but is mainly
-// intended for easy human inspection of values in tests, debug output, etc.
-//
-// The resulting string will include newlines and indentation in order to
-// increase the readability of complex structures. It always ends with a
-// newline, so you can print this result directly to your output.
-func TypeString(ty cty.Type) string {
-	var b strings.Builder
-	writeType(ty, &b, 0)
-	return b.String()
-}
-
-func writeType(ty cty.Type, b *strings.Builder, indent int) {
-	switch {
-	case ty == cty.NilType:
-		b.WriteString("nil")
-		return
-	case ty.IsObjectType():
-		atys := ty.AttributeTypes()
-		if len(atys) == 0 {
-			b.WriteString("object({})")
-			return
-		}
-		attrNames := make([]string, 0, len(atys))
-		for name := range atys {
-			attrNames = append(attrNames, name)
-		}
-		sort.Strings(attrNames)
-		b.WriteString("object({\n")
-		indent++
-		for _, name := range attrNames {
-			aty := atys[name]
-			b.WriteString(indentSpaces(indent))
-			fmt.Fprintf(b, "%s: ", name)
-			writeType(aty, b, indent)
-			b.WriteString(",\n")
-		}
-		indent--
-		b.WriteString(indentSpaces(indent))
-		b.WriteString("})")
-	case ty.IsTupleType():
-		etys := ty.TupleElementTypes()
-		if len(etys) == 0 {
-			b.WriteString("tuple([])")
-			return
-		}
-		b.WriteString("tuple([\n")
-		indent++
-		for _, ety := range etys {
-			b.WriteString(indentSpaces(indent))
-			writeType(ety, b, indent)
-			b.WriteString(",\n")
-		}
-		indent--
-		b.WriteString(indentSpaces(indent))
-		b.WriteString("])")
-	case ty.IsCollectionType():
-		ety := ty.ElementType()
-		switch {
-		case ty.IsListType():
-			b.WriteString("list(")
-		case ty.IsMapType():
-			b.WriteString("map(")
-		case ty.IsSetType():
-			b.WriteString("set(")
-		default:
-			// At the time of writing there are no other collection types,
-			// but we'll be robust here and just pass through the GoString
-			// of anything we don't recognize.
-			b.WriteString(ty.FriendlyName())
-			return
-		}
-		// Because object and tuple types render split over multiple
-		// lines, a collection type container around them can end up
-		// being hard to see when scanning, so we'll generate some extra
-		// indentation to make a collection of structural type more visually
-		// distinct from the structural type alone.
-		complexElem := ety.IsObjectType() || ety.IsTupleType()
-		if complexElem {
-			indent++
-			b.WriteString("\n")
-			b.WriteString(indentSpaces(indent))
-		}
-		writeType(ty.ElementType(), b, indent)
-		if complexElem {
-			indent--
-			b.WriteString(",\n")
-			b.WriteString(indentSpaces(indent))
-		}
-		b.WriteString(")")
-	default:
-		// For any other type we'll just use its GoString and assume it'll
-		// follow the usual GoString conventions.
-		b.WriteString(ty.FriendlyName())
-	}
-}
-
-func indentSpaces(level int) string {
-	return strings.Repeat("    ", level)
-}
-
 func Type(input []cty.Value) (cty.Value, error) {
 	return TypeFunc.Call(input)
 }

internal/lang/funcs/conversion_test.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform/internal/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
@@ -191,92 +190,3 @@ func TestTo(t *testing.T) {
 		})
 	}
 }
-
-func TestType(t *testing.T) {
-	tests := []struct {
-		Input cty.Value
-		Want  string
-	}{
-		// Primititves
-		{
-			cty.StringVal("a"),
-			"string",
-		},
-		{
-			cty.NumberIntVal(42),
-			"number",
-		},
-		{
-			cty.BoolVal(true),
-			"bool",
-		},
-		// Collections
-		{
-			cty.EmptyObjectVal,
-			`object({})`,
-		},
-		{
-			cty.EmptyTupleVal,
-			`tuple([])`,
-		},
-		{
-			cty.ListValEmpty(cty.String),
-			`list(string)`,
-		},
-		{
-			cty.MapValEmpty(cty.String),
-			`map(string)`,
-		},
-		{
-			cty.SetValEmpty(cty.String),
-			`set(string)`,
-		},
-		{
-			cty.ListVal([]cty.Value{cty.StringVal("a")}),
-			`list(string)`,
-		},
-		{
-			cty.ListVal([]cty.Value{cty.ListVal([]cty.Value{cty.NumberIntVal(42)})}),
-			`list(list(number))`,
-		},
-		{
-			cty.ListVal([]cty.Value{cty.MapValEmpty(cty.String)}),
-			`list(map(string))`,
-		},
-		{
-			cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
-				"foo": cty.StringVal("bar"),
-			})}),
-			"list(\n    object({\n        foo: string,\n    }),\n)",
-		},
-		// Unknowns and Nulls
-		{
-			cty.UnknownVal(cty.String),
-			"string",
-		},
-		{
-			cty.NullVal(cty.Object(map[string]cty.Type{
-				"foo": cty.String,
-			})),
-			"object({\n    foo: string,\n})",
-		},
-		{ // irrelevant marks do nothing
-			cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
-				"foo": cty.StringVal("bar").Mark("ignore me"),
-			})}),
-			"list(\n    object({\n        foo: string,\n    }),\n)",
-		},
-	}
-	for _, test := range tests {
-		got, err := Type([]cty.Value{test.Input})
-		if err != nil {
-			t.Fatalf("unexpected error: %s", err)
-		}
-		// The value is marked to help with formatting
-		got, _ = got.Unmark()
-
-		if got.AsString() != test.Want {
-			t.Errorf("wrong result:\n%s", cmp.Diff(got.AsString(), test.Want))
-		}
-	}
-}

internal/lang/funcs/number_test.go

@@ -215,9 +215,9 @@ func TestParseInt(t *testing.T) {
 			``,
 		},
 		{
-			cty.StringVal("128").Mark(marks.Raw),
+			cty.StringVal("128").Mark("boop"),
 			cty.NumberIntVal(10).Mark(marks.Sensitive),
-			cty.NumberIntVal(128).WithMarks(cty.NewValueMarks(marks.Raw, marks.Sensitive)),
+			cty.NumberIntVal(128).WithMarks(cty.NewValueMarks("boop", marks.Sensitive)),
 			``,
 		},
 		{

internal/lang/funcs/redact_test.go

@@ -18,14 +18,14 @@ func TestRedactIfSensitive(t *testing.T) {
 			marks: []cty.ValueMarks{cty.NewValueMarks(marks.Sensitive)},
 			want:  "(sensitive value)",
 		},
-		"raw non-sensitive string": {
+		"marked non-sensitive string": {
 			value: "foo",
-			marks: []cty.ValueMarks{cty.NewValueMarks(marks.Raw)},
+			marks: []cty.ValueMarks{cty.NewValueMarks("boop")},
 			want:  `"foo"`,
 		},
-		"raw sensitive string": {
+		"sensitive string with other marks": {
 			value: "foo",
-			marks: []cty.ValueMarks{cty.NewValueMarks(marks.Raw), cty.NewValueMarks(marks.Sensitive)},
+			marks: []cty.ValueMarks{cty.NewValueMarks("boop"), cty.NewValueMarks(marks.Sensitive)},
 			want:  "(sensitive value)",
 		},
 		"sensitive number": {

internal/lang/marks/marks.go

@@ -38,6 +38,7 @@ func Contains(val cty.Value, mark valueMark) bool {
 // Terraform.
 var Sensitive = valueMark("sensitive")
 
-// Raw is used to indicate to the repl that the value should be written without
-// any formatting.
-var Raw = valueMark("raw")
+// TypeType is used to indicate that the value contains a representation of
+// another value's type. This is part of the implementation of the console-only
+// `type` function.
+var TypeType = valueMark("typeType")

internal/lang/types/type_type.go

@@ -0,0 +1,12 @@
+package types
+
+import (
+	"reflect"
+
+	"github.com/zclconf/go-cty/cty"
+)
+
+// TypeType is a capsule type used to represent a cty.Type as a cty.Value. This
+// is used by the `type()` console function to smuggle cty.Type values to the
+// REPL session, where it can be displayed to the user directly.
+var TypeType = cty.Capsule("type", reflect.TypeOf(cty.Type{}))

internal/lang/types/types.go

@@ -0,0 +1,2 @@
+// Package types contains non-standard cty types used only within Terraform.
+package types

internal/repl/format.go

@@ -17,10 +17,6 @@ func FormatValue(v cty.Value, indent int) string {
 	if !v.IsKnown() {
 		return "(known after apply)"
 	}
-	if v.Type().Equals(cty.String) && v.HasMark(marks.Raw) {
-		raw, _ := v.Unmark()
-		return raw.AsString()
-	}
 	if v.HasMark(marks.Sensitive) {
 		return "(sensitive)"
 	}