From d777022dd83705401fce69afbdf354e796dcdeb8 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 8 Oct 2018 21:17:35 -0400 Subject: [PATCH] go/analysis/passes/structtag: split out from vet Change-Id: Ib3c4c783e4bca2bf2b3c76bea8f19959b7e39539 Reviewed-on: https://go-review.googlesource.com/c/142097 Reviewed-by: Michael Matloob --- .../passes/{vet => structtag}/structtag.go | 67 ++++++----- .../passes/structtag/structtag_test.go | 13 ++ .../passes/structtag/testdata/src/a/a.go | 113 ++++++++++++++++++ go/analysis/passes/vet/testdata/structtag.go | 113 ------------------ 4 files changed, 166 insertions(+), 140 deletions(-) rename go/analysis/passes/{vet => structtag}/structtag.go (74%) create mode 100644 go/analysis/passes/structtag/structtag_test.go create mode 100644 go/analysis/passes/structtag/testdata/src/a/a.go delete mode 100644 go/analysis/passes/vet/testdata/structtag.go diff --git a/go/analysis/passes/vet/structtag.go b/go/analysis/passes/structtag/structtag.go similarity index 74% rename from go/analysis/passes/vet/structtag.go rename to go/analysis/passes/structtag/structtag.go index bdd5f311..48b64771 100644 --- a/go/analysis/passes/vet/structtag.go +++ b/go/analysis/passes/structtag/structtag.go @@ -1,60 +1,70 @@ -// +build ignore - // Copyright 2010 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. -// This file contains the test for canonical struct tags. - -package main +package structtag import ( "errors" "go/ast" "go/token" "go/types" + "path/filepath" "reflect" "strconv" "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" ) -func init() { - register("structtags", - "check that struct field tags have canonical format and apply to exported fields as needed", - checkStructFieldTags, - structType) +var Analyzer = &analysis.Analyzer{ + Name: "structtag", + Doc: `check that struct field tags conform to reflect.StructTag.Get + +Also report certain struct tags (json, xml) used with unexported fields.`, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + RunDespiteErrors: true, + Run: run, } -// checkStructFieldTags checks all the field tags of a struct, including checking for duplicates. -func checkStructFieldTags(f *File, node ast.Node) { - astType := node.(*ast.StructType) - typ := f.pkg.types[astType].Type.(*types.Struct) - var seen map[[2]string]token.Pos - for i := 0; i < typ.NumFields(); i++ { - field := typ.Field(i) - tag := typ.Tag(i) - checkCanonicalFieldTag(f, astType, field, tag, &seen) +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.StructType)(nil), } + inspect.Preorder(nodeFilter, func(n ast.Node) { + styp := pass.TypesInfo.Types[n.(*ast.StructType)].Type.(*types.Struct) + var seen map[[2]string]token.Pos + for i := 0; i < styp.NumFields(); i++ { + field := styp.Field(i) + tag := styp.Tag(i) + checkCanonicalFieldTag(pass, field, tag, &seen) + } + }) + return nil, nil } var checkTagDups = []string{"json", "xml"} var checkTagSpaces = map[string]bool{"json": true, "xml": true, "asn1": true} // checkCanonicalFieldTag checks a single struct field tag. -// top is the top-level struct type that is currently being checked. -func checkCanonicalFieldTag(f *File, top *ast.StructType, field *types.Var, tag string, seen *map[[2]string]token.Pos) { +func checkCanonicalFieldTag(pass *analysis.Pass, field *types.Var, tag string, seen *map[[2]string]token.Pos) { for _, key := range checkTagDups { - checkTagDuplicates(f, tag, key, field, field, seen) + checkTagDuplicates(pass, tag, key, field, field, seen) } if err := validateStructTag(tag); err != nil { - f.Badf(field.Pos(), "struct field tag %#q not compatible with reflect.StructTag.Get: %s", tag, err) + pass.Reportf(field.Pos(), "struct field tag %#q not compatible with reflect.StructTag.Get: %s", tag, err) } // Check for use of json or xml tags with unexported fields. // Embedded struct. Nothing to do for now, but that // may change, depending on what happens with issue 7363. + // TODO(adonovan): investigate, now that that issue is fixed. if field.Anonymous() { return } @@ -65,7 +75,7 @@ func checkCanonicalFieldTag(f *File, top *ast.StructType, field *types.Var, tag for _, enc := range [...]string{"json", "xml"} { if reflect.StructTag(tag).Get(enc) != "" { - f.Badf(field.Pos(), "struct field %s has %s tag but is not exported", field.Name(), enc) + pass.Reportf(field.Pos(), "struct field %s has %s tag but is not exported", field.Name(), enc) return } } @@ -74,7 +84,7 @@ func checkCanonicalFieldTag(f *File, top *ast.StructType, field *types.Var, tag // checkTagDuplicates checks a single struct field tag to see if any tags are // duplicated. nearest is the field that's closest to the field being checked, // while still being part of the top-level struct type. -func checkTagDuplicates(f *File, tag, key string, nearest, field *types.Var, seen *map[[2]string]token.Pos) { +func checkTagDuplicates(pass *analysis.Pass, tag, key string, nearest, field *types.Var, seen *map[[2]string]token.Pos) { val := reflect.StructTag(tag).Get(key) if val == "-" { // Ignored, even if the field is anonymous. @@ -92,7 +102,7 @@ func checkTagDuplicates(f *File, tag, key string, nearest, field *types.Var, see continue } tag := typ.Tag(i) - checkTagDuplicates(f, tag, key, nearest, field, seen) + checkTagDuplicates(pass, tag, key, nearest, field, seen) } } // Ignored if the field isn't anonymous. @@ -122,7 +132,10 @@ func checkTagDuplicates(f *File, tag, key string, nearest, field *types.Var, see *seen = map[[2]string]token.Pos{} } if pos, ok := (*seen)[[2]string{key, val}]; ok { - f.Badf(nearest.Pos(), "struct field %s repeats %s tag %q also at %s", field.Name(), key, val, f.loc(pos)) + 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) } else { (*seen)[[2]string{key, val}] = field.Pos() } diff --git a/go/analysis/passes/structtag/structtag_test.go b/go/analysis/passes/structtag/structtag_test.go new file mode 100644 index 00000000..7ce6d0b1 --- /dev/null +++ b/go/analysis/passes/structtag/structtag_test.go @@ -0,0 +1,13 @@ +package structtag_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/structtag" +) + +func TestFromFileSystem(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, structtag.Analyzer, "a") +} diff --git a/go/analysis/passes/structtag/testdata/src/a/a.go b/go/analysis/passes/structtag/testdata/src/a/a.go new file mode 100644 index 00000000..fe418ddc --- /dev/null +++ b/go/analysis/passes/structtag/testdata/src/a/a.go @@ -0,0 +1,113 @@ +// Copyright 2010 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. + +// This file contains the test for canonical struct tags. + +package a + +import "encoding/xml" + +type StructTagTest struct { + A int "hello" // want "`hello` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair" + B int "\tx:\"y\"" // want "not compatible with reflect.StructTag.Get: bad syntax for struct tag key" + C int "x:\"y\"\tx:\"y\"" // want "not compatible with reflect.StructTag.Get" + D int "x:`y`" // want "not compatible with reflect.StructTag.Get: bad syntax for struct tag value" + E int "ct\brl:\"char\"" // want "not compatible with reflect.StructTag.Get: bad syntax for struct tag pair" + F int `:"emptykey"` // want "not compatible with reflect.StructTag.Get: bad syntax for struct tag key" + G int `x:"noEndQuote` // want "not compatible with reflect.StructTag.Get: bad syntax for struct tag value" + H int `x:"trunc\x0"` // want "not compatible with reflect.StructTag.Get: bad syntax for struct tag value" + I int `x:"foo",y:"bar"` // want "not compatible with reflect.StructTag.Get: key:.value. pairs not separated by spaces" + J int `x:"foo"y:"bar"` // want "not compatible with reflect.StructTag.Get: key:.value. pairs not separated by spaces" + OK0 int `x:"y" u:"v" w:""` + OK1 int `x:"y:z" u:"v" w:""` // note multiple colons. + OK2 int "k0:\"values contain spaces\" k1:\"literal\ttabs\" k2:\"and\\tescaped\\tabs\"" + OK3 int `under_scores:"and" CAPS:"ARE_OK"` +} + +type UnexportedEncodingTagTest struct { + x int `json:"xx"` // want "struct field x has json tag but is not exported" + y int `xml:"yy"` // want "struct field y has xml tag but is not exported" + z int + A int `json:"aa" xml:"bb"` +} + +type unexp struct{} + +type JSONEmbeddedField struct { + UnexportedEncodingTagTest `is:"embedded"` + unexp `is:"embedded,notexported" json:"unexp"` // OK for now, see issue 7363 +} + +type AnonymousJSON struct{} +type AnonymousXML struct{} + +type AnonymousJSONField struct { + DuplicateAnonJSON int `json:"a"` + + A int "hello" // want "`hello` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair" +} + +type DuplicateJSONFields struct { + JSON int `json:"a"` + DuplicateJSON int `json:"a"` // want "struct field DuplicateJSON repeats json tag .a. also at a.go:52" + 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" + 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" + + AnonymousJSONField // want "struct field DuplicateAnonJSON repeats json tag .a. also at a.go:52" + + XML int `xml:"a"` + DuplicateXML int `xml:"a"` // want "struct field DuplicateXML repeats xml tag .a. also at a.go:68" + 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" + 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" + 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 0 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" + + AnonymousXML `xml:"b,attr"` // want "struct field AnonymousXML repeats xml attribute tag .b. also at a.go:84" + } + + AnonymousJSONField `json:"not_anon"` // ok; fields aren't embedded in JSON + AnonymousJSONField `json:"-"` // ok; entire field is ignored in JSON +} + +type UnexpectedSpacetest struct { + A int `json:"a,omitempty"` + B int `json:"b, omitempty"` // want "suspicious space in struct tag value" + C int `json:"c ,omitempty"` + D int `json:"d,omitempty, string"` // want "suspicious space in struct tag value" + E int `xml:"e local"` + F int `xml:"f "` // want "suspicious space in struct tag value" + G int `xml:" g"` // want "suspicious space in struct tag value" + H int `xml:"h ,omitempty"` // want "suspicious space in struct tag value" + I int `xml:"i, omitempty"` // want "suspicious space in struct tag value" + J int `xml:"j local ,omitempty"` // want "suspicious space in struct tag value" + K int `xml:"k local, omitempty"` // want "suspicious space in struct tag value" + L int `xml:" l local,omitempty"` // want "suspicious space in struct tag value" + M int `xml:"m local,omitempty"` // want "suspicious space in struct tag value" + N int `xml:" "` // want "suspicious space in struct tag value" + O int `xml:""` + P int `xml:","` + Q int `foo:" doesn't care "` +} diff --git a/go/analysis/passes/vet/testdata/structtag.go b/go/analysis/passes/vet/testdata/structtag.go deleted file mode 100644 index ad55c4ab..00000000 --- a/go/analysis/passes/vet/testdata/structtag.go +++ /dev/null @@ -1,113 +0,0 @@ -// Copyright 2010 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. - -// This file contains the test for canonical struct tags. - -package testdata - -import "encoding/xml" - -type StructTagTest struct { - A int "hello" // ERROR "`hello` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair" - B int "\tx:\"y\"" // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag key" - C int "x:\"y\"\tx:\"y\"" // ERROR "not compatible with reflect.StructTag.Get" - D int "x:`y`" // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag value" - E int "ct\brl:\"char\"" // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag pair" - F int `:"emptykey"` // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag key" - G int `x:"noEndQuote` // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag value" - H int `x:"trunc\x0"` // ERROR "not compatible with reflect.StructTag.Get: bad syntax for struct tag value" - I int `x:"foo",y:"bar"` // ERROR "not compatible with reflect.StructTag.Get: key:.value. pairs not separated by spaces" - J int `x:"foo"y:"bar"` // ERROR "not compatible with reflect.StructTag.Get: key:.value. pairs not separated by spaces" - OK0 int `x:"y" u:"v" w:""` - OK1 int `x:"y:z" u:"v" w:""` // note multiple colons. - OK2 int "k0:\"values contain spaces\" k1:\"literal\ttabs\" k2:\"and\\tescaped\\tabs\"" - OK3 int `under_scores:"and" CAPS:"ARE_OK"` -} - -type UnexportedEncodingTagTest struct { - x int `json:"xx"` // ERROR "struct field x has json tag but is not exported" - y int `xml:"yy"` // ERROR "struct field y has xml tag but is not exported" - z int - A int `json:"aa" xml:"bb"` -} - -type unexp struct{} - -type JSONEmbeddedField struct { - UnexportedEncodingTagTest `is:"embedded"` - unexp `is:"embedded,notexported" json:"unexp"` // OK for now, see issue 7363 -} - -type AnonymousJSON struct{} -type AnonymousXML struct{} - -type AnonymousJSONField struct { - DuplicateAnonJSON int `json:"a"` - - A int "hello" // ERROR "`hello` not compatible with reflect.StructTag.Get: bad syntax for struct tag pair" -} - -type DuplicateJSONFields struct { - JSON int `json:"a"` - DuplicateJSON int `json:"a"` // ERROR "struct field DuplicateJSON repeats json tag .a. also at structtag.go:52" - IgnoredJSON int `json:"-"` - OtherIgnoredJSON int `json:"-"` - OmitJSON int `json:",omitempty"` - OtherOmitJSON int `json:",omitempty"` - DuplicateOmitJSON int `json:"a,omitempty"` // ERROR "struct field DuplicateOmitJSON repeats json tag .a. also at structtag.go:52" - NonJSON int `foo:"a"` - DuplicateNonJSON int `foo:"a"` - Embedded struct { - DuplicateJSON int `json:"a"` // OK because its not in the same struct type - } - AnonymousJSON `json:"a"` // ERROR "struct field AnonymousJSON repeats json tag .a. also at structtag.go:52" - - AnonymousJSONField // ERROR "struct field DuplicateAnonJSON repeats json tag .a. also at structtag.go:52" - - XML int `xml:"a"` - DuplicateXML int `xml:"a"` // ERROR "struct field DuplicateXML repeats xml tag .a. also at structtag.go:68" - IgnoredXML int `xml:"-"` - OtherIgnoredXML int `xml:"-"` - OmitXML int `xml:",omitempty"` - OtherOmitXML int `xml:",omitempty"` - DuplicateOmitXML int `xml:"a,omitempty"` // ERROR "struct field DuplicateOmitXML repeats xml tag .a. also at structtag.go:68" - NonXML int `foo:"a"` - DuplicateNonXML int `foo:"a"` - Embedded2 struct { - DuplicateXML int `xml:"a"` // OK because its not in the same struct type - } - AnonymousXML `xml:"a"` // ERROR "struct field AnonymousXML repeats xml tag .a. also at structtag.go:68" - 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 0 is valid. - DupAttr int `xml:"b,attr"` // ERROR "struct field DupAttr repeats xml attribute tag .b. also at structtag.go:84" - DupOmitAttr int `xml:"b,omitempty,attr"` // ERROR "struct field DupOmitAttr repeats xml attribute tag .b. also at structtag.go:84" - - AnonymousXML `xml:"b,attr"` // ERROR "struct field AnonymousXML repeats xml attribute tag .b. also at structtag.go:84" - } - - AnonymousJSONField `json:"not_anon"` // ok; fields aren't embedded in JSON - AnonymousJSONField `json:"-"` // ok; entire field is ignored in JSON -} - -type UnexpectedSpacetest struct { - A int `json:"a,omitempty"` - B int `json:"b, omitempty"` // ERROR "suspicious space in struct tag value" - C int `json:"c ,omitempty"` - D int `json:"d,omitempty, string"` // ERROR "suspicious space in struct tag value" - E int `xml:"e local"` - F int `xml:"f "` // ERROR "suspicious space in struct tag value" - G int `xml:" g"` // ERROR "suspicious space in struct tag value" - H int `xml:"h ,omitempty"` // ERROR "suspicious space in struct tag value" - I int `xml:"i, omitempty"` // ERROR "suspicious space in struct tag value" - J int `xml:"j local ,omitempty"` // ERROR "suspicious space in struct tag value" - K int `xml:"k local, omitempty"` // ERROR "suspicious space in struct tag value" - L int `xml:" l local,omitempty"` // ERROR "suspicious space in struct tag value" - M int `xml:"m local,omitempty"` // ERROR "suspicious space in struct tag value" - N int `xml:" "` // ERROR "suspicious space in struct tag value" - O int `xml:""` - P int `xml:","` - Q int `foo:" doesn't care "` -}