go/printer: consider empty lines in table layout computation

In previous versions of Go including 1.10, an empty line would break the
alignment of elements within an expression list.

golang.org/cl/104755 changed the heuristic, with the side effect that
empty lines no longer broke the table alignment.

A prior fix (https://go-review.googlesource.com/c/go/+/125260, reverted)
introduced another regression (#26930) which this change doesn't produce.

Added test cases for both #26352 and #26930.

Fixes #26352.
Updates #26930.

Change-Id: I371f48e6f3620ebbab53f2128ec5e58bcd4a62f1
Reviewed-on: https://go-review.googlesource.com/129256
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Alan Donovan <adonovan@google.com>

3 files changed, 116 insertions, 16 deletions

diff --git a/src/go/printer/nodes.go b/src/go/printer/nodes.go
index 18f2371d24..1de7cd81b2 100644
--- a/src/go/printer/nodes.go
+++ b/src/go/printer/nodes.go
@@ -32,8 +32,10 @@ import (
 
 // Print as many newlines as necessary (but at least min newlines) to get to
 // the current line. ws is printed before the first line break. If newSection
-// is set, the first line break is printed as formfeed. Returns true if any
-// line break was printed; returns false otherwise.
+// is set, the first line break is printed as formfeed. Returns 0 if no line
+// breaks were printed, returns 1 if there was exactly one newline printed,
+// and returns a value > 1 if there was a formfeed or more than one newline
+// printed.
 //
 // TODO(gri): linebreak may add too many lines if the next statement at "line"
 //            is preceded by comments because the computation of n assumes
@@ -43,7 +45,7 @@ import (
 //            linebreaks. At the moment there is no easy way to know about
 //            future (not yet interspersed) comments in this function.
 //
-func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (printedBreak bool) {
+func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (nbreaks int) {
 	n := nlimit(line - p.pos.Line)
 	if n < min {
 		n = min
@@ -53,11 +55,12 @@ func (p *printer) linebreak(line, min int, ws whiteSpace, newSection bool) (prin
 		if newSection {
 			p.print(formfeed)
 			n--
+			nbreaks = 2
 		}
+		nbreaks += n
 		for ; n > 0; n-- {
 			p.print(newline)
 		}
-		printedBreak = true
 	}
 	return
 }
@@ -173,7 +176,7 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 	// The first linebreak is always a formfeed since this section must not
 	// depend on any previous formatting.
 	prevBreak := -1 // index of last expression that was followed by a linebreak
-	if prev.IsValid() && prev.Line < line && p.linebreak(line, 0, ws, true) {
+	if prev.IsValid() && prev.Line < line && p.linebreak(line, 0, ws, true) > 0 {
 		ws = ignore
 		prevBreak = 0
 	}
@@ -234,14 +237,6 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 				useFF = r*ratio <= 1 || r <= ratio
 			}
 		}
-		if useFF {
-			lnsum = 0
-			count = 0
-		}
-		if size > 0 {
-			lnsum += math.Log(float64(size))
-			count++
-		}
 
 		needsLinebreak := 0 < prevLine && prevLine < line
 		if i > 0 {
@@ -257,11 +252,20 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 				// Lines are broken using newlines so comments remain aligned
 				// unless useFF is set or there are multiple expressions on
 				// the same line in which case formfeed is used.
-				if p.linebreak(line, 0, ws, useFF || prevBreak+1 < i) {
+				nbreaks := p.linebreak(line, 0, ws, useFF || prevBreak+1 < i)
+				if nbreaks > 0 {
 					ws = ignore
 					prevBreak = i
 					needsBlank = false // we got a line break instead
 				}
+				// If there was a new section or more than one new line
+				// (which means that the tabwriter will implicitly break
+				// the section), reset the geomean variables since we are
+				// starting a new group of elements with the next element.
+				if nbreaks > 1 {
+					lnsum = 0
+					count = 0
+				}
 			}
 			if needsBlank {
 				p.print(blank)
@@ -281,6 +285,11 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 			p.expr0(x, depth)
 		}
 
+		if size > 0 {
+			lnsum += math.Log(float64(size))
+			count++
+		}
+
 		prevLine = line
 	}
 
@@ -338,7 +347,7 @@ func (p *printer) parameters(fields *ast.FieldList) {
 				p.print(token.COMMA)
 			}
 			// separator if needed (linebreak or blank)
-			if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) {
+			if needsLinebreak && p.linebreak(parLineBeg, 0, ws, true) > 0 {
 				// break line if the opening "(" or previous parameter ended on a different line
 				ws = ignore
 			} else if i > 0 {
@@ -709,7 +718,7 @@ func (p *printer) binaryExpr(x *ast.BinaryExpr, prec1, cutoff, depth int) {
 	if xline != yline && xline > 0 && yline > 0 {
 		// at least one line break, but respect an extra empty line
 		// in the source
-		if p.linebreak(yline, 1, ws, true) {
+		if p.linebreak(yline, 1, ws, true) > 0 {
 			ws = ignore
 			printBlank = false // no blank after line break
 		}
diff --git a/src/go/printer/testdata/alignment.golden b/src/go/printer/testdata/alignment.golden
index c65defe6ae..96086ed906 100644
--- a/src/go/printer/testdata/alignment.golden
+++ b/src/go/printer/testdata/alignment.golden
@@ -128,3 +128,45 @@ func main() {
 		abcdefghijklmnopqrstuvwxyz:		"foo",
 	}
 }
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26352.
+
+var _ = map[int]string{
+	1:	"",
+
+	12345678901234567890123456789:		"",
+	12345678901234567890123456789012345678:	"",
+}
+
+func f() {
+	_ = map[int]string{
+		1:	"",
+
+		12345678901234567:				"",
+		12345678901234567890123456789012345678901:	"",
+	}
+}
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26930.
+
+var _ = S{
+	F1:	[]string{},
+	F2____:	[]string{},
+}
+
+var _ = S{
+	F1:	[]string{},
+	F2____:	[]string{},
+}
+
+var _ = S{
+	F1____:	[]string{},
+	F2:	[]string{},
+}
+
+var _ = S{
+	F1____:	[]string{},
+	F2:	[]string{},
+}
diff --git a/src/go/printer/testdata/alignment.input b/src/go/printer/testdata/alignment.input
index 9b0aae6bec..323d2689e0 100644
--- a/src/go/printer/testdata/alignment.input
+++ b/src/go/printer/testdata/alignment.input
@@ -128,3 +128,52 @@ func main() {
 		abcdefghijklmnopqrstuvwxyz: "foo",
 	}
 }
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26352.
+
+var _ = map[int]string{
+	1: "",
+
+	12345678901234567890123456789: "",
+	12345678901234567890123456789012345678: "",
+}
+
+func f() {
+	_ = map[int]string{
+		1: "",
+
+		12345678901234567: "",
+		12345678901234567890123456789012345678901: "",
+	}
+}
+
+// ----------------------------------------------------------------------------
+// Examples from issue #26930.
+
+var _ = S{
+	F1: []string{
+	},
+	F2____: []string{},
+}
+
+var _ = S{
+	F1: []string{
+
+
+	},
+	F2____: []string{},
+}
+
+var _ = S{
+	F1____: []string{
+	},
+	F2: []string{},
+}
+
+var _ = S{
+	F1____: []string{
+
+	},
+	F2: []string{},
+}