go/analysis/passes/modernize: slicescontains: omit fix with empty body

In the slicescontains modernizer, if the inspected RangeStmt's
child IfStmt contains an unlabeled break statement, the fix will
remove it. If the IfStmt contains only the break and no other
statements, the resulting fix will have a call to
"if slices.Contains(...) {}" with an empty body.

This code is confusing and produces a linter error,
so we should avoid suggesting a fix.

Fixes golang/go#77677

Change-Id: I7c0368678b9034aa698febca2acf20c0071e29c9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/749160
TryBot-Bypass: Madeline Kalil <mkalil@google.com>
Auto-Submit: Madeline Kalil <mkalil@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>

Author: madelinekalil

go/analysis/passes/modernize/slicescontains.go

@@ -231,7 +231,9 @@ func slicescontains(pass *analysis.Pass) (any, error) {
 		// that might affected by melting down the loop.
 		//
 		// TODO(adonovan): relax check by analyzing branch target.
+		numBodyStmts := 0
 		for curBodyStmt := range curBody.Children() {
+			numBodyStmts += 1
 			if curBodyStmt != curLastStmt {
 				for range curBodyStmt.Preorder((*ast.BranchStmt)(nil), (*ast.ReturnStmt)(nil)) {
 					return
@@ -292,7 +294,16 @@ func slicescontains(pass *analysis.Pass) (any, error) {
 		case *ast.BranchStmt:
 			if lastStmt.Tok == token.BREAK && lastStmt.Label == nil { // unlabeled break
 				// Have: for ... { if ... { stmts; break } }
-
+				if numBodyStmts == 1 {
+					// If the only stmt in the body is an unlabeled "break" that
+					// will get deleted in the fix, don't suggest a fix, as it
+					// produces confusing code:
+					//    if slices.Contains(slice, f) {}
+					// Explicitly discarding the result isn't much better:
+					//    _ = slices.Contains(slice, f) // just for effects
+					// See https://go.dev/issue/77677.
+					return
+				}
 				var prevStmt ast.Stmt // previous statement to range (if any)
 				if curPrev, ok := curRange.PrevSibling(); ok {
 					// If the RangeStmt's previous sibling is a Stmt,

go/analysis/passes/modernize/testdata/src/slicescontains/slicescontains.go

@@ -224,3 +224,11 @@ func issue76210negation(haystack []string, needle string) bool {
 	}
 	return res
 }
+
+func issue77677emptybody(slice []int, needle int) {
+	for _, elem := range slice { // nope: would produce "if slices.Contains" with an empty body
+		if elem == needle {
+			break
+		}
+	}
+}

go/analysis/passes/modernize/testdata/src/slicescontains/slicescontains.go.golden

@@ -166,3 +166,11 @@ func issue76210negation(haystack []string, needle string) bool {
 	}
 	return res
 }
+
+func issue77677emptybody(slice []int, needle int) {
+	for _, elem := range slice { // nope: would produce "if slices.Contains" with an empty body
+		if elem == needle {
+			break
+		}
+	}
+}