- improved handling of white space around declarations and statements

- extra tests

R=rsc
DELTA=366  (264 added, 37 deleted, 65 changed)
OCL=35299
CL=35301

src/pkg/go/printer/printer.go

@@ -112,8 +112,7 @@ func (p *printer) write0(data []byte) {
 
 
 // write interprets data and writes it to p.output. It inserts indentation
-// after newline or formfeed, converts formfeed characters into newlines if
-// RawFormat is set, and HTML-escapes data if GenHTML is set.
+// after newline or formfeed and HTML-escapes characters if GenHTML is set.
 //
 func (p *printer) write(data []byte) {
 	i0 := 0;
@@ -217,7 +216,7 @@ func (p *printer) writeItem(pos token.Position, data []byte, setLineTag bool) {
 }
 
 
-// TODO(gri) decide if this is needed - keep around for now
+// TODO(gri): decide if this is needed - keep around for now
 /*
 // Reduce contiguous sequences of '\t' in a []byte to a single '\t'.
 func untabify(src []byte) []byte {
@@ -347,7 +346,7 @@ func (p *printer) print(args ...) {
 			// indentation delta
 			p.indent += x;
 			if p.indent < 0 {
-				panic("print: negative indentation");
+				panicln("print: negative indentation", p.indent);
 			}
 		case whiteSpace:
 			if p.buflen >= len(p.buffer) {
@@ -410,9 +409,32 @@ func (p *printer) flush(next token.Position) {
 // ----------------------------------------------------------------------------
 // Printing of common AST nodes.
 
-// TODO(gri) The code for printing lead and line comments
-//           should be eliminated in favor of reusing the
-//           comment intersperse mechanism above somehow.
+
+// Print as many newlines as necessary (at least one and and at most
+// max newlines) to get to the current line. If newSection is set, the
+// first newline is printed as a formfeed.
+//
+// TODO(gri): Reconsider signature (provide position instead of line)
+//
+func (p *printer) linebreak(line, max int, newSection bool) {
+	n := line - p.last.Line;
+	switch {
+	case n < 1: n = 1;
+	case n > max: n = max;
+	}
+	if newSection {
+		p.print(formfeed);
+		n--;
+	}
+	for ; n > 0; n-- {
+		p.print(newline);
+	}
+}
+
+
+// TODO(gri): The code for printing lead and line comments
+//            should be eliminated in favor of reusing the
+//            comment intersperse mechanism above somehow.
 
 // Print a list of individual comments.
 func (p *printer) commentList(list []*ast.Comment) {
@@ -538,7 +560,7 @@ func (p *printer) parameters(list []*ast.Field) {
 			if len(par.Names) > 0 {
 				// at least one identifier
 				p.print(blank);
-			};
+			}
 			p.expr(par.Type);
 		}
 	}
@@ -655,7 +677,7 @@ func needsBlanks(expr ast.Expr) bool {
 }
 
 
-// TODO(gri) Write this recursively; get rid of vector use.
+// TODO(gri): Write this recursively; get rid of vector use.
 func (p *printer) binaryExpr(x *ast.BinaryExpr, prec1 int) {
 	prec := x.Op.Precedence();
 	if prec < prec1 {
@@ -863,47 +885,28 @@ func (p *printer) expr(x ast.Expr) (optSemi bool) {
 // ----------------------------------------------------------------------------
 // Statements
 
+const maxStmtNewlines = 2  // maximum number of newlines between statements
+
 // Print the statement list indented, but without a newline after the last statement.
-func (p *printer) stmtList(list []ast.Stmt) {
-	if len(list) > 0 {
-		p.print(+1, formfeed);  // the next lines have different structure
-		optSemi := false;
-		for i, s := range list {
-			if i > 0 {
-				if !optSemi {
-					p.print(token.SEMICOLON);
-				}
-				p.print(newline);
-			}
-			optSemi = p.stmt(s);
-		}
-		if !optSemi {
+// Extra line breaks between statements in the source are respected but at most one
+// empty line is printed between statements.
+func (p *printer) stmtList(list []ast.Stmt, indent int) {
+	p.print(+indent);
+	for i, s := range list {
+		p.linebreak(s.Pos().Line, maxStmtNewlines, i == 0);
+		if !p.stmt(s) {
 			p.print(token.SEMICOLON);
 		}
-		p.print(-1);
 	}
+	p.print(-indent);
 }
 
 
-func (p *printer) block(s *ast.BlockStmt) {
+func (p *printer) block(s *ast.BlockStmt, indent int) {
 	p.print(s.Pos(), token.LBRACE);
 	if len(s.List) > 0 {
-		p.stmtList(s.List);
-		p.print(formfeed);
-	}
-	p.print(s.Rbrace, token.RBRACE);
-}
-
-
-func (p *printer) switchBlock(s *ast.BlockStmt) {
-	p.print(s.Pos(), token.LBRACE);
-	if len(s.List) > 0 {
-		for _, s := range s.List {
-			// s is one of *ast.CaseClause, *ast.TypeCaseClause, *ast.CommClause;
-			p.print(formfeed);
-			p.stmt(s);
-		}
-		p.print(formfeed);
+		p.stmtList(s.List, indent);
+		p.linebreak(s.Rbrace.Line, maxStmtNewlines, true);
 	}
 	p.print(s.Rbrace, token.RBRACE);
 }
@@ -953,16 +956,7 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 		p.print("BadStmt");
 
 	case *ast.DeclStmt:
-		var comment *ast.CommentGroup;
-		comment, optSemi = p.decl(s.Decl);
-		if comment != nil {
-			// Line comments of declarations in statement lists
-			// are not associated with the declaration in the parser;
-			// this case should never happen. Print anyway to continue
-			// gracefully.
-			p.lineComment(comment);
-			p.print(newline);
-		}
+		optSemi = p.decl(s.Decl);
 
 	case *ast.EmptyStmt:
 		// nothing to do
@@ -1007,13 +1001,13 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 		}
 
 	case *ast.BlockStmt:
-		p.block(s);
+		p.block(s, 1);
 		optSemi = true;
 
 	case *ast.IfStmt:
 		p.print(token.IF);
 		p.controlClause(false, s.Init, s.Cond, nil);
-		p.block(s.Body);
+		p.block(s.Body, 1);
 		optSemi = true;
 		if s.Else != nil {
 			p.print(blank, token.ELSE, blank);
@@ -1028,12 +1022,13 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 			p.print(token.DEFAULT);
 		}
 		p.print(s.Colon, token.COLON);
-		p.stmtList(s.Body);
+		p.stmtList(s.Body, 1);
+		optSemi = true;  // "block" without {}'s
 
 	case *ast.SwitchStmt:
 		p.print(token.SWITCH);
 		p.controlClause(false, s.Init, s.Tag, nil);
-		p.switchBlock(s.Body);
+		p.block(s.Body, 0);
 		optSemi = true;
 
 	case *ast.TypeCaseClause:
@@ -1044,7 +1039,8 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 			p.print(token.DEFAULT);
 		}
 		p.print(s.Colon, token.COLON);
-		p.stmtList(s.Body);
+		p.stmtList(s.Body, 1);
+		optSemi = true;  // "block" without {}'s
 
 	case *ast.TypeSwitchStmt:
 		p.print(token.SWITCH);
@@ -1056,7 +1052,7 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 		p.print(blank);
 		p.stmt(s.Assign);
 		p.print(blank);
-		p.switchBlock(s.Body);
+		p.block(s.Body, 0);
 		optSemi = true;
 
 	case *ast.CommClause:
@@ -1071,17 +1067,18 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 			p.print(token.DEFAULT);
 		}
 		p.print(s.Colon, token.COLON);
-		p.stmtList(s.Body);
+		p.stmtList(s.Body, 1);
+		optSemi = true;  // "block" without {}'s
 
 	case *ast.SelectStmt:
 		p.print(token.SELECT, blank);
-		p.switchBlock(s.Body);
+		p.block(s.Body, 0);
 		optSemi = true;
 
 	case *ast.ForStmt:
 		p.print(token.FOR);
 		p.controlClause(true, s.Init, s.Cond, s.Post);
-		p.block(s.Body);
+		p.block(s.Body, 1);
 		optSemi = true;
 
 	case *ast.RangeStmt:
@@ -1094,7 +1091,7 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 		p.print(blank, s.TokPos, s.Tok, blank, token.RANGE, blank);
 		p.expr(s.X);
 		p.print(blank);
-		p.block(s.Body);
+		p.block(s.Body, 1);
 		optSemi = true;
 
 	default:
@@ -1189,7 +1186,7 @@ func countValueTypes(list []ast.Spec) (n int) {
 
 
 // Returns true if a separating semicolon is optional.
-func (p *printer) decl(decl ast.Decl) (comment *ast.CommentGroup, optSemi bool) {
+func (p *printer) decl(decl ast.Decl) (optSemi bool) {
 	switch d := decl.(type) {
 	case *ast.BadDecl:
 		p.print(d.Pos(), "BadDecl");
@@ -1214,23 +1211,27 @@ func (p *printer) decl(decl ast.Decl) (comment *ast.CommentGroup, optSemi bool)
 				p.print(+1, formfeed);
 				for i, s := range d.Specs {
 					if i > 0 {
-						p.print(token.SEMICOLON);
-						p.lineComment(comment);
 						p.print(newline);
 					}
-					comment, _ = p.spec(s, m, len(d.Specs));
+					comment, _ := p.spec(s, m, len(d.Specs));
+					p.print(token.SEMICOLON);
+					p.lineComment(comment);
 				}
-				p.print(token.SEMICOLON);
-				p.lineComment(comment);
 				p.print(-1, formfeed);
 			}
 			p.print(d.Rparen, token.RPAREN);
-			comment = nil;  // comment was already printed
 			optSemi = true;
 
 		} else {
 			// single declaration
+			var comment *ast.CommentGroup;
 			comment, optSemi = p.spec(d.Specs[0], m, 1);
+			// If this declaration is inside a statement list, the parser
+			// does not associate a line comment with the declaration but
+			// handles it as ordinary unassociated comment. Thus, in that
+			// case, comment == nil and any trailing semicolon is not part
+			// of a comment.
+			p.lineComment(comment);
 		}
 
 	case *ast.FuncDecl:
@@ -1257,22 +1258,25 @@ func (p *printer) decl(decl ast.Decl) (comment *ast.CommentGroup, optSemi bool)
 		panic("unreachable");
 	}
 
-	return comment, optSemi;
+	return;
 }
 
 
 // ----------------------------------------------------------------------------
 // Files
 
+const maxDeclNewlines = 3  // maximum number of newlines between declarations
+
 func (p *printer) file(src *ast.File) {
 	p.leadComment(src.Doc);
 	p.print(src.Pos(), token.PACKAGE, blank);
 	p.expr(src.Name);
 
-	for _, d := range src.Decls {
-		p.print(newline, newline);
-		comment, _ := p.decl(d);
-		p.lineComment(comment);
+	if len(src.Decls) > 0 {
+		for _, d := range src.Decls {
+			p.linebreak(d.Pos().Line, maxDeclNewlines, false);
+			p.decl(d);
+		}
 	}
 
 	p.print(newline);
@@ -1395,8 +1399,7 @@ func Fprint(output io.Writer, node interface{}, mode uint, tabwidth int) (int, o
 		case ast.Stmt:
 			p.stmt(n);
 		case ast.Decl:
-			comment, _ := p.decl(n);
-			p.lineComment(comment);  // no newline at end
+			p.decl(n);
 		case *ast.File:
 			p.comment = n.Comments;
 			p.file(n);

src/pkg/go/printer/printer_test.go

@@ -98,6 +98,7 @@ type entry struct {
 
 // Use gotest -update to create/update the respective golden files.
 var data = []entry{
+	entry{ "empty.go", "empty.golden", false },
 	entry{ "comments.go", "comments.golden", false },
 	entry{ "comments.go", "comments.x", true },
 	entry{ "linebreaks.go", "linebreaks.golden", false },

src/pkg/go/printer/testdata/comments.golden

@@ -90,6 +90,7 @@ func f1() {
 	f0();
 }
 
+
 func abs(x int) int {
 	if x < 0 {	// the tab printed before this comment's // must not affect the remaining lines
 		return -x;	// this statement should be properly indented
@@ -97,14 +98,18 @@ func abs(x int) int {
 	return x;
 }
 
+
 func typeswitch(x interface{}) {
 	switch v := x.(type) {
 	case bool, int, float:
 	case string:
 	default:
 	}
+
 	switch x.(type) {}
+
 	switch v0, ok := x.(int); v := x.(type) {}
+
 	switch v0, ok := x.(int); x.(type) {
 	case bool, int, float:
 	case string:

src/pkg/go/printer/testdata/comments.x

@@ -2,14 +2,17 @@
 //
 package main
 
+
 // The SZ struct; it is empty.
 type SZ struct{}
 
+
 // The S0 struct; no field is exported.
 type S0 struct {
 	// contains unexported fields
 }
 
+
 // The S1 struct; some fields are not exported.
 type S1 struct {
 	S0;
@@ -18,20 +21,24 @@ type S1 struct {
 	// contains unexported fields
 }
 
+
 // The S2 struct; all fields are exported.
 type S2 struct {
 	S1;
 	A, B, C	float;	// 3 exported fields
 }
 
+
 // The IZ interface; it is empty.
 type SZ interface{}
 
+
 // The I0 interface; no method is exported.
 type I0 interface {
 	// contains unexported methods
 }
 
+
 // The I1 interface; some methods are not exported.
 type I1 interface {
 	I0;
@@ -39,6 +46,7 @@ type I1 interface {
 	// contains unexported methods
 }
 
+
 // The I2 interface; all methods are exported.
 type I1 interface {
 	I0;

src/pkg/go/printer/testdata/declarations.go

@@ -25,6 +25,26 @@ import (
 	c "i" "o";
 )
 
+// no newlines between consecutive single imports, but
+// respect extra line breaks in the source (at most one empty line)
+import _ "io"
+import _ "io"
+import _ "io"
+
+import _ "os"
+import _ "os"
+import _ "os"
+
+
+import _ "fmt"
+import _ "fmt"
+import _ "fmt"
+
+
+// at least one empty line between declarations of different kind
+import _ "io"
+var _ int;
+
 
 func _() {
 	// the following decls need a semicolon at the end
@@ -73,6 +93,8 @@ func _() {
 }
 
 
+
+
 // no tabs for single or ungrouped decls
 func _() {
 	const xxxxxx = 0;

src/pkg/go/printer/testdata/declarations.golden

@@ -25,6 +25,27 @@ import (
 	c			"i" "o";
 )
 
+// no newlines between consecutive single imports, but
+// respect extra line breaks in the source (at most one empty line)
+import _ "io"
+import _ "io"
+import _ "io"
+
+import _ "os"
+import _ "os"
+import _ "os"
+
+
+import _ "fmt"
+import _ "fmt"
+import _ "fmt"
+
+
+// at least one empty line between declarations of different kind
+import _ "io"
+var _ int
+
+
 func _() {
 	// the following decls need a semicolon at the end
 	type _ int;
@@ -33,6 +54,7 @@ func _() {
 	type _ map[string]int;
 	type _ chan int;
 	type _ func() int;
+
 	var _ int;
 	var _ *int;
 	var _ []int;
@@ -47,18 +69,21 @@ func _() {
 	type _ map[string]struct{}
 	type _ chan struct{}
 	type _ func() struct{}
+
 	type _ interface{}
 	type _ *interface{}
 	type _ []interface{}
 	type _ map[string]interface{}
 	type _ chan interface{}
 	type _ func() interface{}
+
 	var _ struct{}
 	var _ *struct{}
 	var _ []struct{}
 	var _ map[string]struct{}
 	var _ chan struct{}
 	var _ func() struct{}
+
 	var _ interface{}
 	var _ *interface{}
 	var _ []interface{}
@@ -75,6 +100,7 @@ func _() {
 	var xxx int;
 	var yyyy float = 3.14;
 	var zzzzz = "bar";
+
 	const (
 		xxxxxx = 0;
 	)

src/pkg/go/printer/testdata/empty.go

+// a comment at the beginning of the file
+
+package empty
+
+// a comment at the end of the file

src/pkg/go/printer/testdata/empty.golden

+// a comment at the beginning of the file
+
+package empty
+
+// a comment at the end of the file

src/pkg/go/printer/testdata/expressions.golden

@@ -16,6 +16,7 @@ var (
 	p													*int;
 )
 
+
 func _() {
 	// no spaces around simple or parenthesized expressions
 	_ = a+b;
@@ -74,6 +75,7 @@ func _() {
 	_ = a - b + c - d + (a+b+c) + d&e;
 }
 
+
 func _() {
 	_ = T{};
 	_ = struct{}{};
@@ -81,6 +83,7 @@ func _() {
 	_ = [...]T{};
 	_ = []T{};
 	_ = map[int]T{};
+
 	_ = (T){};
 	_ = (struct{}){};
 	_ = ([10]T){};

src/pkg/go/printer/testdata/statements.go

@@ -6,32 +6,98 @@ package statements
 
 var expr bool;
 
+func use(x interface{}) {}
+
+// Formatting of if-statement headers.
 func _() {
 	if {}
+	if;{}  // no semicolon printed
 	if expr{}
-	if _:=expr;{}
-	if _:=expr; expr {}
+	if;expr{}  // no semicolon printed
+	if x:=expr;{
+	use(x)}
+	if x:=expr; expr {use(x)}
 }
 
 
+// Formatting of switch-statement headers.
 func _() {
 	switch {}
+	switch;{}  // no semicolon printed
 	switch expr {}
-	switch _ := expr; {}
-	switch _ := expr; expr {}
+	switch;expr{}  // no semicolon printed
+	switch x := expr; { default:use(
+x)
+	}
+	switch x := expr; expr {default:use(x)}
 }
 
 
+// Formatting of switch statement bodies.
+func _() {
+	switch {
+	}
+
+	switch x := 0; x {
+	case 1:
+		use(x);
+		use(x);  // followed by an empty line
+
+	case 2:  // followed by an empty line
+
+		use(x);  // followed by an empty line
+
+	case 3:  // no empty lines
+		use(x);
+		use(x);
+	}
+}
+
+
+// Formatting of for-statement headers.
 func _() {
 	for{}
 	for expr {}
-	for;;{}  // TODO ok to lose the semicolons here?
-	for _ :=expr;; {}
-	for; expr;{}  // TODO ok to lose the semicolons here?
+	for;;{}  // no semicolon printed
+	for x :=expr;; {use( x)}
+	for; expr;{}  // no semicolon printed
 	for; ; expr = false {}
-	for _ :=expr; expr; {}
-	for _ := expr;; expr=false {}
-	for;expr;expr =false {}
-	for _ := expr;expr;expr = false {}
-	for _ := range []int{} {}
+	for x :=expr; expr; {use(x)}
+	for x := expr;; expr=false {use(x)}
+	for;expr;expr =false {
+	}
+	for x := expr;expr;expr = false { use(x) }
+	for x := range []int{} { use(x) }
+}
+
+
+// Extra empty lines inside functions. Do respect source code line
+// breaks between statement boundaries but print at most one empty
+// line at a time.
+func _() {
+
+	const _ = 0;
+
+	const _ = 1;
+	type _ int;
+	type _ float;
+
+	var _ = 0;
+	var x = 1;
+
+	// Each use(x) call below should have at most one empty line before and after.
+
+
+
+	use(x);
+
+	if x < x {
+
+		use(x);
+
+	} else {
+
+		use(x);
+
+	}
 }

src/pkg/go/printer/testdata/statements.golden

@@ -6,30 +6,113 @@ package statements
 
 var expr bool
 
+func use(x interface{}) {}
+
+// Formatting of if-statement headers.
 func _() {
 	if {}
+	if {}	// no semicolon printed
 	if expr {}
-	if _ := expr; {}
-	if _ := expr; expr {}
+	if expr {}	// no semicolon printed
+	if x := expr; {
+		use(x);
+	}
+	if x := expr; expr {
+		use(x);
+	}
 }
 
+
+// Formatting of switch-statement headers.
 func _() {
 	switch {}
+	switch {}	// no semicolon printed
 	switch expr {}
-	switch _ := expr; {}
-	switch _ := expr; expr {}
+	switch expr {}	// no semicolon printed
+	switch x := expr; {
+	default:
+		use(x);
+	}
+	switch x := expr; expr {
+	default:
+		use(x);
+	}
+}
+
+
+// Formatting of switch statement bodies.
+func _() {
+	switch {}
+
+	switch x := 0; x {
+	case 1:
+		use(x);
+		use(x);	// followed by an empty line
+
+	case 2:	// followed by an empty line
+
+		use(x);	// followed by an empty line
+
+	case 3:	// no empty lines
+		use(x);
+		use(x);
+	}
 }
 
+
+// Formatting of for-statement headers.
 func _() {
 	for {}
 	for expr {}
-	for {}	// TODO ok to lose the semicolons here?
-	for _ := expr; ; {}
-	for expr {}	// TODO ok to lose the semicolons here?
+	for {}	// no semicolon printed
+	for x := expr; ; {
+		use(x);
+	}
+	for expr {}	// no semicolon printed
 	for ; ; expr = false {}
-	for _ := expr; expr; {}
-	for _ := expr; ; expr = false {}
+	for x := expr; expr; {
+		use(x);
+	}
+	for x := expr; ; expr = false {
+		use(x);
+	}
 	for ; expr; expr = false {}
-	for _ := expr; expr; expr = false {}
-	for _ := range []int{} {}
+	for x := expr; expr; expr = false {
+		use(x);
+	}
+	for x := range []int{} {
+		use(x);
+	}
+}
+
+
+// Extra empty lines inside functions. Do respect source code line
+// breaks between statement boundaries but print at most one empty
+// line at a time.
+func _() {
+
+	const _ = 0;
+
+	const _ = 1;
+	type _ int;
+	type _ float;
+
+	var _ = 0;
+	var x = 1;
+
+	// Each use(x) call below should have at most one empty line before and after.
+
+
+
+	use(x);
+
+	if x < x {
+
+		use(x);
+
+	} else {
+
+		use(x);
+
+	}
 }