Skip to content

Commit 3a73c35

Browse files
authored
fix: decoder panics on tag not found (#100)
1 parent 602b24c commit 3a73c35

File tree

4 files changed

+91
-58
lines changed

4 files changed

+91
-58
lines changed

.golangci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ issues:
5050
exclude-rules:
5151
- path: (.+)_test.go
5252
linters:
53+
- cyclop
5354
- errcheck
5455
- errorlint
5556
- funlen

decoder/compile.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import (
99
"github.com/abemedia/go-don/internal/byteconv"
1010
)
1111

12-
type decoder func(reflect.Value, Getter) error
12+
type decoder = func(reflect.Value, Getter) error
13+
14+
func noopDecoder(reflect.Value, Getter) error { return nil }
1315

1416
var unmarshalerType = reflect.TypeOf((*encoding.TextUnmarshaler)(nil)).Elem()
1517

decoder/decode.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ func New(tag string) *Decoder {
2626
}
2727

2828
func (d *Decoder) Decode(data Getter, v any) error {
29-
val := reflect.ValueOf(v).Elem()
29+
val := reflect.ValueOf(v)
30+
if val.Kind() != reflect.Pointer {
31+
return ErrUnsupportedType
32+
}
33+
34+
val = val.Elem()
3035
if val.Kind() != reflect.Struct {
3136
return ErrUnsupportedType
3237
}
@@ -37,8 +42,11 @@ func (d *Decoder) Decode(data Getter, v any) error {
3742
if !ok {
3843
var err error
3944
dec, err = compile(t, d.tag, t.Kind() == reflect.Ptr)
40-
if err != nil && err != ErrTagNotFound { //nolint:errorlint,goerr113
41-
return err
45+
if err != nil {
46+
if err != ErrTagNotFound { //nolint:errorlint,goerr113
47+
return err
48+
}
49+
dec = noopDecoder
4250
}
4351

4452
d.cache.Store(t, dec)

decoder/decode_test.go

Lines changed: 76 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package decoder_test
22

33
import (
4+
"reflect"
45
"testing"
56

67
"github.com/abemedia/go-don/decoder"
@@ -80,62 +81,83 @@ func TestDecode(t *testing.T) {
8081
},
8182
}
8283

83-
dec := decoder.New("field")
84-
85-
actual := &test{}
86-
if err := dec.Decode(in, actual); err != nil {
87-
t.Fatal(err)
88-
}
89-
90-
if diff := cmp.Diff(expected, actual); diff != "" {
91-
t.Errorf(diff)
92-
}
93-
}
94-
95-
func TestCached(t *testing.T) {
96-
type test struct {
97-
String string `field:"string"`
98-
}
99-
100-
in := decoder.Map{"string": {"string"}}
101-
expected := &test{String: "string"}
102-
103-
dec, err := decoder.NewCached(test{}, "field")
104-
if err != nil {
105-
t.Fatal(err)
106-
}
107-
108-
actual := &test{}
109-
110-
if err = dec.Decode(in, actual); err != nil {
111-
t.Fatal(err)
112-
}
113-
114-
if diff := cmp.Diff(expected, actual); diff != "" {
115-
t.Errorf(diff)
116-
}
84+
t.Run("Decoder", func(t *testing.T) {
85+
dec := decoder.New("field")
86+
actual := &test{}
87+
if err := dec.Decode(in, actual); err != nil {
88+
t.Fatal(err)
89+
}
90+
if diff := cmp.Diff(expected, actual); diff != "" {
91+
t.Errorf(diff)
92+
}
93+
})
94+
95+
t.Run("CachedDecoder", func(t *testing.T) {
96+
dec, err := decoder.NewCached(test{}, "field")
97+
if err != nil {
98+
t.Fatal(err)
99+
}
100+
101+
actual := &test{}
102+
if err := dec.Decode(in, actual); err != nil {
103+
t.Fatal(err)
104+
}
105+
if diff := cmp.Diff(expected, actual); diff != "" {
106+
t.Errorf(diff)
107+
}
108+
109+
actual = &test{}
110+
val := reflect.ValueOf(actual).Elem()
111+
if err = dec.DecodeValue(in, val); err != nil {
112+
t.Fatal(err)
113+
}
114+
if diff := cmp.Diff(expected, actual); diff != "" {
115+
t.Errorf(diff)
116+
}
117+
})
118+
119+
t.Run("CachedDecoder_NilPointer", func(t *testing.T) {
120+
dec, err := decoder.NewCached(&test{}, "field")
121+
if err != nil {
122+
t.Fatal(err)
123+
}
124+
125+
var actual *test
126+
if err := dec.Decode(in, &actual); err != nil {
127+
t.Fatal(err)
128+
}
129+
if diff := cmp.Diff(expected, actual); diff != "" {
130+
t.Errorf(diff)
131+
}
132+
})
117133
}
118134

119-
func TestCachedNil(t *testing.T) {
120-
type test struct {
121-
String string `field:"string"`
135+
func TestDecodeError(t *testing.T) {
136+
type noTag struct {
137+
Test string `json:"test"`
122138
}
123-
124-
in := decoder.Map{"string": {"string"}}
125-
expected := &test{String: "string"}
126-
127-
dec, err := decoder.NewCached(&test{}, "field")
128-
if err != nil {
129-
t.Fatal(err)
130-
}
131-
132-
var actual *test
133-
134-
if err = dec.Decode(in, &actual); err != nil {
135-
t.Fatal(err)
136-
}
137-
138-
if diff := cmp.Diff(expected, actual); diff != "" {
139-
t.Errorf(diff)
139+
type unsupportedType struct {
140+
Test chan string `field:"test"`
140141
}
142+
s := ""
143+
tests := []any{"", &s, 1, noTag{}, &unsupportedType{}}
144+
145+
t.Run("Decoder", func(t *testing.T) {
146+
for _, test := range tests {
147+
dec := decoder.New("field")
148+
err := dec.Decode(nil, test)
149+
if err == nil {
150+
t.Errorf("should return error for %T", test)
151+
}
152+
}
153+
})
154+
155+
t.Run("CachedDecoder", func(t *testing.T) {
156+
for _, test := range tests {
157+
_, err := decoder.NewCached(test, "field")
158+
if err == nil {
159+
t.Errorf("should return error for %T", test)
160+
}
161+
}
162+
})
141163
}

0 commit comments

Comments
 (0)