Repository: amzn/ion-go
Pull request #99 (2020-07-22 01:37:27)

Preserve annotations when unmarshalling values
```Decoder``` drops annotations and consequently the output of Unmarshal does not have any of the annotations.

To get the annotations when Unmarshalling, user should send a struct to ```unmarshal()``` whereby one field of the struct is tagged as ``` `ion:"annotations"` ```.

For instance:
```	
type foo struct {
   Value   int
   AnyName []string `ion:"annotations"`
}
...
var val foo
err := UnmarshalStr("age::2, &val)
```



codecov-commenter: # [Codecov](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=h1) Report
> Merging [#99](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=desc) into [master](https://codecov.io/gh/amzn/ion-go/commit/0c30b5d01a9d6790add04dae077b4398ed11d28e&el=desc) will **increase** coverage by `0.53%`.
> The diff coverage is `80.00%`.

[![Impacted file tree graph](https://codecov.io/gh/amzn/ion-go/pull/99/graphs/tree.svg?width=650&height=150&src=pr&token=OBDT1TNoNE)](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
+ Coverage   77.11%   77.65%   +0.53%     
==========================================
  Files          22       22              
  Lines        4720     4766      +46     
==========================================
+ Hits         3640     3701      +61     
+ Misses        664      643      -21     
- Partials      416      422       +6     
```


| [Impacted Files](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [ion/unmarshal.go](https://codecov.io/gh/amzn/ion-go/pull/99/diff?src=pr&el=tree#diff-aW9uL3VubWFyc2hhbC5nbw==) | `69.44% <80.00%> (+7.52%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=footer). Last update [0c30b5d...057120b](https://codecov.io/gh/amzn/ion-go/pull/99?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Armanbqt: Created #103 for nested structs.

Patch:
diff --git a/ion/consts.go b/ion/consts.go
index 1a49dec..d4029f1 100644
--- a/ion/consts.go
+++ b/ion/consts.go
@@ -1,6 +1,7 @@
 package ion
 
 import (
+	"math/big"
 	"reflect"
 	"time"
 )
@@ -50,3 +51,4 @@ var hexChars = []byte{
 
 var timeType = reflect.TypeOf(time.Time{})
 var decimalType = reflect.TypeOf(Decimal{})
+var bigIntType = reflect.TypeOf(big.Int{})
diff --git a/ion/unmarshal.go b/ion/unmarshal.go
index c1e01cc..d52fc51 100644
--- a/ion/unmarshal.go
+++ b/ion/unmarshal.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"math/big"
 	"reflect"
 	"strconv"
 	"strings"
@@ -202,6 +201,9 @@ func (d *Decoder) decodeTo(v reflect.Value) error {
 	v = indirect(v, isNull)
 	if isNull {
 		v.Set(reflect.Zero(v.Type()))
+		if v.Type().Kind() == reflect.Struct {
+			return d.attachAnnotations(v)
+		}
 		return nil
 	}
 
@@ -250,6 +252,9 @@ func (d *Decoder) decodeBoolTo(v reflect.Value) error {
 		v.SetBool(val)
 		return nil
 
+	case reflect.Struct:
+		return d.decodeToStructWithAnnotation(v, reflect.Bool)
+
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
 			v.Set(reflect.ValueOf(val))
@@ -259,9 +264,9 @@ func (d *Decoder) decodeBoolTo(v reflect.Value) error {
 	return fmt.Errorf("ion: cannot decode bool to %v", v.Type().String())
 }
 
-var bigIntType = reflect.TypeOf(big.Int{})
-
 func (d *Decoder) decodeIntTo(v reflect.Value) error {
+	intAcceptableKinds := []reflect.Kind{reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
+		reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint, reflect.Uint64, reflect.Uintptr}
 	switch v.Kind() {
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
 		val, err := d.r.Int64Value()
@@ -309,6 +314,7 @@ func (d *Decoder) decodeIntTo(v reflect.Value) error {
 			v.Set(reflect.ValueOf(*val))
 			return nil
 		}
+		return d.decodeToStructWithAnnotation(v, intAcceptableKinds...)
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
@@ -324,6 +330,7 @@ func (d *Decoder) decodeIntTo(v reflect.Value) error {
 }
 
 func (d *Decoder) decodeFloatTo(v reflect.Value) error {
+	floatAcceptableKinds := []reflect.Kind{reflect.Float32, reflect.Float64}
 	val, err := d.r.FloatValue()
 	if err != nil {
 		return err
@@ -345,8 +352,9 @@ func (d *Decoder) decodeFloatTo(v reflect.Value) error {
 				return err
 			}
 			v.Set(reflect.ValueOf(*dec))
-			return nil
+			return d.attachAnnotations(v)
 		}
+		return d.decodeToStructWithAnnotation(v, floatAcceptableKinds...)
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
@@ -367,8 +375,9 @@ func (d *Decoder) decodeDecimalTo(v reflect.Value) error {
 	case reflect.Struct:
 		if v.Type() == decimalType {
 			v.Set(reflect.ValueOf(*val))
-			return nil
+			return d.attachAnnotations(v)
 		}
+		return d.decodeToStructWithAnnotation(v, decimalType.Kind())
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
@@ -389,8 +398,9 @@ func (d *Decoder) decodeTimestampTo(v reflect.Value) error {
 	case reflect.Struct:
 		if v.Type() == timeType {
 			v.Set(reflect.ValueOf(val))
-			return nil
+			return d.attachAnnotations(v)
 		}
+		return d.decodeToStructWithAnnotation(v, timeType.Kind())
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
@@ -412,6 +422,9 @@ func (d *Decoder) decodeStringTo(v reflect.Value) error {
 		v.SetString(val)
 		return nil
 
+	case reflect.Struct:
+		return d.decodeToStructWithAnnotation(v, reflect.String)
+
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
 			v.Set(reflect.ValueOf(val))
@@ -422,6 +435,7 @@ func (d *Decoder) decodeStringTo(v reflect.Value) error {
 }
 
 func (d *Decoder) decodeLobTo(v reflect.Value) error {
+	lobAcceptableKind := []reflect.Kind{reflect.Slice, reflect.Array}
 	val, err := d.r.ByteValue()
 	if err != nil {
 		return err
@@ -443,6 +457,9 @@ func (d *Decoder) decodeLobTo(v reflect.Value) error {
 			return nil
 		}
 
+	case reflect.Struct:
+		return d.decodeToStructWithAnnotation(v, lobAcceptableKind...)
+
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
 			v.Set(reflect.ValueOf(val))
@@ -476,6 +493,11 @@ func (d *Decoder) decodeStructTo(v reflect.Value) error {
 func (d *Decoder) decodeStructToStruct(v reflect.Value) error {
 	fields := fieldsFor(v.Type())
 
+	err := d.attachAnnotations(v)
+	if err != nil {
+		return err
+	}
+
 	if err := d.r.StepIn(); err != nil {
 		return err
 	}
@@ -569,6 +591,7 @@ func (d *Decoder) decodeStructToMap(v reflect.Value) error {
 }
 
 func (d *Decoder) decodeSliceTo(v reflect.Value) error {
+	listSexpAcceptableKind := []reflect.Kind{reflect.Slice, reflect.Array}
 	k := v.Kind()
 
 	// If all we know is we need an interface{}, decode an []interface{} with
@@ -582,6 +605,10 @@ func (d *Decoder) decodeSliceTo(v reflect.Value) error {
 		return nil
 	}
 
+	if k == reflect.Struct {
+		return d.decodeToStructWithAnnotation(v, listSexpAcceptableKind...)
+	}
+
 	// Only other valid targets are arrays and slices.
 	if k != reflect.Array && k != reflect.Slice {
 		return fmt.Errorf("ion: cannot unmarshal slice to %v", v.Type().String())
@@ -671,3 +698,86 @@ func indirect(v reflect.Value, wantPtr bool) reflect.Value {
 
 	return v
 }
+
+func (d *Decoder) decodeToStructWithAnnotation(v reflect.Value, valueAcceptableKinds ...reflect.Kind) error {
+	validAnnotatableStruct, err := isValidAnnotatableStruct(v, valueAcceptableKinds)
+	if err != nil {
+		return err
+	}
+	if !validAnnotatableStruct {
+		return fmt.Errorf("ion: cannot decode %v into %v", v, v.Type().String())
+	}
+
+	// populate annotations to the struct
+	if err := d.attachAnnotations(v); err != nil {
+		return err
+	}
+
+	fields := fieldsFor(v.Type())
+	for _, field := range fields {
+		if !field.annotations {
+			field := findField(fields, field.name)
+			if field != nil {
+				subValue, err := findSubvalue(v, field)
+				if err != nil {
+					return err
+				}
+
+				if err := d.decodeTo(subValue); err != nil {
+					return err
+				}
+				break
+			}
+		}
+	}
+	return nil
+}
+
+func (d *Decoder) attachAnnotations(v reflect.Value) error {
+	fields := fieldsFor(v.Type())
+	for _, field := range fields {
+		if field.annotations {
+			subValue, err := findSubvalue(v, &field)
+			if err != nil {
+				return err
+			}
+			annotations := d.r.Annotations()
+			subValue.Set(reflect.ValueOf(annotations))
+			break
+		}
+	}
+	return nil
+}
+
+// expected struct for decoding Ion values must have only 2 fields: one has `ion:",annotation"`
+// tag, and the other field must be of a type where Ion value can be decoded to.
+func isValidAnnotatableStruct(v reflect.Value, listofkinds []reflect.Kind) (bool, error) {
+	fields := fieldsFor(v.Type())
+	hasAnnotation := false
+	acceptableValueType := false
+	if len(fields) == 2 {
+		for _, field := range fields {
+			subValue, err := findSubvalue(v, &field)
+			if err != nil {
+				return false, err
+			}
+			if field.annotations {
+				hasAnnotation = true
+				continue
+			}
+			if isAcceptableKind(listofkinds, subValue.Type().Kind()) {
+				acceptableValueType = true
+			}
+		}
+	}
+	return hasAnnotation && acceptableValueType, nil
+}
+
+func isAcceptableKind(valueAcceptableKinds []reflect.Kind, valueKind reflect.Kind) bool {
+	for _, kind := range valueAcceptableKinds {
+		if kind == valueKind || reflect.Interface == valueKind {
+			return true
+		}
+	}
+	return false
+}


Test patch:
diff --git a/ion/unmarshal_test.go b/ion/unmarshal_test.go
index 2f51668..b2ea187 100644
--- a/ion/unmarshal_test.go
+++ b/ion/unmarshal_test.go
@@ -568,3 +568,105 @@ func TestDecodeLotsOfInts(t *testing.T) {
 		}
 	}
 }
+
+func TestUnmarshalWithAnnotation(t *testing.T) {
+	type foo struct {
+		Value   interface{}
+		AnyName []string `ion:",annotations"`
+	}
+
+	test := func(str, testName string, eval foo) {
+		t.Run(testName, func(t *testing.T) {
+			var val foo
+			err := UnmarshalStr(str, &val)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !reflect.DeepEqual(val, eval) {
+				t.Errorf("expected %v, got %v", eval, val)
+			}
+		})
+	}
+
+	test("with::multiple::annotations::null", "null", foo{nil, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::true", "bool", foo{true, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::2", "int", foo{2, []string{"with", "multiple", "annotations"}})
+	bi := new(big.Int).Neg(new(big.Int).SetUint64(0xFFFFFFFFFFFFFFFF))
+	test("with::multiple::annotations::-18446744073709551615", "big.Int", foo{bi, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::2.1e1", "float", foo{2.1e1, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::2.2", "decimal", foo{MustParseDecimal("2.2"), []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::\"abc\"", "string", foo{"abc", []string{"with", "multiple", "annotations"}})
+	timestamp := time.Date(2000, 1, 2, 3, 4, 5, 0, time.UTC)
+	test("with::multiple::annotations::2000-01-02T03:04:05Z", "timestamp", foo{timestamp, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::{{'''abc'''}}", "clob", foo{[]byte{97, 98, 99}, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::{{/w==}}", "blob", foo{[]byte{255}, []string{"with", "multiple", "annotations"}})
+}
+
+func TestUnmarshalContainersWithAnnotation(t *testing.T) {
+	type foo struct {
+		Value   []int
+		AnyName []string `ion:",annotations"`
+	}
+
+	test := func(str, testName string, eval interface{}) {
+		t.Run(testName, func(t *testing.T) {
+			var val foo
+			err := UnmarshalStr(str, &val)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !reflect.DeepEqual(val, eval) {
+				t.Errorf("expected %v, got %v", eval, val)
+			}
+		})
+	}
+
+	test("with::multiple::annotations::[1, 2, 3]", "list", foo{[]int{1, 2, 3}, []string{"with", "multiple", "annotations"}})
+	test("with::multiple::annotations::(1 2 3)", "sexp", foo{[]int{1, 2, 3}, []string{"with", "multiple", "annotations"}})
+}
+
+func TestUnmarshalNestedStructsWithAnnotation(t *testing.T) {
+	type nestedInt struct {
+		Value           int
+		ValueAnnotation []string `ion:",annotations"`
+	}
+
+	type nestedStruct struct {
+		Field2                nestedInt
+		InnerStructAnnotation []string `ion:",annotations"`
+	}
+
+	type topLevelStruct struct {
+		Field1             nestedStruct
+		TopLevelAnnotation []string `ion:",annotations"`
+	}
+
+	test := func(str, testName string, eval interface{}) {
+		t.Run(testName, func(t *testing.T) {
+			var val topLevelStruct
+			err := UnmarshalStr(str, &val)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !reflect.DeepEqual(val, eval) {
+				t.Errorf("expected %v, got %v", eval, val)
+			}
+		})
+	}
+
+	/*
+		foo::{
+		  field1: bar::{
+		    field2: baz::5
+		  }
+		}
+	*/
+	innerStructVal := nestedInt{Value: 5, ValueAnnotation: []string{"baz"}}
+	mainStructVal := nestedStruct{Field2: innerStructVal, InnerStructAnnotation: []string{"bar"}}
+	expectedValue := topLevelStruct{Field1: mainStructVal, TopLevelAnnotation: []string{"foo"}}
+
+	test("foo::{Field1:bar::{Field2:baz::5}}", "nested structs", expectedValue)
+}