Skip to content

Commit 96bfb60

Browse files
adonovangopherbot
authored andcommitted
gopls/internal/analysis/modernize: fix minmax bug
The matcher for pattern 2 forgot to check that the IfStmt.Else subtree was nil, leading to unsound fixes. Updates golang/go#71847 Change-Id: I0919076c1af38012cedf3072ef5d1117e96a64b9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/651375 Reviewed-by: Jonathan Amsterdam <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 1f6c6d6 commit 96bfb60

File tree

3 files changed

+25
-1
lines changed

3 files changed

+25
-1
lines changed

gopls/internal/analysis/modernize/minmax.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func minmax(pass *analysis.Pass) {
9595
})
9696
}
9797

98-
} else if prev, ok := curIfStmt.PrevSibling(); ok && isSimpleAssign(prev.Node()) {
98+
} else if prev, ok := curIfStmt.PrevSibling(); ok && isSimpleAssign(prev.Node()) && ifStmt.Else == nil {
9999
fassign := prev.Node().(*ast.AssignStmt)
100100

101101
// Have: lhs0 = rhs0; if a < b { lhs = rhs }

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

+12
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,15 @@ func nopeNotAMinimum(x, y int) int {
103103
}
104104
return y
105105
}
106+
107+
// Regression test for https://github.com/golang/go/issues/71847#issuecomment-2673491596
108+
func nopeHasElseBlock(x int) int {
109+
y := x
110+
// Before, this was erroneously reduced to y = max(x, 0)
111+
if y < 0 {
112+
y = 0
113+
} else {
114+
y += 2
115+
}
116+
return y
117+
}

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

+12
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,15 @@ func nopeNotAMinimum(x, y int) int {
8080
}
8181
return y
8282
}
83+
84+
// Regression test for https://github.com/golang/go/issues/71847#issuecomment-2673491596
85+
func nopeHasElseBlock(x int) int {
86+
y := x
87+
// Before, this was erroneously reduced to y = max(x, 0)
88+
if y < 0 {
89+
y = 0
90+
} else {
91+
y += 2
92+
}
93+
return y
94+
}

0 commit comments

Comments
 (0)