go/analysis: fix ambiguous paths in structtag pass
Now that we support checking for duplicate struct field tags across
anonymous struct fields, we must deal with the fact that the files
involved in such a warning may not be in the same package or directory.
This could lead to errors where a file was mentioned in a package where
it didn't exist. Or even worse, point at a location within an existing
file that doesn't contain the field we want, causing even further
confusion to the user.
To fix this, always make the "also at" positions relative to the current
warning, if possible.
Fixes #29130.
Change-Id: Iaa29b406978f1671bdfb2ddddb7058eeffec92a9
Reviewed-on: https://go-review.googlesource.com/c/155899
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/go/analysis/passes/structtag/structtag.go b/go/analysis/passes/structtag/structtag.go
index 78133fe..2b67c37 100644
--- a/go/analysis/passes/structtag/structtag.go
+++ b/go/analysis/passes/structtag/structtag.go
@@ -136,10 +136,23 @@
*seen = map[[2]string]token.Pos{}
}
if pos, ok := (*seen)[[2]string{key, val}]; ok {
- posn := pass.Fset.Position(pos)
- posn.Filename = filepath.Base(posn.Filename)
- posn.Column = 0
- pass.Reportf(nearest.Pos(), "struct field %s repeats %s tag %q also at %s", field.Name(), key, val, posn)
+ alsoPos := pass.Fset.Position(pos)
+ alsoPos.Column = 0
+
+ // Make the "also at" position relative to the current position,
+ // to ensure that all warnings are unambiguous and correct. For
+ // example, via anonymous struct fields, it's possible for the
+ // two fields to be in different packages and directories.
+ thisPos := pass.Fset.Position(field.Pos())
+ rel, err := filepath.Rel(filepath.Dir(thisPos.Filename), alsoPos.Filename)
+ if err != nil {
+ // Possibly because the paths are relative; leave the
+ // filename alone.
+ } else {
+ alsoPos.Filename = rel
+ }
+
+ pass.Reportf(nearest.Pos(), "struct field %s repeats %s tag %q also at %s", field.Name(), key, val, alsoPos)
} else {
(*seen)[[2]string{key, val}] = field.Pos()
}
diff --git a/go/analysis/passes/structtag/testdata/src/a/a.go b/go/analysis/passes/structtag/testdata/src/a/a.go
index fe418dd..abb67fe 100644
--- a/go/analysis/passes/structtag/testdata/src/a/a.go
+++ b/go/analysis/passes/structtag/testdata/src/a/a.go
@@ -6,7 +6,10 @@
package a
-import "encoding/xml"
+import (
+ "a/b"
+ "encoding/xml"
+)
type StructTagTest struct {
A int "hello" // want "`hello` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair"
@@ -48,48 +51,57 @@
A int "hello" // want "`hello` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair"
}
+// With different names to allow using as anonymous fields multiple times.
+
+type AnonymousJSONField2 struct {
+ DuplicateAnonJSON int `json:"a"`
+}
+type AnonymousJSONField3 struct {
+ DuplicateAnonJSON int `json:"a"`
+}
+
type DuplicateJSONFields struct {
JSON int `json:"a"`
- DuplicateJSON int `json:"a"` // want "struct field DuplicateJSON repeats json tag .a. also at a.go:52"
+ DuplicateJSON int `json:"a"` // want "struct field DuplicateJSON repeats json tag .a. also at a.go:64"
IgnoredJSON int `json:"-"`
OtherIgnoredJSON int `json:"-"`
OmitJSON int `json:",omitempty"`
OtherOmitJSON int `json:",omitempty"`
- DuplicateOmitJSON int `json:"a,omitempty"` // want "struct field DuplicateOmitJSON repeats json tag .a. also at a.go:52"
+ DuplicateOmitJSON int `json:"a,omitempty"` // want "struct field DuplicateOmitJSON repeats json tag .a. also at a.go:64"
NonJSON int `foo:"a"`
DuplicateNonJSON int `foo:"a"`
Embedded struct {
DuplicateJSON int `json:"a"` // OK because it's not in the same struct type
}
- AnonymousJSON `json:"a"` // want "struct field AnonymousJSON repeats json tag .a. also at a.go:52"
+ AnonymousJSON `json:"a"` // want "struct field AnonymousJSON repeats json tag .a. also at a.go:64"
- AnonymousJSONField // want "struct field DuplicateAnonJSON repeats json tag .a. also at a.go:52"
+ AnonymousJSONField // want "struct field DuplicateAnonJSON repeats json tag .a. also at a.go:64"
XML int `xml:"a"`
- DuplicateXML int `xml:"a"` // want "struct field DuplicateXML repeats xml tag .a. also at a.go:68"
+ DuplicateXML int `xml:"a"` // want "struct field DuplicateXML repeats xml tag .a. also at a.go:80"
IgnoredXML int `xml:"-"`
OtherIgnoredXML int `xml:"-"`
OmitXML int `xml:",omitempty"`
OtherOmitXML int `xml:",omitempty"`
- DuplicateOmitXML int `xml:"a,omitempty"` // want "struct field DuplicateOmitXML repeats xml tag .a. also at a.go:68"
+ DuplicateOmitXML int `xml:"a,omitempty"` // want "struct field DuplicateOmitXML repeats xml tag .a. also at a.go:80"
NonXML int `foo:"a"`
DuplicateNonXML int `foo:"a"`
Embedded2 struct {
DuplicateXML int `xml:"a"` // OK because it's not in the same struct type
}
- AnonymousXML `xml:"a"` // want "struct field AnonymousXML repeats xml tag .a. also at a.go:68"
+ AnonymousXML `xml:"a"` // want "struct field AnonymousXML repeats xml tag .a. also at a.go:80"
Attribute struct {
XMLName xml.Name `xml:"b"`
NoDup int `xml:"b"` // OK because XMLName above affects enclosing struct.
Attr int `xml:"b,attr"` // OK because <b b="0"><b>0</b></b> is valid.
- DupAttr int `xml:"b,attr"` // want "struct field DupAttr repeats xml attribute tag .b. also at a.go:84"
- DupOmitAttr int `xml:"b,omitempty,attr"` // want "struct field DupOmitAttr repeats xml attribute tag .b. also at a.go:84"
+ DupAttr int `xml:"b,attr"` // want "struct field DupAttr repeats xml attribute tag .b. also at a.go:96"
+ DupOmitAttr int `xml:"b,omitempty,attr"` // want "struct field DupOmitAttr repeats xml attribute tag .b. also at a.go:96"
- AnonymousXML `xml:"b,attr"` // want "struct field AnonymousXML repeats xml attribute tag .b. also at a.go:84"
+ AnonymousXML `xml:"b,attr"` // want "struct field AnonymousXML repeats xml attribute tag .b. also at a.go:96"
}
- AnonymousJSONField `json:"not_anon"` // ok; fields aren't embedded in JSON
- AnonymousJSONField `json:"-"` // ok; entire field is ignored in JSON
+ AnonymousJSONField2 `json:"not_anon"` // ok; fields aren't embedded in JSON
+ AnonymousJSONField3 `json:"-"` // ok; entire field is ignored in JSON
}
type UnexpectedSpacetest struct {
@@ -111,3 +123,11 @@
P int `xml:","`
Q int `foo:" doesn't care "`
}
+
+type DuplicateWithAnotherPackage struct {
+ b.AnonymousJSONField
+
+ // The "also at" position is in a different package and directory. Use
+ // "b.b" instead of "b/b" to match the relative path on Windows easily.
+ DuplicateJSON int `json:"a"` // want "struct field DuplicateJSON repeats json tag .a. also at b.b.go:8"
+}
diff --git a/go/analysis/passes/structtag/testdata/src/a/b/b.go b/go/analysis/passes/structtag/testdata/src/a/b/b.go
new file mode 100644
index 0000000..1040374
--- /dev/null
+++ b/go/analysis/passes/structtag/testdata/src/a/b/b.go
@@ -0,0 +1,9 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package b
+
+type AnonymousJSONField struct {
+ DuplicateAnonJSON int `json:"a"`
+}
