🏦 👴 🤚 MicrosoftはXamarin.Formsのソースを公開しました。 PVS-Studioでテストする機会を逃すことはできませんでした 🤞🏻 🤸🏽 👨🏾‍🚒

ご存知のように、MicrosoftはXamarinを買収しました。マイクロソフトは最近、製品のソースコードを徐々にオープンし始めましたが、Xamarin.Formsコードのオープンは大きな驚きでした。私はそのようなイベントを通り抜けることができなかったため、静的コードアナライザーを使用してこのプロジェクトのソースコードをチェックすることにしました。

分析されたプロジェクト

Xamarin.Formsは、さまざまなプラットフォーム（Windows、Windows Phone、iOS、Android）に共通のユーザーインターフェイスを作成できるクロスプラットフォームツールキットです。ユーザーインターフェイスは、最終プラットフォームのネイティブコンポーネントを使用してレンダリングされます。これにより、Xamarin.Formsを使用して作成されたアプリケーションは、各プラットフォームの共通ビューを維持できます。 C＃またはXAMLマークアップを使用して、データバインディングとさまざまなスタイルのユーザーインターフェイスを作成できます。

フレームワーク自体のコードもC＃で記述されており、GitHubのリポジトリで入手できます。

分析ツール

プロジェクトはPVS-Studio静的コードアナライザーを使用してチェックされ、その開発では私が積極的に参加しています。既存のルールの変更や新しい診断ルールの追加など、常に改善に取り組んでいます。したがって、新しいプロジェクトの各テストで、より多くの種類のエラーを特定することができます。

各診断ルールには、エラーの説明と、正しくないコードと正しいコードの例を含むドキュメントが含まれています。アナライザーの試用版はこちらからダウンロードできます。また、最近同僚が書いたメモを知ることを提案します。彼女は、デモバージョンのこのような制限が選択された理由と、すべての機能を試すために何をする必要があるかを説明します。読むのが面倒な人のために、すぐにあなたに話します-私たちに書いてください。

PSさらに、このサイトには、オープンソースプロジェクトで見つかったエラーのデータベースと、記事のカタログ（オープンソースプロジェクト、技術フォーカスなどのチェックに関する）があります。よく理解しておくことをお勧めします。

疑わしいコードスニペット

診断ルールV3001で検出された「クラシック」エラーから始めましょう。

const int RwWait = 1; const int RwWrite = 2; const int RwRead = 4; .... public void EnterReadLock() { .... if ((Interlocked.Add(ref _rwlock, RwRead) & (RwWait | RwWait)) == 0) return; .... }

PVS-Studio警告： V3001 「|」の左と右に同じ副次式「RwWait」があります演算子。 SplitOrderedList.cs 458

コードからわかるように、一部の式の値はビット演算を使用して計算されます。さらに、部分式の1つ-RwWait | RwWaitには、同じ定数フィールドが含まれます。意味がありません上記で宣言された定数のセットは2の累乗に等しい値を持つため、フラグとしての使用が暗示されていることは明らかです（ビット操作の使用例で見るように）。 [Flags]属性でマークされた列挙にそれらを配置する方がより実用的だと思います。これは、この列挙で作業するときに多くの利点をもたらします（ V3059のドキュメントを参照）。

現在の例に関しては、おそらく、 RwWrite定数の使用が暗示されています。これは、IntelliSenseの欠点の1つに起因する可能性があります。このツールはコードの記述に非常に役立ちますが、間違った変数を「伝える」ことがあるため、不注意でミスを犯す可能性があります。

同様のエラーが発生した次のコード例：

 public double Left { get; set; } public double Top { get; set; } public double Right { get; set; } public double Bottom { get; set; } internal bool IsDefault { get { return Left == 0 && Top == 0 && Right == 0 && Left == 0; } }

PVS-Studio警告： V3001 「&&」演算子の左側と右側に同一の副次式「Left == 0」があります。 Thickness.cs 29

式に2回、サブ式Left == 0があります。明らかに、これは間違いです。最後の部分式の代わりに、次のコードを配置する必要があります-Bottom == 0。これは、この式でチェックされない唯一のプロパティ（ロジックに従い、一連のプロパティに基づく）であるためです。

次のエラーは、同じ名前で部分的に類似したコードを持つ2つのファイルにあるという点で興味深いものです。そして、それはエラーが増加することがわかります-彼らは1つの場所で間違いを犯し、このコードを別の場所にコピーしました-op！ -ここにあなたのための別の間違った場所があります。

 public override SizeRequest GetDesiredSize(int widthConstraint, int heightConstraint) { .... int width = widthConstraint; if (widthConstraint <= 0) width = (int)Context.GetThemeAttributeDp(global::Android .Resource .Attribute .SwitchMinWidth); else if (widthConstraint <= 0) width = 100; .... }

PVS-Studio 警告： V3003 「if（A）{...} else if（A）{...}」パターンの使用が検出されました。論理エラーが存在する可能性があります。行を確認してください：28、30。Xamarin.Forms.Platform.Android SwitchRenderer.cs 28

このコードでは、 ifステートメントに奇妙なロジックがあります。特定の条件がチェックされ（ widthConstraint <= 0 ）、満たされない場合は、同じ条件が再度チェックされます。エラー？エラー。しかし、それを修正する方法は言うのがより困難です。このタスクは、すでにコードを書いたプログラマーの肩にかかっています。

私が言ったように、まったく同じエラーが同じ名前のファイルで見つかりました。対応するアナライザーメッセージは次のとおりです。V3003 'if（A）{...} else if（A）{...}'パターンの使用が検出されました。論理エラーが存在する可能性があります。 26、28行を確認してください。Xamarin.Forms.Platform.AndroidSwitchRenderer.cs 26

仮想値メカニズムにより、式の値が常にtrueまたはfalseであると判断するV3022診断を含む、多くの診断ルールが大幅に改善されました。その助けを借りて発見されたいくつかの例を見ることをお勧めします：

 public TypeReference ResolveWithContext(TypeReference type) { .... if (genericParameter.Owner.GenericParameterType == GenericParameterType.Type) return TypeArguments[genericParameter.Position]; else return genericParameter.Owner.GenericParameterType == GenericParameterType.Type ? UnresolvedGenericTypeParameter : UnresolvedGenericMethodParameter; .... }

PVS-Studio 警告： V3022式 'genericParameter.Owner.GenericParameterType == GenericParameterType.Type'は常にfalseです。 ICSharpCode.Decompiler TypesHierarchyHelpers.cs 441

興味のないメソッドの部分を削除したという事実にもかかわらず、今でもエラーはあまり目立たないかもしれません。この状況を修正するために、短い変数名でコードを書き直して、コードをさらに単純化することを提案します。

 if (a == enVal) return b; else return a == enVal ? c : d;

これですべてが少し明確になりました。問題の原因は、三項演算子にある2番目のチェックa == enVal（genericParameter.Owner.GenericParameterType == GenericParameterType.Type）です。 ifステートメントの elseブランチの三項演算子は意味がありません-この場合、メソッドは常にd （ UnresolvedGenericMethodParameter ）を返します。

あなたは問題が何であるかを推測していない場合-私は説明します。プログラムが三項演算子の値の計算に達すると、式a == enValがfalseであることはすでにわかっているため、三項演算子では同じ値になります。結論：三項演算子の結果は常に同じです。間違い

メソッドから余分なコードをカットしたとしても、残りの部分ではエラーが巧妙に隠されているため、このような欠陥をすぐに検出することは困難です。この「落とし穴」を特定するために、さらに簡略化する必要がありました。ただし、アナライザーにはこのような問題はなく、タスクに簡単に対処できました。

もちろん、これはそのような場合だけではありません。ここに別のものがあります：

 TypeReference DoInferTypeForExpression(ILExpression expr, TypeReference expectedType, bool forceInferChildren = false) { .... if (forceInferChildren) { .... if (forceInferChildren) { InferTypeForExpression(expr.Arguments.Single(), lengthType); } } .... }

PVS-Studio警告： V3022式「forceInferChildren」は常にtrueです。 ICSharpCode.Decompiler TypeAnalysis.cs 632

繰り返しますが、キャッチに気づきやすくするために、余分なコードをすべて切り取ります。そしてここにあります-forceInferChildren条件は2回チェックされますが、この変数はifステートメントの間では使用されません。これがメソッドのパラメーターであると考えると、他のスレッドもメソッドも直接アクセスせずに変更することはできないと結論付けることができます。したがって、最初のifステートメントが実行されると、2番目のステートメントが常に実行されます。奇妙な論理。

V3022-V3063に類似した診断があります。この診断ルールは、条件式の一部が常にtrueまたはfalseであることを決定します。彼女のおかげで、いくつかの興味深いコードを検出することができました。

 static BindableProperty GetBindableProperty(Type elementType, string localName, IXmlLineInfo lineInfo, bool throwOnError = false) { .... Exception exception = null; if (exception == null && bindableFieldInfo == null) { exception = new XamlParseException( string.Format("BindableProperty {0} not found on {1}", localName + "Property", elementType.Name), lineInfo); } .... }

PVS-Studio警告： V3063条件式の一部は常に真です：例外== null。 Xamarin.Forms.Xaml ApplyPropertiesVisitor.cs 280

副式の例外== nullに興味があります。明らかに、それは常に真実です。なぜこのチェックですか？不明。ちなみに、デバッグ中に値を変更できることを何らかの形で知らせるコメントはありません（ // new Exception（）;など ）はここにありません。

これらは、診断ルールV3022およびV3063によって検出された唯一の疑わしいスポットではありません。しかし、それらに焦点を合わせることはしませんが、他に何が面白いと思ったのか見てみましょう。

 void WriteSecurityDeclarationArgument(CustomAttributeNamedArgument na) { .... output.Write("string('{0}')", NRefactory.CSharp .TextWriterTokenWriter .ConvertString( (string)na.Argument.Value).Replace("'", "\'")); .... }

警告PVS-Studio： V3038 'Replace'関数の最初の引数は2番目の引数と同じです。 ICSharpCode.Decompiler ReflectionDisassembler.cs 349

このコードから、文字列に対して呼び出されるReplaceメソッドに興味があります。どうやら、プログラマーはすべての単一引用符文字をスラッシュと引用 符に置き換えたかったようです。しかし、実際には、2番目の場合、スラッシュ文字がエスケープされるため、このメソッドを呼び出すと、単一引用符がそれに置き換えられます。信じられない？ 等しい（ "'"、 "\'"） 。一部の人にとっては、これは明らかではないかもしれませんが、アナライザーにとってはそうではありません。エスケープを回避するには、文字列リテラルの前に@文字を使用できます。次に、 Replaceメソッドの正しい呼び出しは次のようになります。

 Replace("'", @"\'")

常に同じ値を返すメソッドがありました。例：

 static bool Unprocessed(ICollection<string> extra, Option def, OptionContext c, string argument) { if (def == null) { .... return false; } .... return false; }

PVS-Studio警告： V3009このメソッドが常に1つの同じ値「false」を返すのは奇妙です。 Xamarin.Forms.UITest.TestCloud OptionSet.cs 239

このメソッドでどの引数が入力され、何が行われても、常にfalseを返します。 同意して、それはなんとなく奇妙に見える。

ところで、このコードは再会しました-メソッドは完全にコピーされ、別の場所に移動されました。アナライザーメッセージ：V3009このメソッドが常に1つの同じ値「false」を返すのは奇妙です。 Xamarin.Forms.Xaml.Xamlg Options.cs 1020

潜在的にエラーを含む例外の再生成で、いくつかのコードフラグメントが検出されました。

 static async Task<Stream> GetStreamAsync (Uri uri, CancellationToken cancellationToken) { try { await Task.Delay (5000, cancellationToken); } catch (TaskCanceledException ex) { cancelled = true; throw ex; } .... }

PVS-Studio警告： V3052元の例外オブジェクト「ex」が飲み込まれました。元の例外のスタックが失われる可能性があります。 Xamarin.Forms.Core.UnitTests ImageTests.cs 221

ロジックは単純に思えます。例外が発生した場合、いくつかのアクションを実行して再生成します。しかし、悪魔は詳細にあります。この場合、例外が再度スローされると、元の例外のスタックは完全に上書きされます。これを回避するために、同じ例外を新たにスローする必要はなく、 スロー演算子を呼び出して既存の例外をスローするだけです。次に、 catchブロックのコードは次のようになります。

 cancelled = true; throw;

同様の例：

 public void Visit(ValueNode node, INode parentNode) { .... try { .... } catch (ArgumentException ae) { if (ae.ParamName != "name") throw ae; throw new XamlParseException( string.Format("An element with the name \"{0}\" already exists in this NameScope", (string)node.Value), node); } }

PVS-Studio警告： V3052元の例外オブジェクト 'ae'が飲み込まれました。元の例外のスタックが失われる可能性があります。 Xamarin.Forms.Xaml RegisterXNamesVisitor.cs 38

どちらの場合も、以前の例外に関する情報は失われます。そして、2番目のケースで、この情報が関連していないとまだ仮定できる場合（まだ奇妙ですが）、1番目のケースでは、例外をスローしたい可能性は低いですが、代わりに新しい例外を生成します。解決策は前の例と同じです-引数なしでthrow演算子を呼び出します。

次の断片を犠牲にして-これが間違いであるかどうかは確かに言えませんが、奇妙に見えます。

 void UpdateTitle() { if (Element?.Detail == null) return; ((ITitleProvider)this).Title = (Element.Detail as NavigationPage) ?.CurrentPage?.Title ?? Element.Title ?? Element?.Title; }

PVS-Studio警告： V3042 NullReferenceExceptionの可能性があります。「？。」および「。」演算子は、Elementオブジェクトのメンバーにアクセスするために使用されますXamarin.Forms.Platform.WinRT MasterDetailPageRenderer.cs 288

アナライザーは、 TitleプロパティがElement.TitleとElement？.Titleのさまざまな方法でアクセスされ、最初は直接アクセスされ、次にヌル条件演算子を使用してアクセスされることを警告しました。しかし、ここではすべてがそれほど明確ではありません。

お気づきかもしれませんが、メソッドの開始時にElement？.Detail == nullチェックが実行されます。Element== nullの場合、ここで出口が実行され、以降の操作に到達しないと想定されます。

同時に、式Element？.Titleは、実行時にElementがnullであることを示唆しています。この場合、前の段階でTitleプロパティに直接アクセスすると、 NullReferenceException型の例外が生成されるため、null条件演算子を使用しても意味がありません。

いずれにせよ、このコードは非常に奇妙に見え、修正する必要があります。

オブジェクトが独自の型にキャストされると、奇妙に見えました。そのようなコードの例を次に示します。

 public FormsPivot Control { get; private set; } Brush ITitleProvider.BarBackgroundBrush { set { (Control as FormsPivot).ToolbarBackground = value; } }

警告PVS-Studio： V3051過剰な型キャスト。オブジェクトはすでに「FormsPivot」タイプです。 Xamarin.Forms.Platform.UAP TabbedPageRenderer.cs 73

この場合、これはエラーではありませんが、 ControlオブジェクトにFormsPivotタイプが既にあるため、コードは少なくとも疑わしく見えます。ところで、これはこの種の唯一の警告ではなく、他の人も出会った：

V3051過剰な型キャスト。オブジェクトはすでに「FormsPivot」タイプです。 Xamarin.Forms.Platform.UAP TabbedPageRenderer.cs 78
V3051過剰な型キャスト。オブジェクトはすでに「FormsPivot」タイプです。 Xamarin.Forms.Platform.UAP TabbedPageRenderer.cs 282
V3051過剰な型キャスト。オブジェクトはすでに「FormsPivot」タイプです。 Xamarin.Forms.Platform.WinRT.Phone TabbedPageRenderer.cs 175
V3051過剰な型キャスト。オブジェクトはすでに「FormsPivot」タイプです。 Xamarin.Forms.Platform.WinRT.Phone TabbedPageRenderer.cs 197
V3051過剰な型キャスト。オブジェクトはすでに「FormsPivot」タイプです。 Xamarin.Forms.Platform.WinRT.Phone TabbedPageRenderer.cs 205

単純化できる条件があります。それらの1つの例：

 public override void LayoutSubviews() { .... if (_scroller == null || (_scroller != null && _scroller.Frame == Bounds)) return; .... }

PVS-Studio警告： V3031過剰なチェックは単純化できます。「||」演算子は反対の式で囲まれています。 Xamarin.Forms.Platform.iOS.Classic ContextActionCell.cs 102

この式は、 _scroller！= Null部分式を削除することで簡略化できます。「||」演算子の左側の式が偽の場合にのみ計算されます -_scroller == null 、したがって-_scrollerは nullではなく、 NullReferenceExceptionを取得する心配はありません。次に、簡略化されたコードは次のようになります。

 if (_scroller == null || _scroller.Frame == Bounds))

軟膏で飛ぶ

残念ながら、ソリューションを完全に組み立てることはできませんでした-約6つのプロジェクトが未検証のままであり、クラスが何らかの方法で使用された場所は、可能な限り徹底的な分析を受けませんでした。おそらく、彼らは何か他の興味深いものを見つけることができたかもしれませんが、悲しいかな。

ところで、レベル3にあるメッセージV051によって、分析の問題について調べることができます。原則として、このような警告の存在は、C＃プロジェクトに何らかのコンパイルエラーが含まれていることを示すシグナルであり、アナライザーは複雑な分析を実行するために必要なすべての情報を取得できません。それでも、彼は、タイプとオブジェクトに関する詳細情報が不要なチェックを実行しようとします。

プロジェクトをチェックするとき、警告V051がないことを確認することをお勧めします。そして、それらが存在する場合は、それらを削除してみてください（プロジェクトがコンパイルされていることを確認し、すべての依存関係がロードされていることを確認してください）。

おわりに

Xamarin.Formsチェックは功を奏しました-明らかに間違いがあり、非常に疑わしい、または奇妙なさまざまな興味深い場所がありました。開発者がこの記事を迂回して、ここで書かれたコードの断片を修正しないことを願っています。アナライザーの試用版をダウンロードすることで、発見されたすべての疑わしい場所を表示できます。 PVS-Studioを継続的に実装することは、エラーが表示された直後に検出および修正できるようにする、さらに優れた、より正確なソリューションです。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：セルゲイヴァシリエフ。 MicrosoftはXamarin.Formsのソースコードを開きました。 PVS-Studioで確認する機会を逃すことはできませんでした。

記事を読んで質問がありますか？

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

MicrosoftはXamarin.Formsのソースを公開しました。 PVS-Studioでテストする機会を逃すことはできませんでした

分析されたプロジェクト

分析ツール

疑わしいコードスニペット

軟膏で飛ぶ

おわりに

More articles: