go/ast: don't MergeLine in SortImports when last import on the same line as RParen

Fixes #69183

Change-Id: I8b78dadaa8ba91e74ea2bfc21abd6abe72b7e38b
GitHub-Last-Rev: 1a41f9e8e3bef9926993813568902d1a77c991c1
GitHub-Pull-Request: golang/go#69187
Reviewed-on: https://go-review.googlesource.com/c/go/+/610035
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/go/ast/import.go b/src/go/ast/import.go
index 00f44ee3388aac5acb7d14627caca3dc1f1d6a8c..3e53f10677c9ab8efbaa86e822380be746bc333c 100644 (file)
--- a/src/go/ast/import.go
+++ b/src/go/ast/import.go
@@ -33,11 +33,11 @@ func SortImports(fset *token.FileSet, f *File) {
                for j, s := range d.Specs {
                        if j > i && lineAt(fset, s.Pos()) > 1+lineAt(fset, d.Specs[j-1].End()) {
                                // j begins a new run. End this one.
-                               specs = append(specs, sortSpecs(fset, f, d.Specs[i:j])...)
+                               specs = append(specs, sortSpecs(fset, f, d, d.Specs[i:j])...)
                                i = j
                        }
                }
-               specs = append(specs, sortSpecs(fset, f, d.Specs[i:])...)
+               specs = append(specs, sortSpecs(fset, f, d, d.Specs[i:])...)
                d.Specs = specs
 
                // Deduping can leave a blank line before the rparen; clean that up.
@@ -109,7 +109,7 @@ type cgPos struct {
        cg   *CommentGroup
 }
 
-func sortSpecs(fset *token.FileSet, f *File, specs []Spec) []Spec {
+func sortSpecs(fset *token.FileSet, f *File, d *GenDecl, specs []Spec) []Spec {
        // Can't short-circuit here even if specs are already sorted,
        // since they might yet need deduplication.
        // A lone import, however, may be safely ignored.
@@ -207,7 +207,11 @@ func sortSpecs(fset *token.FileSet, f *File, specs []Spec) []Spec {
                        deduped = append(deduped, s)
                } else {
                        p := s.Pos()
-                       fset.File(p).MergeLine(lineAt(fset, p))
+                       // This function is exited early when len(specs) <= 1,
+                       // so d.Rparen must be populated (d.Rparen.IsValid() == true).
+                       if l := lineAt(fset, p); l != lineAt(fset, d.Rparen) {
+                               fset.File(p).MergeLine(l)
+                       }
                }
        }
        specs = deduped

diff --git a/src/go/ast/import_test.go b/src/go/ast/import_test.go
index 02fde4efb927b31d4929650616a73cb3e4fff42e..8f0d9e2c3353bba4c935f37e1a3f5bd36a6eb530 100644 (file)
--- a/src/go/ast/import_test.go
+++ b/src/go/ast/import_test.go
@@ -79,3 +79,44 @@ import (
                }
        })
 }
+
+func TestIssue69183(t *testing.T) {
+       const src = `package A
+import (
+"a"//a
+"a")
+`
+       fset := token.NewFileSet()
+       f, err := parser.ParseFile(fset, "test.go", src, parser.ParseComments|parser.SkipObjectResolution)
+       if err != nil {
+               t.Fatal(err)
+       }
+       ast.SortImports(fset, f) // should not panic
+}
+
+func TestSortImportsSameLastLine(t *testing.T) {
+       const src = `package A
+import (
+"a"//a
+"a")
+func a() {}
+`
+
+       fset := token.NewFileSet()
+       f, err := parser.ParseFile(fset, "test.go", src, parser.ParseComments|parser.SkipObjectResolution)
+       if err != nil {
+               t.Fatal(err)
+       }
+       ast.SortImports(fset, f)
+       fd := f.Decls[1].(*ast.FuncDecl)
+       fdPos := fset.Position(fd.Pos())
+       // After SortImports, the Position of the func, should still be at Column == 1.
+       // This is related to the issue: https://go.dev/issue/69183, we were merging lines
+       // incorrectly, which caused the position to be Column = 6, Line = 4.
+       if fdPos.Column != 1 {
+               t.Errorf("invalid fdPos.Column = %v; want = 1", fdPos.Column)
+       }
+       if fdPos.Line != 5 {
+               t.Errorf("invalid fdPos.Line = %v; want = 5", fdPos.Line)
+       }
+}