Commit d2825329 authored by Rob Pike's avatar Rob Pike

vet: improve flag handling

Simplify the internal logic for flags controlling what to vet,
by introducing a map of flags that gathers them all together.
This change should simplify the process of adding further flags.

Add a test for untagged struct literals.
Delete a redundant test that was also in the wrong file.
Clean up some ERROR patterns that weren't working.

"make test" passes again.

R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/7305075
parent 6c119a9e
...@@ -13,7 +13,7 @@ import ( ...@@ -13,7 +13,7 @@ import (
// checkAtomicAssignment walks the assignment statement checking for comomon // checkAtomicAssignment walks the assignment statement checking for comomon
// mistaken usage of atomic package, such as: x = atomic.AddUint64(&x, 1) // mistaken usage of atomic package, such as: x = atomic.AddUint64(&x, 1)
func (f *File) checkAtomicAssignment(n *ast.AssignStmt) { func (f *File) checkAtomicAssignment(n *ast.AssignStmt) {
if !*vetAtomic && !*vetAll { if !vet("atomic") {
return return
} }
......
...@@ -25,7 +25,7 @@ var ( ...@@ -25,7 +25,7 @@ var (
// checkBuildTag checks that build tags are in the correct location and well-formed. // checkBuildTag checks that build tags are in the correct location and well-formed.
func checkBuildTag(name string, data []byte) { func checkBuildTag(name string, data []byte) {
if !*vetBuildTags && !*vetAll { if !vet("buildtags") {
return return
} }
lines := bytes.SplitAfter(data, nl) lines := bytes.SplitAfter(data, nl)
......
...@@ -25,18 +25,23 @@ import ( ...@@ -25,18 +25,23 @@ import (
var verbose = flag.Bool("v", false, "verbose") var verbose = flag.Bool("v", false, "verbose")
var exitCode = 0 var exitCode = 0
// Flags to control which checks to perform. // Flags to control which checks to perform. "all" is set to true here, and disabled later if
// NOTE: Add new flags to the if statement at the top of func main too. // a flag is set explicitly.
var ( var report = map[string]*bool{
vetAll = flag.Bool("all", true, "check everything; disabled if any explicit check is requested") "all": flag.Bool("all", true, "check everything; disabled if any explicit check is requested"),
vetAtomic = flag.Bool("atomic", false, "check for common mistaken usages of the sync/atomic package") "atomic": flag.Bool("atomic", false, "check for common mistaken usages of the sync/atomic package"),
vetBuildTags = flag.Bool("buildtags", false, "check that +build tags are valid") "buildtags": flag.Bool("buildtags", false, "check that +build tags are valid"),
vetMethods = flag.Bool("methods", false, "check that canonically named methods are canonically defined") "composites": flag.Bool("composites", false, "check that composite literals used type-tagged elements"),
vetPrintf = flag.Bool("printf", false, "check printf-like invocations") "methods": flag.Bool("methods", false, "check that canonically named methods are canonically defined"),
vetStructTags = flag.Bool("structtags", false, "check that struct field tags have canonical format") "printf": flag.Bool("printf", false, "check printf-like invocations"),
vetRangeLoops = flag.Bool("rangeloops", false, "check that range loop variables are used correctly") "structtags": flag.Bool("structtags", false, "check that struct field tags have canonical format"),
vetUntaggedLiteral = flag.Bool("composites", false, "check that composite literals used type-tagged elements") "rangeloops": flag.Bool("rangeloops", false, "check that range loop variables are used correctly"),
) }
// vet tells whether to report errors for the named check, a flag name.
func vet(name string) bool {
return *report["all"] || *report[name]
}
// setExit sets the value for os.Exit when it is called, later. It // setExit sets the value for os.Exit when it is called, later. It
// remembers the highest value. // remembers the highest value.
...@@ -66,8 +71,11 @@ func main() { ...@@ -66,8 +71,11 @@ func main() {
flag.Parse() flag.Parse()
// If a check is named explicitly, turn off the 'all' flag. // If a check is named explicitly, turn off the 'all' flag.
if *vetAtomic || *vetBuildTags || *vetMethods || *vetPrintf || *vetStructTags || *vetRangeLoops || *vetUntaggedLiteral { for name, ptr := range report {
*vetAll = false if name != "all" && *ptr {
*report["all"] = false
break
}
} }
if *printfuncs != "" { if *printfuncs != "" {
......
...@@ -55,7 +55,7 @@ var canonicalMethods = map[string]MethodSig{ ...@@ -55,7 +55,7 @@ var canonicalMethods = map[string]MethodSig{
} }
func (f *File) checkCanonicalMethod(id *ast.Ident, t *ast.FuncType) { func (f *File) checkCanonicalMethod(id *ast.Ident, t *ast.FuncType) {
if !*vetMethods && !*vetAll { if !vet("methods") {
return return
} }
// Expected input/output. // Expected input/output.
...@@ -161,9 +161,9 @@ func (f *File) matchParamType(expect string, actual ast.Expr) bool { ...@@ -161,9 +161,9 @@ func (f *File) matchParamType(expect string, actual ast.Expr) bool {
return f.b.String() == expect return f.b.String() == expect
} }
func (t *BadTypeUsedInTests) Scan(x fmt.ScanState, c byte) { // ERROR "method Scan[(]x fmt.ScanState, c byte[)] should have signature Scan[(]fmt.ScanState, rune[)] error" func (t *BadTypeUsedInTests) Scan(x fmt.ScanState, c byte) { // ERROR "should have signature Scan"
} }
type BadInterfaceUsedInTests interface { type BadInterfaceUsedInTests interface {
ReadByte() byte // ERROR "method ReadByte[(][)] byte should have signature ReadByte[(][)] [(]byte, error[)]" ReadByte() byte // ERROR "should have signature ReadByte"
} }
...@@ -44,7 +44,7 @@ var printList = map[string]int{ ...@@ -44,7 +44,7 @@ var printList = map[string]int{
// checkCall triggers the print-specific checks if the call invokes a print function. // checkCall triggers the print-specific checks if the call invokes a print function.
func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) { func (f *File) checkFmtPrintfCall(call *ast.CallExpr, Name string) {
if !*vetPrintf && !*vetAll { if !vet("printf") {
return return
} }
name := strings.ToLower(Name) name := strings.ToLower(Name)
......
...@@ -27,7 +27,7 @@ import "go/ast" ...@@ -27,7 +27,7 @@ import "go/ast"
// its index or value variables are used unsafely inside goroutines or deferred // its index or value variables are used unsafely inside goroutines or deferred
// function literals. // function literals.
func checkRangeLoop(f *File, n *ast.RangeStmt) { func checkRangeLoop(f *File, n *ast.RangeStmt) {
if !*vetRangeLoops && !*vetAll { if !vet("rangeloops") {
return return
} }
key, _ := n.Key.(*ast.Ident) key, _ := n.Key.(*ast.Ident)
......
...@@ -14,7 +14,7 @@ import ( ...@@ -14,7 +14,7 @@ import (
// checkField checks a struct field tag. // checkField checks a struct field tag.
func (f *File) checkCanonicalFieldTag(field *ast.Field) { func (f *File) checkCanonicalFieldTag(field *ast.Field) {
if !*vetStructTags && !*vetAll { if !vet("structtags") {
return return
} }
if field.Tag == nil { if field.Tag == nil {
...@@ -37,5 +37,5 @@ func (f *File) checkCanonicalFieldTag(field *ast.Field) { ...@@ -37,5 +37,5 @@ func (f *File) checkCanonicalFieldTag(field *ast.Field) {
} }
type BadTypeUsedInTests struct { type BadTypeUsedInTests struct {
X int "hello" // ERROR "struct field tag" X int "hello" // ERROR "not compatible with reflect.StructTag.Get"
} }
...@@ -9,12 +9,14 @@ package main ...@@ -9,12 +9,14 @@ package main
import ( import (
"go/ast" "go/ast"
"strings" "strings"
"flag" // for test
) )
// checkUntaggedLiteral checks if a composite literal is an struct literal with // checkUntaggedLiteral checks if a composite literal is an struct literal with
// untagged fields. // untagged fields.
func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) { func (f *File) checkUntaggedLiteral(c *ast.CompositeLit) {
if !*vetUntaggedLiteral && !*vetAll { if !vet("composites") {
return return
} }
// Check if the CompositeLit contains an untagged field. // Check if the CompositeLit contains an untagged field.
...@@ -123,6 +125,9 @@ var untaggedLiteralWhitelist = map[string]bool{ ...@@ -123,6 +125,9 @@ var untaggedLiteralWhitelist = map[string]bool{
"image.Rectangle": true, "image.Rectangle": true,
} }
type BadTag struct { var BadStructLiteralUsedInTests = flag.Flag{ // ERROR "untagged fields"
S string `this is a bad tag` // ERROR "not compatible with reflect.StructTag.Get" "Name",
"Usage",
nil, // Value
"DefValue",
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment