2020年にC＃プロジェクトで見つかったトップ10エラー

最後に、このような困難な2020年が終わりに近づいています。つまり、株式を取得するときです。今年、PVS-Studioチームは、オープンソースプロジェクトでアナライザーによって発見されたさまざまなバグを扱った多くの記事を作成しました。2020年にC＃プロジェクトで見つかったTOPエラーで、それらの中で最も興味深いものをここで見ることができます。ハッピービューイング！

トップがどのように形成されたか

このリストには、私の意見では、同僚と私が2020年の記事で書いた最も興味深いトリガーが含まれています。重要な選択基準は、対応するコードフラグメントで実際に間違いがあったという確信度でした。そしてもちろん、場所を選択したり配置したりする際には、トリガーの「面白さ」が考慮されましたが、これはすでに私の主観的な意見です。コメントでいつでも挑戦できます。



PVS-Studioメッセージと、コード警告が発行されたプロジェクトの両方の観点から、トップをできるだけ多様化しようとしました。このリストには、8つの検証済みプロジェクトのソースに関する検出が含まれています。同時に、診断ルールが繰り返されることはほとんどありません。ここで2回会うことができるのは、 V3022と V3106（いいえ、私はそれらを作成しませんでしたが、明らかに、これらは私のお気に入りです）。したがって、ここでは多様性が保証されています:)。

さあ、始めましょう！トップ10！

10位-新しい古いライセンス

私たちのトップの回答は、RavenDBプロジェクトが例として使用されたLinuxおよびmacOSのC＃プロジェクトのチェックに関する1人の非常に優れた人物による記事から始まります 。

private static void UpdateEnvironmentVariableLicenseString(....)
{
  ....
  if (ValidateLicense(newLicense, rsaParameters, oldLicense) == false)
    return;
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3066「ValidateLicense」メソッドに渡される引数の順序が正しくない可能性があります：「newLicense」および「oldLicense」。 LicenseHelper.cs（177）Raven.Server



どうやら、ここで何が問題になっているのでしょうか。コードは非常にうまくコンパイルされます。なぜアナライザーは、最初にoldLicenseを転送し、次にnewLicenseを転送する必要がある と 判断したのですか？あなたはそれを推測しましたね？ValidateLicense宣言を見てみましょう ：

private static bool ValidateLicense(License oldLicense, 
                                    RSAParameters rsaParameters, 
                                    License newLicense)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

うわー、それは本当です-最初に、古いものがパラメータにあり、それから-新しいライセンス。さて、あなたのこの動的分析はこれを捕らえることができますか？:)



いずれにせよ、トリガーは興味深いものです。順序はそれほど重要ではないかもしれませんが、そのようなフラグメントを再確認する方が良いでしょう、同意しますか？

9位-「FirstOrDefault」と予期しない「null」

9位は今年の初めに書かれた記事「Playin "osu！"、間違いを忘れないでください」からの返答でした ：

public ScoreInfo CreateScoreInfo(RulesetStore rulesets)
{
  var ruleset = rulesets.GetRuleset(OnlineRulesetID);

  var mods = Mods != null ? ruleset.CreateInstance() 
                                   .GetAllMods().Where(....)
                                   .ToArray() : Array.Empty<Mod>();
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

エラーが表示されますか？そして彼女はそうです！アナライザーは何と言っていますか？



アナライザーの警告： V3146 [CWE-476]「ルールセット」のnull逆参照の可能性。 'FirstOrDefault'は、デフォルトのnull値を返すことができます。 APILegacyScoreInfo.cs 24



はい、はい、繰り返しますが、必要なすべての情報を一度に提供しませんでした。実際、このコードには疑わしいものは何も表示されません。結局のところ、 アナライザーが教えてくれるFirstOrDefaultは、GetRulesetメソッドの定義に含まれています 。

public RulesetInfo GetRuleset(int id) => 
  AvailableRulesets.FirstOrDefault(....);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

ひどいビジネス！適切なものが見つかった場合、メソッドはRulesetInfoを返し ます。そうでなければ？落ち着いてnullを返し ます。そして、それは呼び出しの結果が使用される場所ですでに撃ちます。この場合、ruleset.CreateInstance（）を呼び出すとき 。



疑問が生じる可能性があります：nullがない場合はどうなり ますか？コレクションに常に特定の要素がある場合はどうなりますか？開発者がこれについて確信している場合は、FirstOrDefaultの代わりにFirstを使用してみ ません か？

8位-Pythonからこんにちは

最初の3つからの最後のトリガーは、RunUOプロジェクトコードに対して発行されました。確認方法に関する記事は2月に書かれたもので、こちらから入手でき ます。



見つかったフラグメントは非常に疑わしいものですが、それが間違いであるかどうかを確実に判断することは困難です。

public override void OnCast()
{
  if ( Core.AOS )
  {
    damage = m.Hits / 2;

    if ( !m.Player )
      damage = Math.Max( Math.Min( damage, 100 ), 15 );
      damage += Utility.RandomMinMax( 0, 15 );
  }
  else { .... }
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3043コードの操作ロジックがそのフォーマットに対応していません。ステートメントは右にインデントされていますが、常に実行されます。中括弧が欠落している可能性があります。地震.cs57



そうです-それはインデントについてです！行の 損傷+ = Utility.RandomMinMax（0、15）は、m.Playerが falseの場合にのみ実行されるべきだったよう です。..。同様に、インデントが美しさのためだけでなく、アプリケーションのロジックを定義するためのPythonコードも機能します。しかし、C＃コンパイラはこの問題に関して異なる意見を持っています！開発者の意見はどうでしたか？



一般に、この状況では、2つのオプションがあります。ここでは中括弧が実際に欠落していて、すべてが正しく機能していないか、すべてが正しく機能していますが、時間の経過とともに、これを間違いと見なして「修正」する人が必ずいるでしょう。



たぶん私は間違っていて、そのようなことを書いても大丈夫な場合が本当にあります。あなたがこれらを知っているならば、コメントを書いてください-私はそのような場合について知りたいと思います。

7位-パーフェクトまたはパーフェクト-それが問題です！

警告を場所に配布することはますます困難になっており、osuに関する記事からこのリストの2番目のアラームに移ってい ます！ ..。



エラーが表示されるまでどのくらいかかりますか？

protected override void CheckForResult(....)
{
  ....
  ApplyResult(r =>
  {
    if (   holdNote.hasBroken
        && (result == HitResult.Perfect || result == HitResult.Perfect))
      result = HitResult.Good;
    ....
  });
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3001「||」の左側と右側に同一のサブ式「result == HitResult.Perfect」があります。オペレーター。DrawableHoldNote.cs 266



それほど多くはない と思いますが、PVS-Studioの警告を読む必要があるだけです。静的分析を使用する開発者は通常これを行います:)。上の前の場所について議論することもできますが、ここでは間違いが明らかです。HitResult列挙のどの特定の要素を2番目のPerfectの代わりに（まあ、または最初の代わりに）ここで使用すべきか を言うのは難しい ですが、結局、何かが明らかに間違って書かれていました。さて、それは大丈夫です-エラーが見つかりました、それはそれが簡単に修正できることを意味します。

6位-null（ではない）合格！

6位は、Open XMLSDKプロジェクトからのコードに対する非常にクールな応答でした。それをチェックすることに関する記事はここで読むことができ ます。



開発者は、直接割り当てられた場合でもnullを返さないプロパティを実装したいと考えていました 。そして、どのような状況でもnullが書き込まれないことを確信できるのは本当に素晴らしい ことです。これがまったく状況ではないのは残念です：

internal string RawOuterXml
{
  get => _rawOuterXml;

  set
  {
    if (string.IsNullOrEmpty(value))
    {
      _rawOuterXml = string.Empty;
    }

    _rawOuterXml = value;
  }
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

パーサー警告： V3008'_rawOuterXml '変数には2回連続して値が割り当てられます。おそらくこれは間違いです。チェック行：164、161。OpenXmlElement.cs 164 _rawOuterXmlには、nullかどうかに関係なく、 値が記録さ



れることが わかります。このコードを注意深く見ると、nullがこのプロパティに書き込まれることは決してないと思うかもしれません -結局のところ、チェックがあります！そうだとすれば、ツリーの下にはギフトではなく、予期しない NullReferenceExceptionがあります:(

5位-内部にアレイがあるアレイからのショット

2020年の上位5つのトリガーは、私が個人的にテストしたTensorFlow.NETプロジェクトのアナライザーによって発行されました。リンクをクリックすると、 このプロジェクトのチェックに関する記事を読むことができます（ああ、私はそこにいるみんなにたくさん会いました）。



ちなみに、実際のC＃プロジェクトからの興味深いエラーの例を見たい場合は、私のtwitterを購読することをお勧めします 。そこで、好奇心をそそるトリガーと興味深いコードフラグメントを投稿する予定ですが、残念ながら、その多くは記事に含まれません。お会いできてうれしいです！ :)



さて、トリガーに移りましょう：

public TensorShape(int[][] dims)
{
  if(dims.Length == 1)
  {
    switch (dims[0].Length)
    {
      case 0: shape = new Shape(new int[0]); break;
      case 1: shape = Shape.Vector((int)dims[0][0]); break;
      case 2: shape = Shape.Matrix(dims[0][0], dims[1][2]); break; // <=
      default: shape = new Shape(dims[0]); break;
    }
  }
  else
  {
    throw new NotImplementedException("TensorShape int[][] dims");
  }
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3106インデックスが範囲外である可能性があります。 「1」インデックスは「dims」の境界を超えています。 TensorShape.cs 107



実際、このトリガーをどこに置くかを選択するのは非常に興味深いので非常に困難でしたが、他の人はこの点でそれほど遅れていません。それで、ここで何が起こっているのかを理解しましょう。



内のアレイの数場合 薄暗くなりが1に等しくない場合、例外NotImplementedExceptionのタイプである スロー。そして、薄暗い場合はどうなりますか 正確に1つの配列？この「配列内の配列」内の要素の数がチェックされます。 2の場合に何が起こるかに注意してください。Shape.Matrixコンストラクターへの引数の1つとして 、予期せず、dims [1] [2]が渡され ます。覚えておきましょう-dims配列にはいくつの要素がありました か？



そうです、ちょうど1つです-チェックしたばかりです！ 1つの要素のみを含む配列から2番目の要素を取得しようとすると、IndexOutOfRangeException例外がスローされます 。明らかに間違いです。しかし、それを修正する明白な方法はありますか？



最初に頭に浮かぶのは変化です dims [1] [2]から dims [0] [2]。エラーはなくなりますか？どんなに！再び同じ例外があります。しかし今回は、この場合、dims [0]の要素数が2に等しいという事実に関連します 。開発者は、インデックスに2つのエラーを続けて作成しましたか？または、他の変数をそこで使用する必要がありますか？誰が知っているか...アナライザーの仕事は間違いを見つけることであり、それを作った人または彼の同僚はそれを修正する必要があります。

4位-存在しないオブジェクトのプロパティ

OpenRAチェックに関する記事から別のトリガーがこのトップに入りました 。おそらくそれはもっと価値があるかもしれませんが、運命の意志によって、このトリガーは4位でした。まあ、それもかなり良いです！今回、PVS-Studioが検出できたエラーを見てみましょう。

public ConnectionSwitchModLogic(....)
{
  ....
  var logo = panel.GetOrNull<RGBASpriteWidget>("MOD_ICON");
  if (logo != null)
  {
    logo.GetSprite = () =>
    {
      ....
    };
  }

  if (logo != null && mod.Icon == null)                    // <=
  {
    // Hide the logo and center just the text
    if (title != null)
      title.Bounds.X = logo.Bounds.Left;

    if (version != null)
      version.Bounds.X = logo.Bounds.X;
    width -= logo.Bounds.Width;
  }
  else
  {
    // Add an equal logo margin on the right of the text
    width += logo.Bounds.Width;                           // <=
  }
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3125'logo 'オブジェクトは、nullに対して検証された後に使用されました。チェックライン：236、222。ConnectionLogic.cs 236



ここで何に注意を払う必要がありますか？さて、初心者にとって、 ロゴにはおそらくnullが含まれている可能性があり ます。これは、定数チェックと、ロゴに書き込まれた値を返すGetOrNullメソッドの名前の両方によって示唆され ます 。もしそうなら、GetOrNullが実際にnullを返した 場合に何が起こるかを考えてみましょう 。最初はすべてが順調ですが、次に状態がチェックされます ロゴ！= null && mod.Icon == null。ご想像のとおり、結果はelse -branchおよび...に遷移し ます。nullが書き込まれている変数の Boundsプロパティにアクセスしようとする と、-bdysh！ NullReferenceExceptionがドアをノックしています。

3位-Schrödingerの要素

最後に、ファイナリストの上位3位に入ります。 2020年の上位3つのバグは、Nethermindプロジェクトで見つかりました。その検証について、興味をそそるタイトル「1行のコード、またはPVS-Studio C＃forLinuxを使用したNethermindのチェック」で記事が書かれました 。エラーは非常に単純ですが、特にプロジェクトのサイズを考えると、人間の目には見えません。このトリガーはその場所に値すると思いますか？

public ReceiptsMessage Deserialize(byte[] bytes)
{
  if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
    return new ReceiptsMessage(null);
    ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3106インデックスが範囲外である可能性があります。 「0」インデックスは「バイト」の境界を超えています。 Nethermind.Network ReceiptsMessageSerializer.cs 50



おそらく、空のボックスで最初のものを取得できるのはすばらしい ことですが、ここでは、そのような要求によってIndexOutOfRangeExceptionがスローされるだけ です。たった1つの些細なこと-オペレーターの小さな間違いであり、アプリケーションはすでに正しく機能していないか、クラッシュすることさえあります。



もちろん、「&&」の代わりに「||」を使用する必要があります。このような論理エラーは、特に複雑な設計では珍しいことではありません。したがって、自動モードでそのような瞬間をチェックすることは非常に便利です。

2位-2未満、3以上

次に、RavenDBプロジェクトのコードにもう1つのトリガーを設定しました。検証の結果については、対応する記事で（だけでなく）読むことができることを思い出させて ください。



さて、今歓迎します-2020年のトップ2の間違い：

private OrderByField ExtractOrderByFromMethod(....)
{
  ....
  if (me.Arguments.Count < 2 && me.Arguments.Count > 3)
    throw new InvalidQueryException(....);
  ....
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3022式 'me.Arguments.Count <2 && me.Arguments.Count> 3'は常にfalseです。おそらく '||'ここでは演算子を使用する必要があります。 QueryMetadata.cs（861）Raven.Server先ほど



、予期しない例外がスローされた瞬間を確認しましたが、逆に、予期された例外がスローされることはありません。そうでなければ、誰かが2未満、3



を超える数を思いついた場合でも、それは破棄され ます。同意しない場合でも驚くことではありませんが、このトリガーは以前のすべてのトリガーよりも本当に気に入っています。はい、エラーは非常に単純です。エラーを修正するには、演算子を変更するだけです。ちなみに、これはコンストラクターに渡されるメッセージによっても示唆されています InvalidQueryException： "無効なORDERBY'spatial.distance（from、to、roundFactor） '呼び出し、2〜3個の引数が必要、取得" + me.Arguments.Count。



私は同意します。これは基本的な見落としですが、少なくともPVS-Studioを使用して見つかるまで、誰も気づいたり修正したりしませんでした。これは、プログラマーがどれほど経験を積んでいても、（残念ながら？）人間であることを思い出させてくれます。そして、人々は、資格に関係なく、さまざまな理由でそのような愚かな間違いさえ見逃す可能性があります。エラーがすぐに発生する場合もあれば、誤ったORDERBY呼び出しに関するメッセージがユーザーに表示されない理由を長い間推測できる場合もあります。

1位-+ 100％コードの安全性の見積もり

やった、やった、やった！私たちはついにトリガーにたどり着きました。それは私が最も興味深く、面白く、かっこいいなどを見つけました。これは、ONLYOFFICEプロジェクトのコードに対して発行されたものであり、その分析は、今年の最新記事の1つである「ONLYOFFICEコミュニティサーバー：バグがセキュリティ問題にどのように寄与するか」に 関連付けられています。



さて、私はあなたにArgumentExceptionについての最も悲しい話を提示します 、それは決して投げられません：

public void SetCredentials(string userName, string password, string domain)
{
  if (string.IsNullOrEmpty(userName))
  {
    throw new ArgumentException("Empty user name.", "userName");
  }
  if (string.IsNullOrEmpty("password"))
  {
    throw new ArgumentException("Empty password.", "password");
  }

  CredentialsUserName = userName;
  CredentialsUserPassword = password;
  CredentialsDomain = domain;
}
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
    

アナライザーの警告： V3022式 'string.IsNullOrEmpty（ "password"）'は常にfalseです。 SmtpSettings.cs 104



どのエラーをどの場所に配置するかを選択するのは非常に困難でしたが、最初からこのトリガーがすべてのリーダーでした。最も単純なマイナーなタイプミス-そしてコードはすでに正しく機能していません。 IDEでの強調表示も、レビューも、古き良き常識も役に立ちませんでした。小さくてシンプルで美しく書かれた関数です。そしてここでも、PVS-Studioは専門家が見逃していたものを見つけることができました。



いつものように、悪魔は細部にあります。そのような詳細がすべて自動的に検索されたら素晴らしいと思いませんか？もちろんそうでしょう！そして、開発者に静的アナライザーではできないことをさせましょう。彼は新しく美しく安全なアプリケーションを作成します。変数をチェックするときに余分な引用符を付けるかどうかを考えずに作成します。

結論

2020年の記事で10の興味深い間違いを見つけるのは簡単でした。しかし、それらを場所に配布することはかなりの作業であることが判明しました。一方では、一部のトリガーは、高度なアナライザーメカニズムの動作をより適切に反映しています。一方で、いくつかの間違いはある程度おかしいように見えます。提示された位置の多くは逆にすることができます-たとえば、トップ2とトップ3。



それとも、このリストに他のポジティブな点があるはずだと思いますか？実際、リンクをたどることで独自のトップを作成することもできます 記事のリストに移動し、あなたの意見で最もおいしいトリガーを見つけます。この場合、コメントで2020年のトップを捨ててください、私はそれを大いに喜んで読みます。リストを私のものよりも面白くすることができますか？



もちろん、警告の「面白さ」の問題はとにかく主観的です。私の意見では、応答を評価するための主な基準は、PVS-Studioからの警告を見たプログラマーが、対応するコードの何かを変更するかどうかです。このリストは、フラグメントのトリガーから作成されたものであり、私の意見では、開発者が静的分析を使用した方が見栄えが良くなります。その上、あなた自身または他のいくつかのプロジェクトをチェックすることによってPVS-Studioを試すことに問題はありません。あなたはただリンクをたどる必要があります 、そこに必要なバージョンのアナライザーをダウンロードし、小さなフォームに記入してトライアルキーをリクエストします。



さて、これですべてです。ご清聴ありがとうございました。またお会いしましょう！

この記事を英語を話す聴衆と共有したい場合は、翻訳リンクを使用してください：NikitaLipilin。 2020年にC＃プロジェクトで見つかったトップ10のバグ。