From c0060eca2c81c0b9077b4152e09329e0f0f84f34 Mon Sep 17 00:00:00 2001
From: Dmitri Shuralyov <shurcooL@gmail.com>
Date: Mon, 19 May 2014 14:04:30 -0700
Subject: [PATCH] go.tools/astutil: fix edge case in DeleteImport causing
 merging of import sections.

The issue occurs only when deleting an import that has a blank line immediately preceding,
and other imports before that.

Currently, DeleteImport assumes there's a blank line-sized hole left behind
where the import was, and always deletes it. That blank line-sized hole is there in all cases
except the above edge case.

This fix checks for that edge case, and does not remove the blank line-sized hole.

The CL also adds a previously failing test case that catches this scenario. After the change to
DeleteImport, the new test passes (along with all other tests).

Fixes golang/go#7679.

Note that there is no attempt to ensure the result *ast.File and *token.FileSet are perfectly
matching to what you would get if you printed the AST and parsed it back. This is how the
rest of the package and the current tests work (i.e., they only check that printing the AST gives
the correct output).
Changing that is very hard, if not impossible, at least not
without resorting to manipulating AST via printing, text manipulation and parsing.
This is okay for most usages, but it does create potential problems. For example,
astutil.Imports() currently only works correctly on freshly parsed AST. If that AST
is manipulated via astutil funcs, then Imports() may not always generate correct
output. However, thas is a separate issue and should be treated as such.

LGTM=bradfitz
R=golang-codereviews, gobot, adonovan, bradfitz
CC=golang-codereviews
https://golang.org/cl/92250045
---
 astutil/imports.go      | 19 +++++++++++---
 astutil/imports_test.go | 55 +++++++++++++++++++++++++++++++++++++++++
 imports/fix_test.go     | 35 ++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/astutil/imports.go b/astutil/imports.go
index 8cc765c58..49bb25dbf 100644
--- a/astutil/imports.go
+++ b/astutil/imports.go
@@ -160,11 +160,22 @@ func DeleteImport(fset *token.FileSet, f *ast.File, path string) (deleted bool)
 				gen.Lparen = token.NoPos // drop parens
 			}
 			if j > 0 {
-				// We deleted an entry but now there will be
+				lastImpspec := gen.Specs[j-1].(*ast.ImportSpec)
+				lastLine := fset.Position(lastImpspec.Path.ValuePos).Line
+				line := fset.Position(impspec.Path.ValuePos).Line
+
+				// We deleted an entry but now there may be
 				// a blank line-sized hole where the import was.
-				// Close the hole by making the previous
-				// import appear to "end" where this one did.
-				gen.Specs[j-1].(*ast.ImportSpec).EndPos = impspec.End()
+				if line-lastLine > 1 {
+					// There was a blank line immediately preceeing the deleted import,
+					// so there's no need to close the hole.
+					// Do nothing.
+				} else {
+					// There was no blank line.
+					// Close the hole by making the previous
+					// import appear to "end" where this one did.
+					lastImpspec.EndPos = impspec.End()
+				}
 			}
 			break
 		}
diff --git a/astutil/imports_test.go b/astutil/imports_test.go
index 55e2edcf4..d7a661cba 100644
--- a/astutil/imports_test.go
+++ b/astutil/imports_test.go
@@ -408,6 +408,61 @@ import (
 		pkg:  "os",
 		in:   "package main\n\nimport `os`",
 		out: `package main
+`,
+	},
+	{
+		name: "import.13",
+		pkg:  "io",
+		in: `package main
+
+import (
+	"fmt"
+
+	"io"
+	"os"
+	"utf8"
+
+	"go/format"
+)
+`,
+		out: `package main
+
+import (
+	"fmt"
+
+	"os"
+	"utf8"
+
+	"go/format"
+)
+`,
+	},
+	{
+		name: "import.14",
+		pkg:  "io",
+		in: `package main
+
+import (
+	"fmt" // a
+
+	"io"   // b
+	"os"   // c
+	"utf8" // d
+
+	"go/format" // e
+)
+`,
+		out: `package main
+
+import (
+	"fmt" // a
+
+	// b
+	"os"   // c
+	"utf8" // d
+
+	"go/format" // e
+)
 `,
 	},
 }
diff --git a/imports/fix_test.go b/imports/fix_test.go
index d46463844..6041f0a86 100644
--- a/imports/fix_test.go
+++ b/imports/fix_test.go
@@ -540,6 +540,41 @@ func main() { fmt.Println("Hello, world") }
 
 func notmain() { fmt.Println("Hello, world") }`,
 	},
+
+	// Remove first import within in a 2nd/3rd/4th/etc. section.
+	// golang.org/issue/7679
+	{
+		name: "issue 7679",
+		in: `package main
+
+import (
+	"fmt"
+
+	"github.com/foo/bar"
+	"github.com/foo/qux"
+)
+
+func main() {
+	var _ = fmt.Println
+	//var _ = bar.A
+	var _ = qux.B
+}
+`,
+		out: `package main
+
+import (
+	"fmt"
+
+	"github.com/foo/qux"
+)
+
+func main() {
+	var _ = fmt.Println
+	//var _ = bar.A
+	var _ = qux.B
+}
+`,
+	},
 }
 
 func TestFixImports(t *testing.T) {