internal/imports: sort fixes for deterministic results

Fixes golang/go#59976

Change-Id: I59c309e126b1ec4cd05c50e5af8e5df8cfe49072
Reviewed-on: https://go-review.googlesource.com/c/tools/+/492738
gopls-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Robert Findley <[email protected]>

Author: findleyr

gopls/internal/regtest/marker/testdata/codeaction/imports.txt

@@ -151,9 +151,7 @@ func _() {
 }
 
 -- removeall.go --
-// TODO(golang/go#59976): this test is flaky because it leaves one comment
-// behind, but non-deterministically.
-package imports // codeaction("source.organizeImports", "package", "", removeall)
+package imports //@codeaction("source.organizeImports", "package", "", removeall)
 
 import (
 	"bytes" //@diag("\"bytes\"", re"not used")
@@ -164,6 +162,14 @@ import (
 func _() {
 }
 
+-- @removeall/removeall.go --
+package imports //@codeaction("source.organizeImports", "package", "", removeall)
+
+//@diag("\"fmt\"", re"not used")
+
+func _() {
+}
+
 -- twolines.go --
 package imports
 func main()  {} //@codeactionerr("source.organizeImports", "main", "", re"found 0")

internal/imports/fix.go

@@ -414,9 +414,16 @@ func (p *pass) fix() ([]*ImportFix, bool) {
 			})
 		}
 	}
-
+	// Collecting fixes involved map iteration, so sort for stability. See
+	// golang/go#59976.
+	sortFixes(fixes)
+
+	// collect selected fixes in a separate slice, so that it can be sorted
+	// separately. Note that these fixes must occur after fixes to existing
+	// imports. TODO(rfindley): figure out why.
+	var selectedFixes []*ImportFix
 	for _, imp := range selected {
-		fixes = append(fixes, &ImportFix{
+		selectedFixes = append(selectedFixes, &ImportFix{
 			StmtInfo: ImportInfo{
 				Name:       p.importSpecName(imp),
 				ImportPath: imp.ImportPath,
@@ -425,8 +432,25 @@ func (p *pass) fix() ([]*ImportFix, bool) {
 			FixType:   AddImport,
 		})
 	}
+	sortFixes(selectedFixes)
+
+	return append(fixes, selectedFixes...), true
+}
 
-	return fixes, true
+func sortFixes(fixes []*ImportFix) {
+	sort.Slice(fixes, func(i, j int) bool {
+		fi, fj := fixes[i], fixes[j]
+		if fi.StmtInfo.ImportPath != fj.StmtInfo.ImportPath {
+			return fi.StmtInfo.ImportPath < fj.StmtInfo.ImportPath
+		}
+		if fi.StmtInfo.Name != fj.StmtInfo.Name {
+			return fi.StmtInfo.Name < fj.StmtInfo.Name
+		}
+		if fi.IdentName != fj.IdentName {
+			return fi.IdentName < fj.IdentName
+		}
+		return fi.FixType < fj.FixType
+	})
 }
 
 // importSpecName gets the import name of imp in the import spec.