diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..5f716b517 --- /dev/null +++ b/.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" diff --git a/Makefile b/Makefile index 582d3dacc..8f2bd607e 100644 --- a/Makefile +++ b/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 diff --git a/server/api.go b/server/api.go index c6b347741..1b860efe0 100644 --- a/server/api.go +++ b/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 } diff --git a/server/command.go b/server/command.go index 33846b965..7639cdd25 100644 --- a/server/command.go +++ b/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 diff --git a/server/template_test.go b/server/template_test.go index 09e226bfa..87092761e 100644 --- a/server/template_test.go +++ b/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,7 +389,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"), @@ -397,7 +397,7 @@ func TestPushedCommitsTemplate(t *testing.T) { Name: sToP("panda"), }, }, - github.PushEventCommit{ + { ID: sToP("a20867b14bb761a232cd80139fbd4c0d33264240"), URL: sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a20867b14bb761a232cd80139fbd4c0d33264240"), Message: sToP("Merge master"), @@ -426,7 +426,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"), @@ -434,7 +434,7 @@ func TestPushedCommitsTemplate(t *testing.T) { Name: sToP("panda"), }, }, - github.PushEventCommit{ + { ID: sToP("a20867b14bb761a232cd80139fbd4c0d33264240"), URL: sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a20867b14bb761a232cd80139fbd4c0d33264240"), Message: sToP("Merge master"), @@ -442,7 +442,7 @@ func TestPushedCommitsTemplate(t *testing.T) { Name: sToP("panda"), }, }, - github.PushEventCommit{ + { ID: sToP("a30867b14bb761a232cd80139fbd4c0d33264240"), URL: sToP("https://github.com/mattermost/mattermost-plugin-github/commit/a30867b14bb761a232cd80139fbd4c0d33264240"), Message: sToP("Fix build"), diff --git a/server/webhook.go b/server/webhook.go index d1a1acc3e..bfaec93f3 100644 --- a/server/webhook.go +++ b/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 }