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

cmd/vet: ambiguous path for filename in repeated json tag warning #29130

Open
mark-rushakoff opened this Issue Dec 7, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@mark-rushakoff
Contributor

mark-rushakoff commented Dec 7, 2018

This is a followup issue to #28995.

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

$ go version
go version devel +bae1e70ac4 Thu Dec 6 20:13:40 2018 +0000 darwin/amd64

What did you do?

I ran go vet ./http and cd ./http; go vet . against github.com/influxdata/platform at revision 5da9d37b5b3d83056313ed5f6e50e529af52734e, master as of sometime today. (Of course, we should have fixed the vet error by now, but it has been helpful to work out some vet issues here, at least.)

$ mgotip vet ./http
# github.com/influxdata/platform/http
http/dashboard_service.go:86:2: struct field Cells repeats json tag "cells" also at dashboard.go:53

# It does in fact match ./dashboard.go line 53, but that isn't necessarily clear...
$ git ls-files | grep dashboard.go
bolt/dashboard.go
dashboard.go
inmem/dashboard.go

# Especially when your working directory does not have dashboard.go:
$ cd ./http
$ mgotip vet .
# github.com/influxdata/platform/http
./dashboard_service.go:86:2: struct field Cells repeats json tag "cells" also at dashboard.go:53

I expected the warning to use either an explicit relative path like ./dashboard.go:53 or ../dashboard.go:53; or a package-relative path like github.com/influxdata/platform/dashboard.go:53.

@mvdan mvdan self-assigned this Dec 7, 2018

@mvdan

This comment has been minimized.

Member

mvdan commented Dec 7, 2018

Indeed a bug. This is because the check was extended to also check anonymous fields, whose type definition may be in a different package. So taking the file path basename is no longer enough to point to the relevant file.

Should be simple enough to fix, and it's a regression in 1.12 since the check was extended in the last few months. Will have a look this weekend.

/cc @alandonovan

@mvdan mvdan added the NeedsFix label Dec 7, 2018

@mvdan mvdan added this to the Go1.12 milestone Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment