format: break long lines

WORK IN PROGRESS. See the test cases for what the intended effect is.
mvdan · Jan 7, 2021 · fe132d1 · fe132d1
1 parent 7e21c17
commit fe132d1
Show file tree

Hide file tree

Showing 2 changed files with 205 additions and 5 deletions.
diff --git a/format/format.go b/format/format.go
@@ -81,24 +81,43 @@ func File(fset *token.FileSet, file *ast.File, opts Options) {
 	}
 	pre := func(c *astutil.Cursor) bool {
 		f.applyPre(c)
-		if _, ok := c.Node().(*ast.BlockStmt); ok {
+		switch node := c.Node().(type) {
+		case *ast.FieldList:
+			ft, _ := c.Parent().(*ast.FuncType)
+			if ft != nil && ft.Params == node {
+				// TODO: investigate if this is worthwhile
+				// f.longLineExtra = 20
+			}
+			if ft != nil && ft.Results == node {
+				f.longLineExtra = -1
+			}
+		case *ast.BlockStmt:
 			f.blockLevel++
 		}
 		return true
 	}
 	post := func(c *astutil.Cursor) bool {
-		if _, ok := c.Node().(*ast.BlockStmt); ok {
+		switch c.Node().(type) {
+		case *ast.FieldList:
+			f.longLineExtra = 0
+		case *ast.BlockStmt:
 			f.blockLevel--
 		}
 		return true
 	}
 	astutil.Apply(file, pre, post)
 }
 
-// Multiline nodes which could fit on a single line under this many
-// bytes may be collapsed onto a single line.
+// Multiline nodes which could easily fit on a single line under this many bytes
+// may be collapsed onto a single line.
 const shortLineLimit = 60
 
+// Single-line nodes which take over this many bytes, and could easily be split
+// into two lines of at least its 40%, may be split.
+const longLineBase = 100
+
+// const minSplitLength = 40
+
 var rxOctalInteger = regexp.MustCompile(`\A0[0-7_]+\z`)
 
 type fumpter struct {
@@ -109,7 +128,8 @@ type fumpter struct {
 
 	astFile *ast.File
 
-	blockLevel int
+	blockLevel    int
+	longLineExtra int
 }
 
 func (f *fumpter) commentsBetween(p1, p2 token.Pos) []*ast.CommentGroup {
@@ -210,6 +230,29 @@ func (f *fumpter) printLength(node ast.Node) int {
 	return int(count) + (f.blockLevel * 8)
 }
 
+func (f *fumpter) tabbedColumn(p token.Pos) int {
+	col := f.Position(p).Column
+
+	// Like in printLength, add an approximation of the indentation level.
+	// Since any existing tabs were already counted as one column, multiply
+	// the level by 7.
+	return col + (f.blockLevel * 7)
+}
+
+func (f *fumpter) lineEnd(line int) token.Pos {
+	if line < 1 {
+		panic("illegal line number")
+	}
+	total := f.LineCount()
+	if line > total {
+		panic("illegal line number")
+	}
+	if line == total {
+		return f.astFile.End()
+	}
+	return f.LineStart(line+1) - 1
+}
+
 // rxCommentDirective covers all common Go comment directives:
 //
 //   //go:        | standard Go directives, like go:noinline
@@ -226,6 +269,8 @@ var rxCommentDirective = regexp.MustCompile(`^([a-z]+:[a-z]+|line\b|export\b|ext
 
 // visit takes either an ast.Node or a []ast.Stmt.
 func (f *fumpter) applyPre(c *astutil.Cursor) {
+	f.splitLongLine(c)
+
 	switch node := c.Node().(type) {
 	case *ast.File:
 		var lastMulti bool
@@ -537,6 +582,55 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 	}
 }
 
+func (f *fumpter) splitLongLine(c *astutil.Cursor) {
+	node := c.Node()
+	if node == nil || f.longLineExtra < 0 {
+		return
+	}
+
+	start := f.Position(node.Pos())
+	end := f.Position(node.End())
+	if c.Index() < 0 || start.Line != end.Line {
+		return
+	}
+	if comp := isComposite(node); comp != nil && len(comp.Elts) > 0 {
+		end = f.Position(comp.Lbrace + 1)
+	}
+	// Like in printLength, add an approximation of the indentation level.
+	// Since any existing tabs were already counted as one column, multiply
+	// the level by 7.
+	startCol := start.Column + f.blockLevel*7
+	endCol := end.Column + f.blockLevel*7
+
+	lineEnd := f.Position(f.lineEnd(start.Line))
+	// We subtract blockLevel to ignore indentation, to match secondLength.
+	firstLength := start.Column - f.blockLevel
+	if firstLength < 0 {
+		panic("negative length")
+	}
+	secondLength := lineEnd.Column - start.Column
+	if secondLength < 0 {
+		panic("negative length")
+	}
+	longLineLimit := longLineBase + f.longLineExtra
+	minSplitLength := int(0.4 * float64(longLineLimit))
+	if startCol > shortLineLimit && endCol > longLineLimit &&
+		firstLength >= minSplitLength && secondLength >= minSplitLength {
+		f.addNewline(node.Pos())
+	}
+}
+
+func isComposite(node ast.Node) *ast.CompositeLit {
+	switch node := node.(type) {
+	case *ast.CompositeLit:
+		return node
+	case *ast.UnaryExpr:
+		return isComposite(node.X) // e.g. &T{}
+	default:
+		return nil
+	}
+}
+
 func (f *fumpter) stmts(list []ast.Stmt) {
 	for i, stmt := range list {
 		ifs, ok := stmt.(*ast.IfStmt)

diff --git a/testdata/scripts/long-lines.txt b/testdata/scripts/long-lines.txt
@@ -0,0 +1,106 @@
+gofumpt -w .
+cmp foo.go foo.go.golden
+
+-- foo.go --
+package p
+
+func _() {
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, argument7, argument8, argument9, argument10); err != nil {
+		panic(err)
+	}
+
+	// Tiny arguments to ensure the length calculation is right.
+	if err := f(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10); err != nil {
+		panic(err)
+	}
+
+	// These wouldn't take significantly less horizontal space if split.
+	f(x, "one single very very very very very very very very very very very very very very very very long literal")
+	if err := f(x, "one single very very very very very very very very very very very very very very very very long literal"); err != nil {
+		panic(err)
+	}
+	{
+		{
+			{
+				{
+					println("first", "one single very very very very very very very very very very very very very long literal")
+				}
+			}
+		}
+	}
+
+	// Allow splitting at the start of sub-lists too.
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, someComplex{argument7, argument8, argument9}); err != nil {
+		panic(err)
+	}
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, &someComplex{argument7, argument8, argument9}); err != nil {
+		panic(err)
+	}
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, []someSlice{argument7, argument8, argument9}); err != nil {
+		panic(err)
+	}
+}
+
+// This line goes beyond the limit of 100, but splitting it would leave the
+// following line with just 20 non-indentation characters. Not worth it.
+func LongButNotWorthSplitting(argument1, argument2, argument3, argument4, argument5, argument6, argument7 int) bool {
+}
+
+// Never split result parameter lists, as that could easily add confusion with
+// extra input parameters.
+func NeverSplitResults(argument1, argument2, argument3, argument4, argument5 int) (result1 int, result2, result3, result4, result5 bool) {
+}
+-- foo.go.golden --
+package p
+
+func _() {
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, argument7,
+		argument8, argument9, argument10); err != nil {
+		panic(err)
+	}
+
+	// Tiny arguments to ensure the length calculation is right.
+	if err := f(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, 0,
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10); err != nil {
+		panic(err)
+	}
+
+	// These wouldn't take significantly less horizontal space if split.
+	f(x, "one single very very very very very very very very very very very very very very very very long literal")
+	if err := f(x, "one single very very very very very very very very very very very very very very very very long literal"); err != nil {
+		panic(err)
+	}
+	{
+		{
+			{
+				{
+					println("first", "one single very very very very very very very very very very very very very long literal")
+				}
+			}
+		}
+	}
+
+	// Allow splitting at the start of sub-lists too.
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, someComplex{
+		argument7, argument8, argument9}); err != nil {
+		panic(err)
+	}
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, &someComplex{
+		argument7, argument8, argument9}); err != nil {
+		panic(err)
+	}
+	if err := f(argument1, argument2, argument3, argument4, argument5, argument6, []someSlice{
+		argument7, argument8, argument9}); err != nil {
+		panic(err)
+	}
+}
+
+// This line goes beyond the limit of 100, but splitting it would leave the
+// following line with just 20 non-indentation characters. Not worth it.
+func LongButNotWorthSplitting(argument1, argument2, argument3, argument4, argument5, argument6, argument7 int) bool {
+}
+
+// Never split result parameter lists, as that could easily add confusion with
+// extra input parameters.
+func NeverSplitResults(argument1, argument2, argument3, argument4, argument5 int) (result1 int, result2, result3, result4, result5 bool) {
+}