build: Add staticcheck, "The best static analysis for Go that money can't buy"

fed60931 · Addshore · 440c417c · fed60931 · fed60931 · fed60931
Commit fed60931 authored Sep 21, 2021 by Addshore 🏄
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -47,11 +47,25 @@ test:
      reports:
        cobertura: coverage.xml
    script:
-      - go get -u golang.org/x/lint/golint
      - make test
      - go get github.com/boumenot/gocover-cobertura
      - gocover-cobertura < coverage.txt > coverage.xml
+checks:
+    stage: test
+    needs: []
+    cache: {}
+    image: docker-registry.wikimedia.org/golang:1.13-3
+    parallel:
+      matrix:
+        - CHECK: lint
+        - CHECK: vet
+        - CHECK: staticcheck
+    script:
+      # XDG_CACHE_HOME is needed by staticcheck
+      - export XDG_CACHE_HOME=/tmp/mwcli-cache
+      - make $CHECK
 build:
    stage: build
    needs: []
@@ -65,7 +79,7 @@ build:
 integration:
    stage: integration
-    needs: [test,build]
+    needs: [checks,test,build]
    dependencies:
      - build
    parallel:

--- a/Makefile
+++ b/Makefile
@@ -18,16 +18,26 @@ all: get-dev generate get build
 get:
 	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get .
-get-dev:
+# Go 1.16 onwards includes a change that makes it possible to install a binary without affecting go.mod.
+# https://stackoverflow.com/a/65734439/4746236
+# TODO use this when we are running on Go 1.16 or later
+get-dev: get-dev-govvv get-dev-gox get-dev-staticfiles get-dev-lint get-dev-staticcheck
+get-dev-govvv:
 	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get github.com/ahmetb/govvv@v0.3.0
-	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get bou.ke/staticfiles@v0.0.0-20210106104248-dd04075d4104
+get-dev-gox:
 	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get github.com/mitchellh/gox@v1.0.1
+get-dev-staticfiles:
+	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get bou.ke/staticfiles@v0.0.0-20210106104248-dd04075d4104
+get-dev-lint:
+	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get golang.org/x/lint/golint
+get-dev-staticcheck:
+	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go get honnef.co/go/tools/cmd/staticcheck
-build:
+build: get-dev-govvv
 	@echo "Building $(GOFILES) to ./bin"
 	GOPATH=$(GOPATH) GOBIN=$(GOBIN) go build -v -ldflags "$(shell ./bin/govvv -flags -version $(SEMVER))" -o bin/mw ./
-release:
+release: get-dev-govvv
 	GOPATH=$(GOPATH) GOBIN=$(GOBIN) ./bin/gox -output="$(RELEASE_DIR)/$(SEMVER)/mw_$(VERSION)_{{.OS}}_{{.Arch}}" -osarch='$(TARGETS)' -ldflags '$(shell ./bin/govvv -flags -version $(SEMVER))' $(GO_PACKAGES)
 	cp LICENSE "$(RELEASE_DIR)"
 	for f in "$(RELEASE_DIR)"/$(SEMVER)/mw_*; do \
@@ -40,9 +50,11 @@ install: all
 generate: generate-staticfiles
 	GOPATH=$(GOPATH) GOBIN=$(GOBIN) go generate $(GO_PACKAGES)
-generate-staticfiles:
+generate-staticfiles: get-dev-staticfiles
 	rm -f internal/mwdd/files/files.go || true
 	./bin/staticfiles -o internal/mwdd/files/files.go static/mwdd/
+	echo "//lint:file-ignore ST1005 It's generated code"|cat - internal/mwdd/files/files.go > internal/mwdd/files/files.go.tmp && mv internal/mwdd/files/files.go.tmp internal/mwdd/files/files.go
+	echo "//Package files ..."|cat - internal/mwdd/files/files.go > internal/mwdd/files/files.go.tmp && mv internal/mwdd/files/files.go.tmp internal/mwdd/files/files.go
 clean:
 	GOPATH=$(GOPATH) GOBIN=$(GOBIN) go clean $(GO_PACKAGES)
@@ -50,19 +62,22 @@ clean:
 	rm -rf _release || true
 	rm internal/mwdd/files/files.go || true
-test: get-dev generate unit lint
+test: get-dev-govvv generate
-unit:
 	GOPATH=$(GOPATH) GOBIN=$(GOBIN) go test -covermode=count -coverprofile "coverage.txt" -ldflags "$(shell ./bin/govvv -flags)" $(GO_PACKAGES)
-lint:
+lint: get-dev-lint generate
 	@echo > .lint-gofmt.diff
 	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) go list -f $(GO_LIST_GOFILES) $(GO_PACKAGES) | while read f; do \
 		gofmt -e -d "$${f}" >> .lint-gofmt.diff; \
 	done
 	@test -z "$(grep '[^[:blank:]]' .lint-gofmt.diff)" || (echo "gofmt found errors:"; cat .lint-gofmt.diff; exit 1)
-	GOPATH=$(GOPATH) GOBIN=$(GOBIN) golint -set_exit_status $(GO_PACKAGES)
+	GOPATH=$(GOPATH) GOBIN=$(GOBIN) ./bin/golint -set_exit_status $(GO_PACKAGES)
+vet: generate
 	GOPATH=$(GOPATH) GOBIN=$(GOBIN) go vet -composites=false $(GO_PACKAGES)
+staticcheck: get-dev-staticcheck generate
+	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) ./bin/staticcheck -version
+	@GOPATH=$(GOPATH) GOBIN=$(GOBIN) ./bin/staticcheck -- ./...
-.PHONY: all get build release install generate generate-staticfiles clean test unit lint
+.PHONY: all get build release install generate generate-staticfiles clean test lint vet staticcheck
--- a/README.md
+++ b/README.md
@@ -9,18 +9,33 @@ Take a look at the user facing docs https://www.mediawiki.org/wiki/Cli
 There is currently 1 subcommand:
- `docker` allows interacting with a new version of the MediaWiki-docker-dev development environment. (See `mw help docker`)
+- `docker` (alias: dev) Allows interacting with a new version of the MediaWiki-docker-dev development environment. (See `mw help docker`)
 ## Contributing
 Clone this repository to your `$GOPATH` (probably `~/go`), so it would be at
 `~/go/src/gerrit.wikimedia.org/r/mediawiki/tools/cli`.
-Within the `~/go/src/gerrit.wikimedia.org/r/mediawiki/tools/cli/cmd` directory:
+Within the `~/go/src/gerrit.wikimedia.org/r/mediawiki/tools/cli` directory:
- run `make` to download dependencies and build an initial binary
+Run `make` to build a binary to `~/go/src/gerrit.wikimedia.org/r/mediawiki/tools/cli/bin/mw`.
-Execute the tool without building from any directory by running the `./dev.sh` script.
+We recommend that you create a development alias for this binary, and run `make` after you make changes to the codebase.
+```sh
+alias mwdev='~/go/src/gerrit.wikimedia.org/r/mediawiki/tools/cli/bin/mw'
+```
+### Makefile commands
+Many other Makefile commands exist that you might find useful:
+- `make build`: Just builds a new binary
+- `make release`: Builds multiple release binaries to `_release`
+- `make test`: Run unit tests
+- `make lint`: Run basic linting
+- `make vet`: Run `go vet`
+- `make staticcheck`: Run https://staticcheck.io/
 ### Packages & Directories
@@ -43,12 +58,6 @@ No naming structured is enforced in CI but a convention exists that should be fo
 - Complex sub commands will be split out into their own file. For example `docker_env.go`.
 - This is a recursive solution.
-### Using a binary
-Make a binary by running `make`
-Execute the binary from any directory with `~/go/src/gerrit.wikimedia.org/r/mediawiki/tools/cli/bin/cli`
 ## Support
 - Documentation: [Cli page on mediawiki.org](https://www.mediawiki.org/wiki/Cli)

--- a/dev.sh
+++ b/dev.sh
-#!/bin/bash
-GOPATH=$(pwd)/vendor:$(pwd)
-GOBIN=$(pwd)/bin
-SCRIPTPATH=$(dirname $(realpath $0))
-WORKINGPATH=$(echo $PWD)
-cd $SCRIPTPATH
-# Force remake of the mwdd files
-rm internal/mwdd/files/files.go
-make internal/mwdd/files/files.go
-# Run from source from the origional directory
-cd $WORKINGPATH
-go run -ldflags "$(${SCRIPTPATH}/bin/govvv -flags)" ${SCRIPTPATH}/main.go $@
\ No newline at end of file
--- a/go.mod
+++ b/go.mod
@@ -27,7 +27,10 @@ require (
 	github.com/stretchr/testify v1.6.1 // indirect
 	github.com/txn2/txeh v1.3.0
 	github.com/xanzy/go-gitlab v0.50.4
-	golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad
+	golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d // indirect
+	golang.org/x/sys v0.0.0-20210921065528-437939a70204 // indirect
+	golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1
+	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	gotest.tools/v3 v3.0.3 // indirect
 )

--- a/go.sum
+++ b/go.sum
@@ -3,6 +3,7 @@ bou.ke/staticfiles v0.0.0-20210106104248-dd04075d4104/go.mod h1:JpKWzdMX3ZBe8DYU
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
+github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/Microsoft/go-winio v0.5.0 h1:Elr9Wn+sGKPlkaBvwu4mTrxtmOp3F3yV9qhaHbXGjwU=
 github.com/Microsoft/go-winio v0.5.0/go.mod h1:JPGBdM1cNvN/6ISo+n8V5iA4v8pBzdOpzfwIujj1a84=
@@ -224,6 +225,7 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
 golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY=
 golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
+golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 h1:XQyxROzUlZH+WIQwySDgnISgOivlhjIEwaQaJEJrrN0=
 golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -236,6 +238,8 @@ golang.org/x/net v0.0.0-20190522155817-f3200d17e092 h1:4QSRKanuywn15aTZvI/mIDEgP
 golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI=
 golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
+golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d h1:20cMwl2fHAzkJMEA+8J4JgqBQcQGzbisXo31MIeenXI=
+golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288 h1:JIqe8uIcRBHXDQVvZtHwp80ai3Lw3IJAeJEs55Dc1W0=
 golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
@@ -259,14 +263,22 @@ golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200831180312-196b9ba8737a h1:i47hUS795cOydZI4AwJQCKXOr4BvxzvikwDoDtHhP2Y=
 golang.org/x/sys v0.0.0-20200831180312-196b9ba8737a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c h1:VwygUrnw9jn88c4u8GD3rZQbqrP/tgas88tPUbBxQrk=
 golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210921065528-437939a70204 h1:JJhkWtBuTQKyz2bd5WG9H8iUsJRU3En/KRfN8B2RnDs=
+golang.org/x/sys v0.0.0-20210921065528-437939a70204/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
+golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
+golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.5 h1:i6eZZ+zk0SOf0xgBpEpPD18qWcJda6q1sxt3S0kzyUQ=
 golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
+golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4 h1:SvFZT6jyqRaOeXpc5h/JSfZenJ2O330aBsf7JfSUXmQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20191024005414-555d28b269f0 h1:/5xXl8Y5W96D+TtHSlonuFqGHIWVuyCkGJLwGh9JJFs=
@@ -275,9 +287,12 @@ golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGm
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
+golang.org/x/tools v0.0.0-20190624222133-a101b041ded4 h1:1mMox4TgefDwqluYCv677yNXwlfTkija4owZve/jr78=
 golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
+golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.3.0 h1:FBSsiFRMz3LBeXIomRnVzrQwSDj4ibvcRexLG0LZGQk=
 google.golang.org/appengine v1.3.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
@@ -308,4 +323,5 @@ gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
 gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
 gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
+honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099 h1:XJP7lxbSxWLOMNdBE4B/STaqVy6L73o0knwj2vIlxnw=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
--- a/internal/mwdd/docker.go
+++ b/internal/mwdd/docker.go
@@ -31,7 +31,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/signal"
-	"golang.org/x/crypto/ssh/terminal"
+	terminal "golang.org/x/term"
 )
 // DockerExecCommand to be run with Docker, which directly uses the docker SDK
@@ -110,10 +110,7 @@ func (m MWDD) DockerExec(command DockerExecCommand) {
 	fd := int(os.Stdin.Fd())
 	var oldState *terminal.State
 	if terminal.IsTerminal(fd) {
-		oldState, err = terminal.MakeRaw(fd)
+		oldState, _ = terminal.MakeRaw(fd)
-		if err != nil {
-			// print error
-		}
 		defer terminal.Restore(fd, oldState)
 	}

--- a/internal/mwdd/files/files-packaged.go
+++ b/internal/mwdd/files/files-packaged.go
@@ -22,12 +22,3 @@ func packagedFileNames() []string {
 	}
 	return keys
 }
-func filter(ss []string, test func(string) bool) (ret []string) {
-	for _, s := range ss {
-		if test(s) {
-			ret = append(ret, s)
-		}
-	}
-	return
-}
--- a/internal/util/dotenv/dotenv_test.go
+++ b/internal/util/dotenv/dotenv_test.go
@@ -102,9 +102,6 @@ func TestFile_GeneralIntegration(t *testing.T) {
 	envFile := File("/tmp/mwcli-test-dotenv-" + randomString())
 	envFile.EnsureExists()
-	type args struct {
-		name string
-	}
 	tests := []struct {
 		name           string
 		f              File