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

encoding/json: tag `json:"-"` doesn't hide an embedded field #35501

Open
ainar-g opened this issue Nov 11, 2019 · 8 comments

Comments

@ainar-g
Copy link
Contributor

@ainar-g ainar-g commented Nov 11, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13.3 linux/amd64
$ gotip version
go version devel +696c41488a Mon Nov 11 15:37:55 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes, see gotip version.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ainar/.cache/go-build"
GOENV="/home/ainar/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/ainar/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/ainar/go/go1.13"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/ainar/go/go1.13/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/ainar/dev/tmp/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build199093238=/tmp/go-build -gno-record-gcc-switches"

What did you do?

type A struct {
	Name string `json:"name"`
}

type B struct {
	A
	Name string `json:"-"`
}

https://play.golang.org/p/uKY3umGYEp3

What did you expect to see?

Either:

<nil> {}

Or:

some error about tag conflict

What did you see instead?

<nil> {"name":"1234"}
@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 11, 2019

Hmm. While I agree that intuitively this is what one would expect, I don't think the docs give a definitive answer. The pieces of the docs that explain "-" and anonymous fields are separate, so it's not clear which takes precedence.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 11, 2019

Here's another tricky question. The docs say:

The Go visibility rules for struct fields are amended for JSON when deciding which field to marshal or unmarshal.

However, does this mean just the original Go field's name, or does it also include the JSON field name one can add via a struct field tag? Right now, the "shadowing" of field names follows what's in the struct tags. That is, your Name string `json:"-"` doesn't affect your Name string `json:"name"`, becaues they don't even share a name. The names in question are - and name.

@ainar-g

This comment has been minimized.

Copy link
Contributor Author

@ainar-g ainar-g commented Nov 11, 2019

The names in question are - and name.

Well, - is technically a “directive” and not a name, is it? But I agree, it's a lot of undefined behaviour.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 11, 2019

Here's an example of my reasoning above: https://play.golang.org/p/lEDqVGEVxAR

The shadowing happens based on what's in the struct field tags. If we completely swap that and use the original struct names instead, we'd break valid use cases like this one.

Well, - is technically a “directive” and not a name, is it?

You're right there. But even if you say "in that case the name remains the original Name", you still have two different names - Name and name.

@andybons andybons added this to the Unplanned milestone Nov 11, 2019
@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 12, 2019

I'll leave this open for a week if anyone has more thoughts, or a specific way in which this could be changed without breaking existing valid use cases.

The only idea that comes to mind that would be truly backwards-compatible would be to extend the json tag syntax to allow omitting a field name, not just a specific field only. For example, `json:"name,-"` to omit any embedded field with `json:"name"`. "-" already takes the place of a name, so it taking the place of an option doesn't seem like a terrible idea.

The question then would be if it's worth adding this feature. How often does one want to omit fields from anonymous structs? Are there any current workarounds? Do any external json libraries support this feature?

@ainar-g

This comment has been minimized.

Copy link
Contributor Author

@ainar-g ainar-g commented Nov 12, 2019

If the behaviour isn't changed, can it at least be documented better, so that I could bother @dominikh the community could build static analysis tools to detect such idiosyncrasies?

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Nov 12, 2019

Yes, whatever we decide to do, I think it would be good to clarify that the visibility rules apply to the JSON names alone.

@breml

This comment has been minimized.

Copy link
Contributor

@breml breml commented Nov 20, 2019

I looked into this issue myself and I feel like the observation of @ainar-g is valid.
The reason for this is the following:

In the documentation of encoding/json the section explaining "-" is directly following the section about ",omitempty" and there it says, that a field is omitted, if it contains the zero value:

The "omitempty" option specifies that the field should be omitted from the encoding if the field has an empty value, defined as false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string.

As a special case, if the field tag is "-", the field is always omitted. Note that a field with name "-" can still be generated using the tag "-,".

Now, if we extend the examples provided above (by @ainar-g and @mvdan) with the ",omitempty" cases, we see, that in this case the anonymous struct is not considered, even if the field in the "parent" struct is empty (zero value). This is the case regardless of the fact, if the final "name" of the field in the encoded json is defined by the struct field name or the struct tag, the behavior is consistent. See: https://play.golang.org/p/VjMa8H7EI9e

Therefore I would argue, that if the "omitting" for ",omitempty" is always decided based on the value in the "parent" struct, the same should be the case for "-", that is, if in the parent struct we have "-", this named field (evaluated either by the struct tag or by the struct field name, in that order) is never shown in the final encoded json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.