From 83e22f1a47ee3bb7ceb1ceb5fbe1c5e13f7fe131 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Sun, 24 Feb 2013 13:18:21 -0800 Subject: [PATCH] cmd/vet: eliminate false positives for slices in untagged literal test Made possible by go/types, as long as the package type-checks OK. Fixes #4684. R=golang-dev CC=golang-dev https://golang.org/cl/7407045 --- src/cmd/vet/Makefile | 2 +- src/cmd/vet/taglit.go | 38 +++++++++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/cmd/vet/Makefile b/src/cmd/vet/Makefile index c0e3169989..ba86addac8 100644 --- a/src/cmd/vet/Makefile +++ b/src/cmd/vet/Makefile @@ -4,5 +4,5 @@ test testshort: go build -tags unsafe - ../../../test/errchk ./vet -printfuncs='Warn:1,Warnf:1' *.go + ../../../test/errchk ./vet -compositewhitelist=false -printfuncs='Warn:1,Warnf:1' *.go diff --git a/src/cmd/vet/taglit.go b/src/cmd/vet/taglit.go index 6171efea48..1197522d49 100644 --- a/src/cmd/vet/taglit.go +++ b/src/cmd/vet/taglit.go @@ -7,18 +7,42 @@ package main import ( + "flag" "go/ast" + "go/types" "strings" - "flag" // for test + "go/scanner" // for test; chosen because it's already linked in. ) +var compositeWhiteList = flag.Bool("compositewhitelist", true, "use composite white list; for testing only") + // checkUntaggedLiteral checks if a composite literal is an struct literal with // untagged fields. func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { if !vet("composites") { return } + + // Check that the CompositeLit's type is a slice or array (which need no tag), if possible. + if f.pkg != nil { + typ := f.pkg.types[c] + if typ != nil { + // If it's a named type, pull out the underlying type. + if namedType, ok := typ.(*types.NamedType); ok { + typ = namedType.Underlying + } + switch typ.(type) { + case *types.Slice: + return + case *types.Array: + return + } + } + } + + // It's a struct, or we can't tell it's not a struct because we don't have types. + // Check if the CompositeLit contains an untagged field. allKeyValue := true for _, e := range c.Elts { @@ -48,11 +72,11 @@ func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { return } typ := path + "." + s.Sel.Name - if untaggedLiteralWhitelist[typ] { + if *compositeWhiteList && untaggedLiteralWhitelist[typ] { return } - f.Warnf(c.Pos(), "%s struct literal uses untagged fields", typ) + f.Warnf(c.Pos(), "%s composite literal uses untagged fields", typ) } // pkgPath returns the import path "image/png" for the package name "png". @@ -125,9 +149,17 @@ var untaggedLiteralWhitelist = map[string]bool{ "image.Rectangle": true, } +// Testing is awkward because we need to reference things from a separate package +// to trigger the warnings. + var BadStructLiteralUsedInTests = flag.Flag{ // ERROR "untagged fields" "Name", "Usage", nil, // Value "DefValue", } + +// Used to test the check for slices and arrays: If that test is disabled and +// vet is run with --compositewhitelist=false, this line triggers an error. +// Clumsy but sufficient. +var scannerErrorListTest = scanner.ErrorList{nil, nil} -- 2.48.1