From d24d34e18d4443ae5365044aaa0bce40a3dd853c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 12 May 2020 23:47:29 +0100 Subject: [PATCH] make "split long lines" idempotent --- format/format.go | 109 +++++++++++++++++--------------- testdata/scripts/long-lines.txt | 14 ++-- 2 files changed, 68 insertions(+), 55 deletions(-) diff --git a/format/format.go b/format/format.go index 9f5e98c..5576f72 100644 --- a/format/format.go +++ b/format/format.go @@ -97,6 +97,7 @@ func File(fset *token.FileSet, file *ast.File, opts Options) { return true } post := func(c *astutil.Cursor) bool { + f.applyPost(c) switch c.Node().(type) { case *ast.FieldList: f.longLineExtra = 0 @@ -267,7 +268,6 @@ func (f *fumpter) lineEnd(line int) token.Pos { // "go:generate", to prevent matching false positives like "https://site". var rxCommentDirective = regexp.MustCompile(`^([a-z]+:[a-z]+|line\b|export\b|extern\b|sys(nb)?\b|nolint\b)`) -// visit takes either an ast.Node or a []ast.Stmt. func (f *fumpter) applyPre(c *astutil.Cursor) { f.splitLongLine(c) @@ -465,6 +465,63 @@ func (f *fumpter) applyPre(c *astutil.Cursor) { f.removeLinesBetween(node.Lbrace, bodyPos) + case *ast.CaseClause: + f.stmts(node.Body) + openLine := f.Line(node.Case) + closeLine := f.Line(node.Colon) + if openLine == closeLine { + // nothing to do + break + } + if len(f.commentsBetween(node.Case, node.Colon)) > 0 { + // don't move comments + break + } + if f.printLength(node) > shortLineLimit { + // too long to collapse + break + } + f.removeLines(openLine, closeLine) + + case *ast.CommClause: + f.stmts(node.Body) + + case *ast.FieldList: + if node.NumFields() == 0 { + // Empty field lists should not contain a newline. + openLine := f.Line(node.Pos()) + closeLine := f.Line(node.End()) + f.removeLines(openLine, closeLine) + } + + // Merging adjacent fields (e.g. parameters) is disabled by default. + if !f.ExtraRules { + break + } + switch c.Parent().(type) { + case *ast.FuncDecl, *ast.FuncType, *ast.InterfaceType: + node.List = f.mergeAdjacentFields(node.List) + c.Replace(node) + case *ast.StructType: + // Do not merge adjacent fields in structs. + } + + case *ast.BasicLit: + // Octal number literals were introduced in 1.13. + if semver.Compare(f.LangVersion, "v1.13") >= 0 { + if node.Kind == token.INT && rxOctalInteger.MatchString(node.Value) { + node.Value = "0o" + node.Value[1:] + c.Replace(node) + } + } + } +} + +func (f *fumpter) applyPost(c *astutil.Cursor) { + switch node := c.Node().(type) { + // Adding newlines to composite literals happens as a "post" step, so + // that we can take into account whether "pre" steps added any newlines + // that would affect us here. case *ast.CompositeLit: if len(node.Elts) == 0 { // doesn't have elements @@ -529,56 +586,6 @@ func (f *fumpter) applyPre(c *astutil.Cursor) { f.addNewline(elem1.End()) } } - - case *ast.CaseClause: - f.stmts(node.Body) - openLine := f.Line(node.Case) - closeLine := f.Line(node.Colon) - if openLine == closeLine { - // nothing to do - break - } - if len(f.commentsBetween(node.Case, node.Colon)) > 0 { - // don't move comments - break - } - if f.printLength(node) > shortLineLimit { - // too long to collapse - break - } - f.removeLines(openLine, closeLine) - - case *ast.CommClause: - f.stmts(node.Body) - - case *ast.FieldList: - if node.NumFields() == 0 { - // Empty field lists should not contain a newline. - openLine := f.Line(node.Pos()) - closeLine := f.Line(node.End()) - f.removeLines(openLine, closeLine) - } - - // Merging adjacent fields (e.g. parameters) is disabled by default. - if !f.ExtraRules { - break - } - switch c.Parent().(type) { - case *ast.FuncDecl, *ast.FuncType, *ast.InterfaceType: - node.List = f.mergeAdjacentFields(node.List) - c.Replace(node) - case *ast.StructType: - // Do not merge adjacent fields in structs. - } - - case *ast.BasicLit: - // Octal number literals were introduced in 1.13. - if semver.Compare(f.LangVersion, "v1.13") >= 0 { - if node.Kind == token.INT && rxOctalInteger.MatchString(node.Value) { - node.Value = "0o" + node.Value[1:] - c.Replace(node) - } - } } } diff --git a/testdata/scripts/long-lines.txt b/testdata/scripts/long-lines.txt index e67978c..ea9f69f 100644 --- a/testdata/scripts/long-lines.txt +++ b/testdata/scripts/long-lines.txt @@ -1,6 +1,9 @@ -gofumpt -w . +gofumpt -w foo.go cmp foo.go foo.go.golden +gofumpt -d foo.go.golden +! stdout . + -- foo.go -- package p @@ -82,15 +85,18 @@ func _() { // Allow splitting at the start of sub-lists too. if err := f(argument1, argument2, argument3, argument4, argument5, argument6, someComplex{ - argument7, argument8, argument9}); err != nil { + argument7, argument8, argument9, + }); err != nil { panic(err) } if err := f(argument1, argument2, argument3, argument4, argument5, argument6, &someComplex{ - argument7, argument8, argument9}); err != nil { + argument7, argument8, argument9, + }); err != nil { panic(err) } if err := f(argument1, argument2, argument3, argument4, argument5, argument6, []someSlice{ - argument7, argument8, argument9}); err != nil { + argument7, argument8, argument9, + }); err != nil { panic(err) } }