cmd/vet: recognise func type conversions

In hasSideEffects, vet has to be taught whether or not a CallExpr is an
actual function call, or just a type conversion.

The previous code knew how to differentiate fn(arg) from int(arg), but
it incorrectly saw (func(T))(fn) as a func call. This edge case is
slightly tricky, since the CallExpr.Fun has a func signature type, just
like in func calls.

However, the difference is that in this case the Fun is a type, not a
value. This information is in types.TypeAndValue, so use it.

Change-Id: I18bb8b23abbe7decc558b726ff2dc31fae2f13d6
Reviewed-on: https://go-review.googlesource.com/111416
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/cmd/vet/bool.go b/src/cmd/vet/bool.go
index 31e81ec4bf1cea5ad9efe334a6673f97984cd0c8..67321c3df4cfba04562340497b2bbf19dc4cf207 100644 (file)
--- a/src/cmd/vet/bool.go
+++ b/src/cmd/vet/bool.go
@@ -145,13 +145,14 @@ func hasSideEffects(f *File, e ast.Expr) bool {
                        // Don't call Type.Underlying(), since its lack
                        // lets us see the NamedFuncType(x) type
                        // conversion as a *types.Named.
-                       _, ok := f.pkg.types[n.Fun].Type.(*types.Signature)
-                       if ok {
-                               // Conservatively assume that all function and
-                               // method calls have side effects for
-                               // now. This will include func type
-                               // conversions, but it's ok given that
-                               // this is the conservative side.
+                       typVal := f.pkg.types[n.Fun]
+                       _, isSig := typVal.Type.(*types.Signature)
+                       if typVal.IsValue() && isSig {
+                               // If we have a value of unnamed signature type,
+                               // this CallExpr is a func call and not a type
+                               // conversion. Conservatively assume that all
+                               // function and method calls have side effects
+                               // for now.
                                safe = false
                                return false
                        }

diff --git a/src/cmd/vet/testdata/bool.go b/src/cmd/vet/testdata/bool.go
index bada13ae0dc2bb810703dfb79caddadab7f1326a..be78caac18e51ae2f838fff924bf9ff36ebe5910 100644 (file)
--- a/src/cmd/vet/testdata/bool.go
+++ b/src/cmd/vet/testdata/bool.go
@@ -24,8 +24,7 @@ func RatherStupidConditions() {
        _ = i == T(2) || i == T(2)       // ERROR "redundant or: i == T(2) || i == T(2)"
        _ = FT(f) == nil || FT(f) == nil // ERROR "redundant or: FT(f) == nil || FT(f) == nil"
 
-       // TODO: distinguish from an actual func call
-       _ = (func() int)(f) == nil || (func() int)(f) == nil
+       _ = (func() int)(f) == nil || (func() int)(f) == nil // ERROR "redundant or: (func() int)(f) == nil || (func() int)(f) == nil"
 
        var namedFuncVar FT
        _ = namedFuncVar() == namedFuncVar() // OK; still func calls