various go printer fixes:

- better handling of line breaks in expression lists
- fixed line breaks around label decls
- remove ()'s around if, for, switch expressions
- simple index expressions don't require blanks
- better line breaks around declarations of different kind

R=rsc
DELTA=404  (369 added, 8 deleted, 27 changed)
OCL=35354
CL=35359

src/pkg/go/printer/printer.go

@@ -410,25 +410,29 @@ func (p *printer) flush(next token.Position) {
 // Printing of common AST nodes.
 
 
-// Print as many newlines as necessary (at least one and and at most
+// Print as many newlines as necessary (but at least min and and at most
 // max newlines) to get to the current line. If newSection is set, the
-// first newline is printed as a formfeed.
+// first newline is printed as a formfeed. Returns true if any linebreak
+// was printed; returns false otherwise.
 //
 // TODO(gri): Reconsider signature (provide position instead of line)
 //
-func (p *printer) linebreak(line, max int, newSection bool) {
-	n := line - p.last.Line;
+func (p *printer) linebreak(line, min, max int, newSection bool) (printedBreak bool) {
+	n := line - p.pos.Line;
 	switch {
-	case n < 1: n = 1;
+	case n < min: n = min;
 	case n > max: n = max;
 	}
-	if newSection {
+	if n > 0 && newSection {
 		p.print(formfeed);
 		n--;
+		printedBreak = true;
 	}
 	for ; n > 0; n-- {
 		p.print(newline);
+		printedBreak = true;
 	}
+	return;
 }
 
 
@@ -506,11 +510,12 @@ func (p *printer) exprList(list []ast.Expr, mode exprListMode) {
 		return;
 	}
 
+	if mode & blankStart != 0 {
+		p.print(blank);
+	}
+
 	if list[0].Pos().Line == list[len(list)-1].Pos().Line {
 		// all list entries on a single line
-		if mode & blankStart != 0 {
-			p.print(blank);
-		}
 		for i, x := range list {
 			if i > 0 {
 				if mode & commaSep != 0 {
@@ -525,8 +530,14 @@ func (p *printer) exprList(list []ast.Expr, mode exprListMode) {
 
 	// list entries span multiple lines;
 	// use source code positions to guide line breaks
-	p.print(+1, formfeed);
 	line := list[0].Pos().Line;
+	indented := false;
+	// there may or may not be a linebreak before the first list
+	// element; in any case indent once after the first linebreak
+	if p.linebreak(line, 0, 2, true) {
+		p.print(+1);
+		indented = true;
+	}
 	for i, x := range list {
 		prev := line;
 		line = x.Pos().Line;
@@ -535,7 +546,12 @@ func (p *printer) exprList(list []ast.Expr, mode exprListMode) {
 				p.print(token.COMMA);
 			}
 			if prev < line {
-				p.print(newline);
+				// at least one linebreak, but respect an extra empty line
+				// in the source
+				if p.linebreak(x.Pos().Line, 1, 2, true) && !indented {
+					p.print(+1);
+					indented = true;
+				}
 			} else {
 				p.print(blank);
 			}
@@ -544,8 +560,15 @@ func (p *printer) exprList(list []ast.Expr, mode exprListMode) {
 	}
 	if mode & commaTerm != 0 {
 		p.print(token.COMMA);
+		if indented {
+			// should always be indented here since we have a multi-line
+			// expression list - be conservative and check anyway
+			p.print(-1);
+		}
+		p.print(formfeed);  // terminating comma needs a line break to look good
+	} else if indented {
+		p.print(-1);
 	}
-	p.print(-1, formfeed);
 }
 
 
@@ -668,6 +691,9 @@ func needsBlanks(expr ast.Expr) bool {
 	case *ast.ParenExpr:
 		// parenthesized expressions don't need blanks around them
 		return false;
+	case *ast.IndexExpr:
+		// index expressions don't need blanks if the indexed expressions are simple
+		return needsBlanks(x.X)
 	case *ast.CallExpr:
 		// call expressions need blanks if they have more than one
 		// argument or if the function or the argument need blanks
@@ -893,7 +919,9 @@ const maxStmtNewlines = 2  // maximum number of newlines 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);
+		// indent == 0 only for lists of switch/select case clauses;
+		// in those cases each clause is a new section
+		p.linebreak(s.Pos().Line, 1, maxStmtNewlines, i == 0 || indent == 0);
 		if !p.stmt(s) {
 			p.print(token.SEMICOLON);
 		}
@@ -906,19 +934,30 @@ func (p *printer) block(s *ast.BlockStmt, indent int) {
 	p.print(s.Pos(), token.LBRACE);
 	if len(s.List) > 0 {
 		p.stmtList(s.List, indent);
-		p.linebreak(s.Rbrace.Line, maxStmtNewlines, true);
+		p.linebreak(s.Rbrace.Line, 1, maxStmtNewlines, true);
 	}
 	p.print(s.Rbrace, token.RBRACE);
 }
 
 
+// TODO(gri): Decide if this should be used more broadly. The printing code
+//            knows when to insert parentheses for precedence reasons, but
+//            need to be careful to keep them around type expressions.
+func stripParens(x ast.Expr) ast.Expr {
+	if px, hasParens := x.(*ast.ParenExpr); hasParens {
+		return stripParens(px.X);
+	}
+	return x;
+}
+
+
 func (p *printer) controlClause(isForStmt bool, init ast.Stmt, expr ast.Expr, post ast.Stmt) {
 	p.print(blank);
 	needsBlank := false;
 	if init == nil && post == nil {
 		// no semicolons required
 		if expr != nil {
-			p.expr(expr);
+			p.expr(stripParens(expr));
 			needsBlank = true;
 		}
 	} else {
@@ -929,7 +968,7 @@ func (p *printer) controlClause(isForStmt bool, init ast.Stmt, expr ast.Expr, po
 		}
 		p.print(token.SEMICOLON, blank);
 		if expr != nil {
-			p.expr(expr);
+			p.expr(stripParens(expr));
 			needsBlank = true;
 		}
 		if isForStmt {
@@ -962,9 +1001,12 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 		// nothing to do
 
 	case *ast.LabeledStmt:
-		p.print(-1, formfeed);
+		// whitespace printing is delayed, thus indentation adjustments
+		// take place before the previous newline/formfeed is printed
+		p.print(-1);
 		p.expr(s.Label);
-		p.print(token.COLON, tab, +1, formfeed);
+		p.print(token.COLON, tab, +1);
+		p.linebreak(s.Stmt.Pos().Line, 0, 1, true);
 		optSemi = p.stmt(s.Stmt);
 
 	case *ast.ExprStmt:
@@ -1011,7 +1053,14 @@ func (p *printer) stmt(stmt ast.Stmt) (optSemi bool) {
 		optSemi = true;
 		if s.Else != nil {
 			p.print(blank, token.ELSE, blank);
-			optSemi = p.stmt(s.Else);
+			switch s.Else.(type) {
+			case *ast.BlockStmt, *ast.IfStmt:
+				optSemi = p.stmt(s.Else);
+			default:
+				p.print(token.LBRACE, +1, formfeed);
+				p.stmt(s.Else);
+				p.print(-1, formfeed, token.RBRACE);
+			}
 		}
 
 	case *ast.CaseClause:
@@ -1267,14 +1316,35 @@ func (p *printer) decl(decl ast.Decl) (optSemi bool) {
 
 const maxDeclNewlines = 3  // maximum number of newlines between declarations
 
+func declToken(decl ast.Decl) (tok token.Token) {
+	tok = token.ILLEGAL;
+	switch d := decl.(type) {
+	case *ast.GenDecl:
+		tok = d.Tok;
+	case *ast.FuncDecl:
+		tok = token.FUNC;
+	}
+	return;
+}
+
+
 func (p *printer) file(src *ast.File) {
 	p.leadComment(src.Doc);
 	p.print(src.Pos(), token.PACKAGE, blank);
 	p.expr(src.Name);
 
 	if len(src.Decls) > 0 {
+		tok := token.ILLEGAL;
 		for _, d := range src.Decls {
-			p.linebreak(d.Pos().Line, maxDeclNewlines, false);
+			prev := tok;
+			tok = declToken(d);
+			// if the declaration token changed (e.g., from CONST to TYPE)
+			// print an empty line between top-level declarations
+			min := 1;
+			if prev != tok {
+				min = 2;
+			}
+			p.linebreak(d.Pos().Line, min, maxDeclNewlines, false);
 			p.decl(d);
 		}
 	}

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

@@ -252,3 +252,27 @@ type _ interface {  // this comment must not change indentation
 	fffff();  // no blank between identifier and ()
 	gggggggggggg(x, y, z int) ();  // hurray
 }
+
+// formatting of variable declarations
+func _() {
+	type day struct { n int; short, long string };
+	var (
+		Sunday = day{ 0, "SUN", "Sunday" };
+		Monday = day{ 1, "MON", "Monday" };
+		Tuesday = day{ 2, "TUE", "Tuesday" };
+		Wednesday = day{ 3, "WED", "Wednesday" };
+		Thursday = day{ 4, "THU", "Thursday" };
+		Friday = day{ 5, "FRI", "Friday" };
+		Saturday = day{ 6, "SAT", "Saturday" };
+	)
+}
+
+
+// formatting of consecutive single-line functions
+func _() {}
+func _() {}
+func _() {}
+
+func _() {}  // an empty line before this function
+func _() {}
+func _() {}

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

@@ -43,6 +43,7 @@ import _ "fmt"
 
 // at least one empty line between declarations of different kind
 import _ "io"
+
 var _ int
 
 
@@ -250,3 +251,30 @@ type _ interface {	// this comment must not change indentation
 	fffff();					// no blank between identifier and ()
 	gggggggggggg(x, y, z int);	// hurray
 }
+
+// formatting of variable declarations
+func _() {
+	type day struct {
+		n			int;
+		short, long	string;
+	}
+	var (
+		Sunday		= day{0, "SUN", "Sunday"};
+		Monday		= day{1, "MON", "Monday"};
+		Tuesday		= day{2, "TUE", "Tuesday"};
+		Wednesday	= day{3, "WED", "Wednesday"};
+		Thursday	= day{4, "THU", "Thursday"};
+		Friday		= day{5, "FRI", "Friday"};
+		Saturday	= day{6, "SAT", "Saturday"};
+	)
+}
+
+
+// formatting of consecutive single-line functions
+func _() {}
+func _() {}
+func _() {}
+
+func _() {}	// an empty line before this function
+func _() {}
+func _() {}

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

@@ -34,6 +34,9 @@ func _() {
 	_ = s[1:2];
 	_ = s[a:b];
 	_ = s[0:len(s)];
+	_ = s[0]<<1;
+	_ = (s[0]<<1)&0xf;
+	_ = s[0] << 2 | s[1] >> 4;
 
 	// spaces around expressions of different precedence or expressions containing spaces
 	_ = a + -b;
@@ -49,6 +52,7 @@ func _() {
 	_ = s[a+b : len(s)];
 	_ = s[len(s) : -a];
 	_ = s[a : len(s)+1];
+	_ = s[a : len(s)+1]+s;
 
 	// spaces around operators with equal or lower precedence than comparisons
 	_ = a == b;
@@ -91,3 +95,12 @@ func _() {
 	_ = ([]T){};
 	_ = (map[int]T){};
 }
+
+
+func _() {
+	// TODO respect source line breaks in multi-line expressions
+	_ = a < b ||
+		b < a;
+	// TODO(gri): add more test cases
+	// TODO(gri): these comments should be indented
+}

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

@@ -34,6 +34,9 @@ func _() {
 	_ = s[1:2];
 	_ = s[a:b];
 	_ = s[0:len(s)];
+	_ = s[0]<<1;
+	_ = (s[0]<<1)&0xf;
+	_ = s[0]<<2 | s[1]>>4;
 
 	// spaces around expressions of different precedence or expressions containing spaces
 	_ = a + -b;
@@ -49,6 +52,7 @@ func _() {
 	_ = s[a+b : len(s)];
 	_ = s[len(s) : -a];
 	_ = s[a : len(s)+1];
+	_ = s[a : len(s)+1]+s;
 
 	// spaces around operators with equal or lower precedence than comparisons
 	_ = a == b;
@@ -91,3 +95,11 @@ func _() {
 	_ = ([]T){};
 	_ = (map[int]T){};
 }
+
+
+func _() {
+	// TODO respect source line breaks in multi-line expressions
+	_ = a < b || b < a;
+// TODO(gri): add more test cases
+// TODO(gri): these comments should be indented
+}

src/pkg/go/printer/testdata/linebreaks.go

@@ -14,6 +14,74 @@ import (
 	"testing";
 )
 
+type writerTestEntry struct {
+	header *Header;
+	contents string;
+}
+
+type writerTest struct {
+	file string;  // filename of expected output
+	entries []*writerTestEntry;
+}
+
+var writerTests = []*writerTest{
+	&writerTest{
+		file: "testdata/writer.tar",
+		entries: []*writerTestEntry{
+			&writerTestEntry{
+				header: &Header{
+					Name: "small.txt",
+					Mode: 0640,
+					Uid: 73025,
+					Gid: 5000,
+					Size: 5,
+					Mtime: 1246508266,
+					Typeflag: '0',
+					Uname: "dsymonds",
+					Gname: "eng",
+				},
+				contents: "Kilts",
+			},
+			&writerTestEntry{
+				header: &Header{
+					Name: "small2.txt",
+					Mode: 0640,
+					Uid: 73025,
+					Gid: 5000,
+					Size: 11,
+					Mtime: 1245217492,
+					Typeflag: '0',
+					Uname: "dsymonds",
+					Gname: "eng",
+				},
+				contents: "Google.com\n",
+			},
+		}
+	},
+	// The truncated test file was produced using these commands:
+	//   dd if=/dev/zero bs=1048576 count=16384 > /tmp/16gig.txt
+	//   tar -b 1 -c -f- /tmp/16gig.txt | dd bs=512 count=8 > writer-big.tar
+	&writerTest{
+		file: "testdata/writer-big.tar",
+		entries: []*writerTestEntry{
+			&writerTestEntry{
+				header: &Header{
+					Name: "tmp/16gig.txt",
+					Mode: 0640,
+					Uid: 73025,
+					Gid: 5000,
+					Size: 16 << 30,
+					Mtime: 1254699560,
+					Typeflag: '0',
+					Uname: "dsymonds",
+					Gname: "eng",
+				},
+				// no contents
+			},
+		},
+	},
+}
+
 type untarTest struct {
 	file string;
 	headers []*Header;
@@ -114,6 +182,16 @@ var facts = map[int] string {
 		"51185210916864000000000000000000000000",
 }
 
+func usage() {
+	fmt.Fprintf(os.Stderr,
+		// TODO(gri): the 2nd string of this string list should not be indented
+		"usage: godoc package [name ...]\n"
+		"	godoc -http=:6060\n"
+	);
+	flag.PrintDefaults();
+	os.Exit(2);
+}
+
 func TestReader(t *testing.T) {
 testLoop:
 	for i, test := range untarTests {

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

@@ -14,6 +14,71 @@ import (
 	"testing";
 )
 
+type writerTestEntry struct {
+	header		*Header;
+	contents	string;
+}
+
+type writerTest struct {
+	file	string;	// filename of expected output
+	entries	[]*writerTestEntry;
+}
+
+var writerTests = []*writerTest{
+	&writerTest{
+		file: "testdata/writer.tar",
+		entries: []*writerTestEntry{
+			&writerTestEntry{
+				header: &Header{
+					Name: "small.txt",
+					Mode: 0640,
+					Uid: 73025,
+					Gid: 5000,
+					Size: 5,
+					Mtime: 1246508266,
+					Typeflag: '0',
+					Uname: "dsymonds",
+					Gname: "eng",
+				},
+				contents: "Kilts",
+			},
+			&writerTestEntry{
+				header: &Header{
+					Name: "small2.txt",
+					Mode: 0640,
+					Uid: 73025,
+					Gid: 5000,
+					Size: 11,
+					Mtime: 1245217492,
+					Typeflag: '0',
+					Uname: "dsymonds",
+					Gname: "eng",
+				},
+				contents: "Google.com\n",
+			},
+		},
+	},
+	// The truncated test file was produced using these commands:
+	//   dd if=/dev/zero bs=1048576 count=16384 > /tmp/16gig.txt
+	//   tar -b 1 -c -f- /tmp/16gig.txt | dd bs=512 count=8 > writer-big.tar
+	&writerTest{
+		file: "testdata/writer-big.tar",
+		entries: []*writerTestEntry{&writerTestEntry{header: &Header{
+			Name: "tmp/16gig.txt",
+			Mode: 0640,
+			Uid: 73025,
+			Gid: 5000,
+			Size: 16<<30,
+			Mtime: 1254699560,
+			Typeflag: '0',
+			Uname: "dsymonds",
+			Gname: "eng",
+		}
+		// no contents
+		}},
+	},
+}
+
 type untarTest struct {
 	file	string;
 	headers	[]*Header;
@@ -109,15 +174,21 @@ var facts = map[int]string{
 	2: "2",
 	10: "3628800",
 	20: "2432902008176640000",
-	100:
-		"933262154439441526816992388562667004907159682643816214685929"
+	100: "933262154439441526816992388562667004907159682643816214685929"
 		"638952175999932299156089414639761565182862536979208272237582"
-		"51185210916864000000000000000000000000"
-	,
+		"51185210916864000000000000000000000000",
 }
 
-func TestReader(t *testing.T) {
+func usage() {
+	fmt.Fprintf(os.Stderr,
+		// TODO(gri): the 2nd string of this string list should not be indented
+		"usage: godoc package [name ...]\n"
+			"	godoc -http=:6060\n");
+	flag.PrintDefaults();
+	os.Exit(2);
+}
 
+func TestReader(t *testing.T) {
 testLoop:
 	for i, test := range untarTests {
 		f, err := os.Open(test.file, os.O_RDONLY, 0444);
@@ -134,10 +205,8 @@ testLoop:
 				continue testLoop;
 			}
 			if !reflect.DeepEqual(hdr, header) {
-				t.Errorf(
-					"test %d, entry %d: Incorrect header:\nhave %+v\nwant %+v",
-					i, j, *hdr, *header
-				);
+				t.Errorf("test %d, entry %d: Incorrect header:\nhave %+v\nwant %+v",
+					i, j, *hdr, *header);
 			}
 		}
 		hdr, err := tr.Next();

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

@@ -14,6 +14,8 @@ func _() {
 	if;{}  // no semicolon printed
 	if expr{}
 	if;expr{}  // no semicolon printed
+	if (expr){}  // no parens printed
+	if;((expr)){}  // no semicolon and parens printed
 	if x:=expr;{
 	use(x)}
 	if x:=expr; expr {use(x)}
@@ -26,6 +28,8 @@ func _() {
 	switch;{}  // no semicolon printed
 	switch expr {}
 	switch;expr{}  // no semicolon printed
+	switch (expr) {}  // no parens printed
+	switch;((expr)){}  // no semicolon and parens printed
 	switch x := expr; { default:use(
 x)
 	}
@@ -51,6 +55,13 @@ func _() {
 		use(x);
 		use(x);
 	}
+
+	switch x {
+	case 0:
+		use(x);
+	case 1:  // this comment should have no effect on the previous or next line
+		use(x);
+	}
 }
 
 
@@ -58,9 +69,11 @@ func _() {
 func _() {
 	for{}
 	for expr {}
-	for;;{}  // no semicolon printed
+	for (expr) {}  // no parens printed
+	for;;{}  // no semicolons printed
 	for x :=expr;; {use( x)}
-	for; expr;{}  // no semicolon printed
+	for; expr;{}  // no semicolons printed
+	for; ((expr));{}  // no semicolons and parens printed
 	for; ; expr = false {}
 	for x :=expr; expr; {use(x)}
 	for x := expr;; expr=false {use(x)}
@@ -101,3 +114,23 @@ func _() {
 
 	}
 }
+
+
+// Formatting around labels.
+func _() {
+	L:
+}
+
+
+func _() {
+	L: _ = 0;
+}
+
+
+func _() {
+	for {
+	L1: _ = 0;
+	L2:
+		_ = 0;
+	}
+}

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

@@ -14,6 +14,8 @@ func _() {
 	if {}	// no semicolon printed
 	if expr {}
 	if expr {}	// no semicolon printed
+	if expr {}	// no parens printed
+	if expr {}	// no semicolon and parens printed
 	if x := expr; {
 		use(x);
 	}
@@ -29,6 +31,8 @@ func _() {
 	switch {}	// no semicolon printed
 	switch expr {}
 	switch expr {}	// no semicolon printed
+	switch expr {}	// no parens printed
+	switch expr {}	// no semicolon and parens printed
 	switch x := expr; {
 	default:
 		use(x);
@@ -57,6 +61,13 @@ func _() {
 		use(x);
 		use(x);
 	}
+
+	switch x {
+	case 0:
+		use(x);
+	case 1:	// this comment should have no effect on the previous or next line
+		use(x);
+	}
 }
 
 
@@ -64,11 +75,13 @@ func _() {
 func _() {
 	for {}
 	for expr {}
-	for {}	// no semicolon printed
+	for expr {}	// no parens printed
+	for {}		// no semicolons printed
 	for x := expr; ; {
 		use(x);
 	}
-	for expr {}	// no semicolon printed
+	for expr {}	// no semicolons printed
+	for expr {}	// no semicolons and parens printed
 	for ; ; expr = false {}
 	for x := expr; expr; {
 		use(x);
@@ -116,3 +129,24 @@ func _() {
 
 	}
 }
+
+
+// Formatting around labels.
+func _() {
+L:
+	;
+}
+
+
+func _() {
+L:	_ = 0;
+}
+
+
+func _() {
+	for {
+	L1:	_ = 0;
+	L2:
+		_ = 0;
+	}
+}