plans/objchange: Decompose type-specific part of assertValueCompatible

This function starts with a general part that deals with conditions that
are common to all types, but then dispatches into different codepaths
depending on the type kind.

To keep the main function shorter, here we decompose the type-kind-specific
handling into separate functions, making assertValueCompatible now end
with a simpler dispatch table.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>

internal/plans/objchange/compatible.go

@@ -281,18 +281,47 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {
 		return errs
 	}
 
+	if !actual.IsKnown() {
+		errs = append(errs, path.NewErrorf("was known, but now unknown"))
+		return errs
+	}
+
+	// We no longer use "errs" after this point, because we should already have returned
+	// if we've added any errors to it. The following is just to minimize the risk of
+	// mistakes under future maintenence.
+	if len(errs) != 0 {
+		return errs
+	}
+
 	ty := planned.Type()
 	switch {
-
-	case !actual.IsKnown():
-		errs = append(errs, path.NewErrorf("was known, but now unknown"))
-
 	case ty.IsPrimitiveType():
+		return assertValueCompatiblePrimitive(planned, actual, path)
+	case ty.IsListType() || ty.IsMapType() || ty.IsTupleType():
+		return assertValueCompatibleCompositeWithKeys(planned, actual, path)
+	case ty.IsObjectType():
+		atys := ty.AttributeTypes()
+		return assertValueCompatibleObject(planned, actual, atys, path)
+	case ty.IsSetType():
+		return assertValueCompatibleSet(planned, actual, path)
+	default:
+		return nil // we don't have specialized checks for any other type kind
+	}
+}
+
+func assertValueCompatiblePrimitive(planned, actual cty.Value, path cty.Path) []error {
+	var errs []error
 	if !actual.Equals(planned).True() {
 		errs = append(errs, path.NewErrorf("was %#v, but now %#v", planned, actual))
 	}
+	return errs
+}
 
-	case ty.IsListType() || ty.IsMapType() || ty.IsTupleType():
+// assertValueCompatibleCompositeWithKeys is the branch of assertValueCompatible for values
+// that are of composite types where elements have comparable keys/indices separate from their
+// values that want to be compared on an element-by-element basis: lists, maps, and tuples.
+func assertValueCompatibleCompositeWithKeys(planned, actual cty.Value, path cty.Path) []error {
+	var errs []error
 	for it := planned.ElementIterator(); it.Next(); {
 		k, plannedV := it.Element()
 		if !actual.HasIndex(k).True() {
@@ -304,16 +333,17 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {
 		moreErrs := assertValueCompatible(plannedV, actualV, append(path, cty.IndexStep{Key: k}))
 		errs = append(errs, moreErrs...)
 	}
-
 	for it := actual.ElementIterator(); it.Next(); {
 		k, _ := it.Element()
 		if !planned.HasIndex(k).True() {
 			errs = append(errs, path.NewErrorf("new element %s has appeared", indexStrForErrors(k)))
 		}
 	}
+	return errs
+}
 
-	case ty.IsObjectType():
+func assertValueCompatibleObject(planned, actual cty.Value, atys map[string]cty.Type, path cty.Path) []error {
-		atys := ty.AttributeTypes()
+	var errs []error
 	for name := range atys {
 		// Because we already tested that the two values have the same type,
 		// we can assume that the same attributes are present in both and
@@ -323,18 +353,15 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {
 		moreErrs := assertValueCompatible(plannedV, actualV, append(path, cty.GetAttrStep{Name: name}))
 		errs = append(errs, moreErrs...)
 	}
+	return errs
+}
 
-	case ty.IsSetType():
+func assertValueCompatibleSet(planned, actual cty.Value, path cty.Path) []error {
-		// We can't really do anything useful for sets here because changing
+	var errs []error
-		// an unknown element to known changes the identity of the element, and
-		// so we can't correlate them properly. However, we will at least check
-		// to ensure that the number of elements is consistent, along with
-		// the general type-match checks we ran earlier in this function.
 	if planned.IsKnown() && !planned.IsNull() && !actual.IsNull() {
-
 		setErrs := assertSetValuesCompatible(planned, actual, path, func(plannedV, actualV cty.Value) bool {
-				errs := assertValueCompatible(plannedV, actualV, append(path, cty.IndexStep{Key: actualV}))
+			moreErrs := assertValueCompatible(plannedV, actualV, append(path, cty.IndexStep{Key: actualV}))
-				return len(errs) == 0
+			return len(moreErrs) == 0
 		})
 		errs = append(errs, setErrs...)
 
@@ -348,8 +375,6 @@ func assertValueCompatible(planned, actual cty.Value, path cty.Path) []error {
 			errs = append(errs, path.NewErrorf("length changed from %d to %d", plannedL, actualL))
 		}
 	}
-	}
-
 	return errs
 }