src

Go monorepo.
git clone git://code.dwrz.net/src
Log | Files | Refs

sa9008.go (3817B)

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