Clever Geek Handbook

Roslynのソースコードの確認

PVS-Studio vsロズリン






時々、以前PVS-Studioでテストしたプロジェクトに戻り、それについての記事を書きました。 これには2つの理由があります。 まず、アナライザーがどの程度改善されているかを理解します。 第二に、プロジェクトの作者が私たちの記事に加えて、私たちが通常提供するバグレポートに注意を払ったかどうかを追跡するためです。 もちろん、私たちが参加しなくてもエラーを修正できます。 しかし、私たちの努力がプロジェクトの改善に役立つのはいつでも素晴らしいことです。 ロズリンも例外ではありませんでした。 このプロジェクトに関する以前のレビュー記事は、2015年12月23日までさかのぼります。 この期間中にアナライザーが開発中に移動したパスを考えると、これは非常に長い時間です。 個人的には、C＃アナライザーPVS-Studioのコアがそれをベースにしているという事実から、Roslynはさらに興味深いものです。 したがって、このプロジェクトのコードの品質に非常に興味があります。 PVS-Studioで2回目のチェックを行い、何が新しくて面白いかを調べます（ただし、重要なものがないことを期待しましょう）。



Roslyn（または.NET Compiler Platform）は、おそらく読者の多くによく知られています。 要するに、これは一連のオープンソースコンパイラと、MicrosoftのC＃およびVisual Basic .NET言語のコード分析のためのAPIです。 プロジェクトのソースコードはGitHubで入手できます 。



このプラットフォームの詳細な説明は行いませんが、同僚のSergey Vasilievによる記事「 Roslyn入門。静的分析ツールを使用して開発する 」に興味があるすべての人に推奨します。 この記事から、Roslynアーキテクチャーの機能だけでなく、このプラットフォームの使用方法についても学ぶことができます。



前述したように、同僚のアンドレイ・カーポフによるRoslynチェック「 PVS-Studio 6.00の新年リリース： Roslynのチェック」に関する最後の記事の執筆から3年以上が経過しました。 この間、C＃PVS-Studioアナライザーは多くの新機能を獲得しました。 一般的に、Andreyの記事は一種の「トライアルボール」でした。なぜなら、C＃アナライザーはPVS-Studioにしか追加されなかったからです。 それにもかかわらず、無条件に高品質のプロジェクトで、Roslynはなんとか興味深いエラーを見つけました。 これまでC＃コードのアナライザーで何が変更されましたか。これにより、より深い分析が可能になる可能性があります。



過去に、アナライザーコアとインフラストラクチャの両方が開発されました。 Visual Studio 2017とRoslyn 2.0のサポート、およびMSBuildとの緊密な統合が追加されました。 MSBuildと統合するためのアプローチと、それを受け入れるようになった理由については、同僚のPavel Yeremeyevの記事「 PVS-StudioでのVisual Studio 2017およびRoslyn 2.0のサポート：既製のソリューションの使用は見た目ほど簡単ではない場合があります」一目で "。



現在、私たちはVisual Studio 2017を最初にサポートした同じスキームに従って、つまり、空のMSBuild.exeファイルの形式の「スタブ」を備えたPVS-Studio配布キットに含まれる独自のツールセットを通じて、Roslyn 3.0への移行に積極的に取り組んでいます。 「松葉杖」のように見えるという事実にもかかわらず（MSBuild APIは、ライブラリの移植性が低いため、サードパーティプロジェクトでの再利用にはあまり適していません）、このアプローチは、Visual Studio 2017の期間中にいくつかの痛みのないRoslyn更新を比較的簡単に再現するのにすでに役立ちました。そして今では、多くのオーバーレイがありますが、Visual Studio 2019へのアップグレードを生き延び、MSBuildの古いバージョンを搭載したシステムで完全な下位互換性とパフォーマンスを維持します。



アナライザーコアにも多くの改良が加えられました。 主な革新の1つは、メソッドの入力値と出力値を考慮し、これらのパラメーターに応じて実行ブランチとリターンポイントの到達可能性を考慮した本格的な手続き間分析です。



メソッド内のパラメーターを追跡するタスクは既に完了に近づいていますが、これらのパラメーターで発生することに対する自動注釈を保持しています（たとえば、潜在的に危険な逆参照）。 これにより、データフローメカニズムを使用して、パラメータをメソッドに渡すときに発生する危険な状況を考慮した診断が可能になります。 以前は、このような危険な場所を分析するとき、そのようなメソッドへのすべての入力値を知ることができなかったため、警告は生成されませんでした。 このメソッドが呼び出されるすべての場所で、これらの入力パラメーターが考慮されるため、危険を検出できます。



注：「 エラーおよび潜在的な脆弱性を見つけるためにPVS-Studioコードアナライザーで使用される技術 」の記事から、データフローなどのアナライザーの主要なメカニズムに慣れることができます。



PVS-Studio Cでのプロシージャー間の分析＃は、入力パラメーターまたは深さによって制限されません。 唯一の制限は、継承のために閉じられず、再帰に陥らないクラスの仮想メソッドです（既に計算されたメソッドの繰り返しの呼び出しをスタックで確認したら停止します）。 この場合、再帰的メソッド自体は、自己再帰の戻り値が不明であるという仮定で最終的に計算されます。



C＃アナライザーのもう1つの大きな革新は、潜在的にnullポインターを逆参照する可能性です。 アナライザーは、すべての実行ブランチで変数の値がnullになることが確実である場合、null参照例外の可能性を宣誓していました。 もちろん、彼は時々間違っていたので、 V3080診断は以前は潜在的なヌル参照と呼ばれていました。



アナライザーは、実行ブランチの1つで変数がnullになる可能性があることを覚えています（たとえば、ifの特定の条件下）。 チェックせずにそのような変数へのアクセスを見つけた場合、メッセージV3080を表示しますが、すべてのブランチでnullを見つけた場合よりも重要度は低くなります。 改善された手続き間分析と組み合わせて、このようなメカニズムにより、エラーを検出するのが非常に困難になります。 例としては、メソッドコールの長いチェーンがあります。最後のメソッドコールはよく知らず、たとえば特定の状況でnullを返しますが、単にそれを知らなかったため、これから身を守りませんでした。 この場合、アナライザーは、nullの割り当てを正確に検出したときにのみ誓います。 私たちの意見では、これは私たちのアプローチを、Nullable参照型としてのC＃8.0の革新から定性的に区別します。実際には、各メソッドでnullチェックを設定することになります。 代替手段を提供します-nullが実際に発生する可能性がある場合にのみチェックを実行し、アナライザーがそのような状況を検索できるようになりました。



それでは、遅滞なく、「デブリーフィング」に移りましょう-Roslynチェックの結果を分析します。 最初に、上記の革新によって見つかったエラーを見てみましょう。 一般に、今回はRoslynコードに対してかなりの数の警告が発行されました。 これは、プラットフォームが非常に活発に開発されているという事実によると思います（コードベースは現在、空のコードを除く約2,770,000行のコードに相当します）。私たちはこのプロジェクトの分析を長い間行っていません。 ただし、重大なエラーはそれほど多くありません。つまり、記事にとって重要なエラーです。 そして、そうです、Roslynには、いつものように、テストから除外したかなりの数のテストがあります。



重大度が中レベルのV3080エラーで開始します。このレベルでは、アナライザーはゼロリンクを介したアクセスの可能性を検出しましたが、可能性のあるすべてのケース（コード分岐）ではありません。



nullの逆参照の可能性-中



V3080 nullの逆参照の可能性。 「現在」の検査を検討してください。 CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70 



private SyntaxNode GetNode(SyntaxNode root) { var current = root; .... while (current.FullSpan.Contains(....)) // <= { .... var nodeOrToken = current.ChildThatContainsPosition(....); .... current = nodeOrToken.AsNode(); // <= } .... } public SyntaxNode AsNode() { if (_token != null) { return null; } return _nodeOrParent; }


GetNodeメソッドを検討してください。 アナライザーは、 whileブロックの状態でヌル参照によるアクセスが可能であると見なします。 whileブロックの本体では、 現在の変数に値（ AsNodeメソッドの実行結果）が割り当てられます。 場合によっては、この値はnullになります 。 実際の手続き間分析の良い例。



次に、プロシージャ間分析が2つのメソッド呼び出しを介して実行された同様のケースを考えます。



V3080 nullの逆参照の可能性。 「ディレクトリ」の検査を検討してください。 CommonCommandLineParser.cs 911 



 private IEnumerable<CommandLineSourceFile> ExpandFileNamePattern(string path, string baseDirectory, ....) { string directory = PathUtilities.GetDirectoryName(path); .... var resolvedDirectoryPath = (directory.Length == 0) ? // <= baseDirectory : FileUtilities.ResolveRelativePath(directory, baseDirectory); .... } public static string GetDirectoryName(string path) { return GetDirectoryName(path, IsUnixLikePlatform); } internal static string GetDirectoryName(string path, bool isUnixLike) { if (path != null) { .... } return null; }


ExpandFileNamePatternメソッドの本体のディレクトリ変数は、 GetDirectoryName（文字列）メソッドから値を取得します。 その結果、オーバーロードされたGetDirectoryName（string、bool）メソッドの結果が返され、その値はnullになる場合があります 。 さらにExpandFileNamePatternメソッドの本体では、 ディレクトリ変数がnullの等価性を事前にチェックせずに使用されるため、アナライザーによる警告の正当性について話すことができます。 これは潜在的に危険な設計です。



エラーV3080を含む別のコード、より正確には、1行のコードに対して2つのエラーが発行されます。 ここでは、プロシージャー間の分析は不要でした。



V3080 nullの逆参照の可能性。 「spanStartLocation」の検査を検討してください。 TestWorkspace.cs 574



V3080 nullの逆参照の可能性。 「spanEndLocationExclusive」の検査を検討してください。 TestWorkspace.cs 574 



 private void MapMarkupSpans(....) { .... foreach (....) { .... foreach (....) { .... int? spanStartLocation = null; int? spanEndLocationExclusive = null; foreach (....) { if (....) { if (spanStartLocation == null && positionInMarkup <= markupSpanStart && ....) { .... spanStartLocation = ....; } if (spanEndLocationExclusive == null && positionInMarkup <= markupSpanEndExclusive && ....) { .... spanEndLocationExclusive = ....; break; } .... } .... } tempMappedMarkupSpans[key]. Add(new TextSpan( spanStartLocation.Value, // <= spanEndLocationExclusive.Value - // <= spanStartLocation.Value)); } } .... }


spanStartLocationおよびspanEndLocationExclusive変数はnullable int型であり、 nullに初期化されます 。 さらにコードでは、特定の条件が満たされた場合にのみ、値を割り当てることができます。 場合によっては、それらの値はnullのままになります 。 さらに、コードでは、アナライザーが示すように、これらの変数は最初にnullの同等性をチェックせずに参照によってアクセスされます。



Roslynコードには、100を超えるこのようなエラーが多数含まれています。多くの場合、これらのエラーのパターンは同じです。 nullを返す可能性のある一般的なメソッドがあります 。 このメソッドの結果は、多くの場所で、時には何十もの中間メソッド呼び出しや追加のチェックを通じて使用されます。 これらのエラーは致命的ではありませんが、ヌルリンクを介したアクセスにつながる可能性があることを理解することが重要です。 そして、そのようなエラーを検出することは非常に困難です。 そのため、場合によっては、メソッドがnullを返した場合に例外がスローされるコードのリファクタリングを検討する必要があります 。 それ以外の場合は、完全なチェックのみでコードを保護できますが、これは非常に面倒で信頼性に欠けます。 もちろん、いずれの場合も、プロジェクトの特性に基づいて決定する必要があります。



ご注意 現時点では、メソッドがnullを返し、実際のエラーがない状況（入力データ）はありません。 ただし、コードに変更が加えられるとすべてが変更される可能性があるため、このようなコードはまだ信頼できません。



V3080でトピックを閉じるために、nullリンクを介したアクセスが発生する可能性が高い、または避けられない場合の、 高信頼レベルの明らかなエラーを見てみましょう。



nullの逆参照の可能性-高



V3080 nullの逆参照の可能性。 「collectionType.Type」の検査を検討してください。 AbstractConvertForToForEachCodeRefactoringProvider.cs 137 



 public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) { .... var collectionType = semanticModel.GetTypeInfo(....); if (collectionType.Type == null && collectionType.Type.TypeKind == TypeKind.Error) { return; } .... }


条件のタイプミス（ &&を使用した||演算子ではなく）により、コードは意図したとおりに機能せず、 collectionType.Type変数がnullの場合にアクセスされます 。 条件は次のように修正する必要があります。 



 if (collectionType.Type == null || collectionType.Type.TypeKind == TypeKind.Error) ....


ところで、2番目のシナリオも可能です。最初の部分では、条件は演算子==と！=によって混同されます。 修正されたコードは次のようになります。 



 if (collectionType.Type != null && collectionType.Type.TypeKind == TypeKind.Error) ....


このバージョンのコードは論理的ではありませんが、エラーも修正しています。 最終決定はプロジェクトの作成者次第です。



別の同様のエラー。



V3080 nullの逆参照の可能性。 「アクション」の検査を検討してください。 TextViewWindow_InProc.cs 372 



 private Func<IWpfTextView, Task> GetLightBulbApplicationAction(....) { .... if (action == null) { throw new InvalidOperationException( $"Unable to find FixAll in {fixAllScope.ToString()} code fix for suggested action '{action.DisplayText}'."); } .... }


例外のメッセージを作成するときにエラーが発生しました。 この場合、 アクション変数を介してaction.DisplayTextプロパティにアクセスしようとしますが、これは明らかにnullです。



そして、最後のV3080エラーは高です。



V3080 nullの逆参照の可能性。 「タイプ」の検査を検討してください。 ObjectFormatterHelpers.cs 91 



 private static bool IsApplicableAttribute( TypeInfo type, TypeInfo targetType, string targetTypeName) { return type != null && AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName; }


メソッドは小さいため、コード全体を提供します。 戻りブロックの条件が正しくありません。 場合によっては、 type.FullNameにアクセスするときにNullReferenceExceptionをスローすることができます。 状況を明確にするために、括弧を使用します（ここでは動作を変更しません）。 



 return (type != null && AreEquivalent(targetType, type)) || (targetTypeName != null && type.FullName == targetTypeName);


これが、操作の優先度に従って、このコードが機能する方法です。 型変数がnullの場合、elseチェックに入ります。そこで、 targetTypeName変数がnull不等式であることを確認して、null 型参照を使用します。 たとえば、次のようにコードを修正できます。 



 return type != null && (AreEquivalent(targetType, type) || targetTypeName != null && type.FullName == targetTypeName);


V3080エラーの調査を完了し、Roslynコードで何が興味深いPVS-Studioアナライザーを見つけたのかを確認できる場所だと思います。



タイプミス



V3005 「SourceCodeKind」変数はそれ自体に割り当てられます。 DynamicFileInfo.cs 17 



 internal sealed class DynamicFileInfo { .... public DynamicFileInfo( string filePath, SourceCodeKind sourceCodeKind, TextLoader textLoader, IDocumentServiceProvider documentServiceProvider) { FilePath = filePath; SourceCodeKind = SourceCodeKind; // <= TextLoader = textLoader; DocumentServiceProvider = documentServiceProvider; } .... }


変数名が失敗したため、 DynamicFileInfoクラスのコンストラクターでタイプミスが行われました。 SourceCodeKindフィールドには 、 sourceCodeKindパラメーターを使用する代わりに、独自の値が割り当てられます。 このようなエラーの可能性を最小限に抑えるために、このような場合にはパラメーター名にアンダースコアプレフィックスを使用することをお勧めします。 コードの修正バージョンの例を示します。 



 public DynamicFileInfo( string _filePath, SourceCodeKind _sourceCodeKind, TextLoader _textLoader, IDocumentServiceProvider _documentServiceProvider) { FilePath = _filePath; SourceCodeKind = _sourceCodeKind; TextLoader = _textLoader; DocumentServiceProvider = _documentServiceProvider; }


不注意



V3006オブジェクトは作成されましたが、使用されていません。 「throw」キーワードが欠落している可能性があります：throw新しいInvalidOperationException（FOO）。 ProjectBuildManager.cs 61 



 ~ProjectBuildManager() { if (_batchBuildStarted) { new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }


特定の条件下では、デストラクタは例外をスローする必要がありますが、これは発生せず、例外オブジェクトが作成されます。 throwキーワードが省略されました。 コードの修正バージョン： 



 ~ProjectBuildManager() { if (_batchBuildStarted) { throw new InvalidOperationException("ProjectBuilderManager.Stop() not called."); } }


C＃でデストラクタを操作し、デストラクタから例外をスローする問題は、別の議論のトピックであり、この記事の範囲外です。



結果が重要でない場合



すべての場合に同じ値を返すメソッドに対して、多くのV3009警告が受信されました。 時にはこれは重要ではないか、戻り値は呼び出しコードで単純にチェックされません。 私はそのような警告を見逃しました。 しかし、いくつかのコードは私には疑わしいようでした。 そのうちの1つを引用します。



V3009このメソッドが常に「true」という同じ値を返すのは奇妙です。 GoToDefinitionCommandHandler.cs 62 



 internal bool TryExecuteCommand(....) { .... using (context.OperationContext.AddScope(....)) { if (....) { return true; } } .... return true; }


TryExecuteCommandメソッドはtrueのみを返し、 true以外は何も返しません。 同時に、戻り値は呼び出し元コードのいくつかのチェックに関与します。 



 public bool ExecuteCommand(....) { .... if (caretPos.HasValue && TryExecuteCommand(....)) { .... } .... }


この振る舞いがどれほど危険であると言うのは難しい。 ただし、結果が不要な場合は、戻り値の型をvoidに置き換え、呼び出し元のメソッドに最小限の変更を加えるだけの価値がある場合があります。 これにより、コードがよりわかりやすく安全になります。



他の同様の警告：





チェックなし



V3019 「as」キーワードを使用した型変換後に、誤った変数がnullと比較される可能性があります。 変数 'value'、 'valueToSerialize'を確認してください。 RoamingVisualStudioProfileOptionPersister.cs 277 



 public bool TryPersist(OptionKey optionKey, object value) { .... var valueToSerialize = value as NamingStylePreferences; if (value != null) { value = valueToSerialize.CreateXElement().ToString(); } .... }


変数値のタイプはNamingStylePreferencesです。 問題はこのチェックに従っています。 値変数がnullでない場合でも、型変換が成功したこと、およびvalueToSerializeに nullが含まれていないことは保証されません。 NullReferenceExceptionがスローされる場合があります 。 コードは次のように修正する必要があります。 



 var valueToSerialize = value as NamingStylePreferences; if (valueToSerialize != null) { value = valueToSerialize.CreateXElement().ToString(); }


そしてもう一つの同様のエラー。



V3019「as」キーワードを使用した型変換後に、誤った変数がnullと比較される可能性があります。 変数「columnState」、「columnState2」を確認してください。 StreamingFindUsagesPresenter.cs 181 



 private void SetDefinitionGroupingPriority(....) { .... foreach (var columnState in ....) { var columnState2 = columnState as ColumnState2; if (columnState?.Name == // <= StandardTableColumnDefinitions2.Definition) { newColumns.Add(new ColumnState2( columnState2.Name, // <= ....)); } .... } .... }


変数columnStateの型はColumnState2です。 ただし、操作の結果である変数columnState2は、さらにnullがチェックされません。 代わりに、条件付きnullステートメントを使用してcolumnState変数がチェックされます 。 このコードの危険性は何ですか？ 前の例のように、 as演算子を使用した型キャストは失敗し、 columnState2変数はnullになり 、さらに例外がスローされます。 ちなみに、タイプミスが原因かもしれません。 ifブロックの状態に注意してください。 おそらくcolumnState？.Nameの代わりにcolumnState2？.Nameを書きたかったのでしょう 。 これは、かなり残念な変数名columnStateとcolumnState2が与えられている可能性が非常に高いです。



冗長チェック



重大ではないが、冗長チェックに関連する潜在的に危険な構造に対して、かなり多数の警告（100以上）が発行されました。 例として、そのうちの1つを挙げます。



V3022式 'navInfo == null'は常にfalseです。 AbstractSyncClassViewCommandHandler.cs 101 



 public bool ExecuteCommand(....) { .... IVsNavInfo navInfo = null; if (symbol != null) { navInfo = libraryService.NavInfoFactory.CreateForSymbol(....); } if (navInfo == null) { navInfo = libraryService.NavInfoFactory.CreateForProject(....); } if (navInfo == null) // <= { return true; } .... } public IVsNavInfo CreateForSymbol(....) { .... return null; } public IVsNavInfo CreateForProject(....) { return new NavInfo(....); }


おそらくここに本当の間違いはないでしょう。 「プロシージャ間分析+データフロー分析」のテクノロジの組み合わせを実際に実行することを示す正当な理由です。 アナライザーは、2番目のチェックnavInfo == nullが冗長であると見なします。 実際、その前に、 navInfoを割り当てるための値はlibraryService.NavInfoFactory.CreateForProjectメソッドから取得されます。 このメソッドは、 NavInfoクラスの新しいオブジェクトを構築して返します。 しかし、 nullではありません。 問題は、アナライザーが最初のテストnavInfo == nullに対して警告を出さなかったのはなぜですか？ これには説明があります。 まず、 シンボル変数がnullであることが判明した場合、 navInfoの値はnull参照のままになります。 次に、 navInfoがlibraryService.NavInfoFactory.CreateForSymbolメソッドから値を受け取った場合でも、この値はnullになる場合があります 。 したがって、 navInfo == nullの最初のチェックは本当に必要です。



不十分なチェック



そして今、状況は上記で検討した状況の反対です。 null参照を介してアクセスできるコードについて、いくつかのV3042警告が受信されました。 さらに、すべてを修正できる小さなチェックは1つまたは2つだけです。



このような2つのエラーを含む興味深いコードの1つを考えてください。



V3042 NullReferenceExceptionの可能性があります。 「？。」 および「。」 演算子は、「レシーバ」オブジェクトBinder_Expressions.cs 7770のメンバーにアクセスするために使用されます



V3042 NullReferenceExceptionの可能性があります。 「？。」 および「。」 演算子は、「レシーバ」オブジェクトBinder_Expressions.cs 7776のメンバーにアクセスするために使用されます 



 private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax != // <= GetConditionalReceiverSyntax(conditionalAccessNode)) { receiver = BindConditionalAccessReceiver(conditionalAccessNode, diagnostics); } var receiverType = receiver.Type; // <= if (receiverType?.IsNullableType() == true) { .... } receiver = new BoundConditionalReceiver(receiver.Syntax, 0, // <= receiverType ?? CreateErrorType(), hasErrors: receiver.HasErrors) // <= { WasCompilerGenerated = true }; return receiver; }


レシーバー変数はnullにできます 。 コードの作成者は、 ifブロックの条件で条件付きnull演算子を使用して、 receiver？.Syntaxにアクセスするため、これを知っています。 さらにコードでは、変数receiverは 、何もチェックせずにreceiver.Type 、 receiver.Syntaxおよびreceiver.HasErrorsにアクセスするために使用されます。 これらのエラーは修正する必要があります： 



 private BoundExpression GetReceiverForConditionalBinding( ExpressionSyntax binding, DiagnosticBag diagnostics) { .... BoundExpression receiver = this.ConditionalReceiverExpression; if (receiver?.Syntax != GetConditionalReceiverSyntax(conditionalAccessNode)) { receiver = BindConditionalAccessReceiver(conditionalAccessNode, diagnostics); } var receiverType = receiver?.Type; if (receiverType?.IsNullableType() == true) { .... } receiver = new BoundConditionalReceiver(receiver?.Syntax, 0, receiverType ?? CreateErrorType(), hasErrors: receiver?.HasErrors) { WasCompilerGenerated = true }; return receiver; }


また、 BoundConditionalReceiverコンストラクターがパラメーターのnull値の取得をサポートしていることを確認するか、追加のリファクタリングを実行する必要があります。



その他の同様のエラー：





状態エラー



V3057 ' Substring '関数は、負でない値が期待されているときに '-1'値を受け取る可能性があります。 2番目の引数を調べます。 CommonCommandLineParser.cs 109 



 internal static bool TryParseOption(....) { .... if (colon >= 0) { name = arg.Substring(1, colon - 1); value = arg.Substring(colon + 1); } .... }


コロン変数が0で、コード内の条件が許可されている場合、 Substringメソッドは ArgumentOutOfRangeException をスローします。 修正が必要： 



 if (colon > 0)


タイプミス可能



V3065パラメーター 't2'はメソッドの本体内では使用されません。 CSharpCodeGenerationHelpers.cs 84 



 private static TypeDeclarationSyntax ReplaceUnterminatedConstructs(....) { .... var updatedToken = lastToken.ReplaceTrivia(lastToken.TrailingTrivia, (t1, t2) => { if (t1.Kind() == SyntaxKind.MultiLineCommentTrivia) { var text = t1.ToString(); .... } else if (t1.Kind() == SyntaxKind.SkippedTokensTrivia) { return ReplaceUnterminatedConstructs(t1); } return t1; }); .... }


2つのパラメーター、t1およびt2がラムダ式に渡されます。 ただし、t1のみが使用されます。 これらの名前の変数を使用するときにミスを犯すのがどれほど簡単かを考えると、疑わしいようです。



不注意



V3083イベント「TagsChanged」の安全でない呼び出し、NullReferenceExceptionが発生する可能性があります。 イベントを呼び出す前に、ローカル変数にイベントを割り当てることを検討してください。 PreviewUpdater.Tagger.cs 37 



 public void OnTextBufferChanged() { if (PreviewUpdater.SpanToShow != default) { if (TagsChanged != null) { var span = _textBuffer.CurrentSnapshot.GetFullSpan(); TagsChanged(this, new SnapshotSpanEventArgs(span)); // <= } } }


TagsChangedイベントは安全ではありません。 nullの等価性をチェックしてからイベントを呼び出すまでの間に、イベントのサブスクリプションを解除する時間があり、例外がスローされます。 さらに、イベントが呼び出される直前にifブロックの本体でさらにいくつかの操作が行われます。 このエラーを「不注意」と呼びました。これは、コード内の他の場所では、たとえば次のように、このイベントでより正確に機能するためです。 



 private void OnTrackingSpansChanged(bool leafChanged) { var handler = TagsChanged; if (handler != null) { var snapshot = _buffer.CurrentSnapshot; handler(this, new SnapshotSpanEventArgs(snapshot.GetFullSpan())); } }


オプションのハンドラー変数を使用すると、問題がなくなります。 OnTextBufferChangedメソッドでは、イベントで同じ安全な操作を編集する必要があります 。



交差する範囲



V3092条件式内で範囲の交差が可能です。 例：if（A> 0 && A <5）{...} else if（A> 3 && A <9）{...}。 ILBuilderEmit.cs 677 



 internal void EmitLongConstant(long value) { if (value >= int.MinValue && value <= int.MaxValue) { .... } else if (value >= uint.MinValue && value <= uint.MaxValue) { .... } else { .... } }


理解を深めるために、このコードを書き直して、定数の名前を実際の値に置き換えます。 



 internal void EmitLongConstant(long value) { if (value >= -2147483648 && value <= 2147483648) { .... } else if (value >= 0 && value <= 4294967295) { .... } else { .... } }


おそらく、本当の間違いはありませんが、状態は奇妙に見えます。 2番目の部分（ else if ）は、2147483648 + 1から4294967295の値の範囲に対してのみ実行されます。



これらの警告をさらにいくつか：





nullの等価性チェック（またはその欠如）の詳細



変数を使用した後にnullをチェックすることに関するいくつかのV3095エラー。 最初のコードはあいまいです。コードを検討してください。



V3095「displayName」オブジェクトは、nullと照合される前に使用されました。 行を確認してください：498、503。FusionAssemblyIdentity.cs 498 



 internal static IAssemblyName ToAssemblyNameObject(string displayName) { if (displayName.IndexOf('\0') >= 0) { return null; } Debug.Assert(displayName != null); .... }


displayName参照はnullであると想定されています。 これを行うには、 Debug.Assertを確認します 。 なぜ文字列を使用した後になるのかは明らかではありません。 また、デバッグ以外の他の構成の場合、コンパイラーはコードからDebug.Assertを削除することに注意してください。 これは、デバッグの場合にのみnull参照を取得できるということですか？ そうでない場合、なぜstring.IsNullOrEmpty（string）をチェックしなかったのか、など。 これらは、コードの作者に対する質問です。



次のエラーはより明白です。



V3095「scriptArgsOpt」オブジェクトは、nullに対して検証される前に使用されました。 行を確認してください：321、325。CommonCommandLineParser.cs 321 



 internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt.Add(arg); // <= continue; } if (scriptArgsOpt != null) { .... } .... } }


このコードには説明は必要ないと思います。 私は修正されたオプションを提供します： 



 internal void FlattenArgs(...., List<string> scriptArgsOpt, ....) { .... while (args.Count > 0) { .... if (parsingScriptArgs) { scriptArgsOpt?.Add(arg); continue; } if (scriptArgsOpt != null) { .... } .... } }


Roslynコードは、このようなエラーをさらに15個見つけました。





V3105のエラーを考慮してください。変数を初期化するときに条件付きnull演算子を使用し、以降のコードでは、変数はnullの等価性をチェックせずに使用されます。



次のエラーは、2つの警告によって直ちに通知されます。



V3105「documentId」変数は、ヌル条件演算子を介して割り当てられた後に使用されました。 NullReferenceExceptionは可能です。 CodeLensReferencesService.cs 138



V3105「documentId」変数は、null条件演算子によって割り当てられた後に使用されました。 NullReferenceExceptionは可能です。 CodeLensReferencesService.cs 139 



 private static async Task<ReferenceLocationDescriptor> GetDescriptorOfEnclosingSymbolAsync(....) { .... var documentId = solution.GetDocument(location.SourceTree)?.Id; return new ReferenceLocationDescriptor( .... documentId.ProjectId.Id, documentId.Id, ....); }


documentId変数はnullに初期化できます。その結果、ReferenceLocationDescriptorを作成すると、例外がスローされます。コードを修正する必要があります。 



 return new ReferenceLocationDescriptor( .... documentId?.ProjectId.Id, documentId?.Id, ....);


また、コード内でさらに、コンストラクターに渡されるヌル変数が同等になる可能性を提供する必要があります。



コード内の他の同様のエラー：





優先順位とブラケット



V3123おそらく、「?:」演算子は予想とは異なる方法で動作します。その優先順位は、その状態の他のオペレーターの優先順位よりも低くなっています。 Edit.cs 70 



 public bool Equals(Edit<TNode> other) { return _kind == other._kind && (_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode) && (_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode); }


開発者が考えたように、戻りブロックの条件はまったく計算されません。最初の条件は_kind == other._kin d（したがって、この条件の後に行の折り返しが行われる）と想定され、その後、「？」演算子を持つ条件ブロックが順次計算されます。実際、最初の条件は_kind == other._kind &&（_oldNode == null）になります。これは、&&演算子の優先順位が「？」演算子よりも高いためです。エラーを修正するには、「？」演算子のすべての式を括弧で囲む必要があります。 



 return _kind == other._kind && ((_oldNode == null) ? other._oldNode == null : _oldNode.Equals(other._oldNode)) && ((_newNode == null) ? other._newNode == null : _newNode.Equals(other._newNode));


これで、見つかったエラーの説明を終了します。



結論



Roslynプロジェクトコードのサイズ（2,770,000行）に関して、私が検出できたエラーは非常に多くありましたが、これはごくわずかです。前の記事のAndreiのように、このプロジェクトの高品質を認識する準備もできています。



このような時折のコードチェックは、静的解析の方法論とは関係がなく、実際には何の利点ももたらさないことに注意してください。静的分析は、ケースからケースではなく、定期的に適用する必要があります。その後、多くのエラーが最も早い段階で修正されるため、それらを修正するコストは10分の1になります。このアイデアについては、この小さな記事で詳しく説明しています。詳しくはこの記事をご覧ください。



検討中のプロジェクトと他のプロジェクトの両方で、他のエラーを個別に検索できます。これを行うには、アナライザをダウンロードして試してください。











この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Sergey Khrenov。 Roslynソースコードのチェック。


More articles:


All Articles