qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 18:44:42 -07:00
Code Releases Activity

go/analysis: fix ambiguous paths in structtag pass

Browse Source 
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>
This commit is contained in:
Daniel Martí 2018-12-28 20:09:56 +01:00
parent 498d954934
commit 3ef6863234
3 changed files with 59 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
go/analysis/passes/structtag/structtag.go
View File

@ -136,10 +136,23 @@ func checkTagDuplicates(pass *analysis.Pass, tag, key string, nearest, field *ty
*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()
}

46
go/analysis/passes/structtag/testdata/src/a/a.go vendored
View File

@ -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 @@ type AnonymousJSONField struct {
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 @@ type UnexpectedSpacetest struct {
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"
}

9
go/analysis/passes/structtag/testdata/src/a/b/b.go vendored Normal file
View File

@ -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"`
}