gopls/internal/analysis/modernize: fix bug in minmax

Fix another bug related to not checking AssignStmt.Op.

+ test

Updates golang/go#70815

Change-Id: I426d27b4879d30f9fecb4d0f131b3c1a0b07773b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/642078
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

gopls/internal/analysis/modernize/maps.go

@@ -177,20 +177,19 @@ func mapsloop(pass *analysis.Pass) {
 		for curRange := range curFile.Preorder((*ast.RangeStmt)(nil)) {
 			rng := curRange.Node().(*ast.RangeStmt)
 
-			if rng.Tok == token.DEFINE && rng.Key != nil && rng.Value != nil && len(rng.Body.List) == 1 {
-				// Have: for k, v := range x { S }
-				if assign, ok := rng.Body.List[0].(*ast.AssignStmt); ok &&
-					assign.Tok == token.ASSIGN &&
-					len(assign.Lhs) == 1 {
-					// Have: for k, v := range x { lhs = rhs }
-
-					if index, ok := assign.Lhs[0].(*ast.IndexExpr); ok &&
-						equalSyntax(rng.Key, index.Index) &&
-						equalSyntax(rng.Value, assign.Rhs[0]) {
-
-						// Have: for k, v := range x { m[k] = v }
-						check(file, curRange, assign, index.X, rng.X)
-					}
+			if rng.Tok == token.DEFINE &&
+				rng.Key != nil &&
+				rng.Value != nil &&
+				isAssignBlock(rng.Body) {
+				// Have: for k, v := range x { lhs = rhs }
+
+				assign := rng.Body.List[0].(*ast.AssignStmt)
+				if index, ok := assign.Lhs[0].(*ast.IndexExpr); ok &&
+					equalSyntax(rng.Key, index.Index) &&
+					equalSyntax(rng.Value, assign.Rhs[0]) {
+
+					// Have: for k, v := range x { m[k] = v }
+					check(file, curRange, assign, index.X, rng.X)
 				}
 			}
 		}

gopls/internal/analysis/modernize/minmax.go

@@ -95,7 +95,7 @@ func minmax(pass *analysis.Pass) {
 				})
 			}
 
-		} else if prev, ok := curIfStmt.PrevSibling(); ok && is[*ast.AssignStmt](prev.Node()) {
+		} else if prev, ok := curIfStmt.PrevSibling(); ok && isSimpleAssign(prev.Node()) {
 			fassign := prev.Node().(*ast.AssignStmt)
 
 			// Have: lhs0 = rhs0; if a < b { lhs = rhs }
@@ -193,8 +193,17 @@ func isAssignBlock(b *ast.BlockStmt) bool {
 	if len(b.List) != 1 {
 		return false
 	}
-	assign, ok := b.List[0].(*ast.AssignStmt)
-	return ok && assign.Tok == token.ASSIGN && len(assign.Lhs) == 1 && len(assign.Rhs) == 1
+	// Inv: the sole statement cannot be { lhs := rhs }.
+	return isSimpleAssign(b.List[0])
+}
+
+// isSimpleAssign reports whether n has the form "lhs = rhs" or "lhs := rhs".
+func isSimpleAssign(n ast.Node) bool {
+	assign, ok := n.(*ast.AssignStmt)
+	return ok &&
+		(assign.Tok == token.ASSIGN || assign.Tok == token.DEFINE) &&
+		len(assign.Lhs) == 1 &&
+		len(assign.Rhs) == 1
 }
 
 // -- utils --

gopls/internal/analysis/modernize/testdata/src/minmax/minmax.go

@@ -81,3 +81,14 @@ func oops() {
 	}
 	print(y)
 }
+
+// Regression test for a bug: += is not a simple assignment.
+func nopeAssignHasIncrementOperator() {
+	x := 1
+	y := 0
+	y += 2
+	if x > y {
+		y = x
+	}
+	print(y)
+}

gopls/internal/analysis/modernize/testdata/src/minmax/minmax.go.golden

@@ -58,3 +58,14 @@ func oops() {
 	y := max(x, 2)
 	print(y)
 }
+
+// Regression test for a bug: += is not a simple assignment.
+func nopeAssignHasIncrementOperator() {
+	x := 1
+	y := 0
+	y += 2
+	if x > y {
+		y = x
+	}
+	print(y)
+}