Repository: amzn/ion-go
Pull request #166 (2020-10-09 22:11:47)

Fixes in unmarshal
There are 2 commits in this pull request to improve Unmarshal:
- [Fix for unameshal string](https://github.com/amzn/ion-go/pull/166/commits/b02f30d8e63a76cdbd2babdefc4f4f22ecd6b833) : Using value of string when unmarshalling string into interface{} and fix some of the corresponding unit tests (the original change, roots in when returning type of [reader.string changed to a pointer](https://github.com/amzn/ion-go/pull/149/files#diff-47907c97adbd5c047dc4ec6220180178R493). 

- [Add null check](https://github.com/amzn/ion-go/pull/166/commits/3119673f3b87299180829d9bdf8da25cc336367d) : Setting nil to ```v.Set()``` can lead to error.



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

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

```diff
@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
+ Coverage   74.90%   74.94%   +0.04%     
==========================================
  Files          25       25              
  Lines        4857     4865       +8     
==========================================
+ Hits         3638     3646       +8     
  Misses        724      724              
  Partials      495      495              
```


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

------

[Continue to review full report at Codecov](https://codecov.io/gh/amzn/ion-go/pull/166?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/166?src=pr&el=footer). Last update [0d00cfc...3119673](https://codecov.io/gh/amzn/ion-go/pull/166?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/ion/unmarshal.go b/ion/unmarshal.go
index 1375b6f..a81cc44 100644
--- a/ion/unmarshal.go
+++ b/ion/unmarshal.go
@@ -348,7 +348,9 @@ func (d *Decoder) decodeBoolTo(v reflect.Value) error {
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
-			v.Set(reflect.ValueOf(*val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return nil
 		}
 	}
@@ -400,7 +402,9 @@ func (d *Decoder) decodeIntTo(v reflect.Value) error {
 			if err != nil {
 				return err
 			}
-			v.Set(reflect.ValueOf(*val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return nil
 		}
 		return d.decodeToStructWithAnnotation(v, typesAcceptableKinds[IntType]...)
@@ -439,14 +443,18 @@ func (d *Decoder) decodeFloatTo(v reflect.Value) error {
 			if err != nil {
 				return err
 			}
-			v.Set(reflect.ValueOf(*dec))
+			if val != nil {
+				v.Set(reflect.ValueOf(*dec))
+			}
 			return d.attachAnnotations(v)
 		}
 		return d.decodeToStructWithAnnotation(v, typesAcceptableKinds[FloatType]...)
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
-			v.Set(reflect.ValueOf(*val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return nil
 		}
 	}
@@ -462,14 +470,18 @@ func (d *Decoder) decodeDecimalTo(v reflect.Value) error {
 	switch v.Kind() {
 	case reflect.Struct:
 		if v.Type() == decimalType {
-			v.Set(reflect.ValueOf(*val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return d.attachAnnotations(v)
 		}
 		return d.decodeToStructWithAnnotation(v, decimalType.Kind())
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
-			v.Set(reflect.ValueOf(val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return nil
 		}
 	}
@@ -485,14 +497,18 @@ func (d *Decoder) decodeTimestampTo(v reflect.Value) error {
 	switch v.Kind() {
 	case reflect.Struct:
 		if v.Type() == timestampType {
-			v.Set(reflect.ValueOf(*val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return d.attachAnnotations(v)
 		}
 		return d.decodeToStructWithAnnotation(v, timestampType.Kind())
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
-			v.Set(reflect.ValueOf(*val))
+			if val != nil {
+				v.Set(reflect.ValueOf(*val))
+			}
 			return nil
 		}
 	}
@@ -538,7 +554,6 @@ func (d *Decoder) decodeStringTo(v reflect.Value) error {
 	case reflect.String:
 		if val != nil {
 			v.SetString(*val)
-
 		}
 		return nil
 
@@ -547,7 +562,7 @@ func (d *Decoder) decodeStringTo(v reflect.Value) error {
 
 	case reflect.Interface:
 		if v.NumMethod() == 0 {
-			v.Set(reflect.ValueOf(val))
+			v.Set(reflect.ValueOf(*val))
 			return nil
 		}
 	}


Test patch:
diff --git a/ion/unmarshal_test.go b/ion/unmarshal_test.go
index 8d69fa8..556b488 100644
--- a/ion/unmarshal_test.go
+++ b/ion/unmarshal_test.go
@@ -265,7 +265,7 @@ func TestUnmarshalBinary(t *testing.T) {
 
 	var decimalVal Decimal
 	decimalBytes := prefixIVM([]byte{0x51, 0xFF}) // 0d-63
-	test(decimalBytes, decimalVal, MustParseDecimal("0d-63"))
+	test(decimalBytes, decimalVal, *MustParseDecimal("0d-63"))
 
 	var timestampValue Timestamp
 	timestampBytes := prefixIVM([]byte{0x67, 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86}) // 0001-02-03T04:05:06Z
@@ -278,7 +278,7 @@ func TestUnmarshalBinary(t *testing.T) {
 
 	var stringVal string
 	stringBytes := prefixIVM([]byte{0x83, 'a', 'b', 'c'}) // "abc"
-	test(stringBytes, stringVal, newString("abc"))
+	test(stringBytes, stringVal, "abc")
 
 	var clobVal []byte
 	clobBytes := prefixIVM([]byte{0x92, 0x0A, 0x0B})
@@ -618,8 +618,8 @@ func TestUnmarshalWithAnnotation(t *testing.T) {
 	bi := new(big.Int).Neg(new(big.Int).SetUint64(0xFFFFFFFFFFFFFFFF))
 	test("with::multiple::annotations::-18446744073709551615", "big.Int", foo{bi, annotations})
 	test("with::multiple::annotations::2.1e1", "float", foo{2.1e1, annotations})
-	test("with::multiple::annotations::2.2", "decimal", foo{MustParseDecimal("2.2"), annotations})
-	test("with::multiple::annotations::\"abc\"", "string", foo{newString("abc"), annotations})
+	test("with::multiple::annotations::2.2", "decimal", foo{*MustParseDecimal("2.2"), annotations})
+	test("with::multiple::annotations::\"abc\"", "string", foo{"abc", annotations})
 	timestamp := NewTimestamp(time.Date(2000, 1, 2, 3, 4, 5, 0, time.UTC), TimestampPrecisionSecond, TimezoneUTC)
 	test("with::multiple::annotations::2000-01-02T03:04:05Z", "timestamp", foo{timestamp, annotations})
 	test("with::multiple::annotations::{{'''abc'''}}", "clob", foo{[]byte{97, 98, 99}, annotations})