Repository: mvdan/gofumpt
Pull request #218 (2022-04-03 05:26:29)

format: add fuzzer and fix two panics
(see commit messages - please do not squash)




Patch:
diff --git a/format/format.go b/format/format.go
index 727acce..7b9f455 100644
--- a/format/format.go
+++ b/format/format.go
@@ -316,23 +316,39 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 	switch node := c.Node().(type) {
 	case *ast.File:
 		// Join contiguous lone var/const/import lines.
-		// Abort if there are empty lines or comments in between,
-		// including a leading comment, which could be a directive.
+		// Abort if there are empty lines in between,
+		// including a leading comment if it's a directive.
 		newDecls := make([]ast.Decl, 0, len(node.Decls))
 		for i := 0; i < len(node.Decls); {
 			newDecls = append(newDecls, node.Decls[i])
 			start, ok := node.Decls[i].(*ast.GenDecl)
-			if !ok || isCgoImport(start) || start.Doc != nil {
+			if !ok || isCgoImport(start) || containsAnyDirective(start.Doc) {
 				i++
 				continue
 			}
 			lastPos := start.Pos()
+		contLoop:
 			for i++; i < len(node.Decls); {
 				cont, ok := node.Decls[i].(*ast.GenDecl)
-				if !ok || cont.Tok != start.Tok || cont.Lparen != token.NoPos ||
-					f.Line(lastPos) < f.Line(cont.Pos())-1 || isCgoImport(cont) {
+				if !ok || cont.Tok != start.Tok || cont.Lparen != token.NoPos || isCgoImport(cont) {
 					break
 				}
+				// Are there things between these two declarations? e.g. empty lines, comments, directives
+				// If so, break the chain on empty lines and directives, continue below for comments.
+				if f.Line(lastPos) < f.Line(cont.Pos())-1 {
+					// break on empty line
+					if cont.Doc == nil {
+						break
+					}
+					// break on directive
+					for i, comment := range cont.Doc.List {
+						if f.Line(comment.Slash) != f.Line(lastPos)+1+i || rxCommentDirective.MatchString(strings.TrimPrefix(comment.Text, "//")) {
+							break contLoop
+						}
+					}
+					// continue below for comments
+				}
+
 				start.Specs = append(start.Specs, cont.Specs...)
 				if c := f.inlineComment(cont.End()); c != nil {
 					// don't move an inline comment outside
@@ -361,7 +377,8 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 				pos = comments[0].Pos()
 			}
 
-			multi := f.Line(pos) < f.Line(decl.End())
+			// Note that we want End-1, as End is the character after the node.
+			multi := f.Line(pos) < f.Line(decl.End()-1)
 			if multi && lastMulti && f.Line(lastEnd)+1 == f.Line(pos) {
 				f.addNewline(lastEnd)
 			}
@@ -553,7 +570,7 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 					}
 				}
 				handleMultiLine(sign.Params)
-				if sign.Results != nil {
+				if sign.Results != nil && len(sign.Results.List) > 0 {
 					lastResultLine := f.Line(sign.Results.List[len(sign.Results.List)-1].End())
 					isLastResultOnParamClosingLine := sign.Params != nil && lastResultLine == f.Line(sign.Params.Closing)
 					if !isLastResultOnParamClosingLine {
@@ -1018,3 +1035,16 @@ func setPos(v reflect.Value, pos token.Pos) {
 		}
 	}
 }
+
+func containsAnyDirective(group *ast.CommentGroup) bool {
+	if group == nil {
+		return false
+	}
+	for _, comment := range group.List {
+		body := strings.TrimPrefix(comment.Text, "//")
+		if rxCommentDirective.MatchString(body) {
+			return true
+		}
+	}
+	return false
+}


Test patch:
diff --git a/format/fuzz_test.go b/format/fuzz_test.go
new file mode 100644
index 0000000..8145944
--- /dev/null
+++ b/format/fuzz_test.go
@@ -0,0 +1,67 @@
+// Copyright (c) 2021, Daniel Martí <mvdan@mvdan.cc>
+// See LICENSE for licensing information
+
+//go:build go1.18
+// +build go1.18
+
+package format
+
+import (
+	"errors"
+	"fmt"
+	"go/scanner"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	qt "github.com/frankban/quicktest"
+	"golang.org/x/tools/txtar"
+)
+
+func FuzzFormat(f *testing.F) {
+	// Initialize the corpus with the Go files from our test scripts.
+	paths, err := filepath.Glob(filepath.Join("..", "testdata", "scripts", "*.txt"))
+	qt.Assert(f, err, qt.IsNil)
+	qt.Assert(f, paths, qt.Not(qt.HasLen), 0)
+	for _, path := range paths {
+		archive, err := txtar.ParseFile(path)
+		qt.Assert(f, err, qt.IsNil)
+		for _, file := range archive.Files {
+			f.Logf("adding %s from %s", file.Name, path)
+			if strings.HasSuffix(file.Name, ".go") || strings.Contains(file.Name, ".go.") {
+				f.Add(string(file.Data), int8(18), false) // -lang=1.18
+				f.Add(string(file.Data), int8(1), false)  // -lang=1.1
+				f.Add(string(file.Data), int8(18), true)  // -lang=1.18 -extra
+			}
+		}
+	}
+
+	f.Fuzz(func(t *testing.T, src string,
+		majorVersion int8, // Empty version if negative, 1.N otherwise.
+		extraRules bool,
+	) {
+		// TODO: also fuzz Options.ModulePath
+		opts := Options{ExtraRules: extraRules}
+		if majorVersion >= 0 {
+			opts.LangVersion = fmt.Sprintf("1.%d", majorVersion)
+		}
+
+		orig := []byte(src)
+		formatted, err := Source(orig, opts)
+		if errors.As(err, &scanner.ErrorList{}) {
+			return // invalid syntax from parsing
+		}
+		qt.Assert(t, err, qt.IsNil)
+		_ = formatted
+
+		// TODO: verify that the result is idempotent
+
+		// TODO: verify that, if the input was valid Go 1.N syntax,
+		// so is the output (how? go/parser lacks an option)
+
+		// TODO: check calling format.Node directly as well
+
+		qt.Assert(t, string(orig), qt.Equals, src,
+			qt.Commentf("input source bytes were modified"))
+	})
+}
diff --git a/format/testdata/fuzz/FuzzFormat/18c862f09f82fe57f536e7ab4b1bd63daecc2cba8189530bb0eb77b8cef6f798 b/format/testdata/fuzz/FuzzFormat/18c862f09f82fe57f536e7ab4b1bd63daecc2cba8189530bb0eb77b8cef6f798
new file mode 100644
index 0000000..6bd1694
--- /dev/null
+++ b/format/testdata/fuzz/FuzzFormat/18c862f09f82fe57f536e7ab4b1bd63daecc2cba8189530bb0eb77b8cef6f798
@@ -0,0 +1,4 @@
+go test fuzz v1
+string("package A\nfunc A000000000(A000000000000,\nA00000000)(){\"\"}")
+int8(62)
+bool(true)
diff --git a/format/testdata/fuzz/FuzzFormat/948d1d5be3c838b207d345a3ac57e97bb3b77788cb5039a65994967490c49baa b/format/testdata/fuzz/FuzzFormat/948d1d5be3c838b207d345a3ac57e97bb3b77788cb5039a65994967490c49baa
new file mode 100644
index 0000000..3c577fb
--- /dev/null
+++ b/format/testdata/fuzz/FuzzFormat/948d1d5be3c838b207d345a3ac57e97bb3b77788cb5039a65994967490c49baa
@@ -0,0 +1,4 @@
+go test fuzz v1
+string("package A\nvar A A\nvar A A")
+int8(18)
+bool(false)
diff --git a/testdata/scripts/decl-group-many.txt b/testdata/scripts/decl-group-many.txt
index 0678b6e..eaaefb9 100644
--- a/testdata/scripts/decl-group-many.txt
+++ b/testdata/scripts/decl-group-many.txt
@@ -18,15 +18,26 @@ const four = 'r'
 var not = 'a'
 
 var v1 = 's'
-// comment, e.g. directive
+//go:embed hello.txt
+var v2 = 'd'
+
+var v1 = 's'
+// comment line 1
+// comment line 2
 var v2 = 'd'
 
 var v1 = "mixed"
 const c1 = "mixed"
 
-// comment, e.g. directive
+//go:embed hello.txt
+var v1 = 's'
+var v2 = 'd'
+var v3 = 'd'
+
+// comment
 var v1 = 's'
 var v2 = 'd'
+/* comment */
 var v3 = 'd'
 
 const inline1 = "s1" // c1
@@ -51,14 +62,21 @@ var not = 'a'
 
 var v1 = 's'
 
-// comment, e.g. directive
+//go:embed hello.txt
 var v2 = 'd'
 
+var (
+	v1 = 's'
+	// comment line 1
+	// comment line 2
+	v2 = 'd'
+)
+
 var v1 = "mixed"
 
 const c1 = "mixed"
 
-// comment, e.g. directive
+//go:embed hello.txt
 var v1 = 's'
 
 var (
@@ -66,6 +84,14 @@ var (
 	v3 = 'd'
 )
 
+// comment
+var (
+	v1 = 's'
+	v2 = 'd'
+	/* comment */
+	v3 = 'd'
+)
+
 const (
 	inline1 = "s1" // c1
 	inline2 = "s2" // c2