]> Cypherpunks repositories - gostls13.git/commitdiff
various go printer fixes:
authorRobert Griesemer <gri@golang.org>
Tue, 6 Oct 2009 02:37:34 +0000 (19:37 -0700)
committerRobert Griesemer <gri@golang.org>
Tue, 6 Oct 2009 02:37:34 +0000 (19:37 -0700)
- 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
src/pkg/go/printer/testdata/declarations.go
src/pkg/go/printer/testdata/declarations.golden
src/pkg/go/printer/testdata/expressions.go
src/pkg/go/printer/testdata/expressions.golden
src/pkg/go/printer/testdata/linebreaks.go
src/pkg/go/printer/testdata/linebreaks.golden
src/pkg/go/printer/testdata/statements.go
src/pkg/go/printer/testdata/statements.golden

index b39fbe4d1dbc5314e1f0242ba6d12208aa473334..443546cd43182057c742dff0f79716e16b91da82 100644 (file)
@@ -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);
                }
        }
index cd7a2338e7aa5e8c929080acd33d7ca2005df5ed..5642412904993e0d1d23d83b18f401b998f33195 100644 (file)
@@ -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 _() {}
index ed012ee8336eed3618aadbbc984dc3de97774dd3..9ea0b597282dc48b3ae3790a4236081460440fc7 100644 (file)
@@ -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 _() {}
index c5e309b8bae64badbed881f43944d11a39c330e7..aa43a3b1551774323b7d601db771eea6bb7d626f 100644 (file)
@@ -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
+}
index 41b027b98acf16fdf98466f5bcf0d23457679ce6..61adaca9cffe478751edaaf4e98bdb3b493f9e35 100644 (file)
@@ -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
+}
index 84dc53d012413124465ea2a47138a613e0ac2d53..6624e55d0b456e08dabede2184b6364a1a54b3b0 100644 (file)
@@ -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 {
index 0aa1c92d83b45dba696c5ce582e3ef97baa26170..54684cef8478939138a802bfce79ae2867b71f06 100644 (file)
@@ -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();
index b4a52058e33443d23d6b7e3aa38ec0c286c2f2b3..5c38a4ac458076881ab5ca2d8440b67cf9e76832 100644 (file)
@@ -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;
+       }
+}
index ef46df6e84601b6426548ebf8cd29ca9f861501c..5826c4abca17d631d4001d39c8d1fd36d071484c 100644 (file)
@@ -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;
+       }
+}