sa9008.go raw

   1  package sa9008
   2  
   3  import (
   4  	"fmt"
   5  	"go/ast"
   6  
   7  	"honnef.co/go/tools/analysis/code"
   8  	"honnef.co/go/tools/analysis/lint"
   9  	"honnef.co/go/tools/analysis/report"
  10  	"honnef.co/go/tools/go/ast/astutil"
  11  	"honnef.co/go/tools/go/ir"
  12  	"honnef.co/go/tools/go/ir/irutil"
  13  	"honnef.co/go/tools/internal/passes/buildir"
  14  	"honnef.co/go/tools/pattern"
  15  
  16  	"golang.org/x/tools/go/analysis"
  17  	"golang.org/x/tools/go/analysis/passes/inspect"
  18  )
  19  
  20  var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
  21  	Analyzer: &analysis.Analyzer{
  22  		Name:     "SA9008",
  23  		Run:      run,
  24  		Requires: []*analysis.Analyzer{inspect.Analyzer, buildir.Analyzer},
  25  	},
  26  	Doc: &lint.RawDocumentation{
  27  		Title: `\'else\' branch of a type assertion is probably not reading the right value`,
  28  		Text: `
  29  When declaring variables as part of an \'if\' statement (like in \"if
  30  foo := ...; foo {\"), the same variables will also be in the scope of
  31  the \'else\' branch. This means that in the following example
  32  
  33      if x, ok := x.(int); ok {
  34          // ...
  35      } else {
  36          fmt.Printf("unexpected type %T", x)
  37      }
  38  
  39  \'x\' in the \'else\' branch will refer to the \'x\' from \'x, ok
  40  :=\'; it will not refer to the \'x\' that is being type-asserted. The
  41  result of a failed type assertion is the zero value of the type that
  42  is being asserted to, so \'x\' in the else branch will always have the
  43  value \'0\' and the type \'int\'.
  44  `,
  45  		Since:    "2022.1",
  46  		Severity: lint.SeverityWarning,
  47  		MergeIf:  lint.MergeIfAny,
  48  	},
  49  })
  50  
  51  var Analyzer = SCAnalyzer.Analyzer
  52  
  53  var typeAssertionShadowingElseQ = pattern.MustParse(`(IfStmt (AssignStmt [obj@(Ident _) ok@(Ident _)] ":=" assert@(TypeAssertExpr obj _)) ok _ elseBranch)`)
  54  
  55  func run(pass *analysis.Pass) (interface{}, error) {
  56  	// TODO(dh): without the IR-based verification, this check is able
  57  	// to find more bugs, but also more prone to false positives. It
  58  	// would be a good candidate for the 'codereview' category of
  59  	// checks.
  60  
  61  	irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR).Pkg
  62  	fn := func(node ast.Node) {
  63  		m, ok := code.Match(pass, typeAssertionShadowingElseQ, node)
  64  		if !ok {
  65  			return
  66  		}
  67  		shadow := pass.TypesInfo.ObjectOf(m.State["obj"].(*ast.Ident))
  68  		shadowed := m.State["assert"].(*ast.TypeAssertExpr).X
  69  
  70  		path, exact := astutil.PathEnclosingInterval(code.File(pass, shadow), shadow.Pos(), shadow.Pos())
  71  		if !exact {
  72  			// TODO(dh): when can this happen?
  73  			return
  74  		}
  75  		irfn := ir.EnclosingFunction(irpkg, path)
  76  		if irfn == nil {
  77  			// For example for functions named "_", because we don't generate IR for them.
  78  			return
  79  		}
  80  
  81  		shadoweeIR, isAddr := irfn.ValueForExpr(m.State["obj"].(*ast.Ident))
  82  		if shadoweeIR == nil || isAddr {
  83  			// TODO(dh): is this possible?
  84  			return
  85  		}
  86  
  87  		var branch ast.Node
  88  		switch br := m.State["elseBranch"].(type) {
  89  		case ast.Node:
  90  			branch = br
  91  		case []ast.Stmt:
  92  			branch = &ast.BlockStmt{List: br}
  93  		case nil:
  94  			return
  95  		default:
  96  			panic(fmt.Sprintf("unexpected type %T", br))
  97  		}
  98  
  99  		ast.Inspect(branch, func(node ast.Node) bool {
 100  			ident, ok := node.(*ast.Ident)
 101  			if !ok {
 102  				return true
 103  			}
 104  			if pass.TypesInfo.ObjectOf(ident) != shadow {
 105  				return true
 106  			}
 107  
 108  			v, isAddr := irfn.ValueForExpr(ident)
 109  			if v == nil || isAddr {
 110  				return true
 111  			}
 112  			if irutil.Flatten(v) != shadoweeIR {
 113  				// Same types.Object, but different IR value. This
 114  				// either means that the variable has been
 115  				// assigned to since the type assertion, or that
 116  				// the variable has escaped to the heap. Either
 117  				// way, we shouldn't flag reads of it.
 118  				return true
 119  			}
 120  
 121  			report.Report(pass, ident,
 122  				fmt.Sprintf("%s refers to the result of a failed type assertion and is a zero value, not the value that was being type-asserted", report.Render(pass, ident)),
 123  				report.Related(shadow, "this is the variable being read"),
 124  				report.Related(shadowed, "this is the variable being shadowed"))
 125  			return true
 126  		})
 127  	}
 128  	code.Preorder(pass, fn, (*ast.IfStmt)(nil))
 129  	return nil, nil
 130  }
 131