fix: complex IfStmt false negatives (#94)

* Add nested BinaryExpr to httpstatus IfStmt tests

Signed-off-by: Spencer Schrock <sschrock@google.com>

* Check ast.BinaryExpr for HTTP Method and StatusCode

This represents a more general case of IfStmt, ForStmt and SwitchStmt

Signed-off-by: Spencer Schrock <sschrock@google.com>

* update httpstatus template for valid IfStmt uses too

Signed-off-by: Spencer Schrock <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>

pkg/analyzer/analyzer.go

@@ -63,9 +63,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		(*ast.CallExpr)(nil),
 		(*ast.BasicLit)(nil),
 		(*ast.CompositeLit)(nil),
-		(*ast.IfStmt)(nil),
+		(*ast.BinaryExpr)(nil),
 		(*ast.SwitchStmt)(nil),
-		(*ast.ForStmt)(nil),
 	}
 
 	insp.Preorder(types, func(node ast.Node) {
@@ -116,54 +115,30 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 			typeElts(pass, x, typ, n.Elts)
 
-		case *ast.IfStmt:
-			cond, ok := n.Cond.(*ast.BinaryExpr)
-			if !ok {
-				return
-			}
-
-			switch cond.Op {
+		case *ast.BinaryExpr:
+			switch n.Op {
 			case token.LSS, token.GTR, token.LEQ, token.GEQ:
 				return
 			}
 
-			x, ok := cond.X.(*ast.SelectorExpr)
+			x, ok := n.X.(*ast.SelectorExpr)
 			if !ok {
 				return
 			}
 
-			y, ok := cond.Y.(*ast.BasicLit)
+			y, ok := n.Y.(*ast.BasicLit)
 			if !ok {
 				return
 			}
 
-			ifElseStmt(pass, x, y)
+			binaryExpr(pass, x, y)
 
 		case *ast.SwitchStmt:
 			x, ok := n.Tag.(*ast.SelectorExpr)
 			if ok {
 				switchStmt(pass, x, n.Body.List)
-			} else {
-				switchStmtAsIfElseStmt(pass, n.Body.List)
-			}
-
-		case *ast.ForStmt:
-			cond, ok := n.Cond.(*ast.BinaryExpr)
-			if !ok {
-				return
-			}
-
-			x, ok := cond.X.(*ast.SelectorExpr)
-			if !ok {
-				return
-			}
-
-			y, ok := cond.Y.(*ast.BasicLit)
-			if !ok {
-				return
 			}
 
-			ifElseStmt(pass, x, y)
 		}
 	})
 
@@ -318,8 +293,11 @@ func typeElts(pass *analysis.Pass, x *ast.Ident, typ *ast.SelectorExpr, elts []a
 	}
 }
 
-// ifElseStmt checks X and Y in if-else-statement.
-func ifElseStmt(pass *analysis.Pass, x *ast.SelectorExpr, y *ast.BasicLit) {
+// binaryExpr checks X and Y in binary expressions, including:
+//   - if-else-statement
+//   - for loops conditions
+//   - switch cases without a tag expression
+func binaryExpr(pass *analysis.Pass, x *ast.SelectorExpr, y *ast.BasicLit) {
 	switch x.Sel.Name {
 	case "StatusCode":
 		if !lookupFlag(pass, HTTPStatusCodeFlag) {
@@ -376,34 +354,6 @@ func switchStmt(pass *analysis.Pass, x *ast.SelectorExpr, cases []ast.Stmt) {
 	}
 }
 
-func switchStmtAsIfElseStmt(pass *analysis.Pass, cases []ast.Stmt) {
-	for _, c := range cases {
-		caseClause, ok := c.(*ast.CaseClause)
-		if !ok {
-			continue
-		}
-
-		for _, expr := range caseClause.List {
-			binaryExpr, ok := expr.(*ast.BinaryExpr)
-			if !ok {
-				continue
-			}
-
-			x, ok := binaryExpr.X.(*ast.SelectorExpr)
-			if !ok {
-				continue
-			}
-
-			y, ok := binaryExpr.Y.(*ast.BasicLit)
-			if !ok {
-				continue
-			}
-
-			ifElseStmt(pass, x, y)
-		}
-	}
-}
-
 func lookupFlag(pass *analysis.Pass, name string) bool {
 	return pass.Analyzer.Flags.Lookup(name).Value.(flag.Getter).Get().(bool)
 }

pkg/analyzer/internal/template/test-httpstatus.go.tmpl

@@ -68,6 +68,8 @@ func _() error {
         return nil
     } else if resp.StatusCode == {{ $key }} { // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}`
         return nil
+    } else if false || resp.StatusCode == {{ $key }} { // want `"{{ quoteMeta $key }}" can be replaced by {{ quoteMeta $value }}`
+        return nil
     }
 {{- end }}
 {{- range $key, $value := .Mapping }}
@@ -89,6 +91,8 @@ func _() error {
         return nil
     } else if resp.StatusCode == {{ $value }} {
         return nil
+    } else if false || resp.StatusCode == {{ $value }} {
+        return nil
     }
 {{- end }}
 {{- range $key, $value := .Mapping }}