dotnet-csharp-code-smells について
C# の開発者やレビュアー向けに、コードスメルとアンチパターンをまとめたガイドです。リソース管理、LINQ、イベント、設計、例外処理などの問題と修正案、関連する CA ルールを示します。
主な機能
- 01リソース破棄の問題を特定
- 02LINQ のアンチパターンを確認
- 03イベント処理のリークを検出
- 04例外処理の不備を確認
- 05関連する CA ルールを参照
活用シーン
- 01C# コードの一般的なスメルをレビュー
- 02リソースリークやイベント購読の問題を調査
- 03CA ルールに沿ってコードを改善
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/インストール後、Claude Code で/dotnet-csharp-code-smellsを入力して読み込まれたか確認します。
同梱ファイルdetails.md
詳細
dotnet-csharp-code-smells
C# コードのコードスメルとアンチパターンを積極的に検出します。このスキルは、計画、実装、レビューのすべてのワークフローモードで起動します。各項目では、コードスメルを特定し、それが有害な理由を説明し、適切な修正方法を示し、関連する CA ルールまたは相互参照を記載します。
相互参照：非同期処理の注意点については[スキル：dotnet-csharp-async-patterns]、命名とスタイルについては[スキル：dotnet-csharp-coding-standards]、DI のライフタイムの誤用については[スキル：dotnet-csharp-dependency-injection]、NRT の注釈ミスについては[スキル：dotnet-csharp-nullable-reference-types]を参照してください。
対象外： LLM 固有の生成ミス（誤った NuGet パッケージ、不適切なプロジェクト構成、MSBuild エラー）は[スキル：dotnet-agent-gotchas]で扱います。このスキルでは、人間か AI かを問わず、あらゆる開発者が避けるべき一般的な .NET のコードスメルを扱います。
1. リソース管理（IDisposable の誤用）
| コードスメル | 有害な理由 | 修正方法 | ルール |
|---|---|---|---|
IDisposable なローカル変数に using がない | アンマネージドハンドル（ソケット、ファイル、DB 接続）がリークします | using 宣言または using ブロックで囲みます | CA2000 |
IDisposable フィールドを破棄しない | クラスが破棄可能なリソースを保持しているのに、破棄しません | IDisposable を実装し、Dispose() 内でフィールドを破棄します | CA2213 |
| 誤った Dispose パターン（ファイナライザーガードがない） | 二重破棄、または GC のファイナライズ時にクリーンアップが漏れます | 標準的な Dispose(bool) パターンに従い、GC.SuppressFinalize(this) を呼び出します | CA1816 |
| あるメソッドで作成した破棄可能オブジェクトをフィールドに格納する | 所有権が不明確になり、破棄を忘れやすくなります | 所有権を明記し、包含するクラスを IDisposable にします | CA2000 |
所有していないリソースに using を使う | 共有リソース（例：注入された HttpClient）を早期に破棄してしまいます | 自分で作成したリソースのみを破棄し、注入されたサービスは DI に管理させます | -- |
各パターンのコード例については、details.md を参照してください。
2. 警告抑制のハック
| コードスメル | 有害な理由 | 修正方法 | ルール |
|---|---|---|---|
CS0067 を抑制するためにイベントを null で呼び出す | 誤解を招く実行時の動作を生み、実際のバグを隠します | #pragma warning disable CS0067 または明示的なイベントアクセサー { add {} remove {} } を使います | CS0067 |
| CS0219 を抑制するためのダミー変数への代入 | 読み手を混乱させるデッドコードです | _ = expression; による破棄、または #pragma warning disable を使います | CS0219 |
復元を伴わない一括 #pragma warning disable | ファイルの残りの部分にあるすべての警告を抑制します | 必ず #pragma warning restore と組み合わせ、特定のコードのみを抑制します | -- |
根拠の記載がない [SuppressMessage] | 将来の保守担当者が、抑制が依然として妥当かどうかを判断できません | 必ず Justification = "reason" を含めます | CA1303 |
CS0067 の背景となる例（悪いパターンから適切な修正への変更）については、details.md を参照してください。
3. LINQ のアンチパターン
| コードスメル | 有害な理由 | 修正方法 | ルール |
|---|---|---|---|
チェーンの途中で早まって .ToList() を呼び出す | すべての要素を強制的に実体化し、メモリを無駄に使います | チェーンを遅延評価のまま保ち、最後にのみ実体化します | CA1851 |
IEnumerable<T> を複数回列挙する | 列挙のたびにクエリまたは DB 呼び出しが再実行されます | .ToList() で一度実体化してから再利用します | CA1851 |
| EF Core でクライアント側評価を行う | テーブル全体をメモリに読み込み、気づきにくい性能上の問題を引き起こします | 変換可能な LINQ にクエリを書き換えるか、明示的な意図を持って AsAsyncEnumerable() を使います | -- |
.Any() の代わりに .Count() > 0 を使う | 短絡評価せず、コレクション全体を列挙します | 存在確認には .Any() を使います | CA1827 |
.Join() または .GroupJoin() の代わりにネストした foreach を使う | O(n+m) で処理できるところを O(n*m) で処理します | LINQ の結合操作または Dictionary による検索を使います | -- |
.First(predicate) の代わりに .Where().First() を使う | 不要な中間イテレーターを作成します | 述語を .First() または .FirstOrDefault() に直接渡します | CA1826 |
4. イベント処理の漏れ
| 問題点 | 有害な理由 | 修正方法 | ルール |
|---|---|---|---|
| イベントの購読を解除しない | メモリリーク：発行元が購読者への参照を保持します | Dispose() で購読を解除するか、弱いイベントパターンを使用します | -- |
| コンストラクターでイベントを発生させる | 購読者がまだ登録されていない可能性があり、派生クラスの構築も完了していません | 完全に初期化されたインスタンスからのみイベントを発生させます | CA2214 |
イベントハンドラーでの async void の誤用 | async void はイベントハンドラーで有効なシグネチャですが、例外を捕捉できません | 本体を try/catch で囲み、例外を明示的に記録して処理します | -- |
| イベントハンドラーで null を確認しない | 購読者がいない場合に NullReferenceException が発生します | event?.Invoke() の null 条件付きパターンを使用します | -- |
| 静的イベントのクリーンアップを行わない | ルート参照によって、アプリケーションの実行中は GC が参照を回収できません | インスタンスイベントを優先するか、WeakEventManager を使用します | -- |
関連項目：[skill:dotnet-csharp-async-patterns] では、async void のファイア・アンド・フォーゲットパターンを詳しく説明しています。
5. 設計上の問題点
| 問題点 | しきい値 | 有害な理由 | 修正方法 |
|---|---|---|---|
| ゴッドクラス | 500 行超 | 責務が多すぎて、テストや保守が困難になります | SRP に従って、まとまりのあるクラスを切り出します |
| 長すぎるメソッド | 30 行超 | 理解、テスト、レビューが困難になります | 説明的な名前を付けたヘルパーメソッドを切り出します |
| 長すぎるパラメーターリスト | 5 個超 | 抽象化が不足していることを示します | パラメーターオブジェクトまたはビルダーを導入します |
| フィーチャー・エンヴィ | メソッドが自身のクラスよりも別のクラスのデータを多く使用する | 責務の配置が不適切で、結合度が高くなります | メソッドを、そのメソッドが依存しているクラスに移動します |
| プリミティブ型偏重 | ドメイン概念を生の string／int で表現する | 型安全性がなく、検証が分散します | 値オブジェクトまたは強く型付けされた ID を導入します |
| 深いネスト | インデントが 3 階層超 | 制御フローを追いにくくなります | ガード節（早期リターン）を使い、メソッドを切り出します |
6. 例外処理の不足
| 問題点 | 有害な理由 | 修正方法 | ルール |
|---|---|---|---|
| 空の catch ブロック | エラーを黙って握りつぶし、バグを隠します | 最低限、例外を記録します。例外を伝播させることを優先します | CA1031 |
基底クラスの Exception を捕捉する | OutOfMemoryException、StackOverflowException なども捕捉します | 具体的な例外型を捕捉します | CA1031 |
記録して握りつぶす（catch { log; }） | 呼び出し元が操作の失敗を把握できません | 記録後に再スローするか、エラー結果を返します | -- |
finally で例外をスローする | 元の例外が新しい例外で隠されます | finally 内で try/catch を使用し、finally から例外をスローしないでください | -- |
throw; ではなく throw ex; を使う | スタックトレースがリセットされ、元の失敗箇所が隠されます | スタックトレースを保持するため、引数なしの throw; を使用します | CA2200 |
| 内部例外を含めない | 例外をラップした際に原因の連鎖が失われます | 元の例外を innerException パラメーターに渡します | -- |
関連項目：[skill:dotnet-csharp-async-patterns] では、ファイア・アンド・フォーゲットおよび async void のシナリオにおける例外処理を説明しています。
クイックリファレンス：CA ルール
| ルール | 説明 |
|---|---|
| CA1031 | 一般的な例外型を捕捉しない |
| CA1816 | GC.SuppressFinalize を正しく呼び出す |
| CA1826 | インデックス可能なコレクションに Enumerable メソッドを使用しない |
| CA1827 | Any() が使える場合に Count()／LongCount() を使用しない |
| CA1851 | IEnumerable コレクションが複数回列挙される可能性がある |
| CA2000 | スコープを抜ける前にオブジェクトを破棄する |
| CA2200 | スタックの詳細を保持するために再スローする |
| CA2213 | 破棄可能なフィールドを破棄する |
| CA2214 | コンストラクターでオーバーライド可能なメソッドを呼び出さない |
プロジェクトで <AnalysisLevel>latest-all</AnalysisLevel> を設定すると、これらを有効にできます。アナライザーの構成については、[skill:dotnet-csharp-coding-standards] を参照してください。