Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls/internal/analysis/modernize: minmax generates incorrect transformation #71721

Closed
adonovan opened this issue Feb 13, 2025 · 4 comments
Assignees
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls/analysis Issues related to running analysis in gopls Refactoring Issues related to refactoring tools
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Feb 13, 2025

(Splitting out of #70815 (comment) on behalf of @spencerschrock.)

Currently at HEAD (44b61a1d174cc06329b20f5de60c2b0c800741a4):

main.go:7:5: if/else statement can be modernized using max

package main

import "fmt"

func main() {
	var x, y int
	if x <= 0 { // a value of -1 or 0 will use a default value (30)
		y = 30
	} else {
		y = x
	}
	fmt.Print(y)
}

Applying the suggested fix doesn't yield an equivalent result:

package main

import "fmt"

func main() {
	var x, y int
	y = max(x, 0)
	fmt.Print(y)
}
@adonovan adonovan self-assigned this Feb 13, 2025
@adonovan adonovan added BugReport Issues describing a possible bug in the Go implementation. gopls/analysis Issues related to running analysis in gopls labels Feb 13, 2025
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Feb 13, 2025
@adonovan adonovan added Refactoring Issues related to refactoring tools and removed gopls Issues related to the Go language server, gopls. BugReport Issues describing a possible bug in the Go implementation. labels Feb 13, 2025
@adonovan adonovan added this to the gopls/v0.18.0 milestone Feb 13, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Feb 13, 2025
@seankhliao seankhliao changed the title gopls/internal/analysis/modernize: minmax generates incorrect transformation x/tools/gopls/internal/analysis/modernize: minmax generates incorrect transformation Feb 13, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/649357 mentions this issue: gopls/internal/analysis/modernize: fix bug in minmax

@findleyr
Copy link
Member

This did not make it into v0.18.0, as it was not cherry-picked.

Moving to v0.18.1, and reopening to track the cherry-pick.

@findleyr findleyr reopened this Feb 20, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/651236 mentions this issue: [gopls-release-branch.0.18] gopls/internal/analysis/modernize: fix bug in minmax

gopherbot pushed a commit to golang/tools that referenced this issue Feb 21, 2025
…g in minmax

Wrong operator. D'oh.

+ test

Fixes golang/go#71721

Change-Id: Ia7fe314df07afa9a9de63c2b6031e678755e9d56
Reviewed-on: https://go-review.googlesource.com/c/tools/+/649357
Reviewed-by: Jonathan Amsterdam <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
(cherry picked from commit 809cde4)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/651236
Reviewed-by: Robert Findley <[email protected]>
@findleyr
Copy link
Member

Closing as the cherry pick has occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls/analysis Issues related to running analysis in gopls Refactoring Issues related to refactoring tools
Projects
None yet
Development

No branches or pull requests

4 participants