Commenting on nolint flags

[matthewpoer]

I've found a behavior and writing style that I'd like to share and include in the project documentation.

I've found that using the //nolint flag can be made more useful by supplying comments on the flag itself, which is accomplished (IMO) most easily and readibly on the same line as the flag itself.

Consider the following definition of a global variables within my main package:

package main

var (
	// ReleaseHash has the hash of the most recent Git commit, i.e. the result of `git rev-parse HEAD`
	ReleaseHash string
)

This is a clear violation of gochecknoglobals, but in this case these variables act similarly to (the known exceptions)[https://github.com/leighmcculloch/gochecknoglobals#exceptions] to gochecknoglobals like version. It should be allowed, so I'll flag the var block with a //nolint:gochecknoglobals.

I know my reasoning is sound, but will I remember in six weeks? Will the next developer understand? I've always been one to leave verbose annotation in my code. I've found that golangci-lint allows the comment to exist on the same line as the flag itself which is IMO the easiest thing to read, e.g.

//nolint:gochecknoglobals // allows setting of application runtime params from Makefile
var (
	// ReleaseHash has the hash of the most recent Git commit, i.e. the result of `git rev-parse HEAD`
	ReleaseHash string
)

To place the same comment above or below the //nolint flag would read as if the comment applied to the var block, but being on the same line makes it more clear that it relates to the flag.

I like this behavior, and if it's not in violation of the maintainers' best practices I'd like this noted in the project README as to encourage others to leave comments explaining and justifying why areas of code should not be linted.

Obligatory environment information

matthewpoer ~/go/src/github.com/golangci/golangci-lint
> git rev-parse HEAD
d2b1eea2c6171a1a1141a448a745335ce2e928a1

matthewpoer ~/go/src/github.com/matthewpoer/plann-pro-backend
> cat .golangci.yml 
linters:
  enable-all: true
linters-settings:
  gocyclo:
    min-complexity: 10
run:
  skip-dirs:
    - bin
    - gen

> go version && go env
go version go1.11.5 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/matthewpoer/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/matthewpoer/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/nx/ch9gcksx2cdc5frh4xlyvrhw0000gn/T/go-build077950777=/tmp/go-build -gno-record-gcc-switches -fno-common"

matthewpoer ~/go/src/github.com/matthewpoer/plann-pro-backend
> golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/matthewpoer/go/src/github.com/matthewpoer/plann-pro-backend /Users/matthewpoer/go/src/github.com/matthewpoer /Users/matthewpoer/go/src/github.com /Users/matthewpoer/go/src /Users/matthewpoer/go /Users/matthewpoer /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 32 linters: [bodyclose deadcode depguard dupl errcheck gochecknoglobals gochecknoinits goconst gocritic gocyclo gofmt goimports golint gosec gosimple govet ineffassign interfacer lll maligned misspell nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck] 
INFO [lintersdb] Optimized sublinters [staticcheck gosimple unused stylecheck] into metalinter megacheck 
INFO [loader] Go packages loading at mode load types and syntax took 1.797997627s 
INFO [loader] SSA repr building timing: packages building 24.901532ms, total 220.841208ms 
INFO [runner] worker.1 took 311.475261ms with stages: misspell: 191.813496ms, gofmt: 74.410808ms, unconvert: 26.54957ms, gocritic: 11.052767ms, ineffassign: 6.215355ms, maligned: 419.928µs, structcheck: 397.878µs, goconst: 281.459µs, nakedret: 218.747µs, gochecknoglobals: 35.595µs, gochecknoinits: 23.682µs, depguard: 11.186µs 
INFO [runner] worker.7 took 311.254082ms with stages: unparam: 306.289174ms, lll: 2.40644ms, deadcode: 1.379947ms, varcheck: 1.156231ms, typecheck: 5.427µs 
INFO [runner] worker.2 took 352.363005ms with stages: gosec: 147.899409ms, goimports: 73.147213ms, errcheck: 68.1441ms, dupl: 62.197299ms, gocyclo: 466.878µs, prealloc: 253.739µs, scopelint: 225.074µs 
INFO [runner] worker.3 took 366.556228ms with stages: interfacer: 366.549962ms 
INFO [runner] worker.8 took 378.90247ms with stages: bodyclose: 378.897015ms 
INFO [runner] worker.6 took 392.326634ms with stages: govet: 392.318439ms 
INFO [runner] worker.5 took 503.462093ms with stages: golint: 503.455518ms 
INFO [runner] worker.4 took 1.111901427s with stages: megacheck: 1.111893237s 
INFO [runner] Workers idle times: #1: 800.238128ms, #2: 759.405669ms, #3: 745.16775ms, #5: 608.394734ms, #6: 719.525328ms, #7: 788.325303ms, #8: 732.919098ms 
INFO [runner/skip dirs] Skipped dir gen/restapi/operations by pattern gen 
INFO [runner/skip dirs] Skipped dir gen/restapi by pattern gen 
INFO [runner] Issues before processing: 36, after processing: 0 
INFO [runner] processing took 36.40611ms with stages: skip_dirs: 28.748497ms, cgo: 3.808617ms, identifier_marker: 3.141093ms, nolint: 220.642µs, exclude: 189.07µs, path_prettifier: 129.121µs, filename_unadjuster: 84.761µs, autogenerated_exclude: 60.548µs, source_code: 8.526µs, skip_files: 2.911µs, exclude-rules: 2.177µs, diff: 2.059µs, max_same_issues: 1.927µs, path_shortener: 1.591µs, uniq_by_line: 1.536µs, max_from_linter: 1.519µs, max_per_file_from_linter: 1.515µs 
INFO File cache stats: 13 entries of total size 37.6KiB 
INFO Memory: 33 samples, avg is 462.1MB, max is 1078.7MB 
INFO Execution took 3.247879324s