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: minmax modernization introduces errors #71111

Open
pjweinb opened this issue Jan 3, 2025 · 3 comments
Open

x/tools/gopls: minmax modernization introduces errors #71111

pjweinb opened this issue Jan 3, 2025 · 3 comments
Assignees
Labels
Bug Issues describing a bug in the Go implementation. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@pjweinb
Copy link

pjweinb commented Jan 3, 2025

The original code is
// Don't refresh more than twice per minute.
delay := 30 * time.Second
// Don't spend more than ~2% of the time refreshing.
if adaptive := 50 * t.duration; adaptive > delay {
delay = adaptive
}
and it says 'if statement can be modernized using max', pointing to 'adaptive > delay'. For the above code it substitutes
// Don't refresh more than twice per minute.
delay := max(adaptive, delay)
which is syntactically (neither delay not adaptive have been defined) and semantically wrong.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 3, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 3, 2025
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.18.0 Jan 3, 2025
@findleyr
Copy link
Member

findleyr commented Jan 3, 2025

Thanks for reporting.

CC @adonovan

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/640038 mentions this issue: internal/analysis/modernize: minmax: don't reduce to y:=min(x, y)

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/640037 mentions this issue: internal/analysis/modernize: minmax: reject IfStmt with Init

gopherbot pushed a commit to golang/tools that referenced this issue Jan 3, 2025
Updates golang/go#71111

Change-Id: I86cdf1731e6057c4c972b91c46e8d3216a18382d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/640037
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Commit-Queue: Alan Donovan <[email protected]>
@gabyhelp gabyhelp added the Bug Issues describing a bug in the Go implementation. label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues describing a bug in the Go implementation. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants