cmd/vet: lostcancel: treat naked return as a use of named results

+ test.

Fixes #16230

Change-Id: Idac995437146a9df9e73f094d2a31abc25b1fa62
Reviewed-on: https://go-review.googlesource.com/24681
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

2 files changed, 38 insertions, 4 deletions

diff --git a/src/cmd/vet/lostcancel.go b/src/cmd/vet/lostcancel.go
index 11c3c47783..d049a3e888 100644
--- a/src/cmd/vet/lostcancel.go
+++ b/src/cmd/vet/lostcancel.go
@@ -101,10 +101,13 @@ func checkLostCancel(f *File, node ast.Node) {
 
 	// Build the CFG.
 	var g *cfg.CFG
+	var sig *types.Signature
 	switch node := node.(type) {
 	case *ast.FuncDecl:
+		sig, _ = f.pkg.defs[node.Name].Type().(*types.Signature)
 		g = cfg.New(node.Body, mayReturn)
 	case *ast.FuncLit:
+		sig, _ = f.pkg.types[node.Type].Type.(*types.Signature)
 		g = cfg.New(node.Body, mayReturn)
 	}
 
@@ -117,7 +120,7 @@ func checkLostCancel(f *File, node ast.Node) {
 	// (It would be more efficient to analyze all cancelvars in a
 	// single pass over the AST, but seldom is there more than one.)
 	for v, stmt := range cancelvars {
-		if ret := lostCancelPath(f, g, v, stmt); ret != nil {
+		if ret := lostCancelPath(f, g, v, stmt, sig); ret != nil {
 			lineno := f.fset.Position(stmt.Pos()).Line
 			f.Badf(stmt.Pos(), "the %s function is not used on all paths (possible context leak)", v.Name())
 			f.Badf(ret.Pos(), "this return statement may be reached without using the %s var defined on line %d", v.Name(), lineno)
@@ -159,14 +162,24 @@ func isContextWithCancel(f *File, n ast.Node) bool {
 // lostCancelPath finds a path through the CFG, from stmt (which defines
 // the 'cancel' variable v) to a return statement, that doesn't "use" v.
 // If it finds one, it returns the return statement (which may be synthetic).
-func lostCancelPath(f *File, g *cfg.CFG, v *types.Var, stmt ast.Node) *ast.ReturnStmt {
+// sig is the function's type, if known.
+func lostCancelPath(f *File, g *cfg.CFG, v *types.Var, stmt ast.Node, sig *types.Signature) *ast.ReturnStmt {
+	vIsNamedResult := sig != nil && tupleContains(sig.Results(), v)
+
 	// uses reports whether stmts contain a "use" of variable v.
 	uses := func(f *File, v *types.Var, stmts []ast.Node) bool {
 		found := false
 		for _, stmt := range stmts {
 			ast.Inspect(stmt, func(n ast.Node) bool {
-				if id, ok := n.(*ast.Ident); ok {
-					if f.pkg.uses[id] == v {
+				switch n := n.(type) {
+				case *ast.Ident:
+					if f.pkg.uses[n] == v {
+						found = true
+					}
+				case *ast.ReturnStmt:
+					// A naked return statement counts as a use
+					// of the named result variables.
+					if n.Results == nil && vIsNamedResult {
 						found = true
 					}
 				}
@@ -251,6 +264,15 @@ outer:
 	return search(defblock.Succs)
 }
 
+func tupleContains(tuple *types.Tuple, v *types.Var) bool {
+	for i := 0; i < tuple.Len(); i++ {
+		if tuple.At(i) == v {
+			return true
+		}
+	}
+	return false
+}
+
 var noReturnFuncs = map[string]bool{
 	"(*testing.common).FailNow": true,
 	"(*testing.common).Fatal":   true,
diff --git a/src/cmd/vet/testdata/lostcancel.go b/src/cmd/vet/testdata/lostcancel.go
index 213dd1832d..b7549c0051 100644
--- a/src/cmd/vet/testdata/lostcancel.go
+++ b/src/cmd/vet/testdata/lostcancel.go
@@ -141,3 +141,15 @@ func _() {
 	var x struct{ f func() }
 	x.f()
 }
+
+// Regression test for Go issue 16230.
+func _() (ctx context.Context, cancel func()) {
+	ctx, cancel = context.WithCancel()
+	return // a naked return counts as a load of the named result values
+}
+
+// Same as above, but for literal function.
+var _ = func() (ctx context.Context, cancel func()) {
+	ctx, cancel = context.WithCancel()
+	return
+}