CruiseControl.NETプロジェクトコードに基づいてPVS-Studio C＃とVisual Studioに組み込まれた静的アナライザーの比較

最近、PascalABC.NETプロジェクトコードに基づいて、PVS-StudioとSonarQubeのC＃アナライザーを比較しました。 この研究は非常に興味深いことがわかったため、この方向で作業を続けることにしました。 今回は、C＃アナライザーPVS-StudioとVisual Studioに組み込まれた静的アナライザーを比較しています。 私の意見では、これは非常に価値のある相手です。 Visual Studioスイートのアナライザーは主にエラーを検索するためではなく、コードの品質を向上させるために設計されているという事実にもかかわらず、これは実際のエラーを見つけるために使用できないことを意味しませんが、これは困難です。 今回の調査の過程で、アナライザーのどの機能が明らかになるかを見てみましょう。 どうぞ！

はじめに

まず、いくつかのポイントを明確にするための小さなFAQセクション。

Q：CruiseControl.NETを使用する理由は何ですか？ これはどのようなプロジェクトですか？

A： CruiseControl.NETは、.NET Frameworkを使用して実装された自動連続統合サーバーです。 CruiseControl.NETソースコードはGitHubで入手できます 。 このプロジェクトは開発されておらず、しばらくサポートされていませんが、最近では一定の人気がありました。 これにより、アナライザーを比較する目的での使用が妨げられることはありませんが、逆に、安定性の要素が研究に導入されます。 PVS-Studioの最新バージョンまたはVisual Studioに組み込まれたアナライザーを使用して、誰もコードを改善していないことを確認できます。 追加のプラスは、CruiseControl.NETの小さなサイズです。プロジェクトには約256万行のコードが含まれます。

Q：Visual Studio 2017を使用しましたか？ 最新バージョンの分析ツールの機能について知りたいと思います。

A：Visual Studio 2017コミュニティを使用して、両方のツールを分析しました。

Q：アナライザーの設定はどうですか？ おそらく、すべてが「特別に構成されている」ので、PVS-Studioの方が優れていると判明したのでしょうか？

A：デフォルト設定は両方のアナライザーで使用されました。 実験の純度を高めるために、Windows 10を搭載した「クリーン」なマシンでインストールと研究を実施しました。

Q：いいですね。 しかし、確かに、結果を少しリグしたか、計算が正しくなかったのでしょうか？ たとえば、PVS-Studioの場合、低警告の信頼性レベルを考慮せず、高と中のみを取得する場合があります。 そうすると、PVS-StudioにはVisual Studioに組み込まれたアナライザーよりも利点があります。後者は同様の構成を持たないからです。

A：結果を分析するとき、絶対にすべての警告レベルが考慮され、利用可能なすべての種類の診断が含まれました。

Q：分析用のファイルの選択はどうですか？ 単体テストなど、例外に何か追加しましたか？

A：両方のアナライザーについて、例外なくソリューション全体がチェックされました。 同時に、CruiseControl.NETには「UnitTests」というプロジェクトが含まれています。 このプロジェクトでは非常に多くの警告が発行されましたが、実際のエラーを検索する際にはすべての警告が考慮されませんでしたが、警告の発行数の全体的なインジケータに表示されます。

Q：本当の間違いですか？ この用語は何ですか？

A：私たちの理解では、これらは実行に不可欠なエラーであり、高い確率で例外のスロー、プログラムの不正な動作、および不正な結果の発行につながります。 ここで修正する必要があるエラー。 これらは設計を改善するための推奨事項ではなく、最終結果に影響を与えないコードの複製などの小さな欠陥ではありません。 CruiseControl.NETコードの実際のエラーの例：

public override string Generate(IIntegrationResult integrationResult) { .... IntegrationSummary lastIntegration = integrationResult.LastIntegration; // <= if (integrationResult == null || ....) // <= { .... } .... } 

多くのアナライザーは、最初にnullをチェックせずにintegrationResult変数が使用されることをコードスニペットに警告します 。 これは正しいですが、通常は多数の誤検知につながりますが、その中で実際のエラーを見つけることは非常に困難です。 私たちのアプローチは、実際のエラーを検出する可能性を高める追加の分析を行うことです。 上記のスニペットでは、変数を使用した後、 nullがチェックされます 。 つまり この場合のプログラマは、メソッドに渡した後、変数の値がnullになる可能性があると想定し、チェックを行います。 これはまさに間違いと考えられる状況です。 メソッドにnullの integrationResultチェックが含まれていない場合、私たちの意見では、これは誤検知になります。

 public override string Generate(IIntegrationResult integrationResult) { .... IntegrationSummary lastIntegration = integrationResult.LastIntegration; .... } 

PVS-Studioアナライザーはこのコードフラグメントの警告を生成しませんが、多くのアナライザーはこれを行います。 そして、その結果、そのようなメッセージは無視されるか、プログラマーによって無効にされます。 多くの警告は良いことを意味しません。

Q：本当にすべてを正しく行ったとしましょう。 しかし、なぜ私はそれをすべて信仰にとらなければならないのですか？ どうすれば研究を繰り返すことができますか？

A：これ以上簡単なことはありません。 Visual Studioに組み込まれているアナライザーは無料です。 Visual Studio 2017 Communityの無料バージョンをインストールするだけです。 PVS-Studioを使用してCruiseControl.NETを分析するには、 ダウンロードしてデモモードで使用するだけです。 はい、制限により完全な分析を行うことはできませんが、私たちに書き込むことができ、一時的なライセンスキーを送信します。

研究と結果

ビジュアルスタジオ

Visual Studioの組み込みアナライザーを使用してプロジェクトコードをチェックするのに数分しかかかりませんでした。 この直後の結果は次のとおりです（フィルターは含まれません）。

アナライザーは10773の警告を発行しました。 はい、ここでエラーを探すのはそれほど簡単ではありません。 最初に、フィルターを使用して、このリストから「UnitTests」プロジェクトから警告を除外します。

いいね 警告のほぼ半分はテストのために受信されましたが、これは驚くことではありません。 しかし、残りの5,000を超えるメッセージでも十分ではありません。 これらの警告は、次のグループに属します。

 Microsoft.Design: CA10XX (: 40, : 1637) Microsoft.Globalization: CA13XX (: 7, : 1406) Microsoft.Interoperability: CA14XX (: 2, : 10) Microsoft.Maintainability: CA15XX (: 3, : 74) Microsoft.Mobility: CA16XX (: 1, : 1) Microsoft.Naming: CA17XX (: 17, : 1071) Microsoft.Performance: CA18XX (: 15, : 489) Microsoft.Portability: CA19XX (: 1, : 4) Microsoft.Reliability: CA20XX (: 4, : 158) Microsoft.Globalization, Microsoft.Security: CA21XX (: 5, : 48) Microsoft.Usage: CA22XX (: 18, : 440) 

多数のコンパイラ警告も発行されています。 どうやら、受け取ったアナライザー診断のそれぞれの説明を調査し、必要な場合にのみ警告の調査を実施する以外に選択肢はありません。

すぐにこの作業を行ったと言いますが、アナライザーによって発行された5,000を超える警告からエラーを見つけるのに役立つものはほとんど見つかりませんでした。 原則として、すべては設計とコードの最適化を改善するための推奨事項に基づいています。 多数の診断ルールのため、説明を含む完全なリストは提供しません。 必要に応じて、Visual Studioに組み込まれたアナライザーを使用してCruiseControl.NETプロジェクトを個別にチェックすることにより、このリストを徹底的に調べることができます。 診断の詳細な説明は、 MSDNで入手できます。

警告の大部分を調査しましたが、すべてではありません。 最後まで、メッセージの多くのグループを表示しても意味がありませんでした。それらはすべて同じタイプであり、明らかにエラーに関するものではなかったからです。 根拠がないように、各グループに1つの例を挙げます。

Microsoft.Design

CA1002「CruiseServerClient.ForceBuild（string、List <NameValuePair>）」の「List <NameValuePair>」を変更して、Collection <T>、ReadOnlyCollection <T>、またはKeyedCollection <K、V> CruiseServerClient.csを使用します118

 public override void ForceBuild(...., List<NameValuePair> parameters) { .... } 

メソッドのparameters パラメーターには、 Listではなく、ユニバーサルコレクション（ Collectionなど ）を使用することをお勧めします。

Microsoft.Globalization

CA1300「AddProjects.RetrieveListOfProjects（BuildServer）」を変更して、MessageBoxOptionsを指定するMessageBox.Showオーバーロードを呼び出し、RightToLeftがRightToLeftに設定されている場合は、MessageBoxOptions.RightAlignおよびMessageBoxOptions.RtlReadingを設定します。 CCTrayLib AddProjects.cs 86

 private void RetrieveListOfProjects(....) { .... MessageBox.Show(this, "Unable to connect to server " + server.DisplayName + ": " + ex.Message, "Error"); .... } 

推奨事項は、 MessageBoxOptions引数を取るMessageBox.Show（）メソッドオーバーロードを使用することです。 これは、右から左への読み取り順序を使用する多言語インターフェイスと言語をより適切にサポートするために必要です。

Microsoft。相互運用性

CA1401 P / Invoke 'NativeMethods.SetForegroundWindow（IntPtr）'のアクセシビリティを変更して、アセンブリの外部から見えなくなるようにします。 CCTrayLib NativeMethods.cs 12

 [DllImport("user32.dll")] [return: MarshalAs(UnmanagedType.Bool)] public static extern bool SetForegroundWindow(IntPtr handle); 

DllImportAttribute属性を持つメソッドのパブリックアクセスレベルを指定しないことをお勧めします。

Microsoft.Maintainability

CA1500「errorMessages」は、「Response.ConcatenateErrors（）」で宣言された変数で、型のインスタンスフィールドと同じ名前を持っています。 これらのアイテムのいずれかの名前を変更します。 Remote Response.cs 152

 private List<ErrorMessage> errorMessages; .... public virtual string ConcatenateErrors() { List<string> errorMessages = new List<string>(); .... } 

ローカル変数の名前がクラスフィールドと同じであるという警告。

Microsoft.Mobility

CA1601メソッドFileChangedWatcher.FileChangedWatcher（params string []）の 'Timer.Timer（double）'の呼び出しを変更して、タイマー間隔を1秒以上の値に設定します。 コアFileChangedWatcher.cs 33

 public FileChangedWatcher(....) { .... timer = new Timer(500); .... } 

タイマーが1秒未満に設定されているという警告。

Microsoft.Naming

CA1702メンバー「Alienbrain.CreateGetProcess（string）」では、パラメーター名「filename」の個別の用語「filename」を複合語「fileName」として表現する必要があります。 コアAlienbrain.cs 378

 public ProcessInfo CreateGetProcess(string filename) { .... } 

キャメルケースを使用して複合変数名を指定することに関する警告。

Microsoft.Performance

CA1800変数「action」は、メソッド「AdministerPlugin.NamedActions.get（）」で「AdministerAction」を複数回入力するようにキャストされます。 冗長なisint命令を排除するために、「as」演算子または直接キャストの結果をキャッシュします。 WebDashboard AdministerPlugin.cs 79

 public INamedAction[] NamedActions { get { .... if (action is AdministerAction) { (action as AdministerAction).Password = password; } .... } .... } 

複数の繰り返しキャストを最適化する必要性に関する警告。

Microsoft.Portability

CA1901コードで宣言されているように、P / Invoke 'Audio.PlaySound（byte []、short、long）'のパラメーター 'fdwSound'は、32ビットプラットフォームでは8バイト幅になります。 このAPIの実際のネイティブ宣言は、32ビットプラットフォームでは4バイト幅でなければならないことを示しているため、これは正しくありません。 「長い」の代わりにどのデータ型を使用すべきかを判断するには、MSDN Platform SDKドキュメントを参照してください。 CCTrayLib Audio.cs 135

 [DllImport ("winmm.dll")] private static extern int PlaySound (byte[] pszSound, Int16 hMod, long fdwSound); 

インポートされたメソッドは、 fdwSoundパラメーターに非ポータブル型を使用することを警告します。 IntPtrまたはUIntPtrを使用する必要があります。

Microsoft.Reliability

CA2000メソッド「About.famfamfamLink_LinkClicked（オブジェクト、LinkLabelLinkClickedEventArgs）」で、オブジェクト「urlLink」のSystem.IDisposable.Disposeを、オブジェクトへのすべての参照がスコープ外になる前に呼び出します。 CCTrayLib About.cs 71

 private void famfamfamLink_LinkClicked(....) { Process urlLink = new Process(); urlLink.StartInfo = new ProcessStartInfo(....); urlLink.Start(); } 

スコープ外になる前にurlLink IDisposableオブジェクトを解放する必要性に関する警告。 たとえば、を使用して使用できます。

Microsoft.Globalization、Microsoft.Security

CA2101セキュリティリスクを軽減するには、DllImport.CharSetをCharSet.Unicodeに設定するか、パラメーターをUnmanagedType.LPWStrとして明示的にマーシャリングすることにより、パラメーター 'lpszDomain'をUnicodeとしてマーシャリングします。 この文字列をANSIまたはシステム依存としてマーシャリングする必要がある場合、MarshalAsを明示的に指定し、BestFitMapping = false;を設定します。 セキュリティを強化するには、ThrowOnUnmappableChar = trueも設定します。 コアImpersonation.cs 100

 [DllImport("advapi32.dll", SetLastError = true)] private static extern bool LogonUser( string lpszUsername, string lpszDomain, string lpszPassword, int dwLogonType, int dwLogonProvider, ref IntPtr phToken); 

たとえば、次のように属性を指定することにより、文字列引数のマーシャリングタイプが指定されていないことを警告します。

 [DllImport("advapi32.dll", SetLastError = true, CharSet = CharSet.Unicode)] 

Microsoft.Usage

CA2201 'CruiseServerClientFactory.GenerateClient（string、ClientStartUpSettings）'は、タイプ 'ApplicationException'の例外を作成します。これは、十分に具体的ではなく、ユーザーコードによって決して発生させない例外タイプです。 この例外インスタンスがスローされる可能性がある場合は、別の例外タイプを使用してください。 リモートCruiseServerClientFactory.cs 97

 public CruiseServerClientBase GenerateClient(....) { .... throw new ApplicationException("Unknown transport protocol"); .... } 

例外スロー警告は一般的すぎます。 この場合、ランタイムで予約されていない、より具体的なタイプの例外をスローすることをお勧めします。

まとめ

完了した作業の結果、実際のエラーを検索するのが理にかなっているという結論に達しました。この場合は、Microsoft.DesignグループのコードCA1062のメッセージのみを検索します。 これらは、メソッドパラメーターが参照型であり、使用前にnullチェックが実行されない場合の状況に関する警告です。 このようなメッセージにフィルターを適用すると、次の結果が得られます。

733はまだたくさんあります。 しかし、あなたが見るように、これはすでに何かです。 また、見つかったコードフラグメントを調べると、実際には潜在的に安全ではありません。 たとえば、ItemStatus.csファイルでは：

 public void AddChild(ItemStatus child) { child.parent = this; childItems.Add(child); } 

ItemStatusクラスのインスタンスへの子の参照は、使用前にテストされません。 はい、これは危険です。 しかし、残念ながら、これは間違いとは言えません。 おそらく、呼び出しコードにチェックを含めることができますが、これは正しくありません。 さらに、メソッドはpublicとして宣言されます。 もちろん、コードの作成者は対策を講じ、何らかの方法でこれらすべての警告に対応する必要がありますが、警告が733あることを思い出させてください。ほとんどの場合、プログラマーは何もしません。 これは、多かれ少なかれ疑わしいものすべてに大量のメッセージを発行する危険性です。 このため、開発者に注意を払う価値のある実際のエラーの例を以前に示しました。 このような動作は、ほとんどの場合偽と見なすことができます。 そして、本当にそうです。

より多くの時間を費やして、発行された733個の警告-5を強調表示しました。これはエラーと解釈できます。 それらの1つ（BuildGraph.csファイル）の例を次に示します。

 public override bool Equals(object obj) { if (obj.GetType() != this.GetType() ) return false; .... } 

obj変数は、使用前にnullがチェックされません。 これはオーバーロードされたEqualsメソッドであるため、エラーを処理しています。 Equalsメソッドは、null参照を正しく処理する必要があります。 おそらくCruiseControl.NETプロジェクトでは、このような状況は発生しませんが、メソッドコードは依然としてエラーであり、修正する必要があります。

細心の注意を払った読者は、メソッドのすべての使用を慎重に調べることなく、ある種の間違いを見逃している可能性があることに反対するかもしれません。 おそらく。 しかし実際には、誰も各警告を注意深く調べないため、誤検知の割合は非常に高くなります。

Visual Studioに組み込まれたアナライザーを使用してCruiseControl.NETコードで実際のエラーを見つけることができたにもかかわらず、プロセス自体は約8時間（1営業日）かかり、注意と集中力が必要でした。 おそらく、プロジェクトの作成者が静的分析を定期的に使用した場合、全体像はよりバラ色になります。

PVS-Studio

私のマシンでPVS-Studioを使用してプロジェクトコードをチェックするには、ちょうど1分かかりました。 この直後、結果は次のようになります（フィルターは含まれません）。

アナライザーは198個の警告を生成しました。 そして、そのうち45件はUnitTestsプロジェクトで受信され、さらに32件の警告の優先度は低くなっています（実際のエラーを見つけるのは困難です）。 合計-分析用の121のメッセージ。私は30分を費やしました。 その結果、19のエラーが特定されました。

それらの1つの例を次に示します。

V3003 「if（A）{...} else if（A）{...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 行を確認してください：120、125。CCTrayLib CCTrayProject.cs 120

 public override bool Equals(object obj) { .... if ((buildServer != null) && (objToCompare.buildServer != null)) { // If both instances have a build server then compare the build // server settings isSame = string.Equals(buildServer.Url, objToCompare.buildServer.Url); } else if ((buildServer != null) && (objToCompare.buildServer != null)) { // If neither instance has a build server then they are the same isSame = true; } .... } 

両方のifブロックに同じ条件が含まれています。 プログラムのロジックに影響を及ぼし、予期しない結果につながる重大なミスが発生しました。

PVS-Studioが検出したエラーの中には、Visual-Studioに組み込まれたアナライザーが検出したエラーから選択したものもあります。

ここにこれ以上追加するものはないと思います。 PVS-Studioは、実際のエラーを見つけるという仕事を迅速かつ効率的に行いました。 しかし、このために彼は存在します！

おわりに

結果を表に示します。

もちろん、PVS-Studioの側面には明らかな利点があります。 ただし、Visual Studioに組み込まれているアナライザーは、まず、設計とコードの最適化を改善することを目的としており、エラーを検索することを目的としていません。 一方、PVS-Studioは、可能な限り低いレベルの誤った警告を発行してエラーを検索するために「投獄」されます。 また、CruiseControl.NETの開発者は、アナライザーをまったく使用しなかったようです。 Visual Studioに組み込まれているアナライザーを定期的に使用すると、コードの品質がはるかに向上し、エラーが発生する可能性が低くなると確信しています。 PVS-Studioについて言うべきこと。 このようなツールを使用すると、「1年に1回」ではなく、定期的に使用して最大限の効果を得ることができます。

→ PVS-Studioをダウンロードしてお試しください

PVS-Studioの商用ライセンスの取得については、メールでお問い合わせください。 デモバージョンの制限を削除する場合は、PVS-Studioの包括的な研究の一時的なライセンスを取得するために私たちに書き込むこともできます。

サイトリンク

1. 静的アナライザーが偽陽性と戦う方法と理由 。
2. SonarQube：SonarC＃およびPVS-Studioのプラグインを使用してPascalABC.NETプロジェクトを確認します 。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Sergey Khrenov。 CruiseControl.NETコードベースを使用したPVS-Studio for C＃と組み込みのVisual Studioアナライザーの比較