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; }

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; }

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)); } } .... }

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

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

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

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}'."); } .... }

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

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

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

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

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

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

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

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

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

V3009 It's odd that this method always returns one and the same value of 'true'. CommentUncommentSelectionCommandHandler.cs 86

V3009 It's odd that this method always returns one and the same value of 'true'. RenameTrackingTaggerProvider.RenameTrackingCommitter.cs 99

V3009 It's odd that this method always returns one and the same value of 'true'. JsonRpcClient.cs 138

V3009 It's odd that this method always returns one and the same value of 'true'. AbstractFormatEngine.OperationApplier.cs 164

V3009 It's odd that this method always returns one and the same value of 'false'. TriviaDataFactory.CodeShapeAnalyzer.cs 254

V3009 It's odd that this method always returns one and the same value of 'true'. ObjectList.cs 173

V3009 It's odd that this method always returns one and the same value of 'true'. ObjectList.cs 249

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

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

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

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(....); }

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; }

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; }

V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'containingType' object SyntaxGeneratorExtensions_Negate.cs 240

V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'expression' object ExpressionSyntaxExtensions.cs 349

V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'expression' object ExpressionSyntaxExtensions.cs 349

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

if (colon > 0)

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; }); .... }

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

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

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 { .... } }

V3092 Range intersections are possible within conditional expressions. Example: if (A > 0 && A < 5) {… } else if (A > 3 && A < 9) {… }. LocalRewriter_Literal.cs 109

V3092 Range intersections are possible within conditional expressions. Example: if (A > 0 && A < 5) {… } else if (A > 3 && A < 9) {… }. LocalRewriter_Literal.cs 66

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

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) { .... } .... } }

V3095 The 'LocalFunctions' object was used before it was verified against null. Check lines: 289, 317. ControlFlowGraphBuilder.RegionBuilder.cs 289

V3095 The 'resolution.OverloadResolutionResult' object was used before it was verified against null. Check lines: 579, 588. Binder_Invocation.cs 579

V3095 The 'resolution.MethodGroup' object was used before it was verified against null. Check lines: 592, 621. Binder_Invocation.cs 592

V3095 The 'touchedFilesLogger' object was used before it was verified against null. Check lines: 111, 126. CSharpCompiler.cs 111

V3095 The 'newExceptionRegionsOpt' object was used before it was verified against null. Check lines: 736, 743. AbstractEditAndContinueAnalyzer.cs 736

V3095 The 'symbol' object was used before it was verified against null. Check lines: 422, 427. AbstractGenerateConstructorService.Editor.cs 422

V3095 The '_state.BaseTypeOrInterfaceOpt' object was used before it was verified against null. Check lines: 132, 140. AbstractGenerateTypeService.GenerateNamedType.cs 132

V3095 The 'element' object was used before it was verified against null. Check lines: 232, 233. ProjectUtil.cs 232

V3095 The 'languages' object was used before it was verified against null. Check lines: 22, 28. ExportCodeCleanupProvider.cs 22

V3095 The 'memberType' object was used before it was verified against null. Check lines: 183, 184. SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183

V3095 The 'validTypeDeclarations' object was used before it was verified against null. Check lines: 223, 228. SyntaxTreeExtensions.cs 223

V3095 The 'text' object was used before it was verified against null. Check lines: 376, 385. MSBuildWorkspace.cs 376

V3095 The 'nameOrMemberAccessExpression' object was used before it was verified against null. Check lines: 206, 223. CSharpGenerateTypeService.cs 206

V3095 The 'simpleName' object was used before it was verified against null. Check lines: 83, 85. CSharpGenerateMethodService.cs 83

V3095 The 'option' object was used before it was verified against null. Check lines: 23, 28. OptionKey.cs 23

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

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

V3105 The 'symbol' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. SymbolFinder_Hierarchy.cs 44

V3105 The 'symbol' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. SymbolFinder_Hierarchy.cs 51

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); }

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

Once in a while we go back to the projects that we have previously checked using PVS-Studio, which results in their descriptions in various articles. Two reasons make these comebacks exciting for us. Firstly, the opportunity to assess the progress of our analyzer. Secondly, monitoring the feedback of the project's authors to our article and the report of errors, which we usually provide them with. Of course, errors can be corrected without our participation. However, it is always nice when our efforts help to make a project better. Roslyn was no exception. The previous article about this project check dates back to December 23, 2015. It's quite a long time, in the view of the progress that our analyzer has made since that time. Since the C# core of the PVS-Studio analyzer is based on Roslyn, it gives us additional interest in this project. As a result, we're as keen as mustard about the code quality of this project. Now let's test it once again and find out some new and interesting issues (but let's hope that nothing significant) that PVS-Studio will be able to find.Many of our readers are likely to be well aware of Roslyn (or .NET Compiler Platform). In short, it is a set of open source compilers and an API for code analysis of C# and Visual Basic .NET languages from Microsoft. The source code of the project is available on GitHub I won't give a detailed description of this platform and I'll recommend checking out the article by my colleague Sergey Vasiliev " Introduction to Roslyn and its use in program development " to all interested readers. From this article, you can find out not only about the features of the Roslyn's architecture, but how exactly we use this platform.As I mentioned earlier, it has been more than 3 years since my colleague Andrey Karpov wrote the last article about the Roslyn check " New Year PVS-Studio 6.00 Release: Scanning Roslyn ". Since then the C# PVS-Studio analyzer had got many new features. Actually, Andrey's article was a test case, as at that time the C# analyzer was just added in PVS-Studio. Despite this, we managed to detect errors in the Roslyn project, which was certainly of high-quality. So what has changed in the analyzer for C# code by this moment that will allow us to perform a more in-depth analysis?Since then, both the core and the infrastructure have been developing. We added support for Visual Studio 2017 and Roslyn 2.0, and a deep integration with MSBuild. The article by my colleague Paul Eremeev " Support of Visual Studio 2017 and Roslyn 2.0 in PVS-Studio: sometimes it's not that easy to use ready-made solutions as it may seem " describes our approach to integration with MSBuild and reasons for this decision.At the moment we are actively working on moving to Roslyn 3.0 in the same way as we initially supported Visual Studio 2017. It requires usage of our own toolset, included in the PVS-Studio distributive as a «stub», which is an empty MSBuild.exe file. Despite the fact that it looks like a «crutch» (MSBuild API is not very friendly for reusing in third-party projects due to low libraries portability), such approach has already helped us to relatively seamlessly overcome multiple Roslyn updates in terms of Visual Studio 2017. Until now it was helping (even with some challenges) to pass through the Visual Studio 2019 update and maintain full backward compatibility and performance for systems with older MSBuild versions.The analyzer core has also undergone a number of improvements. One of the main features is a complete interprocedural analysis with consideration of input and output methods' values, evaluating (depending on these parameters) reachability of the execution branches and return points.We are on our way to completing the task of monitoring parameters inside the methods (for example, potentially dangerous dereferences) along with saving their auto-annotations. For a diagnostic that uses data-flow mechanism, this will allow taking into account dangerous situations, occurring when passing a parameter in a method. Before this, when analyzing such dangerous places, a warning wasn't generated, as we couldn't know about all possible input values in such a method. Now we can detect danger, as in all the places of calling this method, these input parameters will be taken into account.Note: you can read about basic analyzer mechanisms, such as data-flow and others in the article " Technologies used in the PVS-Studio code analyzer for finding bugs and potential vulnerabilities ".Interprocedural analysis in PVS-Studio C# is limited neither by input parameters, nor the depth. The only limitation is virtual methods in classes, open for inheritance, as well as getting into recursion (the analysis stops when it stumbles upon a repeated call of the already evaluated method). In doing so, the recursive method itself will be eventually evaluated assuming that the return value of its recursion is unknown.Another great new feature in the C# analyzer has become taking into account possible dereference of a potentially null pointer. Before that, the analyzer complained of a possible null reference exception, being ensured that in all execution branches the variable value will be null. Of course, it was wrong in some cases, that's why the V3080 diagnostic had been previously called potential null reference.Now the analyzer is aware of the fact that the variable could be null in one of the execution branches (for example, under a certaincondition). If it notices access to such a variable without a check, it will issue the V3080 warning, but at a lower level of certainty, than if it sees null in all branches. Along with the improved interprocedural analysis, such a mechanism allows finding errors that are very difficult to detect. Here is an example — imagine a long chain of method calls, the last of which is unfamiliar to you. Under certain circumstances, it returns null in theblock, but you haven't protected from this, as you simple haven't known. In this case, the analyzer only complains, when it exactly sees null assignment. In our view, it qualitatively distinguishes our approach from such feature of C# 8.0 as nullable type reference, which, in fact, confines to setting checks for null for each method. However, we suggest the alternative — to perform checks only in places where null can really occur, and our analyzer can now search for such cases.So, let's not delay the main point for too long and go to blame-storming — analyzing the results of the Roslyn check. First, let's consider the errors, found due to the features, described above. In sum, there were quite a lot of warnings for the Roslyn code this time. I think it is related to the fact that the platform is very actively evolving (at this point the codebase is about 2 770 000 lines excluding empty), and we haven't analyzed this project for long. Nevertheless, there are not so many critical errors, whereas they are of the most interest for the article. As usual, I excluded tests from the check, there are quite a lot of them in Roslyn.I will start with V3080 errors of the Medium level of certainty, in which the analyzer has detected a possible access by null reference, but not in all possible cases (code branches).V3080 Possible null dereference. Consider inspecting 'current'. CSharpSyntaxTreeFactoryService.PositionalSyntaxReference.cs 70Let's consider the method. The analyzer suggests the access by null reference is possible in the condition of theblockThe variable is assigned a value in the body of theblock, which is a result of themethod. In some cases, this value will be. A good example of interprocedural analysis in action.Now let's consider a similar case, in which the interprocedural analysis was carried out via two method calls.V3080 Possible null dereference. Consider inspecting 'directory'. CommonCommandLineParser.cs 911Thevariable in the body of themethod gets the value from the method. That, in turn, returns the result of the overloaded methodwhose value can be. Since the variableis used without a preliminary check for null in the body of the method— we can proclaim the analyzer correct about issuing the warning. This is a potentially unsafe construction.Another code fragment with the V3080 error, more precisely, with two errors, issued for a single line of code. The interprocedural analysis wasn't needed here.V3080 Possible null dereference. Consider inspecting 'spanStartLocation'. TestWorkspace.cs 574V3080 Possible null dereference. Consider inspecting 'spanEndLocationExclusive'. TestWorkspace.cs 574The variablesandare of thetype and are initialized by. Further along the code they can be assigned values, but only under certain conditions. In some cases, their value remains. After that, these variables are accessed by reference without preliminary check for null, which the analyzer indicates.The Roslyn code contains quite a lot of such errors, more than 100. Often the pattern of these errors is the same. There is some kind of a general method, which potentially returns. The result of this method is used in many places, sometimes through dozens of intermediate method calls or additional checks. It is important to understand that these errors are not fatal, but they can potentially lead to access by null reference. While detecting of such errors is quite challenging. That's why in some cases one should consider code refactoring, in which case ifreturns, the method will throw an exception. Otherwise, you can secure your code only with general checks which is quite tiring and sometimes unreliable. Anyway, it's clear that each specific case requires a solution based on project specifications.Note. It so happens, that at a given point there are no such cases (input data), when the method returnsand there is no actual error. However, such code is still not reliable, because everything can change when introducing some code changes.In order to drop the V3080 subject, let's look at obvious errors of High certainty level, when the access by null reference is the likeliest or even inevitable.V3080 Possible null dereference. Consider inspecting 'collectionType.Type'. AbstractConvertForToForEachCodeRefactoringProvider.cs 137Due to the typo in the condition (is used instead of the operator), the code works differently than intended and the access to thevariable will be executed when it is. The condition should be corrected as follows:By the way, things may unfold in another way: in the first part of the condition the operatorsandare messed upThen the correct code would look like this:This version of the code is less logical, but also corrects the error. The final solution rests for project's authors to decide.Another similar error.V3080 Possible null dereference. Consider inspecting 'action'. TextViewWindow_InProc.cs 372The error is made when generating the message for the exception. It is followed by the attempt to access theproperty via thevariable, which is known to beHere comes the last V3080 error of the High level.V3080 Possible null dereference. Consider inspecting 'type'. ObjectFormatterHelpers.cs 91The method is quite small, so I cite it entirely. The condition in theblock is incorrect. In some cases, when accessing, an exception may occur. I'll use parentheses to make it clear (they won't change the behavior):According to the operations precedence, the code will work exactly like this. In case if thevariable is, we'll fall in the else-check, where we'll use thenull reference, having checked the variablefor. Code might be fixed, for example, as follows:I think, it's enough for reviewing V3080 errors. Now it's high time to see other interesting stuff that the PVS-Studio analyzer managed to find. V3005 The 'SourceCodeKind' variable is assigned to itself. DynamicFileInfo.cs 17Due to failing variables naming, a typo was made in the constructor of theclass. Thefield is assigned its own value instead of using the parameter. To minimize the likelihood of such errors, we recommend that you use the underscore prefix to the parameter names in such cases. Here is an example of a corrected version of the code: V3006 The object was created but it is not being used. The 'throw' keyword could be missing: throw new InvalidOperationException(FOO). ProjectBuildManager.cs 61Under a certain condition, the destructor must throw an exception, but it's not happening while the exception object is simply created. Thekeyword was missed. Here is the corrected version of the code:The issue with destructors in C# and throwing exceptions from them is a topic for another discussion, which is beyond the scope of this article.Methods, which received the same value in all cases, triggered a certain number of V3009 warnings. In some cases it can be not critical or the return value is simply not checked in the calling code. I skipped such warnings. But a few code snippets seemed suspicious. Here is one of them:V3009 It's odd that this method always returns one and the same value of 'true'. GoToDefinitionCommandHandler.cs 62The methodreturns nothing but. In doing so, in the calling code the returned value is involved in some checks.It's hard to say exactly to what extent such behaviour is dangerous. But if the result is not needed, maybe the type of the return value should be changed to void and one should make small edits in the calling method. This will make the code more readable and secure.Similar analyzer warnings: V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'value', 'valueToSerialize'. RoamingVisualStudioProfileOptionPersister.cs 277Thevariable is cast to the type. The problem is in the check that follows this. Even if thevariable was non null, it doesn't guarantee that type casting was successful anddoesn't contain. Possible throwing of the exception. The code needs to be corrected as follows:Another similar bug:V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'columnState', 'columnState2'. StreamingFindUsagesPresenter.cs 181Thevariable is cast to the type. However, the operation result, which is the variableis not checked forfurther. Instead, thevariable is checked using the conditionaloperator. Why is this code dangerous? Just like in the previous example, casting with theoperator may fail and the variable will bewhich will result in an exception. By the way, a typo may be to blame here. Take a look at the condition in theblock.Perhaps, instead ofthe author wanted to write. It is very likely, considering rather faulty variable namesandQuite a large number of warnings (more than 100) were issued on noncritical, but potentially unsafe constructions related to redundant checks. For example, this is one of them. V3022 Expression 'navInfo == null' is always false. AbstractSyncClassViewCommandHandler.cs 101May be there is no actual bug here. It's just a good reason to demonstrate «interprocedural analysis + dataflow analysis» working in a tow. The analyzer suggests the second checkis redundant. Indeed, before it the value assigned towill be obtained from the method, which will construct and return a new object of theclass. No way it will return. Here the question arises, why didn't the analyzer issue a warning for the first check? There are some reasons. Firstly, if thevariable is, thevalue will remain a null reference as well. Secondly, even ifgets the value from the method, this value can also be. Thus, the first checkis really needed.Now the reverse situation from the discussed above. Several V3042 warnings were triggered for the code, in which access by null reference is possible. Even one or two small checks could have fixed everything.Let's consider another interesting code fragment, which has two such errors.V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'receiver' object Binder_Expressions.cs 7770V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'receiver' object Binder_Expressions.cs 7776Thevariable may be null. The author of the code knows about this, as he uses the conditionaloperator in the condition of theblock to access. Further thevariable is used without any checks to accessand. These errors must be corrected:We also have to be sure that the constructor supports gettingvalues for its parameters or we need to perform additional refactoring.Other similar errors: V3057 The 'Substring' function could receive the '-1' value while non-negative value is expected. Inspect the second argument. CommonCommandLineParser.cs 109In case if thevariable is 0, which is fine according to the condition in the code, themethod will throw an exception. This has to be fixed: V3065 Parameter 't2' is not utilized inside method's body. CSharpCodeGenerationHelpers.cs 84The lambda expression accepts two parameters: t1 and t2. However, only t1 is used. It looks suspicious, taking into account the fact how easy it is to make a mistake when using variables with such names. V3083 Unsafe invocation of event 'TagsChanged', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. PreviewUpdater.Tagger.cs 37Theevent is invoked in an unsafe way. Between checking forand invoking the event, someone may unsubscribe from it, then an exception will be thrown. Furthermore, other operations are performed in the body of theblock right before invoking the event. I called this error «Inadvertence», because this event is handled more carefully in other places, as follows:Usage of an additionalvariable prevents the problem. In the methodone has to make edits in order to safely handle the event. V3092 Range intersections are possible within conditional expressions. Example: if (A > 0 && A < 5) {… } else if (A > 3 && A < 9) {… }. ILBuilderEmit.cs 677For better understanding, let me rewrite this code, changing the names of the constants with their actual values:Probably, there is no real error, but the condition looks strange. Its second part () will be executed only for the range from 2147483648 + 1 to 4294967295.Another couple of similar warnings:A couple of V3095 errors on the check of a variable for null right after its usage. The first is ambiguous, let's consider the code.V3095 The 'displayName' object was used before it was verified against null. Check lines: 498, 503. FusionAssemblyIdentity.cs 498It is assumed that the referencecan be null. For this, the checkwas performed. It is not clear why it goes after using a string. It also has to be taken into account that for configurations different from Debug, the compiler will removeat allDoes it mean that getting a null reference is possible only for Debug? If it is not so, why did the author make the check of, for example. It is the question to authors of the code.The following error is more evident.V3095 The 'scriptArgsOpt' object was used before it was verified against null. Check lines: 321, 325. CommonCommandLineParser.cs 321I think this code doesn't need any explanations. Let me give you the fixed version:In Roslyn code, there were 15 more similar errors:Let's consider V3105 errors. Here the conditionaloperator is used when initializing the variable, but further the variable is used without checks forTwo warnings indicate the following error:V3105 The 'documentId' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. CodeLensReferencesService.cs 138V3105 The 'documentId' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. CodeLensReferencesService.cs 139Thevariable can be initialized by. As a result, creating an objectwill result in throwing an exception. The code must be fixed:Developers should also cover the possibility of variables, passed to a constructor, to beOther similar errors in the code: V3123 Perhaps the '?:' operator works in a different way than it was expected. Its priority is lower than the priority of other operators in its condition. Edit.cs 70The condition in the return block is evaluated not as the developer intended. It was assumed that the first condition will bed, (this is why after this condition there is a line break), and after that the blocks of conditions with the operator "" will be evaluated in sequence. In fact, the first condition is. This is due to the fact that the operatorhas a higher priority than operator "". To fix this, a developer should take all expressions of the operator "" in parentheses:That concludes my description of the errors found.Despite the large number of errors, which I managed to find, in terms of the size of the Roslyn project code (2 770 000 lines), it is not too much. As Andrey wrote in a previous article, I am also ready to acknowledge the high quality of this project.I'd like to note that such occasional code checks have nothing to do with the methodology of static analysis and are almost unhelpful. Static analysis should be applied regularly, and not on a case-by-case basis. This way, many errors will be corrected at the earliest stages, and hence the cost of fixing them will be ten times less. This idea is set forth in more detail in this little note , please, check it out.You can check yourself some errors both in this project and in another. To do this, you just need to download and try our analyzer.