Added golangci.yml file to repo (#168)

* Added golangci.yml file to repo

* Update .golangci.yml to use 1.23

Co-Authored-By: Ben Schumacher <ben.schumacher@mattermost.com>

* Remove stale targets from Makefile

* Simpify

Co-authored-by: mattermod <mattermod@users.noreply.github.com>
Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>
Co-authored-by: Hanzei <hanzei@mailbox.org>

.golangci.yml

@@ -0,0 +1,35 @@
+service:
+  golangci-lint-version: 1.23.0
+
+run:
+  timeout: 5m
+
+linters-settings:
+  gofmt:
+    simplify: true
+  govet:
+    check-shadowing: true
+    enable-all: true
+
+linters:
+  disable-all: true
+  enable:
+    - deadcode
+    - gofmt
+    - gosimple
+    - govet
+    - ineffassign
+    - structcheck
+    - unconvert
+    - unused
+    - varcheck
+    # TODO: enable this later
+    # - errcheck
+
+issues:
+  exclude-rules:
+    - linters:
+      # ignore unused warning for manifest
+      - varcheck
+      - deadcode
+      text: "manifest"

Makefile

@@ -22,46 +22,24 @@ all: check-style test dist
 apply:
 	./build/bin/manifest apply
 
-## Runs govet and gofmt against all packages.
+## Runs golangci-lint against all packages.
 .PHONY: check-style
-check-style: webapp/.npminstall gofmt govet
+check-style: webapp/.npminstall golangci-lint
 	@echo Checking for style guide compliance
 
 ifneq ($(HAS_WEBAPP),)
 	cd webapp && npm run lint
 endif
 
-## Runs gofmt against all packages.
-.PHONY: gofmt
-gofmt:
-ifneq ($(HAS_SERVER),)
-	@echo Running gofmt
-	@for package in $$(go list ./server/...); do \
-		echo "Checking "$$package; \
-		files=$$(go list -f '{{range .GoFiles}}{{$$.Dir}}/{{.}} {{end}}' $$package); \
-		if [ "$$files" ]; then \
-			gofmt_output=$$(gofmt -d -s $$files 2>&1); \
-			if [ "$$gofmt_output" ]; then \
-				echo "$$gofmt_output"; \
-				echo "Gofmt failure"; \
-				exit 1; \
-			fi; \
-		fi; \
-	done
-	@echo Gofmt success
-endif
+golangci-lint: ## Run golangci-lint on codebase
+# https://stackoverflow.com/a/677212/1027058 (check if a command exists or not)
+	@if ! [ -x "$$(command -v golangci-lint)" ]; then \
+		echo "golangci-lint is not installed. Please see https://github.com/golangci/golangci-lint#install for installation instructions."; \
+		exit 1; \
+	fi; \
 
-## Runs govet against all packages.
-.PHONY: govet
-govet:
-ifneq ($(HAS_SERVER),)
-	@echo Running govet
-	@# Workaround because you can't install binaries without adding them to go.mod
-	env GO111MODULE=off $(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
-	$(GO) vet ./server/...
-	$(GO) vet -vettool=$(GOPATH)/bin/shadow ./server/...
-	@echo Govet success
-endif
+	@echo Running golangci-lint
+	golangci-lint run ./...
 
 ## Builds the server, if it exists, including support for multiple architectures.
 .PHONY: server

server/api.go

@@ -701,19 +701,14 @@ func getFailReason(code int, repo string, username string) string {
 	switch code {
 	case http.StatusInternalServerError:
 		cause = "Internal server error"
-		break
 	case http.StatusBadRequest:
 		cause = "Bad request"
-		break
 	case http.StatusNotFound:
 		cause = fmt.Sprintf("Sorry, either you don't have access to the repo %s with the user %s or it is no longer available", repo, username)
-		break
 	case http.StatusUnauthorized:
 		cause = fmt.Sprintf("Sorry, your user %s is unauthorized to do this action", username)
-		break
 	case http.StatusForbidden:
 		cause = fmt.Sprintf("Sorry, you don't have enough permissions to comment in the repo %s with the user %s", repo, username)
-		break
 	default:
 		cause = fmt.Sprintf("Unknown status code %d", code)
 	}
@@ -735,9 +730,7 @@ func (p *Plugin) createIssueComment(w http.ResponseWriter, r *http.Request) {
 	req := &CreateIssueCommentRequest{}
 	dec := json.NewDecoder(r.Body)
 	if err := dec.Decode(&req); err != nil {
-		if err != nil {
-			mlog.Error("Error decoding JSON body", mlog.Err(err))
-		}
+		mlog.Error("Error decoding JSON body", mlog.Err(err))
 		writeAPIError(w, &APIErrorResponse{ID: "", Message: "Please provide a JSON object.", StatusCode: http.StatusBadRequest})
 		return
 	}

server/command.go

@@ -196,7 +196,7 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo
 				if !ok {
 					msg := fmt.Sprintf("Invalid feature(s) provided: %s", strings.Join(ifs, ","))
 					if len(ifs) == 0 {
-						msg = fmt.Sprintf("Feature list must have \"pulls\" or \"issues\" when using a label.")
+						msg = "Feature list must have \"pulls\" or \"issues\" when using a label."
 					}
 					p.postCommandResponse(args, msg)
 					return &model.CommandResponse{}, nil

server/template_test.go

@@ -334,7 +334,7 @@ func TestPushedCommitsTemplate(t *testing.T) {
 			Sender: &user,
 			Forced: bToP(false),
 			Commits: []github.PushEventCommit{
-				github.PushEventCommit{
+				{
 					ID:      sToP("a10867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a10867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Leverage git-get-head"),
@@ -361,7 +361,7 @@ func TestPushedCommitsTemplate(t *testing.T) {
 			Sender: &user,
 			Forced: bToP(true),
 			Commits: []github.PushEventCommit{
-				github.PushEventCommit{
+				{
 					ID:      sToP("a10867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a10867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Leverage git-get-head"),
@@ -389,15 +389,15 @@ func TestPushedCommitsTemplate(t *testing.T) {
 			Sender: &user,
 			Forced: bToP(false),
 			Commits: []github.PushEventCommit{
-				github.PushEventCommit{
+				{
 					ID:      sToP("a10867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a10867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Leverage git-get-head"),
 					Committer: &github.CommitAuthor{
 						Name: sToP("panda"),
 					},
 				},
-				github.PushEventCommit{
+				{
 					ID:      sToP("a20867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a20867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Merge master"),
@@ -426,23 +426,23 @@ func TestPushedCommitsTemplate(t *testing.T) {
 			Sender: &user,
 			Forced: bToP(false),
 			Commits: []github.PushEventCommit{
-				github.PushEventCommit{
+				{
 					ID:      sToP("a10867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a10867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Leverage git-get-head"),
 					Committer: &github.CommitAuthor{
 						Name: sToP("panda"),
 					},
 				},
-				github.PushEventCommit{
+				{
 					ID:      sToP("a20867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a20867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Merge master"),
 					Committer: &github.CommitAuthor{
 						Name: sToP("panda"),
 					},
 				},
-				github.PushEventCommit{
+				{
 					ID:      sToP("a30867b14bb761a232cd80139fbd4c0d33264240"),
 					URL:     sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a30867b14bb761a232cd80139fbd4c0d33264240"),
 					Message: sToP("Fix build"),

server/webhook.go

@@ -402,7 +402,7 @@ func (p *Plugin) postCreateEvent(event *github.CreateEvent) {
 
 	subs := p.GetSubscribedChannelsForRepository(repo)
 
-	if subs == nil || len(subs) == 0 {
+	if len(subs) == 0 {
 		return
 	}
 
@@ -445,7 +445,7 @@ func (p *Plugin) postDeleteEvent(event *github.DeleteEvent) {
 
 	subs := p.GetSubscribedChannelsForRepository(repo)
 
-	if subs == nil || len(subs) == 0 {
+	if len(subs) == 0 {
 		return
 	}
 
@@ -488,7 +488,7 @@ func (p *Plugin) postIssueCommentEvent(event *github.IssueCommentEvent) {
 
 	subs := p.GetSubscribedChannelsForRepository(repo)
 
-	if subs == nil || len(subs) == 0 {
+	if len(subs) == 0 {
 		return
 	}
 
@@ -549,7 +549,7 @@ func (p *Plugin) postPullRequestReviewEvent(event *github.PullRequestReviewEvent
 	repo := event.GetRepo()
 
 	subs := p.GetSubscribedChannelsForRepository(repo)
-	if subs == nil || len(subs) == 0 {
+	if len(subs) == 0 {
 		return
 	}
 
@@ -617,7 +617,7 @@ func (p *Plugin) postPullRequestReviewCommentEvent(event *github.PullRequestRevi
 	repo := event.GetRepo()
 
 	subs := p.GetSubscribedChannelsForRepository(repo)
-	if subs == nil || len(subs) == 0 {
+	if len(subs) == 0 {
 		return
 	}