[Go] nilnilで3つ以上の連続するnilを検出できるようになりました
はじめに
こんにちは。CANARY Cloudソフトウェアエンジニアの久嶋です。
今回は自分の提案がきっかけでAntonboom/nilnil(以降nilnil)に「3つ以上の連続するnil値を検出するオプション」が追加されたのでその過程と追加されたオプションの使い方を紹介します。
tl;dr
- nilnilで
return nil, nil, ..., nil
のような3つ以上の連続するnil値を検出できるようになった - golangci-lint経由で呼び出す場合はgolangci-lintの設定ファイルに次のように
only-two: false
を指定することで有効化できるlinters: settings: nilnil: # デフォルト値はtrue only-two: false
-
golangci-lint@v2.0.0
以降で利用可能
ここからは上記の機能提案の経緯を説明します。
直面していた課題
CANARY Cloudでreturn nil, nil, nil
のようなコードが原因で限られた状況において意図しない挙動になることがわかりました。発生頻度は極めて低いものでしたがお客様の業務への影響を最小限にするためにすぐに修正リリースを行いました。
その後、根本原因の解決策を検討しました。
コードレビュー時にチェックして再発を防止する手もありましたが、労力や属人性の観点から静的解析を用いて問題のあるコードを機械的に検出する方針を選択しました。
解決策の候補
まずは既存のlinterやCLIツールでreturn nil, nil, ..., nil
のようなコードを検出できるものがないか調査しましたが見つけることはできませんでした。
そこで、以下の2つの選択肢を検討しました。
-
return nil, nil, ..., nil
のようなコードを検出できる独自linterを作成しCIに組み込む - nilnilに
return nil, nil, ..., nil
のようなコードを検出できる機能を追加する
それぞれ詳しく見ていきましょう。
1. 独自linterを作成する
繰り返しになりますが、return nil, nil, ..., nil
のようなコードを検出できる独自linterを作成しCIに組み込む方法です。
この方法のメリットとデメリットは以下です。
- メリット
- 生成AIやgostaticanalysis/skeletonなどのツールを利用すれば低工数で実装できる見込みがある
- コードレビューなどのやり取りが社内で完結するため
2.
と比較するとリードタイムが短くなる見込みがある - すでにgcpug/zaganeなどの静的解析ツールをCIで動かしていたので独自linterをCIに組み込むハードルが低い
- デメリット
- (独自linterを社外に公開しなければ)独自linterの恩恵を自社内でしか享受できない
- 外部に公開するとなるとドキュメントやコード品質、コミットメッセージなどをある程度整える工数が必要になる
- 公開したとしても利用者がCIに組み込む手間が必要になる
- (独自linterを社外に公開しなければ)独自linterの恩恵を自社内でしか享受できない
以上のことからスピーディーに課題を解決できるものの開発の恩恵が自社内に閉じてしまう点が懸念だと評価しました。
2. nilnilに機能追加を行う
こちらはnilnil側に3つ以上の連続するnil値を検出する機能を追加する方法です。issueで機能追加を提案し、メンテナに承諾されたらPRを送ろうと考えていました。
この方法のメリットとデメリットは以下です。
- メリット
- nilnilはOSSなので機能追加の恩恵をコミュニティ全体が受けることができる
- golangci-lint経由で実行できるためCIに加える変更が最小限で済む
- デメリット
- そもそも機能追加が承諾されない可能性がある
- nilnilのメンテナのapproveが必要になるため
1.
と比較するとリードタイムが長くなる可能性が高い - nilnilの既存コードとの一貫性やコード設計を考慮したPRを作成する必要があるため
1.
と比較すると実装コストが大きくなる可能性がある(当然既存実装がどうなっているかにもよるので一概にはいえないが)
まとめると 1.
と比較してコミュニティへの貢献度合いが大きい一方でリードタイムは多少長くなる見込みで、実装工数も大きくなりそうという印象でした。
選択した解決策
(記事のタイトルから明らかですが)2. nilnilに機能追加を行う
を選択することにしました。
機能追加による恩恵をより広くコミュニティと共有できる点やgolangci-lint経由で手軽に実行できる点が決め手でした。
独自linterの作成よりはリードタイムが長くなりそうでしたが、2週間〜1か月程度あればリリースまで完了できると予想しそこまで問題にはならないと判断したことも後押しとなりました。
ただし、nilnilへの機能追加がメンテナに受け入れられるとは限らないので以下のようなフローで対応を進めることに決めました。
図にするまでもないのですが、issueで機能追加を提案しメンテナから承諾を得られたらPRを送り、意見が合わなかった場合は独自linterを作成しようと考えていました。
方針を決めたところで早速以下のような内容のissueを作成しました。
- nilnilのお陰で潜在的なバグを検出できておりとても感謝している
- nilnilを拡張して3つ以上の連続したnil値のreturnを検出できるように機能追加することを提案したい
- もしこの提案に前向きであればPRを作成しようと思っている
こちらが実際のissueです。
Feature Suggestion: Detecting 3 or More Consecutive nil Values #52
結果
issueの作成後、リポジトリのメンテナからすぐに返信があり「具体的なユースケースを教えてほしい」という連絡がありました。
簡単なサンプルコードを送って返信を待っていたところ、なんとメンテナが「実装してみたから試してみてほしい」という連絡をくれました。「機能追加に賛成してもらえたらラッキーだな」くらいの気持ちでissueを作成したのにまさか実装までやっていただけるとはまさに青天の霹靂でした。
自分たちのコードベースでテストしてみたところ無事にreturn nil, nil, nil
のようなコードを検出できることが確認できました。(検出されたコードはバグではなく意図した実装でした)感謝とともに期待通りに動いていることを報告するとすぐにPRがマージされv1.1.0
としてリリースされました。
そこから2週間ほど経過しgolangci-lint側でもnilnil@v1.1.0を取り込んだバージョンがリリースされたので無事CI環境でも利用できるようになりました。
nilnilに機能追加される結果となったので自分たちが欲しかった機能が利用可能になっただけでなく、他のユーザーからも利用可能な状態になりました。快く提案を受け入れるだけでなく実装までしてくださったAnton Telyshev氏には心から感謝しています。
only-two
オプションの使い方
今回追加されたonly-two
オプションの使い方を紹介します。
- 次のように
return nil, nil, nil
のようなreturn文を含むコードを用意するfunc searchUserAndAddressByUserID(userID string) (*User, *Address, error) { user, err := getUser(userID) if err != nil { return nil, nil, err } if user.Status != "active" { // 本来はnil, nil, ErrNotFoundを返すべきだが、誤ってnil, nil, nilと書いてしまったケースを想定したコード return nil, nil, nil } address, err := getAddress() if err != nil { return nil, nil, err } return user, address, nil }
- golangci-lintの設定ファイルに次のような記述を追加する
linters: settings: nilnil: only-two: false
- golangci-lintを実行すると期待通り
return nil, nil, nil
の箇所に対してエラーが報告される$ GOTOOLCHAIN=go1.24.0 go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.0.0 run ./... --config ./.golangci.yml main.go:37:3: return both a `nil` error and an invalid value: use a sentinel error instead (nilnil) return nil, nil, nil ^ 1 issues: * nilnil: 1 exit status 1
また、nilnilはreturn nil, nil, nil
のようなコード以外に次のようなコードに対してもエラーを報告します。
func searchUserAndAddressByUserID(userID string) (*User, *Address, error) {
return nil, &Address{
Country: "Japan",
City: "Tokyo",
StreetAddress: "1-1",
}, nil
}
エラー内容は上述のケースと同様にreturn both a `nil` error and an invalid value
です。
$ GOTOOLCHAIN=go1.24.0 go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.0.0 run ./... --config ./.golangci.yml
main.go:37:3: return both a `nil` error and an invalid value: use a sentinel error instead (nilnil)
return nil, &Address{
^
1 issues:
* nilnil: 1
exit status 1
これはnilnilのテストコードからもわかるように意図した仕様で、return &User{}, &Address{}, nil
のように最後尾の返り値以外がすべてnon-nilであることを期待しているためです。このようなコードを書きたいケースでは以下のようにnolint
ディレクティブを使用するのも手かもしれません。
//nolint:nilnil //意図したコードなのでnolintする
return nil, &Address{
Country: "Japan",
City: "Tokyo",
StreetAddress: "1-1",
}, nil
おわりに
繰り返しにはなりますが、nilnilに3つ以上の連続するnil値が検出するためのオプションが追加されました。より多くのバグを機械的に検出するために有効化することを検討してみてはいかがでしょうか。
また、今回のやり取りを通してOSSコミュニティは関わることが難しい特殊な領域ではなく貢献が歓迎される開かれたコミュニティであることをあらためて実感しました。[1] nilnilのようにたくさんの企業/プロジェクトのソフトウェアエンジニアリングを支えているにも関わらず個人の活動によって維持されているOSSプロジェクトは数多く存在していると思うので自分にできる貢献は積極的にしていきたいと思います。[2]
以上です。最後まで読んでくださりありがとうございました。
-
当然プロジェクトによっては外部貢献に対してポジティブではないケースもありますが、筆者の経験では多くのOSSプロジェクトで貢献が歓迎されていると感じています。 ↩︎
-
golangci-lintのドキュメントに改善できそうな箇所があったので早速PRを送ってみたところ、無事マージいただけました。 https://github.com/golangci/golangci-lint/pull/5697 ↩︎
Discussion