Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Fix DecodeHook support for decoding map from struct #171

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pamburus
Copy link

@pamburus pamburus commented Aug 1, 2019

No description provided.

@camdencheek
Copy link
Contributor

This issue is also causing me trouble. Effectively, it makes it so even though I can make string -> customType hook functions, hooks that go from customType -> string fail. The test included in the PR does a good job of showing this

@djaglowski
Copy link

I would also greatly appreciate seeing this issue resolved. We are currently having to use a fork of mapstructure in our product.

@mitchellh
Copy link
Owner

Hello! Apologies it has been awhile. I tried to fix the conflicts and merge this but the tests don't pass. If you can rebase and fix that, I'll merge just ping me.

@pamburus
Copy link
Author

pamburus commented Apr 24, 2022

Ok, I've rebased the pull request and added temporary hack to make tests working.
Also it seems that failing tests are connected with some existing contradictory logic.

TestDecode_StructTaggedWithOmitempty_OmitEmptyValues and TestDecode_StructTaggedWithOmitempty_KeepNonEmptyValues expect that typed nil pointer (VisibleNested) is decoded to a map[string]interface{} preserving the type of the pointer, i.e. as (*Nested)(nil).

But Decoder.decode has the following check that explicitly disables decoding of typed nil pointers:

	if input != nil {
		inputVal = reflect.ValueOf(input)

		// We need to check here if input is a typed nil. Typed nils won't
		// match the "input == nil" below so we check that here.
		if inputVal.Kind() == reflect.Ptr && inputVal.IsNil() {
			input = nil
		}
	}

Why do we have 2 opposite decisions for decoding a simple value vs decoding a struct field?
I guess decoding of a struct field of type A into a map with value of type B should reuse the same logic that is used to decode a scalar value of type A to a scalar value of type B.

So, let's decide whether we just fix the expectation of the tests to expect untyped nil values or we change the logic of decoding scalar values so that decoding of a typed nil to an interface{} will create an equivalent typed nil.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants