sa4023.go raw

   1  package sa4023
   2  
   3  import (
   4  	"fmt"
   5  	"go/token"
   6  	"go/types"
   7  
   8  	"honnef.co/go/tools/analysis/code"
   9  	"honnef.co/go/tools/analysis/facts/nilness"
  10  	"honnef.co/go/tools/analysis/facts/typedness"
  11  	"honnef.co/go/tools/analysis/lint"
  12  	"honnef.co/go/tools/analysis/report"
  13  	"honnef.co/go/tools/go/ir"
  14  	"honnef.co/go/tools/go/ir/irutil"
  15  	"honnef.co/go/tools/go/types/typeutil"
  16  	"honnef.co/go/tools/internal/passes/buildir"
  17  
  18  	"golang.org/x/exp/typeparams"
  19  	"golang.org/x/tools/go/analysis"
  20  )
  21  
  22  var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
  23  	Analyzer: &analysis.Analyzer{
  24  		Name:     "SA4023",
  25  		Run:      run,
  26  		Requires: []*analysis.Analyzer{buildir.Analyzer, typedness.Analysis, nilness.Analysis},
  27  	},
  28  	Doc: &lint.RawDocumentation{
  29  		Title: `Impossible comparison of interface value with untyped nil`,
  30  		Text: `Under the covers, interfaces are implemented as two elements, a
  31  type T and a value V. V is a concrete value such as an int,
  32  struct or pointer, never an interface itself, and has type T. For
  33  instance, if we store the int value 3 in an interface, the
  34  resulting interface value has, schematically, (T=int, V=3). The
  35  value V is also known as the interface's dynamic value, since a
  36  given interface variable might hold different values V (and
  37  corresponding types T) during the execution of the program.
  38  
  39  An interface value is nil only if the V and T are both
  40  unset, (T=nil, V is not set), In particular, a nil interface will
  41  always hold a nil type. If we store a nil pointer of type *int
  42  inside an interface value, the inner type will be *int regardless
  43  of the value of the pointer: (T=*int, V=nil). Such an interface
  44  value will therefore be non-nil even when the pointer value V
  45  inside is nil.
  46  
  47  This situation can be confusing, and arises when a nil value is
  48  stored inside an interface value such as an error return:
  49  
  50      func returnsError() error {
  51          var p *MyError = nil
  52          if bad() {
  53              p = ErrBad
  54          }
  55          return p // Will always return a non-nil error.
  56      }
  57  
  58  If all goes well, the function returns a nil p, so the return
  59  value is an error interface value holding (T=*MyError, V=nil).
  60  This means that if the caller compares the returned error to nil,
  61  it will always look as if there was an error even if nothing bad
  62  happened. To return a proper nil error to the caller, the
  63  function must return an explicit nil:
  64  
  65      func returnsError() error {
  66          if bad() {
  67              return ErrBad
  68          }
  69          return nil
  70      }
  71  
  72  It's a good idea for functions that return errors always to use
  73  the error type in their signature (as we did above) rather than a
  74  concrete type such as \'*MyError\', to help guarantee the error is
  75  created correctly. As an example, \'os.Open\' returns an error even
  76  though, if not nil, it's always of concrete type *os.PathError.
  77  
  78  Similar situations to those described here can arise whenever
  79  interfaces are used. Just keep in mind that if any concrete value
  80  has been stored in the interface, the interface will not be nil.
  81  For more information, see The Laws of
  82  Reflection at https://golang.org/doc/articles/laws_of_reflection.html.
  83  
  84  This text has been copied from
  85  https://golang.org/doc/faq#nil_error, licensed under the Creative
  86  Commons Attribution 3.0 License.`,
  87  		Since:    "2020.2",
  88  		Severity: lint.SeverityWarning,
  89  		MergeIf:  lint.MergeIfAny, // TODO should this be MergeIfAll?
  90  	},
  91  })
  92  
  93  var Analyzer = SCAnalyzer.Analyzer
  94  
  95  func run(pass *analysis.Pass) (interface{}, error) {
  96  	// The comparison 'fn() == nil' can never be true if fn() returns
  97  	// an interface value and only returns typed nils. This is usually
  98  	// a mistake in the function itself, but all we can say for
  99  	// certain is that the comparison is pointless.
 100  	//
 101  	// Flag results if no untyped nils are being returned, but either
 102  	// known typed nils, or typed unknown nilness are being returned.
 103  
 104  	irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR)
 105  	typedness := pass.ResultOf[typedness.Analysis].(*typedness.Result)
 106  	nilness := pass.ResultOf[nilness.Analysis].(*nilness.Result)
 107  	for _, fn := range irpkg.SrcFuncs {
 108  		for _, b := range fn.Blocks {
 109  			for _, instr := range b.Instrs {
 110  				binop, ok := instr.(*ir.BinOp)
 111  				if !ok || !(binop.Op == token.EQL || binop.Op == token.NEQ) {
 112  					continue
 113  				}
 114  				if _, ok := binop.X.Type().Underlying().(*types.Interface); !ok || typeparams.IsTypeParam(binop.X.Type()) {
 115  					// TODO support swapped X and Y
 116  					continue
 117  				}
 118  
 119  				k, ok := binop.Y.(*ir.Const)
 120  				if !ok || !k.IsNil() {
 121  					// if binop.X is an interface, then binop.Y can
 122  					// only be a Const if its untyped. A typed nil
 123  					// constant would first be passed to
 124  					// MakeInterface.
 125  					continue
 126  				}
 127  
 128  				var idx int
 129  				var obj *types.Func
 130  				switch x := irutil.Flatten(binop.X).(type) {
 131  				case *ir.Call:
 132  					callee := x.Call.StaticCallee()
 133  					if callee == nil {
 134  						continue
 135  					}
 136  					obj, _ = callee.Object().(*types.Func)
 137  					idx = 0
 138  				case *ir.Extract:
 139  					call, ok := irutil.Flatten(x.Tuple).(*ir.Call)
 140  					if !ok {
 141  						continue
 142  					}
 143  					callee := call.Call.StaticCallee()
 144  					if callee == nil {
 145  						continue
 146  					}
 147  					obj, _ = callee.Object().(*types.Func)
 148  					idx = x.Index
 149  				case *ir.MakeInterface:
 150  					var qualifier string
 151  					switch binop.Op {
 152  					case token.EQL:
 153  						qualifier = "never"
 154  					case token.NEQ:
 155  						qualifier = "always"
 156  					default:
 157  						panic("unreachable")
 158  					}
 159  
 160  					terms, err := typeparams.NormalTerms(x.X.Type())
 161  					if len(terms) == 0 || err != nil {
 162  						// Type is a type parameter with no type terms (or we couldn't determine the terms). Such a type
 163  						// _can_ be nil when put in an interface value.
 164  						continue
 165  					}
 166  
 167  					if report.HasRange(x.X) {
 168  						report.Report(pass, binop, fmt.Sprintf("this comparison is %s true", qualifier),
 169  							report.Related(x.X, "the lhs of the comparison gets its value from here and has a concrete type"))
 170  					} else {
 171  						// we can't generate related information for this, so make the diagnostic itself slightly more useful
 172  						report.Report(pass, binop, fmt.Sprintf("this comparison is %s true; the lhs of the comparison has been assigned a concretely typed value", qualifier))
 173  					}
 174  					continue
 175  				}
 176  				if obj == nil {
 177  					continue
 178  				}
 179  
 180  				isNil, onlyGlobal := nilness.MayReturnNil(obj, idx)
 181  				if typedness.MustReturnTyped(obj, idx) && isNil && !onlyGlobal && !code.IsInTest(pass, binop) {
 182  					// Don't flag these comparisons in tests. Tests
 183  					// may be explicitly enforcing the invariant that
 184  					// a value isn't nil.
 185  
 186  					var qualifier string
 187  					switch binop.Op {
 188  					case token.EQL:
 189  						qualifier = "never"
 190  					case token.NEQ:
 191  						qualifier = "always"
 192  					default:
 193  						panic("unreachable")
 194  					}
 195  					report.Report(pass, binop, fmt.Sprintf("this comparison is %s true", qualifier),
 196  						// TODO support swapped X and Y
 197  						report.Related(binop.X, fmt.Sprintf("the lhs of the comparison is the %s return value of this function call", report.Ordinal(idx+1))),
 198  						report.Related(obj, fmt.Sprintf("%s never returns a nil interface value", typeutil.FuncName(obj))))
 199  				}
 200  			}
 201  		}
 202  	}
 203  
 204  	return nil, nil
 205  }
 206