Add go brace whitespace formatter

_support/Makefile.template

@@ -159,17 +159,25 @@ lint: {{ .GoLint }}
 	go get golang.org/x/lint/golint@959b441ac422379a43da2230f62be024250818b0
 
 .PHONY: check-formatting
-check-formatting: {{ .GoImports }}
+check-formatting: {{ .GoImports }} {{ .BraceFmt }}
 	# goimports
-	@cd {{ .SourceDir }} && goimports -e -l {{ join .GoFiles " " }} | awk '{ print } END { if(NR>0) { print "Formatting error, run make format"; exit(1) } }'
+	@cd {{ .SourceDir }} && goimports -e -l {{ join .GoFiles " " }} | {{ .MakeFormatCheck }}
+	# bracefmt
+	@cd {{ .SourceDir }} && {{ .BraceFmt }} {{ join .GoFiles " " }} | {{ .MakeFormatCheck }}
 
 {{ .GoImports }}:
 	go get golang.org/x/tools/cmd/goimports@2538eef75904eff384a2551359968e40c207d9d2
 
+{{ .BraceFmt }}:
+	@cd {{ .SourceDir }} && go build -o $@ ./internal/cmd/bracefmt
+
 .PHONY: format
-format: {{ .GoImports }}
-	# In addition to fixing imports, goimports also formats your code in the same style as gofmt
-	# so it can be used as a replacement.
+format: {{ .GoImports }} {{ .BraceFmt }}
+	# goimports pass 1
+	@cd {{ .SourceDir }} && goimports -w -l {{ join .GoFiles " " }}
+	# bracefmt
+	@cd {{ .SourceDir }} && {{ .BraceFmt }} -w {{ join .GoFiles " " }}
+	# goimports pass 2
 	@cd {{ .SourceDir }} && goimports -w -l {{ join .GoFiles " " }}
 
 .PHONY: staticcheck

_support/makegen.go

@@ -72,6 +72,7 @@ func (gm *gitalyMake) BuildDir() string {
 
 func (gm *gitalyMake) Pkg() string         { return "gitlab.com/gitlab-org/gitaly" }
 func (gm *gitalyMake) GoImports() string   { return "bin/goimports" }
+func (gm *gitalyMake) BraceFmt() string    { return filepath.Join(gm.BuildDir(), "bin/bracefmt") }
 func (gm *gitalyMake) GoCovMerge() string  { return "bin/gocovmerge" }
 func (gm *gitalyMake) GoLint() string      { return "bin/golint" }
 func (gm *gitalyMake) GoVendor() string    { return "bin/govendor" }
@@ -249,7 +250,7 @@ func (gm *gitalyMake) GoFiles() []string {
 			}
 		}
 
-		if !info.IsDir() && strings.HasSuffix(path, ".go") {
+		if !info.IsDir() && strings.HasSuffix(path, ".go") && !strings.HasSuffix(path, ".pb.go") {
 			rel, err := filepath.Rel(root, path)
 			if err != nil {
 				return err
@@ -309,6 +310,10 @@ func (gm *gitalyMake) ProtoCSHA256() string {
 	return protoCDownload[runtime.GOOS+"/"+runtime.GOARCH].sha256
 }
 
+func (gm *gitalyMake) MakeFormatCheck() string {
+	return `awk '{ print } END { if(NR>0) { print "Formatting error, run make format"; exit(1) } }'`
+}
+
 var templateText = func() string {
 	contents, err := ioutil.ReadFile("../_support/Makefile.template")
 	if err != nil {

auth/extract_test.go

@@ -50,7 +50,6 @@ func TestCheckTokenV1(t *testing.T) {
 			require.Equal(t, tc.code, status.Code(err), "expected grpc code in error %v", err)
 		})
 	}
-
 }
 
 func TestCheckTokenV2(t *testing.T) {

client/dial.go

@@ -76,7 +76,6 @@ func Dial(rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, erro
 				PermitWithoutStream: true,
 			}),
 		)
-
 	}
 
 	conn, err := grpc.Dial(canonicalAddress, connOpts...)

cmd/gitaly-remote/add_test.go

@@ -54,5 +54,4 @@ func TestAddRemote(t *testing.T) {
 			require.Equal(t, tc.url, strings.TrimSpace(string(out)))
 		})
 	}
-
 }

cmd/gitaly-remote/main.go

@@ -27,7 +27,6 @@ func main() {
 	if err != nil {
 		log.Fatalf("error while setting remote: %v", err)
 	}
-
 }
 
 //addOrUpdate adds, or updates if exits, the remote to repository

cmd/gitaly-wrapper/main.go

@@ -116,7 +116,6 @@ func forwardSignals(gitaly *os.Process, log *logrus.Entry) {
 			if err := gitaly.Signal(sig); err != nil {
 				log.WithField("signal", sig).WithError(err).Error("can't forward the signal")
 			}
-
 		}
 	}()
 

cmd/praefect/main.go

@@ -92,7 +92,6 @@ func configure() (config.Config, error) {
 }
 
 func run(listeners []net.Listener, conf config.Config) error {
-
 	clientConnections := conn.NewClientConnections()
 
 	for _, node := range conf.Nodes {

internal/cmd/bracefmt/brace.go

+package main
+
+import (
+	"bytes"
+	"go/scanner"
+	"go/token"
+)
+
+type editCommand int
+
+const (
+	keepLine editCommand = iota
+	addLineBefore
+	removeLine
+)
+
+type edit struct {
+	line int
+	cmd  editCommand
+}
+
+func braceFmt(src []byte) []byte {
+	var s scanner.Scanner
+	fset := token.NewFileSet()
+	file := fset.AddFile("", fset.Base(), len(src))
+	s.Init(file, src, nil, scanner.ScanComments)
+
+	var (
+		edits                            []edit
+		lastNonEmptyLine, lastLBraceLine int
+		lastOuterRBraceLine              = -1
+	)
+
+	for {
+		pos, tok, _ := s.Scan()
+		if tok == token.EOF {
+			break
+		}
+
+		position := fset.Position(pos)
+		currentLine := position.Line
+		var nextEdit edit
+
+		switch tok {
+		case token.RBRACE:
+			if currentLine-lastNonEmptyLine > 1 {
+				// ......foo
+				//
+				// ...}
+				nextEdit = edit{line: currentLine - 1, cmd: removeLine}
+			}
+
+			if position.Column == 1 {
+				lastOuterRBraceLine = currentLine
+			}
+
+			if lastLBraceLine == currentLine {
+				lastLBraceLine = 0
+			}
+		case token.LBRACE:
+			lastLBraceLine = currentLine
+
+			if lastOuterRBraceLine == currentLine {
+				lastOuterRBraceLine = -1
+			}
+		default:
+			if currentLine-lastOuterRBraceLine == 1 {
+				// }
+				// func bar() {
+				nextEdit = edit{line: currentLine, cmd: addLineBefore}
+			} else if currentLine-lastLBraceLine > 1 && lastNonEmptyLine == lastLBraceLine {
+				// ...foo() {
+				//
+				// ......bar
+				nextEdit = edit{line: currentLine - 1, cmd: removeLine}
+			}
+		}
+
+		if nextEdit.cmd != keepLine {
+			if len(edits) == 0 || edits[0] != nextEdit {
+				// Store edits in reverse line order: that way line numbers in edits
+				// won't become invalid when edits get applied.
+				edits = append([]edit{nextEdit}, edits...)
+			}
+		}
+
+		lastNonEmptyLine = currentLine
+	}
+
+	srcLines := bytes.Split(src, []byte("\n"))
+	for _, e := range edits {
+		i := e.line - 1 // scanner uses 1 based indexing; convert to 0 based
+
+		switch e.cmd {
+		case addLineBefore:
+			srcLines = append(srcLines[:i], append([][]byte{nil}, srcLines[i:]...)...)
+		case removeLine:
+			if len(srcLines[i]) == 0 {
+				srcLines = append(srcLines[:i], srcLines[i+1:]...)
+			}
+		}
+	}
+
+	return bytes.Join(srcLines, []byte("\n"))
+}

internal/cmd/bracefmt/brace_test.go

+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestBraceFmt(t *testing.T) {
+	testCases := []struct {
+		desc      string
+		in        string
+		out       string
+		unchanged bool
+	}{
+		{
+			desc: "empty lines inside braces",
+			in: `package main
+
+import "log"
+
+func main() {
+
+	log.Print("hello")
+
+}
+`,
+			out: `package main
+
+import "log"
+
+func main() {
+	log.Print("hello")
+}
+`,
+		},
+		{
+			desc: "missing empty line after top-level closing brace",
+			in: `package main
+
+import "log"
+
+func main() {
+	log.Print("hello")
+}
+func foo() { log.Print("foobar") }
+`,
+			out: `package main
+
+import "log"
+
+func main() {
+	log.Print("hello")
+}
+
+func foo() { log.Print("foobar") }
+`,
+		},
+		{
+			desc: "allow skipping empty line when not at top level",
+			in: `package main
+
+import "log"
+
+func main() {
+	if true {
+		log.Print("hello")
+	}
+	if false {
+		log.Print("world")
+	}
+}
+`,
+			unchanged: true,
+		},
+		{
+			desc: "allow skipping empty line between one-line functions",
+			in: `package main
+
+import "log"
+
+func foo() { log.Print("world") }
+func bar() { log.Print("hello") }
+
+func main() {
+	foo()
+	bar()
+}
+`,
+			unchanged: true,
+		},
+		{
+			desc: "allow }{ at start of line",
+			in: `package main
+
+var anonymousStruct = struct {
+	foo string
+}{
+	foo: "bar",
+}
+`,
+			unchanged: true,
+		},
+		{
+			desc: "allow trailing */ before closing brace",
+			in: `package main
+
+func foo() {
+	return
+
+	/* trailing comment
+	*/
+}
+`,
+			unchanged: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			out := braceFmt([]byte(tc.in))
+
+			if tc.unchanged {
+				require.Equal(t, tc.in, string(out))
+			} else {
+				require.Equal(t, tc.out, string(out))
+			}
+		})
+	}
+}

internal/cmd/bracefmt/main.go

+package main
+
+import (
+	"bytes"
+	"flag"
+	"fmt"
+	"io/ioutil"
+	"os"
+)
+
+const (
+	progName = "bracefmt"
+)
+
+var (
+	writeFiles = flag.Bool("w", false, "write changes to inspected files")
+)
+
+func main() {
+	flag.Parse()
+
+	if len(flag.Args()) == 0 {
+		fmt.Fprintf(os.Stderr, "usage: %s file.go [file.go...]\n", progName)
+		os.Exit(1)
+	}
+
+	if err := _main(flag.Args()); err != nil {
+		fmt.Fprintf(os.Stderr, "%s: fatal: %v", progName, err)
+		os.Exit(1)
+	}
+}
+
+func _main(args []string) error {
+	for _, f := range args {
+		fi, err := os.Stat(f)
+		if err != nil {
+			return err
+		}
+
+		src, err := ioutil.ReadFile(f)
+		if err != nil {
+			return err
+		}
+
+		dst := braceFmt(src)
+		if bytes.Equal(src, dst) {
+			continue
+		}
+
+		fmt.Println(f)
+
+		if !*writeFiles {
+			continue
+		}
+
+		if err := ioutil.WriteFile(f, dst, fi.Mode()); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}

internal/config/config_test.go

@@ -401,7 +401,6 @@ func TestValidateShellPath(t *testing.T) {
 		} else {
 			assert.NoError(t, err)
 		}
-
 	}
 }
 

internal/git/catfile/batch_cache.go

@@ -176,7 +176,6 @@ func (bc *batchCache) lookup(k key) (int, bool) {
 		if ent.key == k {
 			return i, true
 		}
-
 	}
 
 	return -1, false

internal/git/catfile/batch_cache_test.go

@@ -162,7 +162,6 @@ func requireCacheValid(t *testing.T, bc *batchCache) {
 	defer bc.Unlock()
 
 	for _, ent := range bc.entries {
-
 		v := ent.value
 		require.False(t, v.isClosed(), "values in cache should not be closed: %v %v", ent, v)
 	}

internal/git/catfile/catfile_test.go

@@ -119,7 +119,6 @@ func TestCommit(t *testing.T) {
 			require.Equal(t, tc.output, string(contents))
 		})
 	}
-
 }
 
 func TestTag(t *testing.T) {
@@ -190,7 +189,6 @@ func TestTree(t *testing.T) {
 			require.Equal(t, tc.output, string(contents))
 		})
 	}
-
 }
 
 func TestRepeatedCalls(t *testing.T) {

internal/git/hooks/hooks_test.go

@@ -33,5 +33,4 @@ func TestPath(t *testing.T) {
 
 		require.Equal(t, "/var/empty", Path())
 	})
-
 }

internal/git/log/commit_test.go

@@ -97,7 +97,6 @@ func TestParseRawCommit(t *testing.T) {
 			out, err := parseRawCommit(bytes.NewBuffer(tc.in), info)
 			require.NoError(t, err, "parse error")
 			require.Equal(t, tc.out, out)
-
 		})
 	}
 }

internal/git/objectpool/fetch_test.go

@@ -185,7 +185,6 @@ func TestFetchFromOriginRefUpdates(t *testing.T) {
 
 	looseRefs := testhelper.MustRunCommand(t, nil, "find", filepath.Join(poolPath, "refs"), "-type", "f")
 	require.Equal(t, "", string(looseRefs), "there should be no loose refs after the fetch")
-
 }
 
 func resolveRef(t *testing.T, repo string, ref string) string {

internal/git/pktline/pkt_line_test.go

@@ -60,7 +60,6 @@ func TestScanner(t *testing.T) {
 
 			if tc.fail {
 				require.Error(t, scanner.Err())
-
 			} else {
 				require.NoError(t, scanner.Err())
 			}

internal/git/proto.go

@@ -100,7 +100,6 @@ func VersionLessThan(v1Str, v2Str string) (bool, error) {
 
 func versionLessThan(v1, v2 version) bool {
 	switch {
-
 	case v1.major < v2.major:
 		return true
 	case v1.major > v2.major:
@@ -119,7 +118,6 @@ func versionLessThan(v1, v2 version) bool {
 	default:
 		// this should only be reachable when versions are equal
 		return false
-
 	}
 }