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

diff --git a/src/pkg/go/printer/nodes.go b/src/pkg/go/printer/nodes.go
index 05b4ef59a2dc1957e1cf797e7e0a35764cb65651..6be3c09382e06b3b6b826dda59b56771a72f608a 100644 (file)
--- 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 ffca21edba9e3827cfcdf53550380ffa4957e155..4d70617bf13ec191658ecce11dcfb266cbb95711 100644 (file)
--- 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 99945e9551aaf195a65c688f6c538d1d91c7c3ea..bd03bc98b7791fa4b8ab6e4cadba687d20764a31 100644 (file)
--- 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 {}