From 49d2d986978f3b3654ff284fbcbeb4c32ec55fad Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Fri, 9 Mar 2012 11:05:50 -0800 Subject: [PATCH] go/printer, gofmt: nicer formatting of multi-line returns This affects corner (test) cases only; gofmt -w src misc doesn't cause any changes. - added additional test cases - removed doIndent parameter from printer.valueSpec (was always false) - gofmt -w src misc causes no changes Fixes #1207. R=dsymonds, rsc CC=golang-dev https://golang.org/cl/5786060 --- src/pkg/go/printer/nodes.go | 56 ++++++++++++++++-- src/pkg/go/printer/testdata/statements.golden | 59 ++++++++++++++++--- src/pkg/go/printer/testdata/statements.input | 43 ++++++++++++++ 3 files changed, 145 insertions(+), 13 deletions(-) diff --git a/src/pkg/go/printer/nodes.go b/src/pkg/go/printer/nodes.go index 05b4ef59a2..6be3c09382 100644 --- a/src/pkg/go/printer/nodes.go +++ b/src/pkg/go/printer/nodes.go @@ -15,7 +15,7 @@ import ( "unicode/utf8" ) -// Other formatting issues: +// Formatting issues: // - better comment formatting for /*-style comments at the end of a line (e.g. a declaration) // when the comment spans multiple lines; if such a comment is just two lines, formatting is // not idempotent @@ -964,6 +964,41 @@ func (p *printer) controlClause(isForStmt bool, init ast.Stmt, expr ast.Expr, po } } +// indentList reports whether an expression list would look better if it +// were indented wholesale (starting with the very first element, rather +// than starting at the first line break). +// +func (p *printer) indentList(list []ast.Expr) bool { + // Heuristic: indentList returns true if there are more than one multi- + // line element in the list, or if there is any element that is not + // starting on the same line as the previous one ends. + if len(list) >= 2 { + var b = p.lineFor(list[0].Pos()) + var e = p.lineFor(list[len(list)-1].End()) + if 0 < b && b < e { + // list spans multiple lines + n := 0 // multi-line element count + line := b + for _, x := range list { + xb := p.lineFor(x.Pos()) + xe := p.lineFor(x.End()) + if line < xb { + // x is not starting on the same + // line as the previous one ended + return true + } + if xb < xe { + // x is a multi-line element + n++ + } + line = xe + } + return n > 1 + } + } + return false +} + func (p *printer) stmt(stmt ast.Stmt, nextIsRBrace bool) { p.print(stmt.Pos()) @@ -1030,7 +1065,18 @@ func (p *printer) stmt(stmt ast.Stmt, nextIsRBrace bool) { p.print(token.RETURN) if s.Results != nil { p.print(blank) - p.exprList(s.Pos(), s.Results, 1, 0, token.NoPos) + // Use indentList heuristic to make corner cases look + // better (issue 1207). A more systematic approach would + // always indent, but this would cause significant + // reformatting of the code base and not necessarily + // lead to more nicely formatted code in general. + if p.indentList(s.Results) { + p.print(indent) + p.exprList(s.Pos(), s.Results, 1, noIndent, token.NoPos) + p.print(unindent) + } else { + p.exprList(s.Pos(), s.Results, 1, 0, token.NoPos) + } } case *ast.BranchStmt: @@ -1200,9 +1246,9 @@ func keepTypeColumn(specs []ast.Spec) []bool { return m } -func (p *printer) valueSpec(s *ast.ValueSpec, keepType, doIndent bool) { +func (p *printer) valueSpec(s *ast.ValueSpec, keepType bool) { p.setComment(s.Doc) - p.identList(s.Names, doIndent) // always present + p.identList(s.Names, false) // always present extraTabs := 3 if s.Type != nil || keepType { p.print(vtab) @@ -1290,7 +1336,7 @@ func (p *printer) genDecl(d *ast.GenDecl) { if i > 0 { p.linebreak(p.lineFor(s.Pos()), 1, ignore, newSection) } - p.valueSpec(s.(*ast.ValueSpec), keepType[i], false) + p.valueSpec(s.(*ast.ValueSpec), keepType[i]) newSection = p.isMultiLine(s) } } else { diff --git a/src/pkg/go/printer/testdata/statements.golden b/src/pkg/go/printer/testdata/statements.golden index ffca21edba..4d70617bf1 100644 --- a/src/pkg/go/printer/testdata/statements.golden +++ b/src/pkg/go/printer/testdata/statements.golden @@ -55,12 +55,24 @@ func _f() { return T{ 1, 2, - }, + }, nil + return T{ + 1, + 2, + }, + T{ + x: 3, + y: 4, + }, nil + return T{ + 1, + 2, + }, nil return T{ - 1, - 2, - }, + 1, + 2, + }, T{ x: 3, y: 4, @@ -70,10 +82,10 @@ func _f() { z return func() {} return func() { - _ = 0 - }, T{ - 1, 2, - } + _ = 0 + }, T{ + 1, 2, + } return func() { _ = 0 } @@ -84,6 +96,37 @@ func _f() { } } +// Formatting of multi-line returns: test cases from issue 1207. +func F() (*T, os.Error) { + return &T{ + X: 1, + Y: 2, + }, + nil +} + +func G() (*T, *T, os.Error) { + return &T{ + X: 1, + Y: 2, + }, + &T{ + X: 3, + Y: 4, + }, + nil +} + +func _() interface{} { + return &fileStat{ + name: basename(file.name), + size: mkSize(d.FileSizeHigh, d.FileSizeLow), + modTime: mkModTime(d.LastWriteTime), + mode: mkMode(d.FileAttributes), + sys: mkSysFromFI(&d), + }, nil +} + // Formatting of if-statement headers. func _() { if true { diff --git a/src/pkg/go/printer/testdata/statements.input b/src/pkg/go/printer/testdata/statements.input index 99945e9551..bd03bc98b7 100644 --- a/src/pkg/go/printer/testdata/statements.input +++ b/src/pkg/go/printer/testdata/statements.input @@ -52,6 +52,18 @@ func _f() { 3}, 3, } + return T{ + 1, + 2, + }, nil + return T{ + 1, + 2, + }, + T{ + x: 3, + y: 4, + }, nil return T{ 1, 2, @@ -84,6 +96,37 @@ func _f() { } } +// Formatting of multi-line returns: test cases from issue 1207. +func F() (*T, os.Error) { + return &T{ + X: 1, + Y: 2, + }, + nil +} + +func G() (*T, *T, os.Error) { + return &T{ + X: 1, + Y: 2, + }, + &T{ + X: 3, + Y: 4, + }, + nil +} + +func _() interface{} { + return &fileStat{ + name: basename(file.name), + size: mkSize(d.FileSizeHigh, d.FileSizeLow), + modTime: mkModTime(d.LastWriteTime), + mode: mkMode(d.FileAttributes), + sys: mkSysFromFI(&d), + }, nil +} + // Formatting of if-statement headers. func _() { if true {} -- 2.50.0