もうgolangci-lintなんていらねいさ 歌おう別れの歌を

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 で効率よく実行可能です。

例えば

cmd/lint/main.go

// +build lint

package main

import (
  // ...
)

func main() {
  multichecker.Main(
    assign.Analyzer,
    unreachable.Analyzer,
    simple.Analayzer,
    exportloopref.Analyzer,
  )
}

というような lint 専用の entrypoint をリポジトリにおいて、

.github/workflows/lint.yml

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を手軽に追加できる

脱却へのマイルストーン

とはいえ脱却にあたっては、まだまだ不透明な部分が多くあります。
そこで脱却に向けて何をすれば良いのかも検討してみました。
（順不同）

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に別れを告げられるようにしていきましょう。

マイルストンに書いた調査その他は…頑張ります…つらい