C＃プロジェクトのチェックに関する最初の記事

現在、PVS-StudioチームはC＃コードの静的アナライザーを積極的に開発しています。 2015年末までにアナライザーの最初のバージョンをリリースする予定です。 それまでの間、私の仕事は、PVS-StudioツールでC＃プログラマーに事前に興味を持ってもらうために、いくつかの記事を書くことです。 今日、更新されたインストーラーが提供されました。 これで、C＃サポート付きのPVS-Studioをインストールし、何かをチェックすることさえ可能になりました。 私はこれを利用するのが遅くなかったので、腕で最初に現れるものをチェックすることにしました。 Umbracoプロジェクトが最初に現れました。 もちろん、現在のバージョンのアナライザーではほとんど何もできませんが、これですでに小さな記事を書くのに十分です。

ウンブラコ

Umbracoは、コンテンツを世界中に公開するために使用されるオープンソースのコンテンツ管理システムプラットフォームです。 C＃で書かれています。 4.5のリリース以来、システム全体がMITライセンスの下で利用可能になりました。



プロジェクトは中規模です。 ただし、C＃で記述された部分はそれほど大きくありません。 プロジェクトのほとんどはJavaScriptで記述されています。 合計で、プロジェクトには約3200個のファイルがあり、拡張子は「.cs」、合計サイズは15メガバイトです。 C＃コードの行数：400 KLOC。

PVS-Studio 6.00について

検証は、PVS-Studio 6.00アナライザーのアルファバージョンを使用して実行されます。 このバージョンには2つの大きな変更があります。

1. C＃プロジェクトの分析がサポートされます。
2. アナライザはVS2005、VS2008をサポートしなくなります。 エラーが修正された場合、VS2005 / 2008で作業している少数のユーザーに、バージョン5.31以降のバージョン5.xxを引き続き使用することをお勧めします。

価格設定ポリシーは変わりません。 新しい製品を作成するのではなく、既存の製品の機能を拡張します。 もう1つのプログラミング言語をサポートするだけです。 以前は、PVS-Studioを購入して、C、C ++、 C ++ / CLI 、 C ++ / CXの言語で書かれたプロジェクトをテストするために使用できました。 C＃プロジェクトをチェックする追加の機会があります。 これは価格には一切影響しません。 すでにC ++のアナライザーを購入している人は、C＃コードを同時にチェックできます。

なぜC＃なのですか？

会議で、C＃アナライザーの作成はあまりおもしろくないとよく言いました。 C ++のC ++に存在する多くのエラーは、単に不可能です。 本当にそうです。 たとえば、C＃には、それぞれmemset（）などの関数はなく、質量の問題もありません（memset（）に関連する例を参照してください： V511 、 V512 、 V575 、 V579 、 V597 、 V598 ）。



しかし、私は徐々に考えを変えました。 PVS-Studioによって検出される多数のエラーは、プログラミング言語の一部の機能ではなく、プログラマーの不注意に関連しています。 私はタイプミスとコピー＆ペースト後に失敗したコード変更を意味します。 これは、C ++向けPVS-Studioアナライザーが強力な場所であり、これらの開発はC＃にも適用できると判断しました。



C＃言語は、変数名の混乱や、注意の喪失に関連する「 最終行の影響 」から保護しません。



C＃アナライザーの作成を決定したもう1つの重要な要素は、Roslynの登場です。 それがなければ、アナライザーを作成する作業は私たちにとってあまりにも高価になります。



Roslynは、C＃およびVisual Basic向けのオープンソースのコンパイルプラットフォームです。 Roslynは2つの主なアクションを実行します。構文ツリーを構築（解析）し、コンパイルします。 さらに、ソースコードを分析し、再帰的にバイパスし、Visual Studioプロジェクトで作業し、その場でコードを実行できます。

何が面白かった？

C ++には、お気に入りのV501診断プログラムがあります。 C＃：V3001に対応するものがあります。 この診断から始めましょう。



コードスニペットN1



コードにはフォーカルポイントプロパティがあります。

[DataMember(Name = "focalPoint")] public ImageCropFocalPoint FocalPoint { get; set; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

このプロパティのタイプは「ImageCropFocalPoint」で、その定義を以下に示します。

 public class ImageCropFocalPoint { [DataMember(Name = "left")] public decimal Left { get; set; } [DataMember(Name = "top")] public decimal Top { get; set; } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そのようなプロパティを操作するときにミスを犯すことは不可能に思えます。 しかし、違います。 HasFocalPoint（）メソッドに迷惑なタイプミスがあります：

 public bool HasFocalPoint() { return FocalPoint != null && FocalPoint.Top != 0.5m && FocalPoint.Top != 0.5m; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

「トップ」は2回チェックされますが、「左」は忘れていました。



PVS-Studioの対応する警告：V3001「&&」演算子の左側と右側に同一の副次式「FocalPoint.Top！= 0.5m」があります。 ImageCropDataSet.cs 58



コードスニペットN2

 protected virtual void OnBeforeNodeRender(ref XmlTree sender, ref XmlTreeNode node, EventArgs e) { if (node != null && node != null) { if (BeforeNodeRender != null) BeforeNodeRender(ref sender, ref node, e); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V3001「&&」演算子の左側と右側に同一の副次式「node！= Null」があります。 BaseTree.cs 503



「ノード」リンクが二重チェックされます。 最も可能性が高いのは、「送信者」リンクも確認することでした。



コードスニペットN3

 public void Set (ExifTag key, string value) { if (items.ContainsKey (key)) items.Remove (key); if (key == ExifTag.WindowsTitle || <<<<---- key == ExifTag.WindowsTitle || <<<<---- key == ExifTag.WindowsComment || key == ExifTag.WindowsAuthor || key == ExifTag.WindowsKeywords || key == ExifTag.WindowsSubject) { items.Add (key, new WindowsByteString (key, value)); .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V3001「||」の左と右に同一のサブ式「key == ExifTag.WindowsTitle」があります 演算子。 ExifPropertyCollection.cs 78



キーは、定数「ExifTag.WindowsTitle」と2回比較されます。 この間違いがどれほど深刻かを判断するのは難しいです。 おそらく、チェックの1つは不要であり、削除できます。 しかし、おそらく他の定数と比較する必要があります。



コードスニペットN4



エラーの存在がまったくわからない別のケースを考えてみましょう。 ただし、このコードはもう一度確認する価値があります。



4つの名前付き定数を持つ列挙があります。

 public enum DBTypes { Integer, Date, Nvarchar, Ntext }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ただし、何らかの理由でSetProperty（）メソッドが考慮するオプションは3つだけです。 繰り返しますが、これは間違いだと言っているのではありません。 ただし、アナライザーはこのコードに注意を払うことを提案しており、完全に同意します。

 public static Content SetProperty(....) { .... switch (((DefaultData)property.PropertyType. DataTypeDefinition.DataType.Data).DatabaseType) { case DBTypes.Ntext: case DBTypes.Nvarchar: property.Value = preValue.Id.ToString(); break; case DBTypes.Integer: property.Value = preValue.Id; break; } .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

警告PVS-Studio：V3002 switchステートメントは、 'DBTypes'列挙型のすべての値をカバーしていません：日付。 ContentExtensions.cs 286



コードスニペットN5

 public TinyMCE(IData Data, string Configuration) { .... if (p.Alias.StartsWith(".")) styles += p.Text + "=" + p.Alias; else styles += p.Text + "=" + p.Alias; .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V3004「then」ステートメントは「else」ステートメントと同等です。 TinyMCE.cs 170



コードスニペットN6、N7



記事の冒頭で、C＃は「 最後の行効果 」から保護しないと述べました。 そして、対応する例に行きました。

 public void SavePassword(IMember member, string password) { .... member.RawPasswordValue = result.RawPasswordValue; member.LastPasswordChangeDate = result.LastPasswordChangeDate; member.UpdateDate = member.UpdateDate; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V3005「member.UpdateDate」変数はそれ自体に割り当てられます。 MemberService.cs 114



プログラマーは、クラスメンバーを「結果」オブジェクトから「メンバー」オブジェクトにコピーしました。 しかし、最後の瞬間に、その人はリラックスしてメンバー「member.UpdateDate」を自分にコピーしました。



また、SavePassword（）メソッドがパスワードで機能することを警告しています。つまり、それ自体に注意を払う必要があります。



UserService.csファイルでまったく同じコードフラグメントを確認できます（269行目を参照）。 ほとんどの場合、彼はチェックせずに単純にコピーされました。



コードスニペットN8

 private bool ConvertPropertyValueByDataType(....) { if (string.IsNullOrEmpty(string.Format("{0}", result))) { result = false; return true; } .... return true; .... return true; .... return true; .... return true; .... .... return true; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V3009このメソッドが常に同じ値の「true」を返すのは奇妙です。 DynamicNode.cs 695



このメソッドは、多数の「if」ステートメントと多数の「return」ステートメントを使用します。 すべての「return」ステートメントが値「true」を返すことは驚くべきことです。 これに間違いはありますか？ おそらくどこかで「false」を返す必要があったのでしょうか？



コードスニペットN9



読者に注意を確認し、次のコードフラグメントでエラーを見つけることをお勧めします。 この方法を学習しますが、以下のテキストを読んではいけません。 誤ってこれを行わないために、セパレーター（ユニコーン:)を挿入しました。

 public static string GetTreePathFromFilePath(string filePath) { List<string> treePath = new List<string>(); treePath.Add("-1"); treePath.Add("init"); string[] pathPaths = filePath.Split('/'); pathPaths.Reverse(); for (int p = 0; p < pathPaths.Length; p++) { treePath.Add( string.Join("/", pathPaths.Take(p + 1).ToArray())); } string sPath = string.Join(",", treePath.ToArray()); return sPath; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

図1.エラーとは何かの説明からコードを分離します。



PVS-Studio警告：V3010関数「Reverse」の戻り値を使用する必要があります。 DeepLink.cs 19



Reverse（）メソッドを呼び出すことにより、プログラマーは配列「pathPaths」を変更することを計画しました。 おそらく、リスト（ List <T> .Reverse ） に関しては、そのような操作は完全に正しいという事実に混乱しているのでしょう。 ただし、配列に関しては、Reverse（）メソッドは元の配列を変更しません。 配列の場合、このメソッドは 'Enumerable'クラスのReverse（）拡張メソッドを通じて実装されます。 このメソッドは、適切な置換を実行しませんが、変更されたコレクションを返します。



書くのは正しいでしょう：

 string[] pathPaths = filePath.Split('/'); pathPaths = pathPaths.Reverse().ToArray();
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

またはこのように：

 string[] pathPaths = filePath.Split('/').Reverse().ToArray();
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

コードスニペットN10



PVS-Studioアナライザーは、一部のメソッドのボディの不審な一致について、V3013にいくつかの警告を発行しました。 私の意見では、これらの警告はすべて間違っています。 これらの警告の1つだけが注意に値するように思えます。

 public void GetAbsolutePathDecoded(string input, string expected) { var source = new Uri(input, UriKind.RelativeOrAbsolute); var output = source.GetSafeAbsolutePathDecoded(); Assert.AreEqual(expected, output); } public void GetSafeAbsolutePathDecoded(string input, string expected) { var source = new Uri(input, UriKind.RelativeOrAbsolute); var output = source.GetSafeAbsolutePathDecoded(); Assert.AreEqual(expected, output); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V3013「GetAbsolutePathDecoded」関数の本体が「GetSafeAbsolutePathDecoded」関数の本体と完全に同等であることは奇妙です。 UriExtensionsTests.cs 141



おそらくGetAbsolutePathDecoded（）メソッド内では、使用しないでください

 source.GetSafeAbsolutePathDecoded()
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

でも

 source. GetAbsolutePathDecoded()
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

自分が正しいかどうかはわかりませんが、この場所はチェックする価値があります。

質問への回答

この記事は、新しい読者を対象としています。 したがって、発生する可能性のある多くの問題を予見しています。 それらのいくつかに前もって答えようとします。



発見された欠陥をプロジェクト開発者に通知しましたか？



はい、私たちは常にそれを試みます。



PVS-Studioを使用してPVS-Studio自体のコードを検証しますか？



はい



PVS-StudioはMonoをサポートしていますか？



いや



これらの質問やその他の質問に対するより詳細な回答は、「 PVS-Studioに関する記事の読者からの質問への回答 」の注に記載されています。

おわりに

プロジェクトに多くのエラーはありませんでした。 C ++の読者からの読者は、これがなぜ起こっているのかをすでに知っています。 しかし、私たちはまだC＃プログラマーを征服し誘惑していないので、いくつかの重要なポイントをリストします。

1. 静的アナライザーは通常の使用ツールです。 その意味は、早い段階でエラーを見つけることです。 プロジェクトの1回限りのチェックには意味がありません。 原則として、プログラムの動作に重大な影響を与えないエラー、またはコードのめったに使用されないセクションにあるエラーは、この方法で検出されます。 その理由は、この間ずっと本当の間違いが汗と血で修正されたからです。 それらはプログラマーによって発見され、何時間もコードをデバッグし 、テスターはそれら、そしてさらに悪いことにユーザーを発見しました。 これらのエラーの多くは、静的コードアナライザーを定期的に使用すればすぐに修正できます。 C＃コンパイラの警告の拡張としてPVS-Studioを検討してください。 結局のところ、コンパイラーによって発行された警告のリストが年に1回以上表示されることを願っていますか？ これについては、記事「 Leo Tolstoy and Static Code Analysis 」で詳しく説明しています。
2. 記事では、興味深いと思われるコードフラグメントのみを取り上げています。 原則として、アナライザーが非常に正直に疑わしいと判断したコードについては記述しませんが、実際のエラーがないことは明らかです。 これを「匂いコード」と呼びます。 PVS-Studioを使用する場合、このコードは確認する価値があります。 しかし、記事でそのような場所について話すことは不適切です。
3. この項目はC ++用ではありませんが、これまでのところC＃用に存在します。 まだ多くの診断を実装していませんが、急速に動いています。 C＃ユニコーンを少し成長させます。 それから彼はあなたを見せます！

ご清聴ありがとうございました。すべての絶望的なプログラムをお祈りします。





この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 最初のC＃プロジェクト分析 。