VisualStudioのコード分析で識別子のタイプミスなどを検出する

2015/11/24 23:00頃それなりに追記修正
2015/11/25 22:00頃細々と追記修正
確認環境: Visual Studio 2015 Community Version 14.0.23107.0 D14REL

C#プロジェクトで、全てのルールを使ってコード分析したときのメモです。
c# - Visual Studio Code Analysis vs StyleCop + FxCop - Stack Overflow によると、VisualStudioのコード分析とFxCopは同じエンジンを使っているとのことなので、FxCopでも同様の結果を得られると思います。
有益なルール、有用ですが時折抑制したくなるルール、ルールセットから除外したルール、どのように対応すればいいのかわからなかったルール、の順に記載しています。
どのルールを使うかどうかの設定は方法: カスタム規則セットを作成するを参考にしました。

なおルールセット一覧で表示されるルール名は、.rulesetファイルの編集画面にてコンテキストメニューからプロパティウィンドウを開くと編集できます。
ソリューションエクスプローラーから.rulesetファイルのプロパティを開いても編集できませんでした。

(この記事は他の記事と異なり、MSDNへのリンクにja-jpのページを使っています。ページタイトルを記載すると、コード解析の違反内容説明を兼ねられるので。)

有益なルール

ぜひとも有効にしておきたいルールです。
他にも便利なルールが多々ありますが、個人的にインパクトが大きかったものだけ記載して他は割愛します。

CA1702: 複合語では、大文字と小文字が正しく区別されなければなりません

複合語を誤った位置で区切っている箇所を検出できるルールです:

// 正しくは MyDatabase, bは小文字で記述します
public class MyDataBase { }

「規則の説明」にいくつかの例が載っています。他にも"Clipboard"を誤った位置で区切っていた場所を検出できました(引っかかりました)。

CA1704: 識別子は正しく入力されなければなりません

これが記事タイトルでも触れている、タイプミスを検出できるルールです:

public class Test
{
    // typo, 正しくは"directoryName"(eが抜けています)
    public void Foo(string dirctoryName) { }

    // typo, 正しくは"Extension"(6文字目はsです)
    public const string Extention = ".txt";
}

いくつか試してみると、public/protectedに参照できる場所のみがチェックされ、internal/privateな場所やローカル変数はチェックされないようです。

ただし解析に使われるデフォルトの辞書では、よく見る語でも違反とみなされる場合があります。
例えば Directory.Move メソッドで destDirName という仮引数名が使われていますが、デフォルトでは"dest"も"dir"も違反扱いになります。

「違反の修正方法」にある手順に従うと、コード分析辞書をカスタマイズできます。
上の2つの語を違反にしないよう設定するには、

<?xml version="1.0" encoding="utf-8" ?>
<Dictionary>
  <Words>
    <Recognized>
      <Word>dest</Word>
      <Word>dir</Word>
    </Recognized>
  </Words>
</Dictionary>

の内容を持つxmlファイルをプロジェクトに追加し、プロパティウィンドウにてビルドアクションを"CodeAnalysisDictionary"に設定します。
プロジェクト固有の単語があるならそれも設定しておきましょう。

CA1725: パラメーター名は基本宣言と同一でなければなりません

基底クラスやinterfaceのメソッドの仮引数名と、派生クラスや実装クラスのメソッドの仮引数名が異なっている場合に検出されます:

public interface IFoo
{
    void Bar(int piyo);
}
public class Foo : IFoo
{
    // interface側のメソッドの仮引数名と異なっている
    public void Bar(int huga) { }
}

MSDNの説明では「混乱が生じる可能性があります」止まりですが、アップキャストと名前付き引数を併用するとコンパイルエラーになる場合があります:

// IFoo, Fooの定義には上のものを使います
public class Baz
{
    public void Test()
    {
        var foo = new Foo();
        foo.Bar(huga: 0);           // ok
        ((IFoo)foo).Bar(huga: 0);   // コンパイルエラー
    }
}

CA2208: 引数の例外を正しくインスタンス化します

ArgumentExceptionやその派生クラスのコンストラクタのparamName引数に、パラメーター名でない文字列を渡している場所で検出されます:

using System;
using System.IO;

public class Test
{
    public void Foo(string path)
    {
        if (path == null)
        {
            // 引数名のpathではなく、クラス名のPathを指定してしまっている
            throw new ArgumentNullException(nameof(Path));
        }

        if (path.Length == 0)
        {
            // 正しい引数の順番はmessage, paramNameの順、ここでは反対に指定してしまっている
            throw new ArgumentException(nameof(path), $"{nameof(path)}が長さ0の文字列です。");
        }
    }
}

「規則の説明」に各種クラスのコンストラクタの引数順序が記載されていますが、見事にバラバラです。ぜひこのルールに頼りましょう。

有用ですが時折抑制したくなるルール

個別に抑制するには SuppressMessageAttribute クラスを随所に適用します。
とはいえ、属性を手で記述する必要はありません。 Visual Studioのコード分析機能を利用してコードの品質を上げよう (2/3)：CodeZine（コードジン）にあるように、コード分析ウィンドウのコンテキストメニューから自動生成できます。

SuppressMessageのクラス定義を見ると分かりますが [Conditional("CODE_ANALYSIS")] がついています。
方法 : マネージコード分析の特定の警告を有効/無効にするに

コード分析を有効にすると、CODE_ANALYSIS 定数が定義されます。コード分析を有効にした状態でリリースビルドを出荷することのないように注意してください。コード分析を有効にすると、SuppressMessage 属性がバイナリに出力され、パフォーマンスに悪影響が生じます。

と記述されているので、配布用アセンブリを作成するときにはSuppressMessageが含まれないようにしましょう。
ビルド方法を何通りか試し、生成されたアセンブリをILSpyでSuppressMessageの有無を確認してみました:

メニューからコード分析を実行(Alt+F11のもの)したときのビルドでは、当然SuppressMessageが含まれる
プロジェクトのプロパティ設定にて"Enable Code Analysis on Build"をチェックしている場合、F6などでのビルドでも当然SuppressMessageは含まれる
"Enable Code Analysis on Build"をチェックしていない場合、F6などのビルドではSuppressMessageは含まれない

ビルド時にコード分析を行う設定で開発する場合は要注意です。

CA1502: メソッドの実装を複雑にしすぎないでください

混沌としているメソッドにツッコミを入れてくれます。
ただ、ViewModelのコンストラクタで ReactiveProperty のReactiveProperty/ReactiveCommand群の初期化コードを並べているところでツッコミが入ったことがありました。コンストラクタ中で全て初期化したいものですし、個々の初期化コードは非常に素直だったのでで、そこは個別に抑制しました。

CA1811: 呼び出されていないプライベートコードを使用しません

dead codeの検出に役立つので有効にしておきたいルールです。
しかし残念ながら、「規則の論理で識別できないエントリポイントがある場合、誤って規則違反が報告されることがあります」と記述されているように、実際には呼び出されるメソッドで引っかかる場合があります。
試してみると、xamlで自作クラスのイベントハンドラを設定する箇所を検知してくれませんでした:

<Window x:Class="WpfTest.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:local="clr-namespace:WpfTest"
        Title="MainWindow">
    <local:MyButton Click="MyButton_Click" />
</Window>

namespace WpfTest
{
    public partial class MainWindow
    {
        public MainWindow()
        {
            InitializeComponent();
        }

        // どこからも呼びだされていないprivateメソッドを検出してくれます
        private void DeadMethod() { }

        // 自作クラスのイベントハンドラは何故か追跡してくれず引っかかります
        // 個別に抑制するためにSuppressMessage属性を適用します
        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")]
        private void MyButton_Click(object sender, System.Windows.RoutedEventArgs e) { }
    }
    // 何の変哲もない自作クラス
    public class MyButton : System.Windows.Controls.Button { }
}

WPF標準のクラスでは問題なく検知されました。自作マークアップ拡張の件といい、xamlパーサーの挙動が分かりません。

ルールセットから除外したルール

引っかからないように修正したり毎度抑制するのが手間、今後一切検出してほしくない、そう思ったルールです。

CA1006: ジェネリック型をメンバーシグネチャ内で入れ子にしません

System.Collections.Generic名前空間のジェネリック型をネストさせる場所で引っかかってくれます:

public void Foo(IEnumerable<IEnumerable<int>> bar) { }

c# - Alternative to nested type of type Expression<Func<T>> - Stack Overflow にあるように、式木を使った箇所でも指摘してきます:

public void Foo(Expression<Func<int>> bar) { }

他にもDictionaryやList、Tupleなども入れ子になりがちです。

CA2202: オブジェクトを複数回破棄しません

最初のusingでstreamを生成して、続くusingでそのstreamを使う、という場合があります。そのような場合全てで引っ掛けてきます:

using System.IO;
using System.IO.Compression;

public class Test
{
    public void Foo()
    {
        // memoryStreamが複数回disposeされると言ってくる
        using (var memoryStream = new MemoryStream())
        using (var deflateStream = new DeflateStream(memoryStream, CompressionMode.Compress))
        {
        }
    }
}

同様の状況はStreamReaderやBinaryWriterを使う場合でも発生します。.NET Fremework 4.5で追加された、leaveOpenを引数に取るコンストラクタを使っても状況は変わりません。

そもそも Implementing a Dispose Method に

Dispose メソッドが複数回呼び出される場合でも、例外をスローすることなく呼び出されるようにして、リソースが常に適切にクリーンアップされるようにする必要があります。

と記載されています。CA2202ページの「警告を抑制する状況」に

オブジェクトに対して Dispose を複数回呼び出しても安全なことが判明している場合でも、実装が将来変更される可能性があります。

の記述がありますが、杞憂に思えます。

どのように対応すればいいのか分からなかったルール

アセンブリ全体が対象になる指摘は、筆者の知識が足りず対応方法が分かりませんでした。

CA1014: アセンブリに CLSCompliantAttribute を設定します

EXE用のプロジェクトや、C#からしか使うつもりがないDLLの場合でも設定するべきなのかが分かりませんでした。
CLSCompliantAttribute クラスの解説に、既定では「アセンブリは CLS 非準拠」とあるので、
動作を変えないようにルールを通過するには、AssemblyInfo.csにでも

[assembly: System.CLSCompliant(false)]

と記述すればいいのでしょうか。

CA2210: アセンブリには有効な厳密な名前が必要です

BCLにだけ依存しているEXE一個のアセンブリでも署名が必要なのかが分かりませんでした。
「警告を抑制する状況」に「アセンブリの改ざんが問題にならない環境」とありますが、果たしてどんな環境がその状況に合致するのでしょう。

ググッてヒットした C#と諸々アセンブリの署名には

一般的に、共有DLLには、署名を行うことが推奨される。

また、一般的に、EXEファイル、一つのアプリケーション固有のDLLには、署名を付けないことが推奨される。

とありましたが、MSDNにそのような記述は見つかりませんでした。

まとめ

プロジェクトに適合したコード分析ルールを使って、機械的に検出可能なミスは早々と取り除いてしまいましょう。

プログラム系統備忘録ブログ

記事中のコードは自己責任の下でご自由にどうぞ。