/cc @robpike @mvdan @josharian

I'd also wonder if this should be encoding/json's job, which I presume is #17609. But I guess it's too late to make it error on these cases in Go1.

Agreed that this vet enhancement makes sense.

Offhand this seems a minor addition to the struct tags check if it can be done efficiently. Is it actually a problem that shows up in practice?

@robpike I'm working on a patch now, which will let us see how many new vet warnings it would see in existing Go code.

Is it actually a problem that shows up in practice?

It depends on what you mean. I didn't had such a bug yet in my code. But I do use embedded structs a lot, I do use custom 1-3 char json names (saves about 50% of json length and it's important for us because we store 300KB json in mysql and transfer/decode 600KB or 300KB is a noticeable difference) and I do spend a lot of time and attention to manually check tags to ensure they won't conflict. Actually I've asked one of our developers to write a unit test using reflect to automate this check - and before creating that task I've checked how good metalinter's checkers already handle this case, found vet handle it partially and opened this issue.

Change https://golang.org/cl/115676 mentions this issue: cmd/vet: rewrite structtag using go/types

Change https://golang.org/cl/115677 mentions this issue: cmd/vet: check embedded field tags too

A quick fix is implemented above, on top of a small refactor to use go/types. I quickly checked the Go tree, and it found no new cases.

I'm currently downloading https://github.com/rsc/corpus, which should be a large enough corpus of packages to make a decision. GitHub is throttling me to about 100KB/s, so it's going to take a few hours.

I failed to use https://github.com/rsc/corpus after nearly an hour of trying. Even when using the tarball and recursively downloading deps, I always got stuck with some packages not typechecking properly. If anyone knows how to use it, any help will be appreciated.

I settled for some ad-hoc testing on packages that I regularly use and their dependencies. I found that a certain popular package has lots of newly found warnings thanks to this check:

$ go vet github.com/nlopes/slack
# github.com/nlopes/slack
./im.go:25: struct field at ./im.go:27 repeats json tag "is_im" at ./conversation.go:22
./search.go:54: struct field at ./pagination.go:15 repeats json tag "page" at ./pagination.go:7
./search.go:54: struct field at ./search.go:58 repeats json tag "total" at ./pagination.go:6
./search.go:61: struct field at ./pagination.go:15 repeats json tag "page" at ./pagination.go:7
./search.go:61: struct field at ./search.go:65 repeats json tag "total" at ./pagination.go:6
./search.go:68: struct field at ./pagination.go:15 repeats json tag "page" at ./pagination.go:7
./search.go:68: struct field at ./search.go:58 repeats json tag "total" at ./pagination.go:6
./search.go:68: struct field at ./search.go:62 repeats json tag "matches" at ./search.go:55
./search.go:68: struct field at ./pagination.go:5 repeats json tag "count" at ./pagination.go:5
./search.go:68: struct field at ./pagination.go:6 repeats json tag "total" at ./pagination.go:6
./search.go:68: struct field at ./pagination.go:7 repeats json tag "page" at ./pagination.go:7
./search.go:68: struct field at ./pagination.go:8 repeats json tag "pages" at ./pagination.go:8
./search.go:68: struct field at ./search.go:63 repeats json tag "paging" at ./search.go:56
./search.go:68: struct field at ./pagination.go:14 repeats json tag "total_count" at ./pagination.go:14
./search.go:68: struct field at ./pagination.go:15 repeats json tag "page" at ./pagination.go:7
./search.go:68: struct field at ./pagination.go:16 repeats json tag "per_page" at ./pagination.go:16
./search.go:68: struct field at ./pagination.go:17 repeats json tag "page_count" at ./pagination.go:17
./search.go:68: struct field at ./pagination.go:18 repeats json tag "first" at ./pagination.go:18
./search.go:68: struct field at ./pagination.go:19 repeats json tag "last" at ./pagination.go:19
./search.go:68: struct field at ./search.go:64 repeats json tag "pagination" at ./search.go:57
./search.go:68: struct field at ./search.go:65 repeats json tag "total" at ./pagination.go:6
./users.go:171: struct field at ./users.go:130 repeats json tag "presence" at ./users.go:124

I'm pretty confident that this is worth doing - it's very little extra work for vet now that it can rely on go/types, and the check is useful - see for example how I found buggy code in a fairly popular library above.

Ping @alandonovan @josharian (as per dev.golang.org/owners) for a decision. I've got a working CL that you can try; I just want to be sure we want the change before cleaning it up and adding tests.

Seems reasonable to me, for go1.12.

Thanks for the feedback - the CL is now ready to review at https://go-review.googlesource.com/c/go/+/115677.