adding some todos and copy impl

Author: austinvalle

tftypes/refinement/refinement.go

@@ -1,11 +1,23 @@
 package refinement
 
-import "github.com/vmihailenco/msgpack/v5"
+import (
+	"fmt"
+
+	"github.com/vmihailenco/msgpack/v5"
+)
 
 type Key int64
 
 func (k Key) String() string {
-	return "todo"
+	// TODO: Not sure when this is used, double check the names
+	switch k {
+	case KeyNullness:
+		return "nullness"
+	case KeyStringPrefix:
+		return "string_prefix"
+	default:
+		return fmt.Sprintf("unsupported refinement: %d", k)
+	}
 }
 
 const (
@@ -21,7 +33,7 @@ type Refinement interface {
 	Equal(Refinement) bool
 	Encode(*msgpack.Encoder) error
 	String() string
-	unimplementable() // prevent external implementations
+	unimplementable() // prevents external implementations, all refinements are defined in the Terraform/HCL type system go-cty.
 }
 
 type Refinements map[Key]Refinement
@@ -30,5 +42,6 @@ func (r Refinements) Equal(o Refinements) bool {
 	return false
 }
 func (r Refinements) String() string {
+	// TODO: Not sure when this is used, should just aggregate and call all underlying refinements.String() method
 	return "todo"
 }

tftypes/refinement/string_prefix.go

@@ -7,6 +7,8 @@ type stringPrefix struct {
 }
 
 // TODO: What if the prefix is empty? Should we skip encoding? Throw an error earlier? Throw an error here?
+// - I think an empty prefix string is valid, empty string is still more information then wholly unknown?
+// - I wonder what Terraform does in this situation today
 func (s stringPrefix) Encode(enc *msgpack.Encoder) error {
 	// Matching go-cty for the max prefix length allowed here
 	//

tftypes/value.go

@@ -42,11 +42,13 @@ type ValueCreator interface {
 // The recommended usage of a Value is to check that it is known, using
 // Value.IsKnown, then to convert it to a Go type, using Value.As. The Go type
 // can then be manipulated.
+//
+// TODO: add docs for new refinement data
 type Value struct {
 	typ   Type
 	value interface{}
 
-	refinements refinement.Refinements //nolint
+	refinements refinement.Refinements
 }
 
 func (val Value) String() string {
@@ -62,6 +64,7 @@ func (val Value) String() string {
 	}
 
 	// TODO: print refinements
+
 	if !val.IsKnown() {
 		return typ.String() + "<unknown>"
 	}
@@ -226,7 +229,9 @@ func (val Value) Equal(o Value) bool {
 	if !val.Type().Equal(o.Type()) {
 		return false
 	}
+
 	// TODO: compare refinements
+
 	deepEqual, err := val.deepEqual(o)
 	if err != nil {
 		return false
@@ -237,6 +242,9 @@ func (val Value) Equal(o Value) bool {
 // Copy returns a defensively-copied clone of Value that shares no underlying
 // data structures with the original Value and can be mutated without
 // accidentally mutating the original.
+//
+// TODO: Make sure this actually works for refinements. Consuming packages should not be able to mutate refinements of val
+// TODO: Add docs referencing refinements
 func (val Value) Copy() Value {
 	newVal := val.value
 	switch v := val.value.(type) {
@@ -253,7 +261,11 @@ func (val Value) Copy() Value {
 		}
 		newVal = newVals
 	}
-	return NewValue(val.Type(), newVal)
+
+	newTfValue := NewValue(val.Type(), newVal)
+	newTfValue.refinements = val.refinements
+
+	return newTfValue
 }
 
 // NewValue returns a Value constructed using the specified Type and stores the
@@ -599,7 +611,7 @@ func unexpectedValueTypeError(p *AttributePath, expected, got interface{}, typ T
 	return p.NewErrorf("unexpected value type %T, %s values must be of type %T", got, typ, expected)
 }
 
-// TODO: return error?
+// TODO: do we need to return an error? Like if you attempt to refine a value improperly (like string prefix on a number)?
 func (val Value) Refine(refinements refinement.Refinements) Value {
 	newVal := val.Copy()
 
@@ -611,6 +623,8 @@ func (val Value) Refine(refinements refinement.Refinements) Value {
 }
 
 func (val Value) Refinements() refinement.Refinements {
+	// TODO: is this copy really needed?
 	valCopy := val.Copy()
+
 	return valCopy.refinements
 }

tftypes/value_msgpack.go

@@ -434,6 +434,8 @@ func msgpackUnmarshalUnknown(dec *msgpack.Decoder, typ Type, path *AttributePath
 				return Value{}, path.NewErrorf("failed to decode msgpack extension body: string prefix refinement is not valid UTF-8")
 			}
 
+			// TODO: If terraform doesn't support an empty string prefix, then neither should we, potentially return an error here.
+
 			newRefinements[keyCode] = refinement.StringPrefix(prefix)
 		default:
 			// We don't want to error here, as go-cty could introduce new refinements that we'd
@@ -501,7 +503,8 @@ func marshalUnknownValue(val Value, typ Type, p *AttributePath, enc *msgpack.Enc
 	refnEnc := msgpack.NewEncoder(&refnBuf)
 	mapLen := 0
 
-	// TODO: Should the refinement interface be defining the encoding? Should we export the refinement implementations?
+	// TODO: Should the refinement interface be defining the encoding? Should we export the refinement implementation details?
+	// - If we keep it in the interface, then we can simplify this logic
 	for kind, refn := range val.refinements {
 		switch kind {
 		case refinement.KeyNullness: