bools.go

     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  //   e && e
    98  //   e || e
    99  // Exprs must contain only side effect free expressions.
   100  func (op boolOp) checkRedundant(pass *analysis.Pass, exprs []ast.Expr) {
   101  	seen := make(map[string]bool)
   102  	for _, e := range exprs {
   103  		efmt := analysisutil.Format(pass.Fset, e)
   104  		if seen[efmt] {
   105  			pass.ReportRangef(e, "redundant %s: %s %s %s", op.name, efmt, op.tok, efmt)
   106  		} else {
   107  			seen[efmt] = true
   108  		}
   109  	}
   110  }
   111  
   112  // checkSuspect checks for expressions of the form
   113  //   x != c1 || x != c2
   114  //   x == c1 && x == c2
   115  // where c1 and c2 are constant expressions.
   116  // If c1 and c2 are the same then it's redundant;
   117  // if c1 and c2 are different then it's always true or always false.
   118  // Exprs must contain only side effect free expressions.
   119  func (op boolOp) checkSuspect(pass *analysis.Pass, exprs []ast.Expr) {
   120  	// seen maps from expressions 'x' to equality expressions 'x != c'.
   121  	seen := make(map[string]string)
   122  
   123  	for _, e := range exprs {
   124  		bin, ok := e.(*ast.BinaryExpr)
   125  		if !ok || bin.Op != op.badEq {
   126  			continue
   127  		}
   128  
   129  		// In order to avoid false positives, restrict to cases
   130  		// in which one of the operands is constant. We're then
   131  		// interested in the other operand.
   132  		// In the rare case in which both operands are constant
   133  		// (e.g. runtime.GOOS and "windows"), we'll only catch
   134  		// mistakes if the LHS is repeated, which is how most
   135  		// code is written.
   136  		var x ast.Expr
   137  		switch {
   138  		case pass.TypesInfo.Types[bin.Y].Value != nil:
   139  			x = bin.X
   140  		case pass.TypesInfo.Types[bin.X].Value != nil:
   141  			x = bin.Y
   142  		default:
   143  			continue
   144  		}
   145  
   146  		// e is of the form 'x != c' or 'x == c'.
   147  		xfmt := analysisutil.Format(pass.Fset, x)
   148  		efmt := analysisutil.Format(pass.Fset, e)
   149  		if prev, found := seen[xfmt]; found {
   150  			// checkRedundant handles the case in which efmt == prev.
   151  			if efmt != prev {
   152  				pass.ReportRangef(e, "suspect %s: %s %s %s", op.name, efmt, op.tok, prev)
   153  			}
   154  		} else {
   155  			seen[xfmt] = efmt
   156  		}
   157  	}
   158  }
   159  
   160  // hasSideEffects reports whether evaluation of e has side effects.
   161  func hasSideEffects(info *types.Info, e ast.Expr) bool {
   162  	safe := true
   163  	ast.Inspect(e, func(node ast.Node) bool {
   164  		switch n := node.(type) {
   165  		case *ast.CallExpr:
   166  			typVal := info.Types[n.Fun]
   167  			switch {
   168  			case typVal.IsType():
   169  				// Type conversion, which is safe.
   170  			case typVal.IsBuiltin():
   171  				// Builtin func, conservatively assumed to not
   172  				// be safe for now.
   173  				safe = false
   174  				return false
   175  			default:
   176  				// A non-builtin func or method call.
   177  				// Conservatively assume that all of them have
   178  				// side effects for now.
   179  				safe = false
   180  				return false
   181  			}
   182  		case *ast.UnaryExpr:
   183  			if n.Op == token.ARROW {
   184  				safe = false
   185  				return false
   186  			}
   187  		}
   188  		return true
   189  	})
   190  	return !safe
   191  }
   192  
   193  // split returns a slice of all subexpressions in e that are connected by op.
   194  // For example, given 'a || (b || c) || d' with the or op,
   195  // split returns []{d, c, b, a}.
   196  // seen[e] is already true; any newly processed exprs are added to seen.
   197  func (op boolOp) split(e ast.Expr, seen map[*ast.BinaryExpr]bool) (exprs []ast.Expr) {
   198  	for {
   199  		e = unparen(e)
   200  		if b, ok := e.(*ast.BinaryExpr); ok && b.Op == op.tok {
   201  			seen[b] = true
   202  			exprs = append(exprs, op.split(b.Y, seen)...)
   203  			e = b.X
   204  		} else {
   205  			exprs = append(exprs, e)
   206  			break
   207  		}
   208  	}
   209  	return
   210  }
   211  
   212  // unparen returns e with any enclosing parentheses stripped.
   213  func unparen(e ast.Expr) ast.Expr {
   214  	for {
   215  		p, ok := e.(*ast.ParenExpr)
   216  		if !ok {
   217  			return e
   218  		}
   219  		e = p.X
   220  	}
   221  }
   222
View as plain text