Repository: mvdan/gofumpt
Pull request #173 (2021-12-04 09:15:56)

fix a couple of recently reported bugs
(see commit messages - please do not squash)

cc @ainar-g @cckim @riacataquian for reviews if you're up for it :)

ainar-g: I've tried the version from the branch, and I confirm that this fixes #168.  Thanks!
cckim: This fixes the original problem.

Question: is this an intended effect:
```
var _ = map[string]string{
        /*
                joint comment
        */

        "foo": "bar",
}
```
stays the same or should it collapse the space between the two.   [Modified the test case to add a blank line].
mvdan: @cckim indeed it's on purpose; the tests reflect that edge case too; see how "separate comment" in the output file now keeps the empty line afterwards.

The purpose here is to not treat that empty line as a "leading" extra line that we can remove. Perhaps the empty line between the first comment and first element is there as a logical separator. To be conservative, it's best to not touch it.

Patch:
diff --git a/format/format.go b/format/format.go
index d94c471..0837410 100644
--- a/format/format.go
+++ b/format/format.go
@@ -483,7 +483,11 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 				f.removeLines(f.Line(node.Interface)+1, f.Line(removeToPos))
 
 			case len(f.commentsBetween(prev.End(), method.Pos())) > 0:
-				// continue
+				// comments in between; leave newlines alone
+			case len(prev.Names) != len(method.Names):
+				// don't group type unions with methods
+			case len(prev.Names) == 1 && token.IsExported(prev.Names[0].Name) != token.IsExported(method.Names[0].Name):
+				// don't group exported and unexported methods together
 			default:
 				f.removeLinesBetween(prev.End(), method.Pos())
 			}
@@ -651,19 +655,26 @@ func (f *fumpter) applyPost(c *astutil.Cursor) {
 
 		newlineAroundElems := false
 		newlineBetweenElems := false
+		lastEnd := node.Lbrace
 		lastLine := openLine
 		for i, elem := range node.Elts {
-			if elPos := f.Line(elem.Pos()); elPos > lastLine {
+			pos := elem.Pos()
+			comments := f.commentsBetween(lastEnd, pos)
+			if len(comments) > 0 {
+				pos = comments[0].Pos()
+			}
+			if curLine := f.Line(pos); curLine > lastLine {
 				if i == 0 {
 					newlineAroundElems = true
 
 					// remove leading lines if they exist
-					f.removeLines(openLine+1, elPos)
+					f.removeLines(openLine+1, curLine)
 				} else {
 					newlineBetweenElems = true
 				}
 			}
-			lastLine = f.Line(elem.End())
+			lastEnd = elem.End()
+			lastLine = f.Line(lastEnd)
 		}
 		if closeLine > lastLine {
 			newlineAroundElems = true


Test patch:
diff --git a/testdata/scripts/composite-literals-leading-lines.txt b/testdata/scripts/composite-literals-leading-lines.txt
index 2ab7c1c..0b1265c 100644
--- a/testdata/scripts/composite-literals-leading-lines.txt
+++ b/testdata/scripts/composite-literals-leading-lines.txt
@@ -20,12 +20,12 @@ var _ = []string{
 
 var _ = []string{
 
-	// comment
+	// joint comment
 	"foo",
 }
 
 var _ = []string{
-	// comment
+	// separate comment
 
 	"foo",
 }
@@ -43,16 +43,23 @@ var _ = map[string]string{
 
 var _ = map[string]string{
 
-	// comment
+	// joint comment
 	"foo": "bar",
 }
 
 var _ = map[string]string{
-	// comment
+	// separate comment
 
 	"foo": "bar",
 }
 
+var _ = map[string]string{
+	/*
+		joint comment
+	*/
+	"foo": "bar",
+}
+
 -- foo.go.golden --
 package p
 
@@ -65,12 +72,13 @@ var _ = []string{
 }
 
 var _ = []string{
-	// comment
+	// joint comment
 	"foo",
 }
 
 var _ = []string{
-	// comment
+	// separate comment
+
 	"foo",
 }
 
@@ -83,11 +91,19 @@ var _ = map[string]string{
 }
 
 var _ = map[string]string{
-	// comment
+	// joint comment
+	"foo": "bar",
+}
+
+var _ = map[string]string{
+	// separate comment
+
 	"foo": "bar",
 }
 
 var _ = map[string]string{
-	// comment
+	/*
+		joint comment
+	*/
 	"foo": "bar",
 }
diff --git a/testdata/scripts/interface.txt b/testdata/scripts/interface.txt
index 179b6b0..738da12 100644
--- a/testdata/scripts/interface.txt
+++ b/testdata/scripts/interface.txt
@@ -15,6 +15,11 @@ type i1 interface {
 
 	c(x int) int
 
+	D()
+
+	E()
+
+	f()
 }
 
 type i2 interface {
@@ -103,6 +108,11 @@ type i1 interface {
 	a(x int) int
 	b(x int) int
 	c(x int) int
+
+	D()
+	E()
+
+	f()
 }
 
 type i2 interface {
diff --git a/testdata/scripts/typeparams.txt b/testdata/scripts/typeparams.txt
index 09cb614..11bd2c5 100644
--- a/testdata/scripts/typeparams.txt
+++ b/testdata/scripts/typeparams.txt
@@ -22,11 +22,20 @@ type PredeclaredSignedInteger interface {
 }
 
 type StringableSignedInteger interface {
+
 	~int | ~int8 | ~int16 | ~int32 | ~int64
+
 	String() string
 
 }
 
+type CombineEmbeds interface {
+	fmt.Stringer
+	comparable | io.Reader
+
+	Foo()
+}
+
 func Caller() {
 	Foo[int,int](1,2)
 }
@@ -45,9 +54,17 @@ type PredeclaredSignedInteger interface {
 
 type StringableSignedInteger interface {
 	~int | ~int8 | ~int16 | ~int32 | ~int64
+
 	String() string
 }
 
+type CombineEmbeds interface {
+	fmt.Stringer
+	comparable | io.Reader
+
+	Foo()
+}
+
 func Caller() {
 	Foo[int, int](1, 2)
 }