Fix typed nil hook support #278
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adriansmares
commented
Mar 29, 2022
| A *customType2 `mapstructure:"a"` | ||
| } | ||
|
|
||
| for i, marshalledConfig := range []map[string]interface{}{ |
There was a problem hiding this comment.
The first test (which won't use the decoding hooks) works passes just fine without any changes.
| if s == "" { | ||
| // Returning an untyped nil here would cause a panic, as `from.Type()` | ||
| // is invalid for nil. | ||
| return (*customType1)(nil), nil |
There was a problem hiding this comment.
The panic trace is:
--- FAIL: TestTypedNilPostHooks (0.00s)
--- FAIL: TestTypedNilPostHooks/1 (0.00s)
panic: reflect: call of reflect.Value.Type on zero Value [recovered]
panic: reflect: call of reflect.Value.Type on zero Value
goroutine 8 [running]:
testing.tRunner.func1.2({0x56fdc0, 0xc00000e120})
/usr/local/go/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
/usr/local/go/src/testing/testing.go:1392 +0x39f
panic({0x56fdc0, 0xc00000e120})
/usr/local/go/src/runtime/panic.go:838 +0x207
reflect.Value.Type({0x0?, 0x0?, 0x5d4e28?})
/usr/local/go/src/reflect/value.go:2451 +0x12e
github.com/mitchellh/mapstructure.DecodeHookExec({0x571d40?, 0x5a4ed0?}, {0x0?, 0x0?, 0x570840?}, {0x564e80?, 0xc000060df0?, 0x573d00?})
/home/adriansmares/projects/mapstructure/decode_hooks.go:47 +0x11d
github.com/mitchellh/mapstructure.ComposeDecodeHookFunc.func1({0x56a6a0?, 0x5d3ad0?, 0xc00013d358?}, {0x564e80?, 0xc000060df0?, 0xffffffffffffffff?})
/home/adriansmares/projects/mapstructure/decode_hooks.go:69 +0x10c
github.com/mitchellh/mapstructure.DecodeHookExec({0x570840?, 0xc00006a140?}, {0x56a6a0?, 0x5d3ad0?, 0xc00013d3b8?}, {0x564e80?, 0xc000060df0?, 0x55fca9?})
/home/adriansmares/projects/mapstructure/decode_hooks.go:51 +0xd3
github.com/mitchellh/mapstructure.(*Decoder).decode(0xc000010058, {0x55fca9, 0x1}, {0x56a6a0?, 0x5d3ad0?}, {0x564e80?, 0xc000060df0?, 0x10?})
/home/adriansmares/projects/mapstructure/mapstructure.go:440 +0x179
github.com/mitchellh/mapstructure.(*Decoder).decodeStructFromMap(0xc000010058, {0x0, 0x0}, {0x571500?, 0xc00007e870?, 0x1?}, {0x57b740?, 0xc000060df0?, 0xc00006a120?})
/home/adriansmares/projects/mapstructure/mapstructure.go:1382 +0x1470
github.com/mitchellh/mapstructure.(*Decoder).decodeStruct(0x570840?, {0x0, 0x0}, {0x571500?, 0xc00007e870?}, {0x57b740?, 0xc000060df0?, 0x203000?})
/home/adriansmares/projects/mapstructure/mapstructure.go:1208 +0x4ab
github.com/mitchellh/mapstructure.(*Decoder).decode(0xc000010058, {0x0, 0x0}, {0x571500?, 0xc00007e870?}, {0x57b740?, 0xc000060df0?, 0x48?})
/home/adriansmares/projects/mapstructure/mapstructure.go:463 +0x3cf
github.com/mitchellh/mapstructure.(*Decoder).Decode(0xc000010058, {0x571500, 0xc00007e870})
/home/adriansmares/projects/mapstructure/mapstructure.go:398 +0xd8
github.com/mitchellh/mapstructure.TestTypedNilPostHooks.func1(0xc000115380)
/home/adriansmares/projects/mapstructure/mapstructure_test.go:2551 +0x14e
testing.tRunner(0xc000115380, 0xc000060de0)
/usr/local/go/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
/usr/local/go/src/testing/testing.go:1486 +0x35f
FAIL github.com/mitchellh/mapstructure 0.005s
FAIL
It seems to me that returning nil from a type decoding hook is not supported (was it supported in the first place ?)
| @@ -993,7 +993,7 @@ func (d *Decoder) decodePtr(name string, data interface{}, val reflect.Value) (b | |||
| // pointer to be nil as well. | |||
| isNil := data == nil | |||
| if !isNil { | |||
| switch v := reflect.Indirect(reflect.ValueOf(data)); v.Kind() { | |||
| switch v := reflect.ValueOf(data); v.Kind() { | |||
There was a problem hiding this comment.
I'm not entirely sure if this change is correct. I don't understand why Indirect was used here before, given that the nulalbility of data is important here, not the value to which data points.
Are there any other call sites that may require updates ?
5 tasks
adriansmares
force-pushed
the
fix/typed-nil-hook
branch
from
c42f9f1
to
023d3dc
Compare
Signed-off-by: Bhargav Ravuri <vaguecoder0to.n@gmail.com>
|
Pinging @mitchellh . |
5 tasks
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Improve CI
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
chore: fix lint violations
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Add decode hooks for netip Addr and AddrPort
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Decoding array of slices causes panic
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Dereference multiple pointers when decoding to maps
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
fix: StringToSliceHookFunc incorrectly triggered when parsing []byte
…d-fields Fix untagged field decoding in case of decode to struct
Fix Encoding Struct Back to Map Bug (mitchellh#279)
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
feat!: update module path
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
build: update dev env
feature: add StringToBasicTypeHookFunc and support complex
Add an example showing how to use a DecodeHookFunc to parse a custom …
adriansmares
force-pushed
the
fix/typed-nil-hook
branch
from
023d3dc
to
d2bb51a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Typed
nils which are the result of a type decoding hook seem to result in incorrect decoding behavior. Specifically, thenilis turned into an actual value due to the way thenilcheck works, instead of preserving thenil. Previously we were returning just untypednils but ever since #205 this seems to no longer be supported while chaining multiple hooks.I've added a test which reproduces this issue, and a possible fix.
cc @mitchellh