go/analysis/passes/httpresponse: minor clarification
See discussion in https://go-review.googlesource.com/c/tools/+/405314.
Change-Id: Ic5f5182a1cc55b4a2c40f43ebb2250c508388c75
Reviewed-on: https://go-review.googlesource.com/c/tools/+/405537
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Nooras Saba <saba@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/go/analysis/passes/httpresponse/httpresponse.go b/go/analysis/passes/httpresponse/httpresponse.go
index 092ac75..3b9168c 100644
--- a/go/analysis/passes/httpresponse/httpresponse.go
+++ b/go/analysis/passes/httpresponse/httpresponse.go
@@ -62,15 +62,16 @@
// Find the innermost containing block, and get the list
// of statements starting with the one containing call.
- stmts, withinAnotherCall := restOfBlock(stack)
- if withinAnotherCall {
- // We skip cases when the results of a call to http member
- // are passed directly to another call, as that later call
- // could check err != nil and create false positives (#52661).
+ stmts, ncalls := restOfBlock(stack)
+ if len(stmts) < 2 {
+ // The call to the http function is the last statement of the block.
return true
}
- if len(stmts) < 2 {
- return true // the call to the http function is the last statement of the block.
+
+ // Skip cases in which the call is wrapped by another (#52661).
+ // Example: resp, err := checkError(http.Get(url))
+ if ncalls > 1 {
+ return true
}
asg, ok := stmts[0].(*ast.AssignStmt)
@@ -136,34 +137,26 @@
return ok && isNamedType(ptr.Elem(), "net/http", "Client") // method on *http.Client.
}
-// restOfBlock, given a traversal stack, checks if the current node
-// (the last element of stack) appears as an argument to another call.
-// If not, it finds the innermost containing block and returns the
-// suffix of its statements starting with the current node. Otherwise,
-// returns an empty slice.
-func restOfBlock(stack []ast.Node) ([]ast.Stmt, bool) {
+// restOfBlock, given a traversal stack, finds the innermost containing
+// block and returns the suffix of its statements starting with the current
+// node, along with the number of call expressions encountered.
+func restOfBlock(stack []ast.Node) ([]ast.Stmt, int) {
+ var ncalls int
for i := len(stack) - 1; i >= 0; i-- {
- // If the current node appears within another call, then
- // this has to happen within the same block. We can thus
- // immediately return on whichever we see first, a block
- // statement or a call statement.
-
if b, ok := stack[i].(*ast.BlockStmt); ok {
for j, v := range b.List {
if v == stack[i+1] {
- return b.List[j:], false
+ return b.List[j:], ncalls
}
}
break
}
- // The call to an http member currently analyzed is at len(stack)-1.
- if _, ok := stack[i].(*ast.CallExpr); ok && i != len(stack)-1 {
- return nil, true // e.g. "resp, err := wrap(http.Get(...))"
+ if _, ok := stack[i].(*ast.CallExpr); ok {
+ ncalls++
}
-
}
- return nil, false
+ return nil, 0
}
// rootIdent finds the root identifier x in a chain of selections x.y.z, or nil if not found.
