GitExtensionsコードのエラーを検索して分析します







実際のところ、Gitカーネルはパラメーター付きのコマンドラインユーティリティのコレクションです。 快適な作業のために、原則として、使い慣れたグラフィカルインターフェイスを提供するユーティリティが使用されます。 だから、GitExtensionsのようなGitのユーティリティをやがて使用することになりました。 これは私が今まで使った中で最も便利なツールとは言えませんが(たとえば、TortoiseGitが好きです)、Gitを操作するためのお気に入りの信頼できるユーティリティのリストのニッチを明確かつ不当に占有しているわけではありません。



私は最近それを思い出し、静的アナライザーでソースコードのエラーとタイプミスをチェックすることにしました。 検証後に見つかったエラーについては、この記事で説明します。





Gitextxtensions



GitExtensionsは、 オープンソースの Gitバージョン管理システムを操作するためのクロスプラットフォームのビジュアルクライアントです。



GitExtensionsプロジェクトは小規模です。 合計で、これらは10個のメインプロジェクト、20個のプラグイン、2個の追加プロジェクトが補助ライブラリにコンパイルされています。 441ファイルに合計56,091行のコード。



このプロジェクトで興味深いものがPVS-Studio静的アナライザーを見つけることができるかどうか見てみましょう。



検証結果



監査結果に基づいて、121件の警告が受信されました。 さらに詳細に検討すると、最初の(高)レベルで16の警告が受信されました。 それらの11は、問題領域またはエラーを明確に示しました。 第2レベル(平均)では、80件の警告が受信されました。 私の主観的な意見では、69の警告は正しく、エラーやタイプミスのある場所を示していました。 主にエラーが発生する可能性の低い場所を示すため、3番目の(低)レベルの警告は考慮しません。 そして、見つかったエラーの検討に進みます。



















式は常に真です



ヒットパレードは、かなり奇妙なコードで始まります。



string rev1 = ""; string rev2 = ""; var revisions = RevisionGrid.GetSelectedRevisions(); if (revisions.Count > 0) { rev1 = ....; rev2 = ....; .... } else if (string.IsNullOrEmpty(rev1) || string.IsNullOrEmpty(rev2)) // <= { MessageBox.Show(....); return; }
      
      





V3022式 'string.IsNullOrEmpty(rev1)|| string.IsNullOrEmpty(rev2) 'は常に真です。 GitUI FormFormatPatch.cs 155



アナライザーは、変数rev1およびrev2をチェックする式が常にtrueになるという警告を発行しました。 最初は、プログラムの正確性に影響を与えない、通常のタイプミス、アルゴリズムのロジックの監視であると考えました。 しかし、コードをより詳細に調べたところ、明らかに余分なelseステートメントに気付きました。 2番目のチェックの前であり、最初のチェックが不等式の場合にのみ実行されます。



比較の1つは冗長です



チャートの2番目の数字は、通常のタイプミスです。 プログラムのロジックには影響しませんが、この例は静的解析の有用性を示しています。



 public void EditNotes(string revision) { string editor = GetEffectivePathSetting("core.editor").ToLower(); if (editor.Contains("gitextensions") || editor.Contains("notepad") || // <= editor.Contains("notepad++")) // <= { RunGitCmd("notes edit " + revision); } .... }
      
      





V3053過剰な表現。 部分文字列「notepad」および「notepad ++」を調べます。 GitCommands GitModule.cs 691



式は、長い( notepad ++ )および短い( notepad )サブストリングを検索します。 同時に、短い文字列を検索するチェックでは防ぐことができるため、長い文字列でのチェックは機能しません。 すでに述べたように、これは間違いではなく、単なる誤植ですが、別の状況では、無害な冗長チェックが潜在的に潜んでいるバグになる可能性があります。



変数は、 nullをチェックする前に使用されます



3番目の場所はかなり一般的な間違いで占められていますが、このコードのロジックを深く掘り下げていないため、この場合はプログラムの100%の誤動作を引き起こすとは言えません。 変数のヌルがチェックされるという事実のみが、想定されるヌル値について話すことができます。



 void DataReceived(string data) { if (data.StartsWith(CommitBegin)) // <= { .... } if (!string.IsNullOrEmpty(data)) { .... } }
      
      





V3095 nullに対して検証される前に、「データ」オブジェクトが使用されました。 行を確認してください:319、376。GitCommands RevisionGraph.cs 319



nullをチェックする前にデータ変数が使用されます 。これにより、 NullReferenceExceptionが発生する可能性があります。 データ変数が決してnullでない場合、以下のチェックは役に立たないため、誤解を招かないように削除する必要があります。



オーバーライドされたEqualsメソッドでNullReferenceExceptionが発生する場合があります。



このエラーは前のエラーとよく似ています。 オーバーライドされたEqualsメソッドを使用して2つのオブジェクトを比較する場合、常に2番目の比較オブジェクトとしてnullが発生する可能性があります



 public override bool Equals(object obj) { return GetHashCode() == obj.GetHashCode(); // <= }
      
      





V3115 'null'を 'Equals(object obj)'メソッドに渡すと、 'NullReferenceException'になりません。 Git.hub User.cs 16



オーバーライドされたEqualsメソッドをさらに呼び出すと、 objnullの場合にNullReferenceExceptionが発生する場合があります 。 この状況を防ぐには、 nullチェックを使用する必要があります 。 たとえば、次のように:



 public override bool Equals(object obj) { return GetHashCode() == obj?.GetHashCode(); // <= }
      
      





if条件の同一式



5番目の数字では、2つのタイプミスが誇らしげに位置しています。 プログラムの結果には影響しませんが、過度のチェックとして分類できます。



 private void ConfigureRemotes() { .... if (!remoteHead.IsRemote || localHead.IsRemote || !string.IsNullOrEmpty(localHead.GetTrackingRemote(localConfig)) || !string.IsNullOrEmpty(localHead.GetTrackingRemote(localConfig)) || remoteHead.IsTag || localHead.IsTag || !remoteHead.Name.ToLower().Contains(localHead.Name.ToLower()) || !remoteHead.Name.ToLower().Contains(_remote.ToLower())) continue; .... }
      
      





V3001 '||'の左右に同じ部分式があります 演算子。 GitUI FormRemotes.cs 192



よく見ると、テストで2つの同じ条件に気づくでしょう。 ほとんどの場合、これはコピー&ペーストの結果です。 2番目の式がlocalHeadの代わりに変数remoteHeadの使用を暗示していることを考えると、エラーの可能性もありますが、プログラムのアルゴリズムの詳細な分析がなければ、言うことはできません。



別の同様のエラーも見つかりました。



 if (!curItem.IsSourceEqual(item.NeutralValue) && // <= (!String.IsNullOrEmpty(curItem.TranslatedValue) && !curItem.IsSourceEqual(item.NeutralValue))) // <= { curItem.TranslatedValue = ""; }
      
      





V3001 '&&'演算子の左右に同じ副次式があります。 TranslationApp TranslationHelpers.cs 112



文字列の書式設定中のパラメータ形式の不一致



6番目はかなり一般的な間違いです。これは、書式設定された行のテキストをリファクタリングする際のプログラマの不注意の結果として発生します。



 Debug.WriteLine("Loading plugin...", pluginFile.Name); // <=
      
      





V3025形式が正しくありません。 「WriteLine」関数の呼び出し中に、異なる数のフォーマット項目が予期されます。 使用されない引数:pluginFile.Name。 GitUI LoadPlugins.cs 35



この状況では、プログラマーは変数pluginFile.Nameの値がデバッグ用に出力するときにフォーマットされた文字列の最後に追加されると考えたと想定できますが、そうではありません。 正しいコードは次のようになります。



 Debug.WriteLine("Loading '{0}' plugin...", pluginFile.Name); // <=
      
      





式の一部は常に真です



そして最後に、避けることができた別のタイプミス。



 private void toolStripButton(...) { var button = (ToolStripButton)sender; if (!button.Enabled) { StashMessage.ReadOnly = true; } else if (button.Enabled && button.Checked) // <= { StashMessage.ReadOnly = false; } }
      
      





V3063条件式の一部は、評価されると常に真になります:button.Enabled。 GitUI FormStash.cs 301



button.Enabledfalseであることを確認するのでこのチェックの他の 部分でbutton.Enabledtrueであると安全に言うことができ、このチェックを再び除外します。



おわりに



このプロジェクトでは、その他のエラー、タイプミス、欠点が見つかりました。 しかし、彼らは記事でそれらを説明するのに私には興味深そうではなかった。 GitExtensions開発者は、PVS-Studioツールを使用してすべての欠点を簡単に見つけることができます。 上記で提案した静的アナライザーを使用して、プロジェクトのエラーを検索することもできます。



静的解析の主な利点は、定期的に使用することです。 コードを一度ダウンロードして確認してください。これは深刻な問題ではありません。 たとえば、プログラマはコンパイラの警告を定期的に監視し、リリースの1つ前に3年ごとに警告をオンにしません。 通常の使用では、静的アナライザーはタイプミスやエラーの検索にかかる時間を大幅に節約します。





英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください:Ivan Kishchenko。 GitExtensionsのバグが見つかり、分析されました



記事を読んで質問がありますか?
多くの場合、記事には同じ質問が寄せられます。 ここで回答を集めました: PVS-Studioバージョン2015に関する記事の読者からの質問への回答 。 リストをご覧ください。




All Articles