From 70b12541d3fecd5bcb3e66a8234612737629313b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Fri, 16 Nov 2018 16:13:26 +0000 Subject: [PATCH] go/analysis: unindent some pieces of code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All of these were quite heavily indented for no good reason; breaking or returning early makes the code easier to read and follow. Change-Id: Ic539517b07604d71495277b16f1b7eb60d2e3d3c Reviewed-on: https://go-review.googlesource.com/c/149978 Run-TryBot: Daniel Martí TryBot-Result: Gobot Gobot Reviewed-by: Alan Donovan --- go/analysis/passes/lostcancel/lostcancel.go | 109 ++++++++++---------- go/analysis/passes/unsafeptr/unsafeptr.go | 44 ++++---- 2 files changed, 79 insertions(+), 74 deletions(-) diff --git a/go/analysis/passes/lostcancel/lostcancel.go b/go/analysis/passes/lostcancel/lostcancel.go index 996ecc4d..b5161836 100644 --- a/go/analysis/passes/lostcancel/lostcancel.go +++ b/go/analysis/passes/lostcancel/lostcancel.go @@ -93,32 +93,32 @@ func runFunc(pass *analysis.Pass, node ast.Node) { // ctx, cancel = context.WithCancel(...) // var ctx, cancel = context.WithCancel(...) // - if isContextWithCancel(pass.TypesInfo, n) && isCall(stack[len(stack)-2]) { - var id *ast.Ident // id of cancel var - stmt := stack[len(stack)-3] - switch stmt := stmt.(type) { - case *ast.ValueSpec: - if len(stmt.Names) > 1 { - id = stmt.Names[1] - } - case *ast.AssignStmt: - if len(stmt.Lhs) > 1 { - id, _ = stmt.Lhs[1].(*ast.Ident) - } + if !isContextWithCancel(pass.TypesInfo, n) || !isCall(stack[len(stack)-2]) { + return true + } + var id *ast.Ident // id of cancel var + stmt := stack[len(stack)-3] + switch stmt := stmt.(type) { + case *ast.ValueSpec: + if len(stmt.Names) > 1 { + id = stmt.Names[1] } - if id != nil { - if id.Name == "_" { - pass.Reportf(id.Pos(), - "the cancel function returned by context.%s should be called, not discarded, to avoid a context leak", - n.(*ast.SelectorExpr).Sel.Name) - } else if v, ok := pass.TypesInfo.Uses[id].(*types.Var); ok { - cancelvars[v] = stmt - } else if v, ok := pass.TypesInfo.Defs[id].(*types.Var); ok { - cancelvars[v] = stmt - } + case *ast.AssignStmt: + if len(stmt.Lhs) > 1 { + id, _ = stmt.Lhs[1].(*ast.Ident) + } + } + if id != nil { + if id.Name == "_" { + pass.Reportf(id.Pos(), + "the cancel function returned by context.%s should be called, not discarded, to avoid a context leak", + n.(*ast.SelectorExpr).Sel.Name) + } else if v, ok := pass.TypesInfo.Uses[id].(*types.Var); ok { + cancelvars[v] = stmt + } else if v, ok := pass.TypesInfo.Defs[id].(*types.Var); ok { + cancelvars[v] = stmt } } - return true }) @@ -179,18 +179,22 @@ func hasImport(pkg *types.Package, path string) bool { // isContextWithCancel reports whether n is one of the qualified identifiers // context.With{Cancel,Timeout,Deadline}. func isContextWithCancel(info *types.Info, n ast.Node) bool { - if sel, ok := n.(*ast.SelectorExpr); ok { - switch sel.Sel.Name { - case "WithCancel", "WithTimeout", "WithDeadline": - if x, ok := sel.X.(*ast.Ident); ok { - if pkgname, ok := info.Uses[x].(*types.PkgName); ok { - return pkgname.Imported().Path() == contextPackage - } - // Import failed, so we can't check package path. - // Just check the local package name (heuristic). - return x.Name == "context" - } + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return false + } + switch sel.Sel.Name { + case "WithCancel", "WithTimeout", "WithDeadline": + default: + return false + } + if x, ok := sel.X.(*ast.Ident); ok { + if pkgname, ok := info.Uses[x].(*types.PkgName); ok { + return pkgname.Imported().Path() == contextPackage } + // Import failed, so we can't check package path. + // Just check the local package name (heuristic). + return x.Name == "context" } return false } @@ -270,29 +274,30 @@ outer: var search func(blocks []*cfg.Block) *ast.ReturnStmt search = func(blocks []*cfg.Block) *ast.ReturnStmt { for _, b := range blocks { - if !seen[b] { - seen[b] = true + if seen[b] { + continue + } + seen[b] = true - // Prune the search if the block uses v. - if blockUses(pass, v, b) { - continue - } + // Prune the search if the block uses v. + if blockUses(pass, v, b) { + continue + } - // Found path to return statement? - if ret := b.Return(); ret != nil { - if debug { - fmt.Printf("found path to return in block %s\n", b) - } - return ret // found + // Found path to return statement? + if ret := b.Return(); ret != nil { + if debug { + fmt.Printf("found path to return in block %s\n", b) } + return ret // found + } - // Recur - if ret := search(b.Succs); ret != nil { - if debug { - fmt.Printf(" from block %s\n", b) - } - return ret + // Recur + if ret := search(b.Succs); ret != nil { + if debug { + fmt.Printf(" from block %s\n", b) } + return ret } } return nil diff --git a/go/analysis/passes/unsafeptr/unsafeptr.go b/go/analysis/passes/unsafeptr/unsafeptr.go index 116d622b..308bfc69 100644 --- a/go/analysis/passes/unsafeptr/unsafeptr.go +++ b/go/analysis/passes/unsafeptr/unsafeptr.go @@ -62,28 +62,28 @@ func isSafeUintptr(info *types.Info, x ast.Expr) bool { return isSafeUintptr(info, x.X) case *ast.SelectorExpr: - switch x.Sel.Name { - case "Data": - // reflect.SliceHeader and reflect.StringHeader are okay, - // but only if they are pointing at a real slice or string. - // It's not okay to do: - // var x SliceHeader - // x.Data = uintptr(unsafe.Pointer(...)) - // ... use x ... - // p := unsafe.Pointer(x.Data) - // because in the middle the garbage collector doesn't - // see x.Data as a pointer and so x.Data may be dangling - // by the time we get to the conversion at the end. - // For now approximate by saying that *Header is okay - // but Header is not. - pt, ok := info.Types[x.X].Type.(*types.Pointer) - if ok { - t, ok := pt.Elem().(*types.Named) - if ok && t.Obj().Pkg().Path() == "reflect" { - switch t.Obj().Name() { - case "StringHeader", "SliceHeader": - return true - } + if x.Sel.Name != "Data" { + break + } + // reflect.SliceHeader and reflect.StringHeader are okay, + // but only if they are pointing at a real slice or string. + // It's not okay to do: + // var x SliceHeader + // x.Data = uintptr(unsafe.Pointer(...)) + // ... use x ... + // p := unsafe.Pointer(x.Data) + // because in the middle the garbage collector doesn't + // see x.Data as a pointer and so x.Data may be dangling + // by the time we get to the conversion at the end. + // For now approximate by saying that *Header is okay + // but Header is not. + pt, ok := info.Types[x.X].Type.(*types.Pointer) + if ok { + t, ok := pt.Elem().(*types.Named) + if ok && t.Obj().Pkg().Path() == "reflect" { + switch t.Obj().Name() { + case "StringHeader", "SliceHeader": + return true } } }