Merge branch 'megacheck' into 'master'

Added megacheck linter and fixed new linting errors

See merge request !321

12 files changed, 19 insertions, 75 deletions

diff --git a/Makefile b/Makefile
index 3698d58ce..9e672ba5d 100644
--- a/Makefile
+++ b/Makefile
@@ -32,6 +32,7 @@ GOVENDOR = $(BIN_BUILD_DIR)/govendor
 GOLINT = $(BIN_BUILD_DIR)/golint
 GOCOVMERGE = $(BIN_BUILD_DIR)/gocovmerge
 GOIMPORTS = $(BIN_BUILD_DIR)/goimports
+MEGACHECK = $(BIN_BUILD_DIR)/megacheck
 
 .NOTPARALLEL:
 
@@ -60,7 +61,7 @@ install: build
 	cd $(BIN_BUILD_DIR) && install $(COMMANDS) $(INSTALL_DEST_DIR)
 
 .PHONY: verify
-verify: lint check-formatting govendor-status notice-up-to-date
+verify: lint check-formatting megacheck govendor-status notice-up-to-date
 
 .PHONY: govendor-status
 govendor-status: $(TARGET_SETUP) $(GOVENDOR)
@@ -84,6 +85,10 @@ prepare-tests: $(TARGET_SETUP) $(TEST_REPO)
 lint: $(GOLINT)
 	go run _support/lint.go
 
+.PHONY: megacheck
+megacheck: $(MEGACHECK)
+	@$(MEGACHECK) $(LOCAL_PACKAGES)
+
 .PHONY: check-formatting
 check-formatting: $(TARGET_SETUP) $(GOIMPORTS)
 	@test -z "$$($(GOIMPORTS) -e -l $(LOCAL_GO_FILES))" || (echo >&2 "Formatting or imports need fixing: 'make format'" && $(GOIMPORTS) -e -l $(LOCAL_GO_FILES) && false)
@@ -147,3 +152,7 @@ $(GOCOVMERGE): $(TARGET_SETUP)
 # Install goimports
 $(GOIMPORTS): $(TARGET_SETUP)
 	go get -v golang.org/x/tools/cmd/goimports
+
+# Install megacheck
+$(MEGACHECK): $(TARGET_SETUP)
+	go get -v honnef.co/go/tools/cmd/megacheck
diff --git a/internal/diff/diff.go b/internal/diff/diff.go
index a216c7146..9fcc013e9 100644
--- a/internal/diff/diff.go
+++ b/internal/diff/diff.go
@@ -335,8 +335,6 @@ func (parser *Parser) consumeChunkLine() {
 	parser.currentDiff.lineCount++
 	parser.linesProcessed++
 	parser.bytesProcessed += len(line)
-
-	return
 }
 
 func (parser *Parser) consumeBinaryNotice() {
@@ -347,8 +345,6 @@ func (parser *Parser) consumeBinaryNotice() {
 	}
 
 	parser.currentDiff.Binary = true
-
-	return
 }
 
 func (parser *Parser) consumeLine(updateStats bool) {
@@ -363,8 +359,6 @@ func (parser *Parser) consumeLine(updateStats bool) {
 		parser.linesProcessed++
 		parser.bytesProcessed += len(line)
 	}
-
-	return
 }
 
 // unescape unescapes the escape codes used by 'git diff'
diff --git a/internal/helper/command.go b/internal/helper/command.go
index e678fccda..cc4ed735c 100644
--- a/internal/helper/command.go
+++ b/internal/helper/command.go
@@ -200,7 +200,7 @@ func (c *Command) logProcessComplete(ctx context.Context, exitCode int) {
 
 	systemTime := cmd.ProcessState.SystemTime()
 	userTime := cmd.ProcessState.UserTime()
-	realTime := time.Now().Sub(c.startTime)
+	realTime := time.Since(c.startTime)
 
 	entry := grpc_logrus.Extract(ctx).WithFields(log.Fields{
 		"path":                   cmd.Path,
diff --git a/internal/middleware/panichandler/panic_handler.go b/internal/middleware/panichandler/panic_handler.go
index a9b4d223e..68deea146 100644
--- a/internal/middleware/panichandler/panic_handler.go
+++ b/internal/middleware/panichandler/panic_handler.go
@@ -52,10 +52,8 @@ func handleCrash(grpcMethodName string, handler PanicHandler) {
 
 		handler(grpcMethodName, r)
 
-		if additionalHandlers != nil {
-			for _, fn := range additionalHandlers {
-				fn(grpcMethodName, r)
-			}
+		for _, fn := range additionalHandlers {
+			fn(grpcMethodName, r)
 		}
 	}
 }
diff --git a/internal/service/commit/list_files_test.go b/internal/service/commit/list_files_test.go
index 2d9fe1448..2185a2522 100644
--- a/internal/service/commit/list_files_test.go
+++ b/internal/service/commit/list_files_test.go
@@ -141,10 +141,9 @@ func TestListFilesFailure(t *testing.T) {
 	defer conn.Close()
 
 	tests := []struct {
-		repo     *pb.Repository
-		revision string
-		code     codes.Code
-		desc     string
+		repo *pb.Repository
+		code codes.Code
+		desc string
 	}{
 		// Nil Repo
 		{repo: nil, code: codes.InvalidArgument, desc: "nil repo"},
diff --git a/internal/service/commit/testhelper_test.go b/internal/service/commit/testhelper_test.go
index 33fbf3ddd..8630b8239 100644
--- a/internal/service/commit/testhelper_test.go
+++ b/internal/service/commit/testhelper_test.go
@@ -66,21 +66,6 @@ func startTestServices(t *testing.T) *grpc.Server {
 	return server
 }
 
-func newCommitClient(t *testing.T, serviceSocketPath string) pb.CommitClient {
-	connOpts := []grpc.DialOption{
-		grpc.WithInsecure(),
-		grpc.WithDialer(func(addr string, _ time.Duration) (net.Conn, error) {
-			return net.Dial("unix", addr)
-		}),
-	}
-	conn, err := grpc.Dial(serviceSocketPath, connOpts...)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return pb.NewCommitClient(conn)
-}
-
 func newCommitServiceClient(t *testing.T, serviceSocketPath string) (pb.CommitServiceClient, *grpc.ClientConn) {
 	connOpts := []grpc.DialOption{
 		grpc.WithInsecure(),
diff --git a/internal/service/ref/refexists_test.go b/internal/service/ref/refexists_test.go
index f87d32c8a..2204b39ee 100644
--- a/internal/service/ref/refexists_test.go
+++ b/internal/service/ref/refexists_test.go
@@ -12,11 +12,6 @@ import (
 )
 
 func TestRefExists(t *testing.T) {
-	type args struct {
-		ctx context.Context
-		in  *pb.RefExistsRequest
-	}
-
 	badRepo := &pb.Repository{StorageName: "invalid", RelativePath: "/etc/"}
 
 	tests := []struct {
diff --git a/internal/service/ref/testhelper_test.go b/internal/service/ref/testhelper_test.go
index 75a742848..f2de2e634 100644
--- a/internal/service/ref/testhelper_test.go
+++ b/internal/service/ref/testhelper_test.go
@@ -15,7 +15,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/helper/lines"
 	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
-	"gitlab.com/gitlab-org/gitaly/internal/service/renameadapter"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/reflection"
@@ -96,22 +95,6 @@ func TestMain(m *testing.M) {
 	}())
 }
 
-func runRefServer(t *testing.T) *grpc.Server {
-	os.Remove(serverSocketPath)
-	grpcServer := testhelper.NewTestGrpcServer(t, nil, nil)
-	listener, err := net.Listen("unix", serverSocketPath)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	pb.RegisterRefServer(grpcServer, renameadapter.NewRefAdapter(&server{}))
-	reflection.Register(grpcServer)
-
-	go grpcServer.Serve(listener)
-
-	return grpcServer
-}
-
 func runRefServiceServer(t *testing.T) *grpc.Server {
 	os.Remove(serverSocketPath)
 	grpcServer := testhelper.NewTestGrpcServer(t, nil, nil)
diff --git a/internal/service/smarthttp/scan_deepen_test.go b/internal/service/smarthttp/scan_deepen_test.go
index 2458b638c..c5a8103d6 100644
--- a/internal/service/smarthttp/scan_deepen_test.go
+++ b/internal/service/smarthttp/scan_deepen_test.go
@@ -43,7 +43,7 @@ func TestFailedScanDeepen(t *testing.T) {
 
 	for _, example := range examples {
 		t.Run(example, func(t *testing.T) {
-			if scanDeepen(bytes.NewReader([]byte(example))) == true {
+			if scanDeepen(bytes.NewReader([]byte(example))) {
 				t.Fatalf("scanDeepen %q: expected result to be false, got true", example)
 			}
 		})
diff --git a/internal/service/smarthttp/testhelper_test.go b/internal/service/smarthttp/testhelper_test.go
index 49272c64a..316e106e4 100644
--- a/internal/service/smarthttp/testhelper_test.go
+++ b/internal/service/smarthttp/testhelper_test.go
@@ -15,8 +15,7 @@ import (
 )
 
 const (
-	testRepoRoot = "testdata/data"
-	pktFlushStr  = "0000"
+	pktFlushStr = "0000"
 )
 
 var (
@@ -53,18 +52,3 @@ func newSmartHTTPClient(t *testing.T) (pb.SmartHTTPClient, *grpc.ClientConn) {
 
 	return pb.NewSmartHTTPClient(conn), conn
 }
-
-func newRefServiceClient(t *testing.T) pb.RefServiceClient {
-	connOpts := []grpc.DialOption{
-		grpc.WithInsecure(),
-		grpc.WithDialer(func(addr string, _ time.Duration) (net.Conn, error) {
-			return net.Dial("unix", addr)
-		}),
-	}
-	conn, err := grpc.Dial(serverSocketPath, connOpts...)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return pb.NewRefServiceClient(conn)
-}
diff --git a/internal/service/ssh/cmd/gitaly-upload-pack/main.go b/internal/service/ssh/cmd/gitaly-upload-pack/main.go
index ec0fc8d5e..8df5dfe56 100644
--- a/internal/service/ssh/cmd/gitaly-upload-pack/main.go
+++ b/internal/service/ssh/cmd/gitaly-upload-pack/main.go
@@ -42,9 +42,7 @@ func main() {
 	if len(gitConfig) > 0 {
 		configs := strings.Split(gitConfig, " ")
 
-		for _, c := range configs {
-			req.GitConfigOptions = append(req.GitConfigOptions, c)
-		}
+		req.GitConfigOptions = append(req.GitConfigOptions, configs...)
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
diff --git a/internal/service/ssh/testhelper_test.go b/internal/service/ssh/testhelper_test.go
index 885bf4017..51ddb5895 100644
--- a/internal/service/ssh/testhelper_test.go
+++ b/internal/service/ssh/testhelper_test.go
@@ -23,7 +23,6 @@ const (
 
 var (
 	serverSocketPath = testhelper.GetTemporaryGitalySocketFileName()
-	workDir          string
 	testRepo         *pb.Repository
 
 	uploadPackPath  string