diff --git a/cmd/vet/main.go b/cmd/vet/main.go index f91b851cfd..c75f6fe396 100644 --- a/cmd/vet/main.go +++ b/cmd/vet/main.go @@ -40,6 +40,7 @@ var report = map[string]*bool{ "buildtags": flag.Bool("buildtags", false, "check that +build tags are valid"), "composites": flag.Bool("composites", false, "check that composite literals used field-keyed elements"), "methods": flag.Bool("methods", false, "check that canonically named methods are canonically defined"), + "nilfunc": flag.Bool("nilfunc", false, "check for comparisons between functions and nil"), "printf": flag.Bool("printf", false, "check printf-like invocations"), "rangeloops": flag.Bool("rangeloops", false, "check that range loop variables are used correctly"), "shadow": flag.Bool("shadow", false, "check for shadowed variables (experimental; must be set explicitly)"), @@ -356,6 +357,8 @@ func (f *File) Visit(node ast.Node) ast.Visitor { switch n := node.(type) { case *ast.AssignStmt: f.walkAssignStmt(n) + case *ast.BinaryExpr: + f.walkBinaryExpr(n) case *ast.CallExpr: f.walkCallExpr(n) case *ast.CompositeLit: @@ -383,6 +386,10 @@ func (f *File) walkAssignStmt(stmt *ast.AssignStmt) { f.checkShadowAssignment(stmt) } +func (f *File) walkBinaryExpr(expr *ast.BinaryExpr) { + f.checkNilFuncComparison(expr) +} + // walkCall walks a call expression. func (f *File) walkCall(call *ast.CallExpr, name string) { f.checkFmtPrintfCall(call, name) diff --git a/cmd/vet/nilfunc.go b/cmd/vet/nilfunc.go new file mode 100644 index 0000000000..c6b33fdd13 --- /dev/null +++ b/cmd/vet/nilfunc.go @@ -0,0 +1,63 @@ +// Copyright 2013 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 code to check for useless function comparisons. +A useless comparison is one like f == nil as opposed to f() == nil. +*/ + +package main + +import ( + "go/ast" + "go/token" + + "code.google.com/p/go.tools/go/types" +) + +func (f *File) checkNilFuncComparison(e *ast.BinaryExpr) { + if !vet("nilfunc") { + return + } + + // Only want == or != comparisons. + if e.Op != token.EQL && e.Op != token.NEQ { + return + } + + // Only want comparisons with a nil identifier on one side. + var e2 ast.Expr + switch { + case f.isNil(e.X): + e2 = e.Y + case f.isNil(e.Y): + e2 = e.X + default: + return + } + + // Only want identifiers or selector expressions. + var obj types.Object + switch v := e2.(type) { + case *ast.Ident: + obj = f.pkg.idents[v] + case *ast.SelectorExpr: + obj = f.pkg.idents[v.Sel] + default: + return + } + + // Only want functions. + if _, ok := obj.(*types.Func); !ok { + return + } + + f.Badf(e.Pos(), "comparison of function %v %v nil is always %v", obj.Name(), e.Op, e.Op == token.NEQ) +} + +// isNil reports whether the provided expression is the built-in nil +// identifier. +func (f *File) isNil(e ast.Expr) bool { + return f.pkg.types[e] == types.Typ[types.UntypedNil] +} diff --git a/cmd/vet/testdata/nilfunc.go b/cmd/vet/testdata/nilfunc.go new file mode 100644 index 0000000000..8ee921ff85 --- /dev/null +++ b/cmd/vet/testdata/nilfunc.go @@ -0,0 +1,37 @@ +// Copyright 2013 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 testdata + +import "os" + +func F() {} + +type T struct { + F func() +} + +func (T) M() {} + +var Fv = F + +func Comparison() { + var t T + var fn func() + if fn == nil || Fv == nil || t.F == nil { + // no error; these func vars or fields may be nil + } + if F == nil { // ERROR "comparison of function F == nil is always false" + panic("can't happen") + } + if t.M == nil { // ERROR "comparison of function M == nil is always false" + panic("can't happen") + } + if F != nil { // ERROR "comparison of function F != nil is always true" + if t.M != nil { // ERROR "comparison of function M != nil is always true" + return + } + } + panic("can't happen") +} diff --git a/cmd/vet/types.go b/cmd/vet/types.go index df93f02e87..a4abcf88e8 100644 --- a/cmd/vet/types.go +++ b/cmd/vet/types.go @@ -266,7 +266,7 @@ func (f *File) isErrorMethodCall(call *ast.CallExpr) bool { } // There must be a receiver for it to be a method call. Otherwise it is // a function, not something that satisfies the error interface. - if sig.Recv == nil { + if sig.Recv() == nil { return false } // There must be no arguments. Already verified by type checking, but be thorough.