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: mapsloop loses commentary #72958

Open
adonovan opened this issue Mar 19, 2025 · 1 comment
Open

x/tools/gopls/internal/analysis/modernize: mapsloop loses commentary #72958

adonovan opened this issue Mar 19, 2025 · 1 comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) BugReport Issues describing a possible bug in the Go implementation. gopls/imports NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Refactoring Issues related to refactoring tools
Milestone

Comments

@adonovan
Copy link
Member

The mapsloop transformation discards comments:

diff --git a/src/go/doc/reader.go b/src/go/doc/reader.go
index e84d6d9a71..6e8c682087 100644
--- a/src/go/doc/reader.go
+++ b/src/go/doc/reader.go
-                       for name, f := range t.funcs {
-                               // in a correct AST, package-level function names
-                               // are all different - no need to check for conflicts
-                               r.funcs[name] = f
-                       }
+                       maps.Copy(r.funcs, t.funcs)

Until recently, minmax used to do the same (#72727). We should apply a similar fix, here and anywhere else that discards a block.

@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Mar 19, 2025
@adonovan adonovan added Analysis Issues related to static analysis (vet, x/tools/go/analysis) Refactoring Issues related to refactoring tools BugReport Issues describing a possible bug in the Go implementation. and removed gopls Issues related to the Go language server, gopls. labels Mar 19, 2025
@seankhliao seankhliao changed the title gopls/internal/analysis/modernize: mapsloop loses commentary x/tools/gopls/internal/analysis/modernize: mapsloop loses commentary Mar 19, 2025
@gopherbot gopherbot added this to the Unreleased milestone Mar 19, 2025
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 20, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/660255 mentions this issue: gopls/internal/analysis/modernize: preserves comments in mapsloop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) BugReport Issues describing a possible bug in the Go implementation. gopls/imports NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Refactoring Issues related to refactoring tools
Projects
None yet
Development

No branches or pull requests

3 participants