diff --git a/crazy_test.go b/crazy_test.go deleted file mode 100644 index 46538cd..0000000 --- a/crazy_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package param - -import ( - "net/url" - "testing" -) - -type Crazy struct { - A *Crazy - B *Crazy - Value int - Slice []int - Map map[string]Crazy -} - -func TestCrazy(t *testing.T) { - t.Parallel() - - c := Crazy{} - err := Parse(url.Values{ - "A[B][B][A][Value]": {"1"}, - "B[A][A][Slice][]": {"3", "1", "4"}, - "B[Map][hello][A][Value]": {"8"}, - "A[Value]": {"2"}, - "A[Slice][]": {"9", "1", "1"}, - "Value": {"42"}, - }, &c) - if err != nil { - t.Error("Error parsing craziness: ", err) - } - - // Exhaustively checking everything here is going to be a huge pain, so - // let's just hope for the best, pretend NPEs don't exist, and hope that - // this test covers enough stuff that it's actually useful. - assertEqual(t, "c.A.B.B.A.Value", 1, c.A.B.B.A.Value) - assertEqual(t, "c.A.Value", 2, c.A.Value) - assertEqual(t, "c.Value", 42, c.Value) - assertEqual(t, `c.B.Map["hello"].A.Value`, 8, c.B.Map["hello"].A.Value) - - assertEqual(t, "c.A.B.B.B", (*Crazy)(nil), c.A.B.B.B) - assertEqual(t, "c.A.B.A", (*Crazy)(nil), c.A.B.A) - assertEqual(t, "c.A.A", (*Crazy)(nil), c.A.A) - - if c.Slice != nil || c.Map != nil { - t.Error("Map and Slice should not be set") - } - - sl := c.B.A.A.Slice - if len(sl) != 3 || sl[0] != 3 || sl[1] != 1 || sl[2] != 4 { - t.Error("Something is wrong with c.B.A.A.Slice") - } - sl = c.A.Slice - if len(sl) != 3 || sl[0] != 9 || sl[1] != 1 || sl[2] != 1 { - t.Error("Something is wrong with c.A.Slice") - } -} diff --git a/error_helpers.go b/error_helpers.go deleted file mode 100644 index 9477d3a..0000000 --- a/error_helpers.go +++ /dev/null @@ -1,25 +0,0 @@ -package param - -import ( - "errors" - "fmt" - "log" -) - -// Testing log.Fatal in tests is... not a thing. Allow tests to stub it out. -var pebkacTesting bool - -const errPrefix = "param/parse: " -const yourFault = " This is a bug in your use of the param library." - -// Problem exists between keyboard and chair. This function is used in cases of -// programmer error, i.e. an inappropriate use of the param library, to -// immediately force the program to halt with a hopefully helpful error message. -func pebkac(format string, a ...interface{}) { - err := errors.New(errPrefix + fmt.Sprintf(format, a...) + yourFault) - if pebkacTesting { - panic(err) - } else { - log.Fatal(err) - } -} diff --git a/errors.go b/errors.go index e6b9de6..e91dfec 100644 --- a/errors.go +++ b/errors.go @@ -110,3 +110,20 @@ func (k KeyError) Error() string { return fmt.Sprintf("param: error parsing key %q: unknown field %q on "+ "struct %q of type %v", k.FullKey, k.Field, k.Key, k.Type) } + +// InvalidParseError describes an invalid argument passed to Parse. It is always +// the result of programmer error. +type InvalidParseError struct { + Type reflect.Type + Hint string +} + +func (err InvalidParseError) Error() string { + msg := fmt.Sprintf("param/parse: unsupported type %v", err.Type) + + if err.Hint != "" { + msg += ": " + err.Hint + } + + return msg +} diff --git a/param.go b/param.go index 685df90..1c55f6d 100644 --- a/param.go +++ b/param.go @@ -26,34 +26,38 @@ import ( ) // Parse the given arguments into the the given pointer to a struct object. -func Parse(params url.Values, target interface{}) (err error) { +func Parse(params url.Values, target interface{}) error { v := reflect.ValueOf(target) - defer func() { - if r := recover(); r != nil { - var ok bool - err, ok = r.(error) - if !ok { - panic(err) - } + if v.Kind() != reflect.Ptr || v.Elem().Kind() != reflect.Struct { + hint := "target must be a pointer to a struct" + if v.Kind() == reflect.Ptr && v.IsNil() { + hint = "target may not be a nil pointer" } - }() - if v.Kind() != reflect.Ptr || v.Elem().Kind() != reflect.Struct { - pebkac("Target of param.Parse must be a pointer to a struct. "+ - "We instead were passed a %v", v.Type()) + return InvalidParseError{ + Type: v.Type(), + Hint: hint, + } } el := v.Elem() t := el.Type() - cache := cacheStruct(t) + cache, err := cacheStruct(t) + if err != nil { + return err + } for key, values := range params { sk, keytail := key, "" if i := strings.IndexRune(key, '['); i != -1 { sk, keytail = sk[:i], sk[i:] } - parseStructField(cache, key, sk, keytail, values, el) + + err := parseStructField(cache, key, sk, keytail, values, el) + if err != nil { + return err + } } return nil diff --git a/param_test.go b/param_test.go index 7ee81d3..1437bf0 100644 --- a/param_test.go +++ b/param_test.go @@ -71,7 +71,7 @@ func init() { testTime, _ = time.Parse(time.RFC3339, testTimeString) } -func singletonErrors(t *testing.T, field, valid, invalid string) { +func singletonErrorsWithInvalid(t *testing.T, field, valid, invalid string) { e := Everything{} err := Parse(url.Values{field: {invalid}}, &e) @@ -79,17 +79,23 @@ func singletonErrors(t *testing.T, field, valid, invalid string) { t.Errorf("Expected error parsing %q as %s", invalid, field) } - err = Parse(url.Values{field + "[]": {valid}}, &e) + singletonErrors(t, field, valid) +} + +func singletonErrors(t *testing.T, field, val string) { + e := Everything{} + + err := Parse(url.Values{field + "[]": {val}}, &e) if err == nil { t.Errorf("Expected error parsing nested %s", field) } - err = Parse(url.Values{field + "[nested]": {valid}}, &e) + err = Parse(url.Values{field + "[nested]": {val}}, &e) if err == nil { t.Errorf("Expected error parsing nested %s", field) } - err = Parse(url.Values{field: {valid, valid}}, &e) + err = Parse(url.Values{field: {val, val}}, &e) if err == nil { t.Errorf("Expected error passing %s twice", field) } @@ -122,7 +128,7 @@ func TestBoolTyped(t *testing.T) { func TestBoolErrors(t *testing.T) { t.Parallel() - singletonErrors(t, "Bool", "true", "llama") + singletonErrorsWithInvalid(t, "Bool", "true", "llama") } var intAnswers = map[string]int{ @@ -158,7 +164,7 @@ func TestIntTyped(t *testing.T) { func TestIntErrors(t *testing.T) { t.Parallel() - singletonErrors(t, "Int", "1", "llama") + singletonErrorsWithInvalid(t, "Int", "1", "llama") e := Everything{} err := Parse(url.Values{"Int": {"4.2"}}, &e) @@ -199,7 +205,7 @@ func TestUintTyped(t *testing.T) { func TestUintErrors(t *testing.T) { t.Parallel() - singletonErrors(t, "Uint", "1", "llama") + singletonErrorsWithInvalid(t, "Uint", "1", "llama") e := Everything{} err := Parse(url.Values{"Uint": {"4.2"}}, &e) @@ -249,7 +255,7 @@ func TestFloatTyped(t *testing.T) { func TestFloatErrors(t *testing.T) { t.Parallel() - singletonErrors(t, "Float", "1.0", "llama") + singletonErrorsWithInvalid(t, "Float", "1.0", "llama") } func TestMap(t *testing.T) { @@ -434,6 +440,17 @@ func TestStringTyped(t *testing.T) { assertEqual(t, "e.AString", MyString("llama"), e.AString) } +func TestStringErrors(t *testing.T) { + t.Parallel() + singletonErrors(t, "String", "str") + + e := Everything{} + err := Parse(url.Values{"Int": {"4.2"}}, &e) + if err == nil { + t.Error("Expected error parsing float as int") + } +} + func TestStruct(t *testing.T) { t.Parallel() e := Everything{} @@ -500,10 +517,112 @@ func TestTextUnmarshaler(t *testing.T) { func TestTextUnmarshalerError(t *testing.T) { t.Parallel() - e := Everything{} - err := Parse(url.Values{"Time": {"llama"}}, &e) + now := time.Now().Format(time.RFC3339) + singletonErrorsWithInvalid(t, "Time", now, "llama") +} + +type Crazy struct { + A *Crazy + B *Crazy + Value int + Slice []int + Map map[string]Crazy +} + +func TestCrazy(t *testing.T) { + t.Parallel() + + c := Crazy{} + err := Parse(url.Values{ + "A[B][B][A][Value]": {"1"}, + "B[A][A][Slice][]": {"3", "1", "4"}, + "B[Map][hello][A][Value]": {"8"}, + "A[Value]": {"2"}, + "A[Slice][]": {"9", "1", "1"}, + "Value": {"42"}, + }, &c) + if err != nil { + t.Error("Error parsing craziness: ", err) + } + + // Exhaustively checking everything here is going to be a huge pain, so + // let's just hope for the best, pretend NPEs don't exist, and hope that + // this test covers enough stuff that it's actually useful. + assertEqual(t, "c.A.B.B.A.Value", 1, c.A.B.B.A.Value) + assertEqual(t, "c.A.Value", 2, c.A.Value) + assertEqual(t, "c.Value", 42, c.Value) + assertEqual(t, `c.B.Map["hello"].A.Value`, 8, c.B.Map["hello"].A.Value) + + assertEqual(t, "c.A.B.B.B", (*Crazy)(nil), c.A.B.B.B) + assertEqual(t, "c.A.B.A", (*Crazy)(nil), c.A.B.A) + assertEqual(t, "c.A.A", (*Crazy)(nil), c.A.A) + + if c.Slice != nil || c.Map != nil { + t.Error("Map and Slice should not be set") + } + + sl := c.B.A.A.Slice + if len(sl) != 3 || sl[0] != 3 || sl[1] != 1 || sl[2] != 4 { + t.Error("Something is wrong with c.B.A.A.Slice") + } + sl = c.A.Slice + if len(sl) != 3 || sl[0] != 9 || sl[1] != 1 || sl[2] != 1 { + t.Error("Something is wrong with c.A.Slice") + } +} + +func TestParseNilPtr(t *testing.T) { + var s *struct{} + + err := Parse(url.Values{"A": {"123"}}, s) + if err == nil { + t.Errorf("Expected error parsing %T", s) + } +} + +func TestUnsupportedStructKind(t *testing.T) { + var s struct { + Bad chan string + } + + err := Parse(url.Values{"Bad": {"123"}}, &s) + if err == nil { + t.Errorf("Expected error parsing %T", s) + } +} + +func TestUnsupportedNestedStruct(t *testing.T) { + var s struct { + Bad struct { + A chan string + } + } + + err := Parse(url.Values{"Bad[A]": {"123"}}, &s) + if err == nil { + t.Errorf("Expected error parsing %T", s) + } +} + +func TestUnsupportedParseKind(t *testing.T) { + var s struct { + Bad []chan string + } + + err := Parse(url.Values{"Bad[]": {"123"}}, &s) + if err == nil { + t.Errorf("Expected error parsing %T", s) + } +} + +func TestUnsupportedMapKey(t *testing.T) { + var s struct { + Bad map[int]int + } + + err := Parse(url.Values{"Bad[123]": {"123"}}, &s) if err == nil { - t.Error("expected error parsing llama as time") + t.Errorf("Expected error parsing %T", s) } } diff --git a/parse.go b/parse.go index 6386da7..8e25719 100644 --- a/parse.go +++ b/parse.go @@ -16,36 +16,35 @@ var textUnmarshalerType = reflect.TypeOf((*encoding.TextUnmarshaler)(nil)).Elem( // parser is responsible for, for instance "[bar][]". `values` is the list of // values assigned to this key, and `target` is where the resulting typed value // should be Set() to. -func parse(key, keytail string, values []string, target reflect.Value) { +func parse(key, keytail string, values []string, target reflect.Value) error { t := target.Type() if reflect.PtrTo(t).Implements(textUnmarshalerType) { - parseTextUnmarshaler(key, keytail, values, target) - return + return parseTextUnmarshaler(key, keytail, values, target) } switch k := target.Kind(); k { case reflect.Bool: - parseBool(key, keytail, values, target) + return parseBool(key, keytail, values, target) case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - parseInt(key, keytail, values, target) + return parseInt(key, keytail, values, target) case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - parseUint(key, keytail, values, target) + return parseUint(key, keytail, values, target) case reflect.Float32, reflect.Float64: - parseFloat(key, keytail, values, target) + return parseFloat(key, keytail, values, target) case reflect.Map: - parseMap(key, keytail, values, target) + return parseMap(key, keytail, values, target) case reflect.Ptr: - parsePtr(key, keytail, values, target) + return parsePtr(key, keytail, values, target) case reflect.Slice: - parseSlice(key, keytail, values, target) + return parseSlice(key, keytail, values, target) case reflect.String: - parseString(key, keytail, values, target) + return parseString(key, keytail, values, target) case reflect.Struct: - parseStruct(key, keytail, values, target) - + return parseStruct(key, keytail, values, target) default: - pebkac("unsupported object of type %v and kind %v.", - target.Type(), k) + return InvalidParseError{ + Type: target.Type(), + } } } @@ -59,137 +58,168 @@ func kpath(key, keytail string) string { // Helper for validating that a value has been passed exactly once, and that the // user is not attempting to nest on the key. -func primitive(key, keytail string, tipe reflect.Type, values []string) { +func primitive(key, keytail string, tipe reflect.Type, values []string) error { if keytail != "" { - panic(NestingError{ + return NestingError{ Key: kpath(key, keytail), Type: tipe, Nesting: keytail, - }) + } } + if len(values) != 1 { - panic(SingletonError{ + return SingletonError{ Key: kpath(key, keytail), Type: tipe, Values: values, - }) + } } + + return nil } -func keyed(tipe reflect.Type, key, keytail string) (string, string) { +func keyed(tipe reflect.Type, key, keytail string) (string, string, error) { if keytail == "" || keytail[0] != '[' { - panic(SyntaxError{ + return "", "", SyntaxError{ Key: kpath(key, keytail), Subtype: MissingOpeningBracket, ErrorPart: keytail, - }) + } } idx := strings.IndexRune(keytail, ']') if idx == -1 { - panic(SyntaxError{ + return "", "", SyntaxError{ Key: kpath(key, keytail), Subtype: MissingClosingBracket, ErrorPart: keytail[1:], - }) + } } - return keytail[1:idx], keytail[idx+1:] + return keytail[1:idx], keytail[idx+1:], nil } -func parseTextUnmarshaler(key, keytail string, values []string, target reflect.Value) { - primitive(key, keytail, target.Type(), values) +func parseTextUnmarshaler(key, keytail string, values []string, target reflect.Value) error { + err := primitive(key, keytail, target.Type(), values) + if err != nil { + return err + } tu := target.Addr().Interface().(encoding.TextUnmarshaler) - err := tu.UnmarshalText([]byte(values[0])) + err = tu.UnmarshalText([]byte(values[0])) if err != nil { - panic(TypeError{ + return TypeError{ Key: kpath(key, keytail), Type: target.Type(), Err: err, - }) + } } + + return nil } -func parseBool(key, keytail string, values []string, target reflect.Value) { - primitive(key, keytail, target.Type(), values) +func parseBool(key, keytail string, values []string, target reflect.Value) error { + err := primitive(key, keytail, target.Type(), values) + if err != nil { + return err + } switch values[0] { case "true", "1", "on": target.SetBool(true) + return nil case "false", "0", "": target.SetBool(false) + return nil default: - panic(TypeError{ + return TypeError{ Key: kpath(key, keytail), Type: target.Type(), - }) + } } } -func parseInt(key, keytail string, values []string, target reflect.Value) { +func parseInt(key, keytail string, values []string, target reflect.Value) error { t := target.Type() - primitive(key, keytail, t, values) + err := primitive(key, keytail, t, values) + if err != nil { + return err + } i, err := strconv.ParseInt(values[0], 10, t.Bits()) if err != nil { - panic(TypeError{ + return TypeError{ Key: kpath(key, keytail), Type: t, Err: err, - }) + } } + target.SetInt(i) + return nil } -func parseUint(key, keytail string, values []string, target reflect.Value) { +func parseUint(key, keytail string, values []string, target reflect.Value) error { t := target.Type() - primitive(key, keytail, t, values) + err := primitive(key, keytail, t, values) + if err != nil { + return err + } i, err := strconv.ParseUint(values[0], 10, t.Bits()) if err != nil { - panic(TypeError{ + return TypeError{ Key: kpath(key, keytail), Type: t, Err: err, - }) + } } + target.SetUint(i) + return nil } -func parseFloat(key, keytail string, values []string, target reflect.Value) { +func parseFloat(key, keytail string, values []string, target reflect.Value) error { t := target.Type() - primitive(key, keytail, t, values) + err := primitive(key, keytail, t, values) + if err != nil { + return err + } f, err := strconv.ParseFloat(values[0], t.Bits()) if err != nil { - panic(TypeError{ + return TypeError{ Key: kpath(key, keytail), Type: t, Err: err, - }) + } } target.SetFloat(f) + return nil } -func parseString(key, keytail string, values []string, target reflect.Value) { - primitive(key, keytail, target.Type(), values) +func parseString(key, keytail string, values []string, target reflect.Value) error { + err := primitive(key, keytail, target.Type(), values) + if err != nil { + return err + } target.SetString(values[0]) + return nil } -func parseSlice(key, keytail string, values []string, target reflect.Value) { +func parseSlice(key, keytail string, values []string, target reflect.Value) error { t := target.Type() // BUG(carl): We currently do not handle slices of nested types. If // support is needed, the implementation probably could be fleshed out. if keytail != "[]" { - panic(NestingError{ + return NestingError{ Key: kpath(key, keytail), Type: t, Nesting: keytail, - }) + } } slice := reflect.MakeSlice(t, len(values), len(values)) @@ -198,14 +228,22 @@ func parseSlice(key, keytail string, values []string, target reflect.Value) { // We actually cheat a little bit and modify the key so we can // generate better debugging messages later key := fmt.Sprintf("%s[%d]", kp, i) - parse(key, "", values[i:i+1], slice.Index(i)) + err := parse(key, "", values[i:i+1], slice.Index(i)) + if err != nil { + return err + } } + target.Set(slice) + return nil } -func parseMap(key, keytail string, values []string, target reflect.Value) { +func parseMap(key, keytail string, values []string, target reflect.Value) error { t := target.Type() - mapkey, maptail := keyed(t, key, keytail) + mapkey, maptail, err := keyed(t, key, keytail) + if err != nil { + return err + } // BUG(carl): We don't support any map keys except strings, although // there's no reason we shouldn't be able to throw the value through our @@ -214,7 +252,10 @@ func parseMap(key, keytail string, values []string, target reflect.Value) { if t.Key().Kind() == reflect.String { mk = reflect.ValueOf(mapkey).Convert(t.Key()) } else { - pebkac("key for map %v isn't a string (it's a %v).", t, t.Key()) + return InvalidParseError{ + Type: t, + Hint: fmt.Sprintf("map keys must be strings, not %v", t.Key()), + } } if target.IsNil() { @@ -227,23 +268,36 @@ func parseMap(key, keytail string, values []string, target reflect.Value) { // MapIndex isn't Set()table if the key exists. val = reflect.New(t.Elem()).Elem() } - parse(key, maptail, values, val) + + err = parse(key, maptail, values, val) + if err != nil { + return err + } + target.SetMapIndex(mk, val) + return nil } -func parseStruct(key, keytail string, values []string, target reflect.Value) { +func parseStruct(key, keytail string, values []string, target reflect.Value) error { t := target.Type() - sk, skt := keyed(t, key, keytail) - cache := cacheStruct(t) + sk, skt, err := keyed(t, key, keytail) + if err != nil { + return err + } + + cache, err := cacheStruct(t) + if err != nil { + return err + } - parseStructField(cache, key, sk, skt, values, target) + return parseStructField(cache, key, sk, skt, values, target) } -func parsePtr(key, keytail string, values []string, target reflect.Value) { +func parsePtr(key, keytail string, values []string, target reflect.Value) error { t := target.Type() - if target.IsNil() { target.Set(reflect.New(t.Elem())) } - parse(key, keytail, values, target.Elem()) + + return parse(key, keytail, values, target.Elem()) } diff --git a/pebkac_test.go b/pebkac_test.go deleted file mode 100644 index 71d64eb..0000000 --- a/pebkac_test.go +++ /dev/null @@ -1,58 +0,0 @@ -package param - -import ( - "net/url" - "strings" - "testing" -) - -type Bad struct { - Unknown interface{} -} - -type Bad2 struct { - Unknown *interface{} -} - -type Bad3 struct { - BadMap map[int]int -} - -// These tests are not parallel so we can frob pebkac behavior in an isolated -// way - -func assertPebkac(t *testing.T, err error) { - if err == nil { - t.Error("Expected PEBKAC error message") - } else if !strings.HasSuffix(err.Error(), yourFault) { - t.Errorf("Expected PEBKAC error, but got: %v", err) - } -} - -func TestBadInputs(t *testing.T) { - pebkacTesting = true - - err := Parse(url.Values{"Unknown": {"4"}}, Bad{}) - assertPebkac(t, err) - - b := &Bad{} - err = Parse(url.Values{"Unknown": {"4"}}, &b) - assertPebkac(t, err) - - pebkacTesting = false -} - -func TestBadTypes(t *testing.T) { - pebkacTesting = true - - err := Parse(url.Values{"Unknown": {"4"}}, &Bad{}) - assertPebkac(t, err) - - err = Parse(url.Values{"Unknown": {"4"}}, &Bad2{}) - assertPebkac(t, err) - - err = Parse(url.Values{"BadMap[llama]": {"4"}}, &Bad3{}) - assertPebkac(t, err) - - pebkacTesting = false -} diff --git a/struct.go b/struct.go index 56094ba..c1d0af8 100644 --- a/struct.go +++ b/struct.go @@ -1,6 +1,7 @@ package param import ( + "fmt" "reflect" "strings" "sync" @@ -16,19 +17,19 @@ import ( type structCache map[string]cacheLine type cacheLine struct { offset int - parse func(string, string, []string, reflect.Value) + parse func(string, string, []string, reflect.Value) error } var cacheLock sync.RWMutex var cache = make(map[reflect.Type]structCache) -func cacheStruct(t reflect.Type) structCache { +func cacheStruct(t reflect.Type) (structCache, error) { cacheLock.RLock() sc, ok := cache[t] cacheLock.RUnlock() if ok { - return sc + return sc, nil } // It's okay if two people build struct caches simultaneously @@ -42,7 +43,12 @@ func cacheStruct(t reflect.Type) structCache { } name := extractName(sf) if name != "-" { - sc[name] = cacheLine{i, extractHandler(t, sf)} + h, err := extractHandler(t, sf) + if err != nil { + return nil, err + } + + sc[name] = cacheLine{i, h} } } @@ -50,7 +56,7 @@ func cacheStruct(t reflect.Type) structCache { cache[t] = sc cacheLock.Unlock() - return sc + return sc, nil } // Extract the name of the given struct field, looking at struct tags as @@ -71,51 +77,51 @@ func extractName(sf reflect.StructField) string { return name } -func extractHandler(s reflect.Type, sf reflect.StructField) func(string, string, []string, reflect.Value) { +func extractHandler(s reflect.Type, sf reflect.StructField) (func(string, string, []string, reflect.Value) error, error) { if reflect.PtrTo(sf.Type).Implements(textUnmarshalerType) { - return parseTextUnmarshaler + return parseTextUnmarshaler, nil } switch sf.Type.Kind() { case reflect.Bool: - return parseBool + return parseBool, nil case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return parseInt + return parseInt, nil case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return parseUint + return parseUint, nil case reflect.Float32, reflect.Float64: - return parseFloat + return parseFloat, nil case reflect.Map: - return parseMap + return parseMap, nil case reflect.Ptr: - return parsePtr + return parsePtr, nil case reflect.Slice: - return parseSlice + return parseSlice, nil case reflect.String: - return parseString + return parseString, nil case reflect.Struct: - return parseStruct - + return parseStruct, nil default: - pebkac("struct %v has illegal field %q (type %v, kind %v).", - s, sf.Name, sf.Type, sf.Type.Kind()) - return nil + return nil, InvalidParseError{ + Type: s, + Hint: fmt.Sprintf("field %q in struct %v", sf.Name, s), + } } } // We have to parse two types of structs: ones at the top level, whose keys // don't have square brackets around them, and nested structs, which do. -func parseStructField(cache structCache, key, sk, keytail string, values []string, target reflect.Value) { +func parseStructField(cache structCache, key, sk, keytail string, values []string, target reflect.Value) error { l, ok := cache[sk] if !ok { - panic(KeyError{ + return KeyError{ FullKey: key, Key: kpath(key, keytail), Type: target.Type(), Field: sk, - }) + } } - f := target.Field(l.offset) - l.parse(key, keytail, values, f) + f := target.Field(l.offset) + return l.parse(key, keytail, values, f) } diff --git a/struct_test.go b/struct_test.go index ecba3e2..a37271c 100644 --- a/struct_test.go +++ b/struct_test.go @@ -68,7 +68,10 @@ func TestNames(t *testing.T) { func TestCacheStruct(t *testing.T) { t.Parallel() - sc := cacheStruct(fruityType) + sc, err := cacheStruct(fruityType) + if err != nil { + t.Errorf("Failed to cache struct: %v", err) + } if len(sc) != len(fruityCache) { t.Errorf("Cache has %d keys, but expected %d", len(sc), @@ -99,7 +102,10 @@ func TestCacheStruct(t *testing.T) { func TestPrivate(t *testing.T) { t.Parallel() - sc := cacheStruct(privateType) + sc, err := cacheStruct(privateType) + if err != nil { + t.Errorf("Failed to cache struct: %v", err) + } if len(sc) != 1 { t.Error("Expected Private{} to have one cachable field") }