概要

go fixが利用するgo/analysis/passes/modernizeのアナライザ一覧が変更された。[]byte(fmt.Sprintf(...))をfmt.Appendf(nil, ...)へ書き換えるfmtappendfアナライザは、書き換え結果の質に懸念があったため既定の実行対象から削除された。またsync.WaitGroupのAdd/Done呼び出しをWaitGroup.Goへ書き換えるwaitgroupアナライザは、go vet側に別途存在する同名のwaitgroupアナライザ(誤ったAdd呼び出し位置を検出する)との混同を避けるため、waitgroupgoへ改称された。

導入経緯

issue #77581で、報告者がfmtappendfモダナイザを大規模なコードベースへ適用したところ、書き換え候補の半数以上でより適切な代替案が見落とされていること、また残りのケースでも可読性の低下と引き換えの微妙な最適化に留まり、明確な改善(Pareto improvement)とは言えないことを指摘し、アナライザの削除を提案した。

議論のハイライト

• 報告者は、w.Write([]byte(fmt.Sprintf("hello, %s!", name)))のようなコードに対し、fmtappendfはw.Write(fmt.Appendf(nil, "hello, %s!", name))を提案するが、本来望ましいのはfmt.Fprintf(w, "hello, %s!", name)への書き換えである、と指摘した。
• append式の中で使われているケース(append(dst, []byte(fmt.Sprintf(...))...))でも、fmtappendfはappend(dst, fmt.Appendf(nil, ...)...)という遠回りな形を提案するが、fmt.Appendf(dst, ...)と直接書く方が望ましいと指摘した。
• コアチームメンバーから、以前から別のレビュアーも同様の懸念を示していたことを踏まえ、削除に同意するコメントが付いた。
• 対応する変更はCL 771561(go/analysis/passes/modernize: disable fmtappendf modernizer)として提出された。

移行時の注意

go fixやgoplsの「モダナイズ」機能でfmtappendfによる自動書き換えを利用していた場合、当該アナライザは既定の実行対象から外れるため、今後は提案されなくなる。また、sync.WaitGroupのAdd/Done呼び出しをWaitGroup.Goへ書き換える提案をアナライザ名で明示的に指定していた場合(フラグや無視コメントでの指定など)、名称がwaitgroupgoに変わったため指定名の更新が必要になる。

実装解説

modernize.goのアナライザ一覧(Suite)を確認すると、FmtAppendfAnalyzerは完全に削除されたわけではなく、// FmtAppendfAnalyzer, // makes code less clear, see golang/go#77581という形でコメントアウトされている。実装自体(fmtappendf.go)は残ったまま、既定のSuiteから除外されている状態である。一方、waitgroupgo.goではアナライザ名がName: "waitgroupgo"として定義されており、これはgo vet側のgolang.org/x/tools/go/analysis/passes/waitgroupパッケージが提供する、誤ったAdd呼び出し位置を検出する別のwaitgroupアナライザとの名前衝突を避けるための改称である。