SharpDevelopの再確認：新機能

PVS-Studioツールは常に改善されています。 同時に、C＃コードアナライザーは最も動的に開発されています。2016年には、90個の新しい診断ルールが追加されました。 さて、アナライザーの品質の最良の指標は、検出したエラーです。 大規模なオープンプロジェクトを繰り返しチェックし、結果を比較することは、常に興味深く、また非常に便利です。 今日は、SharpDevelopプロジェクトの再確認に焦点を当てます。

はじめに

SharpDevelop検証に関する以前の記事は、2015年11月にAndrey Karpovによって公開されました。 当時は、新しいC＃アナライザーのみをテストし、最初のリリースをリリースする準備をしていました。 それでも、それでも、ベータ版を使用して、AndreiはSharpDevelopプロジェクトでいくつかの興味深いエラーを検出することができました。 その後、SharpDevelopは「棚に置かれ」、新しい診断ルールを開発する際の内部テストにのみ他のプロジェクトと一緒に使用されました。 そして最後に、SharpDevelopをもう一度チェックしますが、 PVS-Studio 6.12アナライザーのより「筋肉質」なバージョンを使用します。

確認のため、 GitHubポータルからSharpDevelopソースコードの現在のバージョンをダウンロードしました。 プロジェクトには、約100万行のC＃コードが含まれています。 動作中、分析装置は809個の警告を発行しました。 これらのうち、最初のレベル-74、2番目-508、3番目-227：


低レベルでの警告は考慮しません。統計的には、偽陽性の割合が大きいです。 MeduimおよびHighレベルの警告（582警告）の分析により、誤ったまたは非常に疑わしい構成要素の約40％の存在が明らかになりました。 これは233の警告に相当します。 言い換えると、PVS-Studioアナライザーは、コード1000行あたり平均0.23エラーを検出しました。 これは、SharpDevelopプロジェクトコードの高品質を示しています。 他の多くのプロジェクトでは、すべてがずっと悲しく見えます。

繰り返し確認した結果、Andreiの記事で以前に説明されたエラーの一部が発見されました。 しかし、見つかったエラーのほとんどは新しいものです。 次に、最も興味深いものを挙げます。

検証結果

参照のコピー/貼り付けエラー

「 重りと措置の部屋 」で間違いなくその地位を占めることができる間違い。 静的コード分析の使用の有用性とコピーペーストの危険性の鮮明な図解。

PVS-Studioアナライザーの警告 ： V3102ループ内の定数インデックスによる「 method。SequencePoints 」オブジェクトの要素への疑わしいアクセス。 CodeCoverageMethodTreeNode.cs 52

public override void ActivateItem() { if (method != null && method.SequencePoints.Count > 0) { CodeCoverageSequencePoint firstSequencePoint = method.SequencePoints[0]; .... for (int i = 1; i < method.SequencePoints.Count; ++i) { CodeCoverageSequencePoint sequencePoint = method.SequencePoints[0]; // <= .... } .... } .... } 

forループの各反復で、コレクションのゼロ要素へのアクセスが使用されます。 if条件の直後に追加のコードを意図的に引用しました。 ループ内に配置された行がどこからコピーされたかを簡単に確認できます。 変数名firstSequencePointはsequencePointに置き換えられましたが、インデックスによってアクセス式を変更するのを忘れていました。 この設計の正しいバージョンは次のとおりです。

 public override void ActivateItem() { if (method != null && method.SequencePoints.Count > 0) { CodeCoverageSequencePoint firstSequencePoint = method.SequencePoints[0]; .... for (int i = 1; i < method.SequencePoints.Count; ++i) { CodeCoverageSequencePoint sequencePoint = method.SequencePoints[i]; .... } .... } .... } 

10の違いを見つける、またはもう一度コピーペースト

PVS-Studioアナライザーの警告 ： V3021同一の条件式を持つ2つの「if」ステートメントがあります。 最初の「if」ステートメントにはメソッドの戻り値が含まれます。 これは、2番目の「if」ステートメントが無意味なNamespaceTreeNode.csであることを意味します87

 public int Compare(SharpTreeNode x, SharpTreeNode y) { .... if (typeNameComparison == 0) { if (x.Text.ToString().Length < y.Text.ToString().Length) // <= return -1; if (x.Text.ToString().Length < y.Text.ToString().Length) // <= return 1; } .... } 

両方のifブロックに同じ条件が含まれています。 この場合、プログラムの正しいバージョンがどのように見えるかを判断することは困難です。 作成者がコードフラグメントを調べる必要があります。

タイムリーなヌル等値チェック

PVS-Studioアナライザーの警告 ： V3095 nullに対して検証される前に「位置」オブジェクトが使用されました。 行を確認してください：204、206。Task.cs 204

 public void JumpToPosition() { if (hasLocation && !position.IsDeleted) // <= .... else if (position != null) .... } 

変数positionは 、等価性nullをチェックせずに使用されます 。 検証は別の条件、 elseコードブロックで行われます。 コードの正しいバージョンは次のとおりです。

 public void JumpToPosition() { if (hasLocation && position != null && !position.IsDeleted) .... else if (position != null) .... } 

null等値チェックがありません

PVS-Studioアナライザーの警告 ： V3125 'mainAssemblyList'オブジェクトは、nullに対して検証された後に使用されました。 行を確認してください：304、291。ClassBrowserPad.cs 304

 void UpdateActiveWorkspace() { var mainAssemblyList = SD.ClassBrowser.MainAssemblyList; if ((mainAssemblyList != null) && (activeWorkspace != null)) { .... } .... mainAssemblyList.Assemblies.Clear(); // <= .... } 

mainAssemblyList変数は 、最初にnullをチェックせずに使用されます 。 ただし、別のコードにはこのようなチェックが含まれています。 コードの正しいバージョン：

 void UpdateActiveWorkspace() { var mainAssemblyList = SD.ClassBrowser.MainAssemblyList; if ((mainAssemblyList != null) && (activeWorkspace != null)) { .... } .... if (mainAssemblyList != null) { mainAssemblyList.Assemblies.Clear(); } .... } 

予期しないソート結果

PVS-Studioアナライザーの警告 ： V3078 「OrderBy」メソッドを繰り返し呼び出すと、元のソート順が失われます。 'ThenBy'メソッドを使用して、元の並べ替えを保持します。 CodeCoverageMethodElement.cs 124

 void Init() { .... this.SequencePoints.OrderBy(item => item.Line) .OrderBy(item => item.Column); // <= } 

このコードフラグメントの結果は、 ColumnフィールドのみでSequencePointsコレクションをソートします。 どうやら、これはコードの作者が期待したものとはまったく異なります。 問題は、 OrderByメソッドを再度呼び出すと、前の並べ替えの結果を考慮せずにコレクションが並べ替えられることです。 状況を改善するには、 OrderByを再度呼び出す代わりにThenByを使用する必要があります。

 void Init() { .... this.SequencePoints.OrderBy(item => item.Line) .ThenBy(item => item.Column); } 

ゼロによる潜在的な除算

PVS-Studioアナライザーの警告 ： V3064潜在的なゼロ除算。 分母「workAmount」の検査を検討してください。 XamlSymbolSearch.cs 60

 public XamlSymbolSearch(IProject project, ISymbol entity) { .... interestingFileNames = new List<FileName>(); .... foreach (var item in ....) interestingFileNames.Add(item.FileName); .... workAmount = interestingFileNames.Count; workAmountInverse = 1.0 / workAmount; // <= } 

interestFileNamesコレクションが空の場合、ゼロによる除算が発生します。 適切なエラー修正オプションを提案するのは十分困難です。 ただし、いずれにしても、変数workAmountがゼロに等しい状況では 、変数workAmountInverseの値を計算するアルゴリズムの改良が必要です 。

再割り当て

PVS-Studioアナライザーの警告 ：V3008 'ignoreDialogIdSelectedInTextEditor'変数には値が連続して2回割り当てられます。 おそらくこれは間違いです。 行を確認：204、201。WixDialogDesigner.cs 204

 void OpenDesigner() { try { ignoreDialogIdSelectedInTextEditor = true; // <= WorkbenchWindow.ActiveViewContent = this; } finally { ignoreDialogIdSelectedInTextEditor = false; // <= } } 

tryブロックの結果に関係なく、変数ignoreDialogIdSelectedInTextEditorはfalseになります 。 「落とし穴」が存在する可能性を排除するために、使用される変数の宣言を確認します。 ignoreDialogIdSelectedInTextEditor宣言は次のとおりです。

 bool ignoreDialogIdSelectedInTextEditor; 

IWorkbenchWindowおよびActiveViewContentの宣言：

 public IWorkbenchWindow WorkbenchWindow { get { return workbenchWindow; } } IViewContent ActiveViewContent { get; set; } 

ご覧のとおり、 ignoreDialogIdSelectedInTextEditor変数を値に再割り当てする明確な理由はありません。 上記の構成の正しいバージョンが、 最終的にではなくcatchキーワードを使用して元のものと異なる可能性があることを提案しようと思います。

 void OpenDesigner() { try { ignoreDialogIdSelectedInTextEditor = true; WorkbenchWindow.ActiveViewContent = this; } catch { ignoreDialogIdSelectedInTextEditor = false; } } 

誤った部分文字列検索

PVS-Studioアナライザーの警告 ： V3053過剰な表現。 サブストリング「/ debug」および「/ debugport」を調べます。 NDebugger.cs 287

 public bool IsKernelDebuggerEnabled { get { .... if (systemStartOptions.Contains("/debug") || systemStartOptions.Contains("/crashdebug") || systemStartOptions.Contains("/debugport") || // <= systemStartOptions.Contains("/baudrate")) { return true; } .... } } 

systemStartOptions行は、サブストリング「/ debug」または「/ debugport」のいずれかを順次検索します。 問題は、文字列「/ debug」自体が「/ debugport」の部分文字列であることです。 したがって、サブストリング「/ debug」を見つけると、サブストリング「/ debugport」をさらに検索しても意味がありません。 おそらくこれはエラーではありませんが、コードは単純化できます。

 public bool IsKernelDebuggerEnabled { get { .... if (systemStartOptions.Contains("/debug") || systemStartOptions.Contains("/crashdebug") || systemStartOptions.Contains("/baudrate")) { return true; } .... } } 

エラー処理例外

PVS-Studioアナライザーの警告 ： V3052元の例外オブジェクト「ex」が飲み込まれました。 元の例外のスタックが失われる可能性があります。 ReferenceFolderNodeCommands.cs 130

 DiscoveryClientProtocol DiscoverWebServices(....) { try { .... } catch (WebException ex) { if (....) { .... } else { throw ex; // <= } } .... } 

この場合、キャッチされた例外が再びスローされるため、 throw exを呼び出すと、元の例外のスタックが上書きされます。 修正されたオプション：

 DiscoveryClientProtocol DiscoverWebServices(....) { try { .... } catch (WebException ex) { if (....) { .... } else { throw; } } .... } 

クラスコンストラクターで初期化されていないフィールドを使用する

PVS-Studioアナライザーの警告 ： V3128 'contentPanel'フィールドは、コンストラクターで初期化される前に使用されます。 SearchResultsPad.cs 66

 Grid contentPanel; public SearchResultsPad() { .... defaultToolbarItems = ToolBarService .CreateToolBarItems(contentPanel, ....); // <= .... contentPanel = new Grid {....}; .... } 

contentPanelフィールドは 、 SearchResultsPadクラスのコンストラクターのCreateToolBarItemsメソッドにパラメーターの1つとして渡されます 。 同時に、このフィールドは使用後に初期化されます。 おそらくこの場合、変数contentPanelとnullが等しい可能性をCreateToolBarItemsメソッドの本体およびスタックに沿って考慮することができるため、エラーはありません。 しかし、コードは疑わしく見え、作成者による検証が必要です。

この記事では、PVS-Studioアナライザーが検出したすべてのエラーからは程遠いものの、私にとって興味深いと思われるエラーのみを説明したことを強調します。 プロジェクトの作成者は私たちに連絡し、一時的なライセンスキーを取得して、プロジェクトのより徹底的な検証を実行できます。

おわりに

そして再びPVS-Studioは失望しませんでした。SharpDevelopプロジェクトの再検討中に、新しい興味深いエラーが見つかりました。 そして、これはアナライザーがその仕事をうまく行い、私たちの周りの世界をもう少し完璧にすることができることを意味します。

PVS-Studio静的アナライザーを無料で使用して自分のプロジェクトをチェックする機会を利用して、いつでもこのプロセスに参加できます。

PVS-Studioをダウンロードして試してください： http : //www.viva64.com/en/pvs-studio/

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



この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Sergey Khrenov。 SharpDevelopの再確認：新しいバグはありますか？