x/tools/go/analysis: -fix creates unnecessary import decls

[adonovan]

Here is a fairly typical diff hunk from an analyzer, as applied by the -fix flag. Observe that it creates a new import decl, even though there's an existing one that it could add to. The original patch was computed by analysisinternal.AddImport, but applying gofmt (as -fix does) didn't smooth the two declarations into one; you need goimports for that.

--- /Users/adonovan/w/xtools/playground/socket/socket.go (old)
+++ /Users/adonovan/w/xtools/playground/socket/socket.go (new)
@@ -14,7 +14,9 @@
 // This will not run on App Engine as WebSockets are not supported there.
 package socket // import "[golang.org/x/tools/playground/socket](http://golang.org/x/tools/playground/socket)"
 
-import (
+import "slices"
+
+import (
     "bytes"
     "encoding/json"
     "errors"

AddImport could do a better job here. Perhaps it could always insert the new imports inside the top of any existing import decl; this may improve the final result, especially for std imports, which will be sorted. However, it may not be better (though nor is it worse) for non-std imports, which often appear in a second or later group separate by a blank line. In any case, there is room for improvement.

cc: @jba

[gabyhelp]

Related Issues

• go/analysis/analysistest: RunWithSuggestedFixes cannot handle overlapping fixes (such as ones that add imports); need diff3 #67049

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[gopherbot]

Change https://go.dev/cl/648136 mentions this issue: internal/analysisinternal: AddImport puts new import in a group