convert: More forthcoming about cause in MismatchMessage

Due to some early experience with this function initially exposing too much
implementation detail that tended to cause readers to get fixated on the
wrong parts of the error message, convert.MismatchMessage switched to
a mode where in some cases it withholds information about what type was
given to a failed type conversion.

However, this moved it to the opposite extreme of being _too_ cautious and
not reporting some information that _is_ more helpful than confusing.

After referring back to my earlier notes on what tended to cause confusion,
this now makes a more thorough effort to classify various "unlikely to
cause confusion" combinations of got and want type and in those cases it
will now report what type was given alongside what value was required.

This is a subjective boundary and so might get negotiated further in
future, but based on the existing tests and some further manual testing
this seems to make a relatively good compromise where it generates useful
information in most cases where the direct type is the most likely problem,
while still avoiding saying confusing things that expose implementation
details like untyped nulls, empty collections with as-yet-unknown element
types, etc.

This work was sponsored by Spacelift as an indirect contribution to the
OpenTofu project. Thanks!

Author: apparentlymart

CHANGELOG.md

@@ -1,5 +1,6 @@
 # 1.16.3 (Unreleased)
 
+* `convert`: Now generates more specific error messages in various cases of type conversion failure, giving additional information about the type that was given as compared to the type that was wanted by the caller.
 
 # 1.16.2 (January 21, 2025)
 

cty/convert/conversion_capsule_test.go

@@ -79,12 +79,12 @@ func TestConvertCapsuleType(t *testing.T) {
 		{
 			From:    cty.True,
 			To:      capTy,
-			WantErr: `test thingy required`,
+			WantErr: `test thingy required, but have bool`,
 		},
 		{
 			From:    capVal("hello"),
 			To:      cty.Bool,
-			WantErr: `bool required`,
+			WantErr: `bool required, but have test thingy`,
 		},
 		{
 			From: cty.UnknownVal(capTy),
@@ -99,22 +99,22 @@ func TestConvertCapsuleType(t *testing.T) {
 		{
 			From:    cty.UnknownVal(cty.Bool),
 			To:      capTy,
-			WantErr: `test thingy required`,
+			WantErr: `test thingy required, but have bool`,
 		},
 		{
 			From:    cty.NullVal(cty.Bool),
 			To:      capTy,
-			WantErr: `test thingy required`,
+			WantErr: `test thingy required, but have bool`,
 		},
 		{
 			From:    cty.UnknownVal(capTy),
 			To:      cty.Bool,
-			WantErr: `bool required`,
+			WantErr: `bool required, but have test thingy`,
 		},
 		{
 			From:    cty.NullVal(capTy),
 			To:      cty.Bool,
-			WantErr: `bool required`,
+			WantErr: `bool required, but have test thingy`,
 		},
 		{
 			From: capIntVal(42),

cty/convert/mismatch_msg.go

@@ -53,6 +53,9 @@ func MismatchMessage(got, want cty.Type) string {
 	case got.IsCollectionType() && want.IsCollectionType():
 		return mismatchMessageCollectionsFromCollections(got, want)
 
+	case !typesAreLikelyToCauseConfusion(got, want):
+		return fmt.Sprintf("%s required, but have %s", want.FriendlyName(), got.FriendlyName())
+
 	default:
 		// If we have nothing better to say, we'll just state what was required.
 		return want.FriendlyNameForConstraint() + " required"
@@ -224,3 +227,55 @@ func mismatchMessageCollectionsFromCollections(got, want cty.Type) string {
 	}
 	return fmt.Sprintf("incorrect %s: %s", noun, MismatchMessage(gotEty, wantEty))
 }
+
+func typesAreLikelyToCauseConfusion(got, want cty.Type) bool {
+	// NOTE: This function is intended to be used as the penultemate test
+	// in MismatchMessage, and so it intentionally does not address
+	// combinations that are already addressed by the more specific
+	// earlier tests.
+	if gotP, wantP := got.IsPrimitiveType(), want.IsPrimitiveType(); gotP != wantP {
+		// There's never any situation where a primitive type could successfully
+		// convert to a non-primitive one, so this is very unlikely to be
+		// intended as an automatic type conversion. This case is doing most
+		// of the useful work of this function, allowing us to report such
+		// things as "string required, but got tuple".
+		return false
+	}
+	if want == cty.String {
+		// All of the primitive types can always successfully convert to
+		// string, regardless of value.
+		return false
+	}
+	if (want == cty.Bool && got == cty.Number) || (got == cty.Bool && want == cty.Number) {
+		// There are no automatic conversions between bool and number, so
+		// describing both directions of this is unlikely to cause confusion.
+		return false
+	}
+	if got.IsCollectionType() && want.IsCollectionType() && !got.ElementType().HasDynamicTypes() && want.ElementType().HasDynamicTypes() {
+		// Describing mismatches of collection element types is helpful
+		// when we have specific types to report, but confusing when
+		// there are any inexact types involved because readers tend to
+		// then think the inexactness of the element type is the problem,
+		// when it's far more likely to be the collection type kind that's
+		// causing the problem.
+		return false
+	}
+	if got.IsTupleType() && want.IsObjectType() || got.IsObjectType() && got.IsTupleType() {
+		// The structural type kinds get described just by their bare names,
+		// "tuple" or "object", and so they are not confusing when they
+		// differ. They _are_ potentially confusing when we have matching kinds
+		// but different element types within, but thankfully that's most
+		// often handled by one of the earlier cases in MismatchMessage.
+		return false
+	}
+	// We are intentionally a little more vague for string-to-number and
+	// string-to-bool conversions because the rules for those are a little
+	// too subtle to be described purely as "need X but want Y", and historical
+	// experience shows that describing it that way causes folks to focus
+	// on the wrong problem (their value is wrong, not their type).
+
+	// If we find that we're getting here in additional specific cases where
+	// it would be more helpful than confusing to report both types then
+	// we'll add additional rules here in a future version.
+	return true
+}

cty/convert/mismatch_msg_test.go

@@ -15,7 +15,7 @@ func TestMismatchMessage(t *testing.T) {
 		{
 			cty.Bool,
 			cty.Number,
-			`number required`,
+			`number required, but have bool`,
 		},
 		{
 			cty.EmptyObject,
@@ -55,7 +55,7 @@ func TestMismatchMessage(t *testing.T) {
 			cty.List(cty.Object(map[string]cty.Type{
 				"foo": cty.String,
 			})),
-			`incorrect list element type: object required`,
+			`incorrect list element type: object required, but have string`,
 		},
 		{
 			cty.List(cty.EmptyObject),

cty/convert/public_test.go

@@ -623,7 +623,7 @@ func TestConvert(t *testing.T) {
 			Type: cty.Object(map[string]cty.Type{
 				"foo": cty.Number,
 			}),
-			WantError: `attribute "foo": number required`,
+			WantError: `attribute "foo": number required, but have bool`,
 		},
 		{
 			Value: cty.ObjectVal(map[string]cty.Value{
@@ -632,7 +632,7 @@ func TestConvert(t *testing.T) {
 			Type: cty.Object(map[string]cty.Type{
 				"foo": cty.Number,
 			}),
-			WantError: `attribute "foo": number required`,
+			WantError: `attribute "foo": number required, but have bool`,
 		},
 		{
 			Value: cty.NullVal(cty.String),
@@ -1064,7 +1064,7 @@ func TestConvert(t *testing.T) {
 					"d": cty.String,
 				}),
 			}, []string{"b", "c"}),
-			WantError: `map element type is incompatible with attribute "c": object required`,
+			WantError: `map element type is incompatible with attribute "c": object required, but have string`,
 		},
 		{
 			Value: cty.TupleVal([]cty.Value{

cty/function/stdlib/format_test.go

@@ -340,7 +340,7 @@ func TestFormat(t *testing.T) {
 			cty.StringVal("%d green bottles standing on the wall"),
 			[]cty.Value{cty.True},
 			cty.NilVal,
-			`unsupported value for "%d" at 0: number required`,
+			`unsupported value for "%d" at 0: number required, but have bool`,
 		},
 		{
 			cty.StringVal("%d green bottles standing on the wall"),
@@ -794,7 +794,7 @@ func TestFormatList(t *testing.T) {
 			cty.StringVal("%s"),
 			[]cty.Value{cty.EmptyObjectVal},
 			cty.ListValEmpty(cty.String),
-			`error on format iteration 0: unsupported value for "%s" at 0: string required`,
+			`error on format iteration 0: unsupported value for "%s" at 0: string required, but have object`,
 		},
 		12: {
 			cty.StringVal("%v"),