internal/lsp: fix imports issue with duplicate package decl

We shouldn't apply formatting fixes when we are
organizing imports.

Fixes golang/go#38412

Change-Id: I082f4d9a7d1d1dd571304733c287b0d5e90ea448
Reviewed-on: https://go-review.googlesource.com/c/tools/+/228264
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

9 files changed, 39 insertions, 7 deletions

diff --git a/internal/lsp/cmd/test/imports.go b/internal/lsp/cmd/test/imports.go
index 04c700b96..56cfda08b 100644
--- a/internal/lsp/cmd/test/imports.go
+++ b/internal/lsp/cmd/test/imports.go
@@ -7,6 +7,8 @@ package cmdtest
 import (
 	"testing"
 
+	"golang.org/x/tools/internal/lsp/diff"
+	"golang.org/x/tools/internal/lsp/diff/myers"
 	"golang.org/x/tools/internal/span"
 )
 
@@ -18,6 +20,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
 		return []byte(got), nil
 	}))
 	if want != got {
-		t.Errorf("imports failed for %s, expected:\n%q\ngot:\n%q", filename, want, got)
+		d := myers.ComputeEdits(uri, want, got)
+		t.Errorf("imports failed for %s, expected:\n%s", filename, diff.ToUnified("want", "got", want, d))
 	}
 }
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 6e9d4b12f..b7b27aa4a 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -18,6 +18,7 @@ import (
 	"golang.org/x/tools/go/packages/packagestest"
 	"golang.org/x/tools/internal/lsp/cache"
 	"golang.org/x/tools/internal/lsp/diff"
+	"golang.org/x/tools/internal/lsp/diff/myers"
 	"golang.org/x/tools/internal/lsp/mod"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
@@ -363,7 +364,8 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
 		return []byte(got), nil
 	}))
 	if want != got {
-		t.Errorf("import failed for %s, expected:\n%v\ngot:\n%v", filename, want, got)
+		d := myers.ComputeEdits(uri, want, got)
+		t.Errorf("import failed for %s: %s", filename, diff.ToUnified("want", "got", want, d))
 	}
 }
 
diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go
index 6240c69bd..c6c96fd4d 100644
--- a/internal/lsp/source/format.go
+++ b/internal/lsp/source/format.go
@@ -175,6 +175,15 @@ func computeFixEdits(view View, ph ParseGoHandle, options *imports.Options, orig
 	// just those sections against each other, then shift the resulting
 	// edits to the right lines in the original file.
 	left, right := origImports, fixedImports
+
+	// If there is no diff, return early, as there's no need to compute edits.
+	// imports.ApplyFixes also formats the file, and this way we avoid
+	// unnecessary formatting, which may cause further issues if we can't
+	// find an import block on which to anchor the diffs.
+	if len(left) == 0 && len(right) == 0 {
+		return nil, nil
+	}
+
 	converter := span.NewContentConverter(filename, origImports)
 	offset := origImportOffset
 
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 619999ecb..505372192 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -17,6 +17,7 @@ import (
 	"golang.org/x/tools/go/packages/packagestest"
 	"golang.org/x/tools/internal/lsp/cache"
 	"golang.org/x/tools/internal/lsp/diff"
+	"golang.org/x/tools/internal/lsp/diff/myers"
 	"golang.org/x/tools/internal/lsp/fuzzy"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
@@ -468,7 +469,8 @@ func (r *runner) Import(t *testing.T, spn span.Span) {
 		return []byte(got), nil
 	}))
 	if want != got {
-		t.Errorf("import failed for %s, expected:\n%v\ngot:\n%v", spn.URI().Filename(), want, got)
+		d := myers.ComputeEdits(spn.URI(), want, got)
+		t.Errorf("import failed for %s: %s", spn.URI().Filename(), diff.ToUnified("want", "got", want, d))
 	}
 }
 
diff --git a/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.golden b/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.golden
index 59aaf148f..60e508158 100644
--- a/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.golden
@@ -1,6 +1,16 @@
 -- goimports --
+
+
+
+
+
 package imports //@import("package")
 
+
+
+
+
+
 func _() {
 	println("Hello, world!")
 }
@@ -10,5 +20,5 @@ func _() {
 
 
 
-      
+
 
diff --git a/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.in b/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.in
index f9b07b884..c4e3bd53d 100644
--- a/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.in
+++ b/internal/lsp/testdata/lsp/primarymod/imports/issue35458.go.in
@@ -1,4 +1,4 @@
-    
+
 
 
 
@@ -19,4 +19,4 @@ func _() {
 
 
 
-      
+
diff --git a/internal/lsp/testdata/lsp/primarymod/imports/two_lines.go.golden b/internal/lsp/testdata/lsp/primarymod/imports/two_lines.go.golden
new file mode 100644
index 000000000..ec118a4dd
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/imports/two_lines.go.golden
@@ -0,0 +1,4 @@
+-- goimports --
+package main
+func main()  {} //@import("main")
+
diff --git a/internal/lsp/testdata/lsp/primarymod/imports/two_lines.go.in b/internal/lsp/testdata/lsp/primarymod/imports/two_lines.go.in
new file mode 100644
index 000000000..eee534569
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/imports/two_lines.go.in
@@ -0,0 +1,2 @@
+package main
+func main()  {} //@import("main")
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 61ef9df75..853af9628 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -10,7 +10,7 @@ CaseSensitiveCompletionsCount = 4
 DiagnosticsCount = 43
 FoldingRangesCount = 2
 FormatCount = 6
-ImportCount = 7
+ImportCount = 8
 SuggestedFixCount = 6
 DefinitionsCount = 46
 TypeDefinitionsCount = 2