"clothe" naked returns for the sake of clarity

When a function has named results, one can use a "naked return"
to implicitly return those variables. That is, the code

    func Foo() (err error) {
        return
    }

is functionally equivalent to

    func Foo() (err error) {
        return err
    }

Naked returns exist primarily for the sake of avoiding repetition,
but even the Go tour recommends that their use should be limited:

    Naked return statements should be used only in short functions,
    as with the example shown here. They can harm readability in longer functions.

Most Go functions out in the wild aren't short, and in practice,
naked returns tend to be avoided in larger or collaborative codebases
as they confuse the reader about whether any values are being returned,
or which values are being returned if any.

Given that gofumpt is already opinionated, and its use tends to be on
Go modules on the larger side or with multiple contributors,
it seems best to rewrite naked returns to "clothe" them.

Fixes #285.

Author: mvdan

README.md

@@ -435,7 +435,25 @@ type ZeroFields struct {
 
 </details>
 
-#### Extra rules behind `-extra`
+**Avoid naked returns for the sake of clarity**
+
+<details><summary><i>Example</i></summary>
+
+```go
+func Foo() (err error) {
+	return
+}
+```
+
+```go
+func Foo() (err error) {
+	return err
+}
+```
+
+</details>
+
+### Extra rules behind `-extra`
 
 **Adjacent parameters with the same type should be grouped together**
 

format/format.go

@@ -114,6 +114,9 @@ func File(fset *token.FileSet, file *ast.File, opts Options) {
 		switch node := c.Node().(type) {
 		case *ast.FuncDecl:
 			topFuncType = node.Type
+			f.parentFuncTypes = append(f.parentFuncTypes, node.Type)
+		case *ast.FuncLit:
+			f.parentFuncTypes = append(f.parentFuncTypes, node.Type)
 		case *ast.FieldList:
 			ft, _ := c.Parent().(*ast.FuncType)
 			if ft == nil || ft != topFuncType {
@@ -149,6 +152,8 @@ func File(fset *token.FileSet, file *ast.File, opts Options) {
 
 		// Reset minSplitFactor and blockLevel.
 		switch node := c.Node().(type) {
+		case *ast.FuncDecl, *ast.FuncLit:
+			f.parentFuncTypes = f.parentFuncTypes[:len(f.parentFuncTypes)-1]
 		case *ast.FuncType:
 			if node == topFuncType {
 				f.minSplitFactor = 0.4
@@ -185,6 +190,10 @@ type fumpter struct {
 	blockLevel int
 
 	minSplitFactor float64
+
+	// parentFuncTypes is a stack of parent function types,
+	// used to determine return type information when clothing naked returns.
+	parentFuncTypes []*ast.FuncType
 }
 
 func (f *fumpter) commentsBetween(p1, p2 token.Pos) []*ast.CommentGroup {
@@ -685,6 +694,36 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 	case *ast.AssignStmt:
 		// Only remove lines between the assignment token and the first right-hand side expression
 		f.removeLines(f.Line(node.TokPos), f.Line(node.Rhs[0].Pos()))
+
+	case *ast.ReturnStmt:
+		if len(node.Results) > 0 {
+			break
+		}
+		results := f.parentFuncTypes[len(f.parentFuncTypes)-1].Results
+		if results.NumFields() == 0 {
+			break
+		}
+
+		// The function has return values; let's clothe the return.
+		node.Results = make([]ast.Expr, 0, results.NumFields())
+	nameLoop:
+		for _, result := range results.List {
+			for _, ident := range result.Names {
+				name := ident.Name
+				if name == "_" { // we can't handle blank names just yet
+					node.Results = nil
+					break nameLoop
+				}
+				node.Results = append(node.Results, &ast.Ident{
+					// Use the Pos of the return statement, to not interfere with comment placement.
+					NamePos: node.Pos(),
+					Name:    name,
+				})
+			}
+		}
+		if len(node.Results) > 0 {
+			c.Replace(node)
+		}
 	}
 }
 

testdata/script/clothe-returns.txtar

@@ -0,0 +1,74 @@
+exec gofumpt -w foo.go
+cmp foo.go foo.go.golden
+
+exec gofumpt -d foo.go.golden
+! stdout .
+
+-- foo.go --
+package p
+
+func foo() (err error) {
+	if true {
+		return
+	}
+	if false {
+		return func() (err2 error) {
+			return
+		}
+	}
+	return
+}
+
+func bar() (_ int, err error) {
+	return
+}
+
+func baz() (a, b, c int) {
+	return
+}
+
+func qux() (file string, b int, err error) {
+	if err == nil {
+		return
+	}
+
+	// A comment
+	return
+}
+
+// quux does quuxy things
+func quux() {}
+-- foo.go.golden --
+package p
+
+func foo() (err error) {
+	if true {
+		return err
+	}
+	if false {
+		return func() (err2 error) {
+			return err2
+		}
+	}
+	return err
+}
+
+func bar() (_ int, err error) {
+	return
+}
+
+func baz() (a, b, c int) {
+	return a, b, c
+}
+
+func qux() (file string, b int, err error) {
+	if err == nil {
+		return file, b, err
+	}
+
+	// A comment
+	return file, b, err
+}
+
+// quux does quuxy things
+func quux() {}