Geekly articles weekly

检查Roslyn的源代码

PVS-Studio VS罗斯林

有时，我们会返回到之前使用PVS-Studio测试过的项目，并撰写了有关该项目的文章。 这样做有两个原因。 首先，要了解我们的分析仪变得更好了。 其次，跟踪项目的作者是否关注我们的文章以及我们通常提供给他们的错误报告。 当然，没有我们的参与就可以纠正错误。 但是，只要我们的努力能使一个项目变得更好，那总是很好。 罗斯林也不例外。 之前有关该项目的评论文章可追溯到2015年12月23日。 考虑到我们的分析仪在这段时间内的发展历程，这已经是很长的时间了。 对于我们个人而言，罗斯林（C＃分析仪PVS-Studio的核心就是基于它的事实）也引起了人们的极大兴趣。 因此，我们对该项目的代码质量非常感兴趣。 我们将安排第二次检查，以发现新的和有趣的（但我们希望没什么大不了的）PVS-Studio在那里可以找到。

Roslyn（或.NET编译器平台）可能是我们许多读者熟悉的。 简而言之，它是一组用于Microsoft C＃和Visual Basic .NET语言代码分析的开源编译器和API。 该项目的源代码可在GitHub上找到 。

我不会对此平台进行详细说明，但我会向所有感兴趣的人推荐我的同事Sergey Vasiliev的文章“ 罗斯林简介。使用静态分析工具进行开发” 。 从本文中，您不仅可以了解Roslyn架构的功能，还可以了解我们如何使用该平台。

正如我前面提到的，自从我的同事Andrei Karpov撰写上一篇有关Roslyn检查“ PVS-Studio 6.00的新年发布：检查Roslyn ”的文章以来，已经过去了三年多。 在此期间，C＃PVS-Studio分析仪获得了许多新功能。 通常，Andrey的文章是一种“测试球”，因为C＃分析器仅在那时才添加到PVS-Studio中。 尽管如此，即便如此，罗斯林还是在一个无条件的高质量项目中设法找到了有趣的错误。 到目前为止，C＃代码分析器中发生了哪些变化，这可能允许进行更深入的分析？

在过去的时间里，分析仪核心和基础设施都得到了发展。 添加了对Visual Studio 2017和Roslyn 2.0的支持，以及与MSBuild的深度集成。 您可以在我的同事Pavel Yeremeyev的文章中了解有关我们与MSBuild集成的方法的更多信息，以及导致我们接受它的原因，“ 在PVS-Studio中对Visual Studio 2017和Roslyn 2.0的支持：有时使用现成的解决方案似乎并不容易一目了然 。“

现在，我们正在按照最初支持Visual Studio 2017的相同方案（即通过我们自己的工具集）积极地过渡到Roslyn 3.0，该工具集包含在PVS-Studio分发工具包中，带有空MSBuild.exe文件形式的“存根”。 尽管它看起来像个“拐杖”（由于库的可移植性很低，MSBuild API对于在第三批项目中重用不是很友好），但是这种方法已经帮助我们在Visual Studio 2017的生命周期中相对轻松地重现了多个Roslyn更新。现在，尽管有大量覆盖，但在升级到Visual Studio 2019的过程中仍然可以生存，并在具有旧版本MSBuild的系统上保持完全的向后兼容性和性能。

分析仪核心也进行了许多改进。 主要创新之一是全面的过程间分析，其中考虑了方法的输入和输出值，并根据这些参数考虑了执行分支和返回点的可达性。

跟踪方法内部参数的任务已经接近完成，同时保留了有关这些参数在那里发生的情况的自动注释（例如，潜在的危险取消引用）。 这将允许使用数据流机制进行任何诊断，以考虑将参数传递给方法时发生的危险情况。 以前，在分析此类危险场所时，不会生成警告，因为我们无法知道该方法的所有可能输入值。 现在我们可以检测出危险了，因为在调用此方法的所有地方，这些输入参数都将被考虑在内。

注意：您可以通过文章“ PVS-Studio代码分析器中用于查找错误和潜在漏洞的技术 ”来熟悉分析器的主要机制，例如数据流和其他机制。

PVS-Studio C＃中的过程间分析不受输入参数或深度的限制。 唯一的限制是类中的虚拟方法，这些类没有为继承而关闭并且没有递归递归（当我们在堆栈上看到对已计算方法的重复调用时，让我们停下来）。 而且，递归方法本身最终将在假设其自递归的返回值未知的情况下进行计算。

C＃分析器的另一个重大创新是可以取消引用潜在的空指针。 以前，如果确定在所有执行分支中变量的值将为null，则分析器会发誓可能会出现null引用异常。 当然，有时他会误会，因此以前将V3080诊断称为潜在的空引用。

现在，分析器会记住该变量在执行分支之一中可能为null（例如，在if中的特定条件下）。 如果他不经检查即可访问此变量，则将显示消息V3080，但重要性低于在所有分支中均看到null的重要性。 结合改进的过程间分析，这种机制可以发现很难检测到的错误。 一个例子是一长串方法调用，您不熟悉其中的最后一个方法调用，例如在某些情况下返回空值，但是您并未对此加以保护，因为您根本不知道该方法。 在这种情况下，分析仪仅在准确看到空值分配时发誓。 我们认为，这在质量上将我们的方法与C＃8.0的可空引用类型的创新区分开来，实际上，归结为每种方法中都设置了空检查。 我们提供了一种替代方法-仅在真正可能出现null的地方进行检查，而我们的分析仪现在可以查找这种情况。

因此，我们毫不拖延地进行“汇报”-分析罗斯林检查的结果。 首先，让我们看看由于上述创新而发现的错误。 总体而言，这次针对Roslyn代码发布了许多警告。 我认为这是由于该平台正在非常积极地进行开发的事实（当前的代码库大约为277万行代码，不包括空代码行），并且我们很长一段时间都没有对该项目进行分析。 但是，没有那么多关键错误，即它们是本文所关注的。 是的，Roslyn中有很多测试，像往常一样，我被排除在测试之外。

我将以中等严重性级别的V3080错误开始，在该错误中，分析仪检测到通过零链路的可能访问，但并非在所有可能的情况下（代码分支）。

可能的空取消引用-中

V3080可能为空的取消引用。 考虑检查“当前”。 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 。 实际的过程间分析的一个很好的例子。

现在考虑类似的情况，其中通过两个方法调用执行过程间分析。

V3080可能为空的取消引用。 考虑检查“目录”。 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（字符串，布尔）方法的结果，该方法的值可以为null 。 由于在ExpandFileNamePattern方法的主体中进一步使用了目录变量，而没有对空相等性进行初步检查，因此我们可以讨论分析器发出的警告是否合法。 这是一个潜在的不安全设计。

更准确地说，另一段错误为V3080的代码针对一行代码立即发出了两个错误。 在这里，不需要过程间分析。

V3080可能为空的取消引用。 考虑检查“ spanStartLocation”。 TestWorkspace.cs 574

V3080可能为空的取消引用。 考虑检查“ 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 。 如代码分析器所指出的那样，在代码的进一步部分中，这些变量是通过引用来访问的，而无需首先检查空相等性。

Roslyn代码包含很多这样的错误，超过100个。这些错误的模式通常是相同的。 有一些通用方法可能会返回null 。 此方法的结果在很多地方使用，有时通过数十个中间方法调用或其他检查使用。 重要的是要了解这些错误不是致命的，但它们可能会导致通过空链接进行访问。 而且要检测此类错误非常困难。 因此，在某些情况下，您应该考虑重构代码，如果该方法返回null，则将引发异常。 否则，您只能通过全面检查来保护代码，这很繁琐且不可靠。 当然，在每种情况下，都应根据项目的特征来做出决定。

注意事项 碰巧的是，目前没有任何情况（输入数据）中方法返回null并且没有实际错误。 但是，这样的代码仍然不可靠，因为当对代码进行更改时，所有内容都会更改。

为了用V3080结束本主题，让我们看一下通过置信度链接进行访问的可能性更高甚至是不可避免的情况下，高置信水平下的明显错误。

可能的空取消引用-高

V3080可能为空的取消引用。 考虑检查“ collectionType.Type”。 137第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) ....

顺便说一下，事件发展的第二种变体也是可能的：在第一部分中，条件由运算符==和！=混合在一起。 然后，更正后的代码将是：

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

此版本的代码逻辑性较差，但也可以纠正错误。 最终决定权取决于项目的作者。

另一个类似的错误。

V3080可能为空的取消引用。 考虑检查“动作”。 文字检视视窗_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变量（显然为null）访问action.DisplayText属性。

最后一个错误是V3080高电平。

V3080可能为空的取消引用。 考虑检查“类型”。 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 ，我们使用空类型引用。 您可以修复代码，例如：

 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'关键字可能丢失：抛出新的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警告。 有时这并不重要，或者根本没有在调用代码中检查返回代码。 我错过了这样的警告。 但是一些代码对我来说似乎很可疑。 我会引用其中之一：

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代替返回类型并对调用方法进行最少的更改。 这将使代码更易于理解和安全。

其他类似的警告：

  • V3009这种方法总是返回一个相同的'true'值，这很奇怪。 CommentUncommentSelectionCommandHandler.cs 86
  • V3009这种方法总是返回一个相同的'true'值，这很奇怪。 RenameTrackingTaggerProvider.RenameTrackingCommitter.cs 99
  • V3009这种方法总是返回一个相同的'true'值，这很奇怪。 第138章
  • V3009这种方法总是返回一个相同的'true'值，这很奇怪。 第164章
  • V3009奇怪的是，此方法总是返回一个相同的'false'值。 TriviaDataFactory.CodeShapeAnalyzer.cs 254
  • V3009这种方法总是返回一个相同的'true'值，这很奇怪。 物件清单.cs 173
  • V3009这种方法总是返回一个相同的'true'值，这很奇怪。 物件清单.cs 249

未检查

V3019可能在使用'as'关键字进行类型转换后将不正确的变量与null进行比较。 检查变量“值”，“ valueToSerialize”。 漫游VisualStudioProfileOptionPersister.cs 277

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

变量值为 NamingStylePreferences类型。 问题出在此检查之后。 即使value变量不为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 。 而是使用条件空语句检查变量columnState 。 此代码有什么危险？ 与前面的示例一样，使用as运算符的类型转换可能会失败，并且columnState2变量将为null ，这将进一步引发异常。 顺便说一句，错字可能是罪魁祸首。 注意if块中的条件。 也许他们想编写columnState2？.Name而不是columnState？.Name 。 考虑到非常不幸的变量名称columnState和columnState2，这很有可能。

冗余检查

对于非关键但与冗余检查相关联的潜在不安全构造，发出了大量警告（超过100个）。 例如，我将给他们之一。

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

也许这里没有真正的错误。 这是充分证明“过程间分析+数据流分析”技术相结合的充分理由。 分析器认为第二次检查navInfo == null是多余的。 实际上，在此之前，将从libraryService.NavInfoFactory.CreateForProject方法获得分配navInfo的值，该方法将构造并返回NavInfo类的新对象。 但无论如何都不为null 。 问题是，为什么分析器没有为第一次检查navInfo == null生成警告？ 对此有一个解释。 首先，如果符号变量结果为null ，则navInfo的值将保持为空引用。 其次，即使navInfo从libraryService.NavInfoFactory.CreateForSymbol方法接收到一个值，该值也可能为null 。 因此，首先需要对navInfo == null进行第一次检查。

没有足够的支票

现在情况与上述情况相反。 收到了一些V3042警告，提示可以通过空引用访问的代码。 而且，只有一两个小支票可以解决所有问题。

考虑包含两个此类错误的一段有趣的代码。

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块的条件下使用条件空运算符来访问接收方？.Syntax 。 在代码的进一步部分，不经任何检查即可使用变量接收器来访问receive.Type ， receiver.Syntax和receive.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构造函数支持为其参数获取空值，或执行其他重构。

其他类似错误：

  • V3042可能为NullReferenceException。 '？。' 和“。” 运算符用于访问“ containsType”对象的成员SyntaxGeneratorExtensions_Negate.cs 240
  • V3042可能为NullReferenceException。 '？。' 和“。” 349运算符用于访问“表达式”对象ExpressionSyntaxExtensions.cs的成员
  • V3042可能为NullReferenceException。 '？。' 和“。” 349运算符用于访问“表达式”对象ExpressionSyntaxExtensions.cs的成员

条件错误

V3057 “子字符串”功能可以接收“ -1”值，而预期为非负值。 检查第二个参数。 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; }); .... }

两个参数传递给lambda表达式：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）{...}否则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 { .... } }

可能没有真正的错误，但条件看起来很奇怪。 第二部分（ 否则为 ）将仅对2147483648 +1到4294967295范围内的值执行。

其中一些警告：

  • V3092在条件表达式中可能存在范围交点。 示例：if（A> 0 && A <5）{...}否则if（A> 3 && A <9）{...}。 本地重写器_Literal.cs 109
  • V3092在条件表达式中可能存在范围交点。 示例：if（A> 0 && A <5）{...}否则if（A> 3 && A <9）{...}。 LocalRewriter_Literal.cs 66

有关空相等检查（或缺少空检查）的更多信息

有关在使用变量后检查变量是否为null的一些V3095错误。 首先是模棱两可，考虑一下代码。

V3095在对null进行验证之前，已使用'displayName'对象。 检查行：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以外的其他配置，编译器将完全从代码中删除Debug.Assert 。 这是否意味着仅对于Debug，才可以获取空引用？ 如果不是这样，那么为什么不检查string.IsNullOrEmpty（字符串） ，例如。 这些是代码作者的问题。

以下错误更加明显。

V3095在验证是否为null之前，已使用'scriptArgsOpt'对象。 检查行：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个此类错误：

  • V3095在对null进行验证之前，已使用'LocalFunctions'对象。 检查行：289、317。ControlFlowGraphBuilder.RegionBuilder.cs 289
  • V3095在对null进行验证之前，已使用'resolution.OverloadResolutionResult'对象。 检查行：579、588。Binder_Invocation.cs 579
  • V3095在对null进行验证之前，已使用'resolution.MethodGroup'对象。 检查行：592、621。Binder_Invocation.cs 592
  • V3095在对null进行验证之前，已使用'touchedFilesLogger'对象。 检查行：111，126。CSharpCompiler.cs 111
  • V3095在对null进行验证之前，已使用'newExceptionRegionsOpt'对象。 检查行：736、743。AbstractEditAndContinueAnalyzer.cs 736
  • V3095在验证是否为null之前，已使用'symbol'对象。 检查行：422、427。AbstractGenerateConstructorService.Editor.cs 422
  • V3095在验证空值之前使用了'_state.BaseTypeOrInterfaceOpt'对象。 检查行：132，140。AbstractGenerateTypeService.GenerateNamedType.cs 132
  • V3095在对null进行验证之前，已使用'element'对象。 检查线：232，233。ProjectUtil.cs 232
  • V3095在验证是否为null之前，已使用“语言”对象。 检查行：22，28。ExportCodeCleanupProvider.cs 22
  • V3095在对null进行验证之前，已使用'memberType'对象。 检查行：183，184。SyntaxGeneratorExtensions_CreateGetHashCodeMethod.cs 183
  • V3095在对null进行验证之前，已使用'validTypeDeclarations'对象。 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

考虑V3105的错误。这里，我们在初始化变量时使用条件空运算符，以下在代码中使用变量时不检查空相等性。

下一个错误由两个警告立即发出信号。

V3105通过空条件运算符分配了'documentId'变量后，就使用了该变量。 NullReferenceException是可能的。 CodeLensReferencesService.cs 138

V3105通过空条件运算符分配了'documentId'变量后，就使用了该变量。 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, ....);

另外，在代码中，还必须提供传递给构造函数的空变量相等的可能性。

代码中的其他类似错误：

  • V3105通过空条件运算符分配了'symbol'变量后，即可使用。NullReferenceException是可能的。SymbolFinder_Hierarchy.cs 44
  • V3105通过空条件运算符分配了'symbol'变量后，即可使用。NullReferenceException是可能的。SymbolFinder_Hierarchy.cs 51

优先级和括号

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一样，我也准备承认这个项目的高质量。

我想指出的是，这种偶然的代码检查与静态分析的方法无关，实际上并没有带来任何好处。应当定期进行静态分析，而不是个案研究。然后，许多错误将在最早的阶段得到纠正，因此，修复它们的成本将降低十倍。在这篇小文章中将详细介绍这种想法，我请您熟悉一下。

您可以在所考虑的项目以及任何其他项目中独立搜索更多错误。为此，您只需下载并试用我们的分析仪。



如果您想与讲英语的读者分享这篇文章，请使用以下链接：Sergey Khrenov。 检查Roslyn源代码 。

Source: https://habr.com/ru/post/zh-CN446592/

More articles:


All Articles