Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nil interfaces #19

Closed
wade-tattersall opened this issue Nov 15, 2019 · 5 comments
Closed

Support nil interfaces #19

wade-tattersall opened this issue Nov 15, 2019 · 5 comments

Comments

@wade-tattersall
Copy link

Marshal currently triggers a panic if it runs into any nil pointers.

I think such values should be stored in the resulting map as nils. This would be consistent with the json package, allowing the resulting map to be passed to json.Marshal and get the same result as if the struct itself were passed directly to json.Marshal.

Example code:


import (
	"encoding/json"
	"fmt"
	"github.com/liip/sheriff"
)

func main() {
	expected := map[string]interface{}{"thing": nil}
	fmt.Println(expected)

	type Example struct {
		Thing *int `json:"thing" groups:"include"`
	}
	x := Example{}
	_, _ = json.Marshal(x) // {"thing":null}

	opt := sheriff.Options{
		Groups: []string{"include"},
	}
	result2, err := sheriff.Marshal(&opt, x)
	if err != nil {panic(err)}
	fmt.Println(result2)
}```
@mweibel
Copy link
Collaborator

mweibel commented Nov 15, 2019

@wade-tattersall
Thanks for the repro case. However I couldn't find any panic. Could you elaborate which panic you get?
Your example seems to work fine:
https://play.golang.org/p/fyoe2I_QG-r

@wade-tattersall
Copy link
Author

Sorry, I messed up somehow - I think I must have forgotten to save before running the repro after I tried simplifying it.

After a bit of digging, I was able to work out what does trigger it: https://play.golang.org/p/7wCzhabD1dF.

That is, it fails for the relatively trivial case of sheriff.Marshal(&opt, nil), and hence also fails when marshalValue calls Marshal for any member field that is a nil interface.

There's a subtlety there that confuses me - marshalValue only calls Marshal for k == reflect.Interface || k == reflect.Struct, but when it does so, the resulting parameter that Marshal receives has a ValueOf(data).Kind() == reflect.Invalid. So it seems to lose the type information as it goes.

@wade-tattersall wade-tattersall changed the title Support nil pointers Support nil interfaces Nov 17, 2019
@mweibel
Copy link
Collaborator

mweibel commented Jul 24, 2020

I don't have much time at the moment - if you provide a PR though (with test cases please), I'll do my best to review it!

@xuancanh
Copy link
Contributor

Hi @mweibel, I've created a PR for this issue. Could you help review it? Thank you a lot.

@mweibel
Copy link
Collaborator

mweibel commented Oct 1, 2020

fixed by #26

@mweibel mweibel closed this as completed Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants