merging adjacent parameters is now behind -extra

Since this rule was more controversial than others; in many cases,
explicitly separate types can be a way to signal that two parameters do
not belong together or are not related.

However, the feature is still useful. For example, when obtaining a
large chunk of generated Go code, or when code is written by new Go
developers who weren't aware of this language feature, it can still be
useful to automatically apply the rule and use tools like 'git diff' or
'git add -p' to manually vet whether or not the changes make sense.

Fixes #53.

Author: mvdan

flag.go

@@ -5,7 +5,10 @@ package main
 
 import "flag"
 
-var langVersion = flag.String("lang", "", "target Go version in the form 1.X (default from go.mod)")
+var (
+	langVersion = flag.String("lang", "", "target Go version in the form 1.X (default from go.mod)")
+	extraRules  = flag.Bool("extra", false, "enable extra rules which should be vetted by a human")
+)
 
 func init() {
 	// make -s default to true

format/format.go

@@ -38,6 +38,8 @@ type Options struct {
 	// is equivalent to "1.14.0". When empty, it is equivalent to "v1", to
 	// not use language features which could break programs.
 	LangVersion string
+
+	ExtraRules bool
 }
 
 // Source formats src in gofumpt's format, assuming that src holds a valid Go
@@ -474,6 +476,10 @@ func (f *fumpter) applyPre(c *astutil.Cursor) {
 		f.stmts(node.Body)
 
 	case *ast.FieldList:
+		// 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)

gen.go

@@ -252,7 +252,10 @@ func copyGofmt(pkg *Package) {
 		// This is the only gofumpt change on gofmt's codebase, besides changing
 		// the name in the usage text.
 		` + extraSrcLangVersion + `
-		gformat.File(fileSet, file, gformat.Options{LangVersion: *langVersion})
+		gformat.File(fileSet, file, gformat.Options{
+			LangVersion: *langVersion,
+			ExtraRules:  *extraRules,
+		})
 		`
 	for _, path := range sourceFiles(pkg) {
 		body := readFile(path)

gofmt.go

@@ -126,7 +126,10 @@ func processFile(filename string, in io.Reader, out io.Writer, stdin bool) error
 			*langVersion = string(out)
 		}
 	}
-	gformat.File(fileSet, file, gformat.Options{LangVersion: *langVersion})
+	gformat.File(fileSet, file, gformat.Options{
+		LangVersion: *langVersion,
+		ExtraRules:  *extraRules,
+	})
 
 	res, err := format(fileSet, file, sourceAdj, indentAdj, src, printer.Config{Mode: printerMode, Tabwidth: tabWidth})
 	if err != nil {

gofumports/internal/event/core/fast.go

@@ -22,7 +22,7 @@ func Log1(ctx context.Context, message string, t1 label.Label) {
 
 // Log2 takes a message and two labels and delivers a log event to the exporter.
 // It is a customized version of Print that is faster and does no allocation.
-func Log2(ctx context.Context, message string, t1, t2 label.Label) {
+func Log2(ctx context.Context, message string, t1 label.Label, t2 label.Label) {
 	Export(ctx, MakeEvent([3]label.Label{
 		keys.Msg.Of(message),
 		t1,

gofumports/internal/gocommand/invoke.go

@@ -135,7 +135,7 @@ type Invocation struct {
 	Logf       func(format string, args ...interface{})
 }
 
-func (i *Invocation) runWithFriendlyError(ctx context.Context, stdout, stderr io.Writer) (friendlyError, rawError error) {
+func (i *Invocation) runWithFriendlyError(ctx context.Context, stdout, stderr io.Writer) (friendlyError error, rawError error) {
 	rawError = i.run(ctx, stdout, stderr)
 	if rawError != nil {
 		friendlyError = rawError

gofumports/internal/imports/imports.go

@@ -165,7 +165,7 @@ func initialize(filename string, src []byte, opt *Options) ([]byte, *Options, er
 	return src, opt, nil
 }
 
-func formatFile(fileSet *token.FileSet, file *ast.File, src []byte, adjust func(orig, src []byte) []byte, opt *Options) ([]byte, error) {
+func formatFile(fileSet *token.FileSet, file *ast.File, src []byte, adjust func(orig []byte, src []byte) []byte, opt *Options) ([]byte, error) {
 	mergeImports(opt.Env, fileSet, file)
 	sortImports(opt.Env, fileSet, file)
 	imps := astutil.Imports(fileSet, file)
@@ -343,7 +343,7 @@ func cutSpace(b []byte) (before, middle, after []byte) {
 //    to every non-blank line in src.
 // 3) matchSpace copies the trailing space from orig and uses it in place
 //   of src's trailing space.
-func matchSpace(orig, src []byte) []byte {
+func matchSpace(orig []byte, src []byte) []byte {
 	before, _, after := cutSpace(orig)
 	i := bytes.LastIndex(before, []byte{'\n'})
 	before, indent := before[:i+1], before[i+1:]

gofumports/internal/imports/mod.go

@@ -331,7 +331,7 @@ func (r *ModuleResolver) dirIsNestedModule(dir string, mod *gocommand.ModuleJSON
 	return modDir != mod.Dir
 }
 
-func (r *ModuleResolver) modInfo(dir string) (modDir, modName string) {
+func (r *ModuleResolver) modInfo(dir string) (modDir string, modName string) {
 	readModName := func(modFile string) string {
 		modBytes, err := ioutil.ReadFile(modFile)
 		if err != nil {

rewrite.go

@@ -245,7 +245,7 @@ func match(m map[string]reflect.Value, pattern, val reflect.Value) bool {
 // of wildcards and pos used as the position of tokens from the pattern.
 // if m == nil, subst returns a copy of pattern and doesn't change the line
 // number information.
-func subst(m map[string]reflect.Value, pattern, pos reflect.Value) reflect.Value {
+func subst(m map[string]reflect.Value, pattern reflect.Value, pos reflect.Value) reflect.Value {
 	if !pattern.IsValid() {
 		return reflect.Value{}
 	}

testdata/scripts/comment-spaced.txt

@@ -70,7 +70,7 @@ package p
 //export CgoFunc
 
 //extern open
-func c_open(name *byte, mode, perm int) int
+func c_open(name *byte, mode int, perm int) int
 
 //line 123
 

testdata/scripts/func-merge-parameters.txt

@@ -1,5 +1,10 @@
-gofumpt -w foo.go
-cmp foo.go foo.go.golden
+# By default, this rule isn't enabled.
+gofumpt foo.go
+cmp stdout foo.go
+
+# It's run with -extra.
+gofumpt -extra foo.go
+cmp stdout foo.go.golden
 
 gofumpt -d foo.go.golden
 ! stdout .