tl;dr
- go vet (go/analysis/passes) がとても充実している
- 惰性でgolangci-lintに頼っているかもしれない
- 何が必要で何がいらないのか見極めが必要そう
- go vetをannotationsに飛ばすGitHub Action作ったよ
背景
go の静的解析と群雄割拠のlinter群
過去のgo界隈では、様々な開発者が go/types や go/ast を駆使して思い思いのI/Fのlinterを作り、今でもそれらの多くは生き残っています。
それらのI/F差を吸収・統合する目的で gometalinter などが作られてきました。
今は gometalinter は後継の golangci-lint にその座を譲り、 golangci-lint のほぼ一強となっています。
go/analysis の登場
ですがそれも昔の話。
@tenntennさんなどがもうさんざんいろいろな記事を書いておられるのでいまさら語るまでもないことですが、goにおけるlinterの作成は golang.org/x/tools/go/analysis の登場により、構文解析処理のラップ（Pass/inspect）、解析間の依存I/Fの定義（Fact）、ユーザーI/Fの共通定義（xxxchecker）といった部分が整理されています。
goの公式から提供されている linter go vet も今では go/analysis の共通I/Fに則っています。
go vet と golangci-lint
ところでかの golangci-lint ですが、利用していると 同じ lint を違うlinterから同時に指摘されるケースが目につく 気がしています。
golangci-lint 自体が、 go vet を利用しており、どうも go vet と他のlinterとで同じ lint を同時に検出するケースがあるようです。
go vet の中身を見てみると、なかなか面白い構成です。
$ go tool vet help
asmdecl report mismatches between assembly files and Go declarations
assign check for useless assignments
atomic check for common mistakes using the sync/atomic package
bools check for common mistakes involving boolean operators
buildtag check that +build tags are well-formed and correctly located
cgocall detect some violations of the cgo pointer passing rules
composites check for unkeyed composite literals
copylocks check for locks erroneously passed by value
httpresponse check for mistakes using HTTP responses
loopclosure check references to loop variables from within nested functions
lostcancel check cancel func returned by context.WithCancel is called
nilfunc check for useless comparisons between functions and nil
printf check consistency of Printf format strings and arguments
shift check for shifts that equal or exceed the width of the integer
stdmethods check signature of methods of well-known interfaces
structtag check that struct field tags conform to reflect.StructTag.Get
tests check for common mistaken usages of tests and examples
unmarshal report passing non-pointer or non-interface values to unmarshal
unreachable check for unreachable code
unsafeptr check for invalid conversions of uintptr to unsafe.Pointer
unusedresult check for unused results of calls to some functions
久々にそのラインナップを確認して、そんなにチェックしてたのか、と思うくらいに多いので、少し面食らいました。
一方の golangci-lintを見てみます。
govet - Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
errcheck - Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
staticcheck - Staticcheck is a go vet on steroids, applying a ton of static analysis checks
unused - Checks Go code for unused constants, variables, functions and types
gosimple - Linter for Go source code that specializes in simplifying a code
structcheck - Finds unused struct fields
varcheck - Finds unused global variables and constants
ineffassign - Detects when assignments to existing variables are not used
deadcode - Finds unused code
typecheck - Like the front-end of a Go compiler, parses and type-checks Go code
bodyclose - checks whether HTTP response body is closed successfully
noctx - noctx finds sending http request without context.Context
golint - Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes
rowserrcheck - checks whether Err of rows is checked successfully
stylecheck - Stylecheck is a replacement for golint
gosec - Inspects source code for security problems
interfacer - Linter that suggests narrower interface types
unconvert - Remove unnecessary type conversions
dupl - Tool for code clone detection
goconst - Finds repeated strings that could be replaced by a constant
gocyclo - Computes and checks the cyclomatic complexity of functions
gocognit - Computes and checks the cognitive complexity of functions
asciicheck - Simple linter to check that your code does not contain non-ASCII identifiers
gofmt - Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
gofumpt - Gofumpt checks whether code was gofumpt-ed.
goimports - Goimports does everything that gofmt does. Additionally it checks unused imports
goheader - Checks is file header matches to pattern
maligned - Tool to detect Go structs that would take less memory if their fields were sorted
depguard - Go linter that checks if package imports are in a list of acceptable packages
misspell - Finds commonly misspelled English words in comments
lll - Reports long lines
unparam - Reports unused function parameters
dogsled - Checks assignments with too many blank identifiers (e.g. x, , , _, := f())
nakedret - Finds naked returns in functions greater than a specified function length
prealloc - Finds slice declarations that could potentially be preallocated
scopelint - Scopelint checks for unpinned variables in go programs
gocritic - The most opinionated Go source code linter
gochecknoinits - Checks that no init functions are present in Go code
gochecknoglobals - Checks that no globals are present in Go code
godox - Tool for detection of FIXME, TODO and other comment keywords
funlen - Tool for detection of long functions
whitespace - Tool for detection of leading and trailing whitespace
wsl - Whitespace Linter - Forces you to use empty lines!
goprintffuncname - Checks that printf-like functions are named with f at the end
gomnd - An analyzer to detect magic numbers.
goerr113 - Golang linter to check the errors handling expressions
gomodguard - Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations.
godot - Check if comments end in a period
testpackage - linter that makes you use a separate _test package
nestif - Reports deeply nested if statements
exportloopref - An analyzer that finds exporting pointers for loop variables.
exhaustive - check exhaustiveness of enum switch statements
sqlclosecheck - Checks that sql.Rows and sql.Stmt are closed.
nolintlint - Reports ill-formed or insufficient nolint directives
数はやたらめったら多いですが、いくつか「これは実は要らないのでは？」と思えるものもちらほら見受けられます。
-
deadcode:go vetのunreachableで代えられないか？ -
ineffassign:go vetのassignってこれ相当なのでは？ - などなど
また、いくつかの linter は go/analysis に準拠したI/Fを提供しているため、 go/analysis/xxxchecker で呼び出すことが可能です。
（これを調べるのにひどく苦労しました）
- govet
- staticcheck
- unused
- gosimple
- bodyclose
- noctx
- rowserrcheck
- stylecheck
- asciicheck
- goprintffuncname
- gomnd
- goerr113
- testpackage
- exportloopref
- exhaustive
golangci-lint からの脱却
これらの背景を踏まえ、 golangci-lint から脱却できる可能性を模索しています。
go vet only
最低限のlinterは go vet で揃っているのだから、実は golangci-lint を使わなくても良いかもしれません。
その他の linterを組み合わせる
必要な(go/analysis に準拠した）linterがあれば、 go/analysis/xxxchecker で効率よく実行可能です。
例えば
// +build lint
package main
import (
// ...
)
func main() {
multichecker.Main(
assign.Analyzer,
unreachable.Analyzer,
simple.Analayzer,
exportloopref.Analyzer,
)
}
というような lint 専用の entrypoint をリポジトリにおいて、
name: Lint
on: [push]
jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/setup-go@v2
- name: run linter
run: go run ./cmd/lint/main.go ./...
とでもすれば、golangci-lintよろしくカスタムのlintを実行できます。
脱却のpros./cons.
別に今あるものが動いてるんだから良いじゃないか、という話は大いにあるので、pros./cons.をまとめてみました。
golangci-lintの優位性
- 無思考で利用できる
- 公式のGitHub Actionがある
- なんとなくアップデートすればそれなりに新しいlinterに追随できる
- GitHub/super-linters を利用すれば勝手についてくる
-
// nolintコメントによる細やかな調整が可能 - GitHub Annotation が見やすい
-
go/analysisに準拠していない linter を統合できる
脱却の優位性
- Contributorに
golangci-lintのinstallを求めない- installしなくてもできるけど、CI待つことにはなる
- .golangci.yml のような設定ファイルを覚える必要がない
- 独自に
go.modでバージョン指定ができる- Third-party linterも（
go/analysis準拠ならば）自分で取り込める - 新しいバージョンのlinterにバグがあったら巻き戻すことも可能
- プロジェクトローカルのlinterを手軽に追加できる
- Third-party linterも（
脱却へのマイルストーン
とはいえ脱却にあたっては、まだまだ不透明な部分が多くあります。
そこで脱却に向けて何をすれば良いのかも検討してみました。
（順不同）
GitHub Action の作成
golangci-lint はその名の通り、CIでのサポートを充実させています。
- GitHub Actionで実行する
- GitHubのAnnotationに投稿する
といった機能があります。
先走って後者だけは作ってしまいました。
step:
- name: go vet
run: go vet -json ./... 2> diagnostics.json
- name: annotate diagnostics
uses: kyoh86/go-check-action/annotate@v1
with:
level: error
exit-code: 1
のように、 go vet など（go/analysis/xxxchecker準拠の）linterであれば、
その -json フラグの出力をParseしてGitHub Annotationに投稿してくれます。
前者も用意したいですね。（まだ作ってないよ）
step:
- name: run analyzers
uses: kyoh86/go-check-action/check@v1
with:
analyzer-packages: |
golang.org/x/tools/go/analysis/passes/assign
golang.org/x/tools/go/analysis/passes/unreachable
github.com/dominikh/go-tools/simple
github.com/kyoh86/exportloopref
こんなふうにできたら最高なんですが。多分やれる。
何が検出されなくなるのか見極める
golangci-lintで見つけられたlintのうち、いくつかは go/analysis に準拠していないので、使えなくなります。
その時
- deadcode : go vet の unreachable で代えられないか？
- ineffassign : go vet の assign ってこれ相当なのでは？
というように、相当する準拠したlinterと置換したとき（あるいは置換できなかったとき）、何が検出されなくなるのかはきちんと見極める必要があるでしょう。
古い linter を go/analysis 準拠のものに作り変える
漏れるlintのうち、どうしても困るものは新しい linter を作る必要が生じる可能性,もあります。
きっつい。
まとめ
go/analysisの充実で、golangci-lintのような辛いラッパーを撲滅できる可能性が出てきました。
gophersのみんなで新しいlinterをどんどん作って、いっそgolangci-lintに別れを告げられるようにしていきましょう。
マイルストンに書いた調査その他は…頑張ります…つらい