From 77b516425597da3c093a666c11608112e91604de Mon Sep 17 00:00:00 2001 From: Gustavo Niemeyer Date: Wed, 19 Jun 2013 13:52:23 -0400 Subject: [PATCH] Review by Roger plus overall naming improvements. --- decode.go | 5 ++--- decode_test.go | 15 ++++++++++++--- encode.go | 4 ++-- encode_test.go | 36 ++++++++++++++++++++-------------- goyaml.go | 52 ++++++++++++++++++++++++++------------------------ 5 files changed, 65 insertions(+), 47 deletions(-) diff --git a/decode.go b/decode.go index 15f4a04..5dd2ce5 100644 --- a/decode.go +++ b/decode.go @@ -433,18 +433,17 @@ func (d *decoder) mapping(n *node, out reflect.Value) (good bool) { } func (d *decoder) mappingStruct(n *node, out reflect.Value) (good bool) { - fields, err := getStructFields(out.Type()) + sinfo, err := getStructInfo(out.Type()) if err != nil { panic(err) } name := settableValueOf("") - fieldsMap := fields.Map l := len(n.children) for i := 0; i < l; i += 2 { if !d.unmarshal(n.children[i], name) { continue } - if info, ok := fieldsMap[name.String()]; ok { + if info, ok := sinfo.FieldsMap[name.String()]; ok { var field reflect.Value if info.Inline == nil { field = out.Field(info.Num) diff --git a/decode_test.go b/decode_test.go index 965032c..718b679 100644 --- a/decode_test.go +++ b/decode_test.go @@ -320,14 +320,23 @@ var unmarshalTests = []struct { // Struct inlining { - "a: 1\nb: 2\n", + "a: 1\nb: 2\nc: 3\n", &struct { A int - C struct{ B int } `yaml:",inline"` - }{1, struct{ B int }{2}}, + C inlineB `yaml:",inline"` + }{1, inlineB{2, inlineC{3}}}, }, } +type inlineB struct { + B int + inlineC `yaml:",inline"` +} + +type inlineC struct { + C int +} + func (s *S) TestUnmarshal(c *C) { for i, item := range unmarshalTests { t := reflect.ValueOf(item.value).Type() diff --git a/encode.go b/encode.go index 6a9c8fa..b228a10 100644 --- a/encode.go +++ b/encode.go @@ -109,12 +109,12 @@ func (e *encoder) mapv(tag string, in reflect.Value) { } func (e *encoder) structv(tag string, in reflect.Value) { - fields, err := getStructFields(in.Type()) + sinfo, err := getStructInfo(in.Type()) if err != nil { panic(err) } e.mappingv(tag, func() { - for _, info := range fields.List { + for _, info := range sinfo.FieldsList { var value reflect.Value if info.Inline == nil { value = in.Field(info.Num) diff --git a/encode_test.go b/encode_test.go index ad44241..5d16426 100644 --- a/encode_test.go +++ b/encode_test.go @@ -205,9 +205,9 @@ var marshalTests = []struct { { &struct { A int - C struct{ B int } `yaml:",inline"` - }{1, struct{ B int }{2}}, - "a: 1\nb: 2\n", + C inlineB `yaml:",inline"` + }{1, inlineB{2, inlineC{3}}}, + "a: 1\nb: 2\nc: 3\n", }, } @@ -219,17 +219,25 @@ func (s *S) TestMarshal(c *C) { } } -//var unmarshalErrorTests = []struct{data, error string}{ -// {"v: !!float 'error'", "Can't decode !!str 'error' as a !!float"}, -//} -// -//func (s *S) TestUnmarshalErrors(c *C) { -// for _, item := range unmarshalErrorTests { -// var value interface{} -// err := goyaml.Unmarshal([]byte(item.data), &value) -// c.Assert(err, Matches, item.error) -// } -//} +var marshalErrorTests = []struct { + value interface{} + error string +}{ + { + &struct { + B int + inlineB ",inline" + }{1, inlineB{2, inlineC{3}}}, + `Duplicated key 'b' in struct struct \{ B int; .*`, + }, +} + +func (s *S) TestMarshalErrors(c *C) { + for _, item := range marshalErrorTests { + _, err := goyaml.Marshal(item.value) + c.Assert(err, ErrorMatches, item.error) + } +} var marshalTaggedIfaceTest interface{} = &struct{ A string }{"B"} diff --git a/goyaml.go b/goyaml.go index 3981d71..e52a2dd 100644 --- a/goyaml.go +++ b/goyaml.go @@ -1,12 +1,4 @@ -// -// goyaml - YAML support for the Go language -// -// https://wiki.ubuntu.com/goyaml -// -// Copyright (c) 2011 Canonical Ltd. -// -// Written by Gustavo Niemeyer -// +// Package goyaml implements YAML support for the Go language. package goyaml import ( @@ -74,16 +66,20 @@ type Getter interface { // tag value may be used to tweak the name. Everything before the first // comma in the field tag will be used as the name. The values following // the comma are used to tweak the marshalling process (see Marshal). +// Conflicting names result in a runtime error. // // For example: // // type T struct { -// F int "a,omitempty" +// F int `yaml:"a,omitempty"` // B int // } // var T t // goyaml.Unmarshal([]byte("a: 1\nb: 2"), &t) // +// See the documentation of Marshal for the format of tags and a list of +// supported tag options. +// func Unmarshal(in []byte, out interface{}) (err error) { defer handleErr(&err) d := newDecoder() @@ -104,9 +100,8 @@ func Unmarshal(in []byte, out interface{}) (err error) { // The lowercased field name is used as the key for each exported field, // but this behavior may be changed using the respective field tag. // The tag may also contain flags to tweak the marshalling behavior for -// the field. The tag formats accepted are: -// -// "[][,[,]]" +// the field. Conflicting names result in a runtime error. The tag format +// accepted is: // // `(...) yaml:"[][,[,]]" (...)` // @@ -149,9 +144,14 @@ func Marshal(in interface{}) (out []byte, err error) { // The code in this section was copied from gobson. -type structFields struct { - Map map[string]fieldInfo - List []fieldInfo +// structInfo holds details for the serialization of fields of +// a given struct. +type structInfo struct { + FieldsMap map[string]fieldInfo + FieldsList []fieldInfo + + // InlineMap is the number of the field in the struct that + // contains an ,inline map, or -1 if there's none. InlineMap int } @@ -160,10 +160,12 @@ type fieldInfo struct { Num int OmitEmpty bool Flow bool + + // Inline holds the field index if the field is part of an inlined struct. Inline []int } -var fieldMap = make(map[reflect.Type]*structFields) +var structMap = make(map[reflect.Type]*structInfo) var fieldMapMutex sync.RWMutex type externalPanic string @@ -172,12 +174,12 @@ func (e externalPanic) String() string { return string(e) } -func getStructFields(st reflect.Type) (*structFields, error) { +func getStructInfo(st reflect.Type) (*structInfo, error) { fieldMapMutex.RLock() - fields, found := fieldMap[st] + sinfo, found := structMap[st] fieldMapMutex.RUnlock() if found { - return fields, nil + return sinfo, nil } n := st.NumField() @@ -230,11 +232,11 @@ func getStructFields(st reflect.Type) (*structFields, error) { // } // inlineMap = info.Num case reflect.Struct: - sfields, err := getStructFields(field.Type) + sinfo, err := getStructInfo(field.Type) if err != nil { return nil, err } - for _, finfo := range sfields.List { + for _, finfo := range sinfo.FieldsList { if _, found := fieldsMap[finfo.Key]; found { msg := "Duplicated key '" + finfo.Key + "' in struct " + st.String() return nil, errors.New(msg) @@ -269,12 +271,12 @@ func getStructFields(st reflect.Type) (*structFields, error) { fieldsMap[info.Key] = info } - fields = &structFields{fieldsMap, fieldsList, inlineMap} + sinfo = &structInfo{fieldsMap, fieldsList, inlineMap} fieldMapMutex.Lock() - fieldMap[st] = fields + structMap[st] = sinfo fieldMapMutex.Unlock() - return fields, nil + return sinfo, nil } func isZero(v reflect.Value) bool {