| 1 | // Copyright 2014 The Go Authors. All rights reserved. |
|---|---|
| 2 | // Use of this source code is governed by a BSD-style |
| 3 | // license that can be found in the LICENSE file. |
| 4 | |
| 5 | // Package bools defines an Analyzer that detects common mistakes |
| 6 | // involving boolean operators. |
| 7 | package bools |
| 8 | |
| 9 | import ( |
| 10 | "go/ast" |
| 11 | "go/token" |
| 12 | "go/types" |
| 13 | |
| 14 | "golang.org/x/tools/go/analysis" |
| 15 | "golang.org/x/tools/go/analysis/passes/inspect" |
| 16 | "golang.org/x/tools/go/analysis/passes/internal/analysisutil" |
| 17 | "golang.org/x/tools/go/ast/inspector" |
| 18 | ) |
| 19 | |
| 20 | const Doc = "check for common mistakes involving boolean operators" |
| 21 | |
| 22 | var Analyzer = &analysis.Analyzer{ |
| 23 | Name: "bools", |
| 24 | Doc: Doc, |
| 25 | Requires: []*analysis.Analyzer{inspect.Analyzer}, |
| 26 | Run: run, |
| 27 | } |
| 28 | |
| 29 | func run(pass *analysis.Pass) (interface{}, error) { |
| 30 | inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) |
| 31 | |
| 32 | nodeFilter := []ast.Node{ |
| 33 | (*ast.BinaryExpr)(nil), |
| 34 | } |
| 35 | seen := make(map[*ast.BinaryExpr]bool) |
| 36 | inspect.Preorder(nodeFilter, func(n ast.Node) { |
| 37 | e := n.(*ast.BinaryExpr) |
| 38 | if seen[e] { |
| 39 | // Already processed as a subexpression of an earlier node. |
| 40 | return |
| 41 | } |
| 42 | |
| 43 | var op boolOp |
| 44 | switch e.Op { |
| 45 | case token.LOR: |
| 46 | op = or |
| 47 | case token.LAND: |
| 48 | op = and |
| 49 | default: |
| 50 | return |
| 51 | } |
| 52 | |
| 53 | comm := op.commutativeSets(pass.TypesInfo, e, seen) |
| 54 | for _, exprs := range comm { |
| 55 | op.checkRedundant(pass, exprs) |
| 56 | op.checkSuspect(pass, exprs) |
| 57 | } |
| 58 | }) |
| 59 | return nil, nil |
| 60 | } |
| 61 | |
| 62 | type boolOp struct { |
| 63 | name string |
| 64 | tok token.Token // token corresponding to this operator |
| 65 | badEq token.Token // token corresponding to the equality test that should not be used with this operator |
| 66 | } |
| 67 | |
| 68 | var ( |
| 69 | or = boolOp{"or", token.LOR, token.NEQ} |
| 70 | and = boolOp{"and", token.LAND, token.EQL} |
| 71 | ) |
| 72 | |
| 73 | // commutativeSets returns all side effect free sets of |
| 74 | // expressions in e that are connected by op. |
| 75 | // For example, given 'a || b || f() || c || d' with the or op, |
| 76 | // commutativeSets returns {{b, a}, {d, c}}. |
| 77 | // commutativeSets adds any expanded BinaryExprs to seen. |
| 78 | func (op boolOp) commutativeSets(info *types.Info, e *ast.BinaryExpr, seen map[*ast.BinaryExpr]bool) [][]ast.Expr { |
| 79 | exprs := op.split(e, seen) |
| 80 | |
| 81 | // Partition the slice of expressions into commutative sets. |
| 82 | i := 0 |
| 83 | var sets [][]ast.Expr |
| 84 | for j := 0; j <= len(exprs); j++ { |
| 85 | if j == len(exprs) || hasSideEffects(info, exprs[j]) { |
| 86 | if i < j { |
| 87 | sets = append(sets, exprs[i:j]) |
| 88 | } |
| 89 | i = j + 1 |
| 90 | } |
| 91 | } |
| 92 | |
| 93 | return sets |
| 94 | } |
| 95 | |
| 96 | // checkRedundant checks for expressions of the form |
| 97 | // |
| 98 | // e && e |
| 99 | // e || e |
| 100 | // |
| 101 | // Exprs must contain only side effect free expressions. |
| 102 | func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) { |
| 103 | seen := make(map[string]bool) |
| 104 | for _, e := range exprs { |
| 105 | efmt := analysisutil.Format(pass.Fset, e) |
| 106 | if seen[efmt] { |
| 107 | pass.ReportRangef(e, "redundant %s: %s %s %s", op.name, efmt, op.tok, efmt) |
| 108 | } else { |
| 109 | seen[efmt] = true |
| 110 | } |
| 111 | } |
| 112 | } |
| 113 | |
| 114 | // checkSuspect checks for expressions of the form |
| 115 | // |
| 116 | // x != c1 || x != c2 |
| 117 | // x == c1 && x == c2 |
| 118 | // |
| 119 | // where c1 and c2 are constant expressions. |
| 120 | // If c1 and c2 are the same then it's redundant; |
| 121 | // if c1 and c2 are different then it's always true or always false. |
| 122 | // Exprs must contain only side effect free expressions. |
| 123 | func (op boolOp) checkSuspect(pass *analysis.Pass, exprs []ast.Expr) { |
| 124 | // seen maps from expressions 'x' to equality expressions 'x != c'. |
| 125 | seen := make(map[string]string) |
| 126 | |
| 127 | for _, e := range exprs { |
| 128 | bin, ok := e.(*ast.BinaryExpr) |
| 129 | if !ok || bin.Op != op.badEq { |
| 130 | continue |
| 131 | } |
| 132 | |
| 133 | // In order to avoid false positives, restrict to cases |
| 134 | // in which one of the operands is constant. We're then |
| 135 | // interested in the other operand. |
| 136 | // In the rare case in which both operands are constant |
| 137 | // (e.g. runtime.GOOS and "windows"), we'll only catch |
| 138 | // mistakes if the LHS is repeated, which is how most |
| 139 | // code is written. |
| 140 | var x ast.Expr |
| 141 | switch { |
| 142 | case pass.TypesInfo.Types[bin.Y].Value != nil: |
| 143 | x = bin.X |
| 144 | case pass.TypesInfo.Types[bin.X].Value != nil: |
| 145 | x = bin.Y |
| 146 | default: |
| 147 | continue |
| 148 | } |
| 149 | |
| 150 | // e is of the form 'x != c' or 'x == c'. |
| 151 | xfmt := analysisutil.Format(pass.Fset, x) |
| 152 | efmt := analysisutil.Format(pass.Fset, e) |
| 153 | if prev, found := seen[xfmt]; found { |
| 154 | // checkRedundant handles the case in which efmt == prev. |
| 155 | if efmt != prev { |
| 156 | pass.ReportRangef(e, "suspect %s: %s %s %s", op.name, efmt, op.tok, prev) |
| 157 | } |
| 158 | } else { |
| 159 | seen[xfmt] = efmt |
| 160 | } |
| 161 | } |
| 162 | } |
| 163 | |
| 164 | // hasSideEffects reports whether evaluation of e has side effects. |
| 165 | func hasSideEffects(info *types.Info, e ast.Expr) bool { |
| 166 | safe := true |
| 167 | ast.Inspect(e, func(node ast.Node) bool { |
| 168 | switch n := node.(type) { |
| 169 | case *ast.CallExpr: |
| 170 | typVal := info.Types[n.Fun] |
| 171 | switch { |
| 172 | case typVal.IsType(): |
| 173 | // Type conversion, which is safe. |
| 174 | case typVal.IsBuiltin(): |
| 175 | // Builtin func, conservatively assumed to not |
| 176 | // be safe for now. |
| 177 | safe = false |
| 178 | return false |
| 179 | default: |
| 180 | // A non-builtin func or method call. |
| 181 | // Conservatively assume that all of them have |
| 182 | // side effects for now. |
| 183 | safe = false |
| 184 | return false |
| 185 | } |
| 186 | case *ast.UnaryExpr: |
| 187 | if n.Op == token.ARROW { |
| 188 | safe = false |
| 189 | return false |
| 190 | } |
| 191 | } |
| 192 | return true |
| 193 | }) |
| 194 | return !safe |
| 195 | } |
| 196 | |
| 197 | // split returns a slice of all subexpressions in e that are connected by op. |
| 198 | // For example, given 'a || (b || c) || d' with the or op, |
| 199 | // split returns []{d, c, b, a}. |
| 200 | // seen[e] is already true; any newly processed exprs are added to seen. |
| 201 | func (op boolOp) split(e ast.Expr, seen map[*ast.BinaryExpr]bool) (exprs []ast.Expr) { |
| 202 | for { |
| 203 | e = unparen(e) |
| 204 | if b, ok := e.(*ast.BinaryExpr); ok && b.Op == op.tok { |
| 205 | seen[b] = true |
| 206 | exprs = append(exprs, op.split(b.Y, seen)...) |
| 207 | e = b.X |
| 208 | } else { |
| 209 | exprs = append(exprs, e) |
| 210 | break |
| 211 | } |
| 212 | } |
| 213 | return |
| 214 | } |
| 215 | |
| 216 | // unparen returns e with any enclosing parentheses stripped. |
| 217 | func unparen(e ast.Expr) ast.Expr { |
| 218 | for { |
| 219 | p, ok := e.(*ast.ParenExpr) |
| 220 | if !ok { |
| 221 | return e |
| 222 | } |
| 223 | e = p.X |
| 224 | } |
| 225 | } |
| 226 |
Members