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

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

Closed
mark-rushakoff opened this issue Dec 7, 2018 · 2 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mark-rushakoff
Copy link
Contributor

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
Copy link
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 The path to resolution is known, but the work has not been done. label Dec 7, 2018
@mvdan mvdan added this to the Go1.12 milestone Dec 7, 2018
@gopherbot
Copy link

Change https://golang.org/cl/156378 mentions this issue: cmd/vendor: update to golang.org/x/tools@3ef68632

@golang golang locked and limited conversation to collaborators Jan 6, 2020
@rsc rsc unassigned mvdan Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants