Left: | ||
Right: |
LEFT | RIGHT |
---|---|
1 // Copyright 2013 The Go Authors. All rights reserved. | 1 // Copyright 2013 The Go Authors. All rights reserved. |
2 // Use of this source code is governed by a BSD-style | 2 // Use of this source code is governed by a BSD-style |
3 // license that can be found in the LICENSE file. | 3 // license that can be found in the LICENSE file. |
4 | |
5 /* | |
6 This file contains the code to check for useless function comparisons. | |
7 A useless comparison is one like f == nil as opposed to f() == nil. | |
8 */ | |
4 | 9 |
5 package main | 10 package main |
6 | 11 |
7 import ( | 12 import ( |
8 "go/ast" | 13 "go/ast" |
9 "go/token" | 14 "go/token" |
10 | 15 |
11 "code.google.com/p/go.tools/go/types" | 16 "code.google.com/p/go.tools/go/types" |
12 ) | 17 ) |
13 | 18 |
14 func (f *File) checkNilFuncComparison(expr *ast.BinaryExpr) { | 19 func (f *File) checkNilFuncComparison(e *ast.BinaryExpr) { |
15 » // Only want == or != comparisons. | 20 » if !vet("nilfunc") { |
16 » if expr.Op != token.EQL && expr.Op != token.NEQ { | |
17 return | 21 return |
18 } | 22 } |
19 | 23 |
20 » // Only want comparisons with nil identifier on one side. | 24 » // Only want == or != comparisons. |
21 » var e ast.Expr | 25 » if e.Op != token.EQL && e.Op != token.NEQ { |
26 » » return | |
27 » } | |
28 | |
29 » // Only want comparisons with a nil identifier on one side. | |
30 » var e2 ast.Expr | |
22 switch { | 31 switch { |
23 » case isNil(expr.X) && !isNil(expr.Y): | 32 » case f.isNil(e.X): |
24 » » e = expr.Y | 33 » » e2 = e.Y |
25 » case !isNil(expr.X) && isNil(expr.Y): | 34 » case f.isNil(e.Y): |
26 » » e = expr.X | 35 » » e2 = e.X |
27 default: | 36 default: |
28 return | 37 return |
29 } | 38 } |
30 | 39 |
31 // Only want identifiers or selector expressions. | 40 // Only want identifiers or selector expressions. |
32 var obj types.Object | 41 var obj types.Object |
33 » switch v := e.(type) { | 42 » switch v := e2.(type) { |
34 case *ast.Ident: | 43 case *ast.Ident: |
35 obj = f.pkg.idents[v] | 44 obj = f.pkg.idents[v] |
36 case *ast.SelectorExpr: | 45 case *ast.SelectorExpr: |
37 obj = f.pkg.idents[v.Sel] | 46 obj = f.pkg.idents[v.Sel] |
38 default: | 47 default: |
39 return | 48 return |
40 } | 49 } |
41 | 50 |
42 // Only want functions. | 51 // Only want functions. |
43 if _, ok := obj.(*types.Func); !ok { | 52 if _, ok := obj.(*types.Func); !ok { |
44 return | 53 return |
45 } | 54 } |
46 | 55 |
47 » f.Badf(expr.Pos(), "function uselessly compared to nil") | 56 » f.Badf(e.Pos(), "comparison of function %v %v nil is always %v", obj.Nam e(), e.Op, e.Op == token.NEQ) |
dsymonds
2013/08/20 04:30:56
I think this needs better phrasing. It's certainly
adg
2013/08/20 04:44:23
Please suggest better phrasing.
dsymonds
2013/08/20 04:45:53
"function comparison always <X>", depending on whe
adg
2013/08/20 04:56:55
Done.
| |
48 } | 57 } |
49 | 58 |
50 // isNil reports whether the provided expression is the built-in nil | 59 // isNil reports whether the provided expression is the built-in nil |
51 // idenitifer. | 60 // identifier. |
dsymonds
2013/08/20 04:30:56
"identifier"
adg
2013/08/20 04:44:23
Done.
| |
52 func isNil(e ast.Expr) bool { | 61 func (f *File) isNil(e ast.Expr) bool { |
53 » id, ok := e.(*ast.Ident) | 62 » return f.pkg.types[e] == types.Typ[types.UntypedNil] |
54 » if !ok { | |
55 » » return false | |
56 » } | |
57 » return id.Name == "nil" && id.Obj == nil | |
58 } | 63 } |
LEFT | RIGHT |