Golang CI integration, linter fixes

.golangci.yml

@@ -0,0 +1,62 @@
+run:
+  timeout: 5m
+  modules-download-mode: readonly
+
+linters-settings:
+  goconst:
+    min-len: 2
+    min-occurrences: 2
+  gofmt:
+    simplify: true
+  goimports:
+    local-prefixes: https://github.com/mattermost/mattermost-plugin-solar-lottery
+  golint:
+    min-confidence: 0
+  govet:
+    check-shadowing: true
+    enable-all: true
+  misspell:
+    locale: US
+
+linters:
+  disable-all: true
+  enable:
+    - bodyclose
+    - deadcode
+    # - errcheck
+    # - goconst
+    - gocritic
+    # - gofmt
+    # - goimports
+    # - golint
+    # - gosec
+    # - gosimple
+    # - govet
+    # - ineffassign
+    # - interfacer
+    # - misspell
+    - nakedret
+    # - staticcheck
+    - structcheck
+    # - stylecheck
+    - typecheck
+    - unconvert
+    # - unused
+    - varcheck
+    - whitespace
+
+issues:
+  exclude-rules:
+    - path: server/manifest.go
+      linters:
+        - deadcode
+        - unused
+        - varcheck
+    - path: server/configuration.go
+      linters:
+        - unused
+    - path: _test\.go
+      linters:
+        - bodyclose
+        - goconst
+        - scopelint # https://github.com/kyoh86/scopelint/issues/4

Makefile

@@ -44,54 +44,25 @@ all: check-style test dist
 apply:
 	./build/bin/manifest apply
 
-## Runs govet and gofmt against all packages.
+## Runs golangci-lint and eslint.
 .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 ./...); 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
-
-## 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 ./...
-	$(GO) vet -vettool=$(GOPATH)/bin/shadow ./...
-	@echo Govet success
-endif
+## Run golangci-lint on codebase.
+.PHONY: golangci-lint
+golangci-lint:
+	@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 golint against all packages.
-.PHONY: golint
-golint:
-	@echo Running lint
-	env GO111MODULE=off $(GO) get golang.org/x/lint/golint
-	$(GOPATH)/bin/golint -set_exit_status ./...
-	@echo lint success
+	@echo Running golangci-lint
+	golangci-lint run ./...
 
 ## Generates mock golang interfaces for testing
 mock:
@@ -136,6 +107,7 @@ ifneq ($(HAS_WEBAPP),)
 endif
 
 ## Builds the webapp in debug mode, if it exists.
+.PHONY: webapp-debug
 webapp-debug: webapp/.npminstall
 ifneq ($(HAS_WEBAPP),)
 	cd webapp && \
@@ -229,10 +201,12 @@ endif
 clean:
 	rm -fr dist/
 ifneq ($(HAS_SERVER),)
+	rm -fr server/coverage.txt
 	rm -fr server/dist
 endif
 ifneq ($(HAS_WEBAPP),)
 	rm -fr webapp/.npminstall
+	rm -fr webapp/junit.xml
 	rm -fr webapp/dist
 	rm -fr webapp/node_modules
 endif
@@ -303,5 +277,3 @@ endif
 	@curl -s -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins/$(PLUGIN_ID)/disable > /dev/null && \
 		curl -s -H "Authorization: Bearer $(TOKEN)" -X POST $(MM_SERVICESETTINGS_SITEURL)/api/v4/plugins/$(PLUGIN_ID)/enable > /dev/null && \
 		echo "OK." || echo "Sorry, something went wrong. Check that MM_ADMIN_USERNAME and MM_ADMIN_PASSWORD env variables are set correctly."
-
-

build/deploy/main.go

@@ -34,18 +34,38 @@ func deploy() error {
 	bundlePath := os.Args[2]
 
 	siteURL := os.Getenv("MM_SERVICESETTINGS_SITEURL")
+	adminToken := os.Getenv("MM_ADMIN_TOKEN")
 	adminUsername := os.Getenv("MM_ADMIN_USERNAME")
 	adminPassword := os.Getenv("MM_ADMIN_PASSWORD")
 	copyTargetDirectory, _ := filepath.Abs("../mattermost-server")
-	if siteURL != "" && adminUsername != "" && adminPassword != "" {
-		return uploadPlugin(pluginID, bundlePath, siteURL, adminUsername, adminPassword)
+
+	if siteURL != "" {
+		client := model.NewAPIv4Client(siteURL)
+
+		if adminToken != "" {
+			log.Printf("Authenticating using token against %s.", siteURL)
+			client.SetToken(adminToken)
+
+			return uploadPlugin(client, pluginID, bundlePath)
+		}
+
+		if adminUsername != "" && adminPassword != "" {
+			client := model.NewAPIv4Client(siteURL)
+			log.Printf("Authenticating as %s against %s.", adminUsername, siteURL)
+			_, resp := client.Login(adminUsername, adminPassword)
+			if resp.Error != nil {
+				return errors.Wrapf(resp.Error, "failed to login as %s", adminUsername)
+			}
+
+			return uploadPlugin(client, pluginID, bundlePath)
+		}
 	}
 
 	_, err := os.Stat(copyTargetDirectory)
 	if os.IsNotExist(err) {
 		return errors.New("no supported deployment method available, please install plugin manually")
 	} else if err != nil {
-		return errors.Errorf("Failed to stat %s", copyTargetDirectory)
+		return errors.Wrapf(err, "failed to stat %s", copyTargetDirectory)
 	}
 
 	log.Printf("Installing plugin to mattermost-server found in %s.", copyTargetDirectory)
@@ -55,30 +75,23 @@ func deploy() error {
 
 // uploadPlugin attempts to upload and enable a plugin via the Client4 API.
 // It will fail if plugin uploads are disabled.
-func uploadPlugin(pluginID, bundlePath, siteURL, adminUsername, adminPassword string) error {
-	client := model.NewAPIv4Client(siteURL)
-	log.Printf("Authenticating as %s against %s.", adminUsername, siteURL)
-	_, resp := client.Login(adminUsername, adminPassword)
-	if resp.Error != nil {
-		return fmt.Errorf("Failed to login as %s: %s", adminUsername, resp.Error.Error())
-	}
-
+func uploadPlugin(client *model.Client4, pluginID, bundlePath string) error {
 	pluginBundle, err := os.Open(bundlePath)
 	if err != nil {
 		return errors.Wrapf(err, "failed to open %s", bundlePath)
 	}
 	defer pluginBundle.Close()
 
 	log.Print("Uploading plugin via API.")
-	_, resp = client.UploadPluginForced(pluginBundle)
+	_, resp := client.UploadPluginForced(pluginBundle)
 	if resp.Error != nil {
-		return fmt.Errorf("Failed to upload plugin bundle: %s", resp.Error.Error())
+		return errors.Wrap(resp.Error, "failed to upload plugin bundle")
 	}
 
 	log.Print("Enabling plugin.")
 	_, resp = client.EnablePlugin(pluginID)
 	if resp.Error != nil {
-		return fmt.Errorf("Failed to enable plugin: %s", resp.Error.Error())
+		return errors.Wrap(resp.Error, "Failed to enable plugin")
 	}
 
 	return nil

build/manifest/main.go

@@ -106,7 +106,7 @@ func applyManifest(manifest *model.Manifest) error {
 		if err := ioutil.WriteFile(
 			"server/manifest.go",
 			[]byte(fmt.Sprintf(pluginIDGoFileTemplate, manifest.Id, manifest.Version)),
-			0644,
+			0600,
 		); err != nil {
 			return errors.Wrap(err, "failed to write server/manifest.go")
 		}
@@ -116,7 +116,7 @@ func applyManifest(manifest *model.Manifest) error {
 		if err := ioutil.WriteFile(
 			"webapp/src/manifest.js",
 			[]byte(fmt.Sprintf(pluginIDJSFileTemplate, manifest.Id, manifest.Version)),
-			0644,
+			0600,
 		); err != nil {
 			return errors.Wrap(err, "failed to open webapp/src/manifest.js")
 		}

server/command/command.go

@@ -29,7 +29,7 @@ type Command struct {
 	ChannelID   string
 
 	actualTrigger string
-	outputJson    bool
+	outputJSON    bool
 	fs            *pflag.FlagSet
 	now           *types.Time
 }
@@ -109,7 +109,7 @@ func (c *Command) normalOut(out md.Markdowner, err error) (md.MD, error) {
 	if err != nil {
 		return "", err
 	}
-	if c.outputJson {
+	if c.outputJSON {
 		out = md.JSONBlock(out)
 	}
 	return out.Markdown(), nil

server/command/command_flags.go

@@ -43,7 +43,7 @@ func (c *Command) parse(parameters []string) error {
 		return err
 	}
 
-	if (*c.now).IsZero() {
+	if c.now.IsZero() {
 		now := types.NewTime(time.Now())
 		c.now = &now
 	}
@@ -53,7 +53,7 @@ func (c *Command) parse(parameters []string) error {
 func (c *Command) flags() *pflag.FlagSet {
 	if c.fs == nil {
 		c.fs = pflag.NewFlagSet("", pflag.ContinueOnError)
-		c.fs.BoolVar(&c.outputJson, "json", false, "output as JSON")
+		c.fs.BoolVar(&c.outputJSON, "json", false, "output as JSON")
 		c.now, _ = c.withTimeFlag("now", "specify the transaction time (default: now)")
 	}
 	return c.fs

server/command/fair_test.go

@@ -54,7 +54,7 @@ func TestFairSimple(t *testing.T) {
 }
 
 // TODO TestFairSET is effectively failing; for some reason preferring the
-// "MOBILE" team. Need to do some careful analisys, the skills structure may
+// "MOBILE" team. Need to do some careful analysis, the skills structure may
 // indeed be skewed towards non-mobile, but would think randomization would
 // help? Need a better "fairness" test harness anyway.
 //

server/command/rotation_archive_test.go

@@ -49,7 +49,6 @@ func TestRotationArchive(t *testing.T) {
 		_, err = run(t, SL, `/lotto rotation show test-123`)
 		require.Equal(t, kvstore.ErrNotFound, err)
 	})
-
 }
 
 func TestRotationDelete(t *testing.T) {
@@ -83,5 +82,4 @@ func TestRotationDelete(t *testing.T) {
 		_, err = run(t, SL, `/lotto rotation show test-123`)
 		require.Equal(t, kvstore.ErrNotFound, err)
 	})
-
-}
+}

server/command/rotation_list.go

@@ -19,7 +19,7 @@ func (c *Command) rotationList(parameters []string) (md.MD, error) {
 		return "", err
 	}
 
-	if c.outputJson {
+	if c.outputJSON {
 		return md.JSONBlock(rotations), nil
 	}
 	if rotations.Len() == 0 {

server/command/rotation_show.go

@@ -21,7 +21,7 @@ func (c *Command) rotationShow(parameters []string) (md.MD, error) {
 		return "", err
 	}
 
-	if c.outputJson {
+	if c.outputJSON {
 		return md.JSONBlock(r), nil
 	}
 	return r.MarkdownBullets(), nil

server/command/skill.go

@@ -25,7 +25,7 @@ func (c *Command) skillNew(parameters []string) (md.MD, error) {
 		return "", err
 	}
 
-	if c.outputJson {
+	if c.outputJSON {
 		return md.JSONBlock(skill), nil
 	}
 	return md.Markdownf("Added **%s** to known skills.", skill), nil
@@ -45,7 +45,7 @@ func (c *Command) skillDelete(parameters []string) (md.MD, error) {
 	if err != nil {
 		return "", err
 	}
-	if c.outputJson {
+	if c.outputJSON {
 		return md.JSONBlock(skill), nil
 	}
 	return md.Markdownf("Deleted **%s** from known skills. User profiles are not changed.", skill), nil
@@ -60,7 +60,7 @@ func (c *Command) skillList(parameters []string) (md.MD, error) {
 	if err != nil {
 		return "", err
 	}
-	if c.outputJson {
+	if c.outputJSON {
 		return md.JSONBlock(skills), nil
 	}
 	return "Known skills: " + md.JSONBlock(skills), nil

server/command/task_unassign_test.go

@@ -85,7 +85,7 @@ func TestTaskUnassign(t *testing.T) {
 			}
 			force := " "
 			if tc.force {
-				force = force + "--force"
+				force += "--force"
 			}
 			_, err := runJSON(t, SL, "/lotto task unassign test-rotation#1 "+tc.unassign+force, &out)
 			if tc.expectError {

server/command/user_join.go

@@ -4,15 +4,14 @@
 package command
 
 import (
-	"fmt"
 
 	"github.com/mattermost/mattermost-plugin-solar-lottery/server/sl"
 	"github.com/mattermost/mattermost-plugin-solar-lottery/server/utils/md"
 )
 
 func (c *Command) userJoin(parameters []string) (md.MD, error) {
 	c.withFlagRotation()
-	starting, err := c.withTimeFlag("starting", fmt.Sprintf("time for user to start participating"))
+	starting, err := c.withTimeFlag("starting", "time for user to start participating")
 	if err != nil {
 		return c.flagUsage(), err
 	}