From: Robert Griesemer Date: Sat, 3 Oct 2009 05:24:05 +0000 (-0700) Subject: - improved handling of white space around declarations and statements X-Git-Tag: weekly.2009-11-06~425 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=4b3a13d37971fb07468383dc46c8947dda4aac41;p=gostls13.git - 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 --- diff --git a/src/pkg/go/printer/printer.go b/src/pkg/go/printer/printer.go index 21c8912890..b39fbe4d1d 100644 --- a/src/pkg/go/printer/printer.go +++ b/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); diff --git a/src/pkg/go/printer/printer_test.go b/src/pkg/go/printer/printer_test.go index 91e3f2ec35..2f98eacf47 100644 --- a/src/pkg/go/printer/printer_test.go +++ b/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 }, diff --git a/src/pkg/go/printer/testdata/comments.golden b/src/pkg/go/printer/testdata/comments.golden index 40f81c194e..10fce681cf 100644 --- a/src/pkg/go/printer/testdata/comments.golden +++ b/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: diff --git a/src/pkg/go/printer/testdata/comments.x b/src/pkg/go/printer/testdata/comments.x index 9450119c68..5e22c6b8dd 100644 --- a/src/pkg/go/printer/testdata/comments.x +++ b/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; diff --git a/src/pkg/go/printer/testdata/declarations.go b/src/pkg/go/printer/testdata/declarations.go index e853eb55c7..cd7a2338e7 100644 --- a/src/pkg/go/printer/testdata/declarations.go +++ b/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; diff --git a/src/pkg/go/printer/testdata/declarations.golden b/src/pkg/go/printer/testdata/declarations.golden index 4181b05ecd..ed012ee833 100644 --- a/src/pkg/go/printer/testdata/declarations.golden +++ b/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; ) diff --git a/src/pkg/go/printer/testdata/empty.go b/src/pkg/go/printer/testdata/empty.go new file mode 100644 index 0000000000..a055f47581 --- /dev/null +++ b/src/pkg/go/printer/testdata/empty.go @@ -0,0 +1,5 @@ +// a comment at the beginning of the file + +package empty + +// a comment at the end of the file diff --git a/src/pkg/go/printer/testdata/empty.golden b/src/pkg/go/printer/testdata/empty.golden new file mode 100644 index 0000000000..a055f47581 --- /dev/null +++ b/src/pkg/go/printer/testdata/empty.golden @@ -0,0 +1,5 @@ +// a comment at the beginning of the file + +package empty + +// a comment at the end of the file diff --git a/src/pkg/go/printer/testdata/expressions.golden b/src/pkg/go/printer/testdata/expressions.golden index f772953fce..41b027b98a 100644 --- a/src/pkg/go/printer/testdata/expressions.golden +++ b/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){}; diff --git a/src/pkg/go/printer/testdata/statements.go b/src/pkg/go/printer/testdata/statements.go index b568bbf7ab..b4a52058e3 100644 --- a/src/pkg/go/printer/testdata/statements.go +++ b/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); + + } } diff --git a/src/pkg/go/printer/testdata/statements.golden b/src/pkg/go/printer/testdata/statements.golden index 93f1064d78..ef46df6e84 100644 --- a/src/pkg/go/printer/testdata/statements.golden +++ b/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); + + } }