Geekly articles weekly

检查Roslyn源代码

PVS-Studio VS罗斯林

有时，我们返回到先前使用PVS-Studio检查过的项目，这些结果在各篇文章中都有介绍。 有两个原因使这些卷土重来令我们兴奋。 首先，有机会评估我们的分析仪的进度。 其次，监视项目作者对我们的文章和错误报告的反馈，这些错误通常是我们提供给他们的。 当然，没有我们的参与就可以纠正错误。 但是，当我们的努力有助于使一个项目变得更好时，总是很高兴。 罗斯林也不例外。 关于此项目检查的上一篇文章可追溯到2015年12月23日。考虑到自那时以来我们的分析仪所取得的进展，这已经是相当长的时间了。 由于PVS-Studio分析仪的C＃核心基于Roslyn，它使我们对该项目产生了更多的兴趣。 因此，我们对这个项目的代码质量非常满意。 现在，让我们再次进行测试，找出PVS-Studio能够找到的一些新的有趣的问题（但希望没有什么大的问题）。

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

我将不对该平台进行详细描述，我建议所有感兴趣的读者阅读我的同事Sergey Vasiliev的文章“ Roslyn的介绍及其在程序开发中的用途 ”。 从本文中，您不仅可以了解罗斯林架构的功能，还可以了解我们如何使用该平台。

正如我之前提到的，距我的同事Andrey Karpov写了有关Roslyn支票的最新文章“ New Year PVS-Studio 6.00 Release：Scaning Roslyn ”已经3年多了。 从那时起，C＃PVS-Studio分析仪具有许多新功能。 实际上，Andrey的文章只是一个测试案例，因为那时C＃分析器才刚刚添加到PVS-Studio中。 尽管如此，我们还是设法检测出Roslyn项目中的错误，这肯定是高质量的。 那么，到目前为止，C＃代码分析器中发生了哪些变化，这将使我们能够进行更深入的分析？

从那时起，核心和基础设施都在发展。 我们增加了对Visual Studio 2017和Roslyn 2.0的支持，以及与MSBuild的深度集成。 我的同事Paul Eremeev的文章“ 在PVS-Studio中支持Visual Studio 2017和Roslyn 2.0：有时似乎不容易使用现成的解决方案 ”，描述了我们与MSBuild集成的方法以及做出此决定的原因。

目前，我们正在以与最初支持Visual Studio 2017相同的方式，积极向Roslyn 3.0迁移。它要求使用我们自己的工具集，该工具集包含在PVS-Studio发行版中，是一个“存根”，它是空的MSBuild .exe文件。 尽管它看起来像一个“拐杖”（由于库的可移植性很低，MSBuild API对于在第三方项目中重用不是很友好），但是这种方法已经帮助我们相对无缝地克服了Visual Studio方面的多个Roslyn更新2017年。到目前为止，它一直在帮助（即使有一些挑战）通过Visual Studio 2019更新，并为具有较旧MSBuild版本的系统保持完全的向后兼容性和性能。

分析仪核心也进行了许多改进。 主要功能之一是完整的过程间分析，其中要考虑输入和输出方法的值，评估（取决于这些参数）执行分支和返回点的可达性。

我们正在完成监视方法内部参数（例如，潜在的危险引用）以及保存其自动注释的任务。 对于使用数据流机制的诊断，这将考虑到在方法中传递参数时发生的危险情况。 在此之前，在分析此类危险场所时，不会生成警告，因为我们无法知道这种方法中所有可能的输入值。 现在我们可以检测到危险了，因为在所有调用此方法的地方，都将考虑这些输入参数。

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

PVS-Studio C＃中的过程间分析不受输入参数和深度的限制。 唯一的限制是类中的虚方法，可以进行继承以及进行递归（进入递归操作时，分析会因重复调用已求值的方法而停止，从而停止分析）。 这样，假定递归的返回值未知，则将最终评估递归方法本身。

C＃分析器中的另一个重要新功能已经考虑到可能对空指针的取消引用。 在此之前，分析器抱怨可能存在null引用异常，并确保在所有执行分支中变量值将为null。 当然，在某些情况下这是错误的，这就是V3080诊断程序以前曾调用潜在的空引用的原因。

现在，分析器已经意识到以下事实：在执行分支之一中该变量可能为null（例如，在特定的if条件下）。 如果发现未检查就访问了这样的变量，它将发出V3080警告，但它的确定性要低于所有分支中都为null的警告。 与改进的过程间分析一起，这种机制还允许找到很难检测到的错误。 这是一个示例-想象一长串方法调用，您可能不熟悉其中的最后一个。 在某些情况下，它在catch块中返回null，但是您并没有受到保护，因为您还不知道。 在这种情况下，分析器仅在准确看到空分配时才抱怨。 在我们看来，它在质量上将我们的方法与C＃8.0的诸如可为空的类型引用之类的功能区分开来，实际上，它仅限于为每个方法设置是否为null的检查。 但是，我们建议您采取另一种方法-仅在真正可能发生空值的地方执行检查，而我们的分析仪现在可以搜索这种情况。

因此，让我们不要将要点拖延太久，而要去责备一下-分析Roslyn检查的结果。 首先，让我们考虑由于上述功能而发现的错误。 总而言之，这次Roslyn代码有很多警告。 我认为这与该平台的发展非常积极有关（此时，代码库大约为277万行（不包括空白）），并且我们没有对此项目进行长时间的分析。 但是，没有那么多关键错误，而本文中最关注它们。 像往常一样，我从支票中排除了测试，罗斯林中有很多测试。

我将以中等确定性级别的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主体中的null进行初步检查-我们可以宣布分析器在发出警告时是正确的。 这是一种潜在的不安全构造。

为单行代码发布的另一个带有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具有可为null的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时可能会发生异常。 我将使用括号将其弄清楚（它们不会改变行为）：

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

根据操作优先级，代码将完全像这样工作。 如果类型变量为null ，我们将进行else-check，其中将使用类型 null引用，并检查变量targetTypeName是否为null 。 代码可能是固定的，例如，如下所示：

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

我认为，足够检查V3080错误。 现在是时候看看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 。 这样做，在调用代码中，某些检查涉及返回的值。

 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运算符的转换可能会失败，并且变量将为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从方法ibraryService.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块的条件下使用条件null运算符来访问接收者？ 。 此外，无需任何检查就可以使用接收器变量来访问receive.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; }

我们还必须确保构造函数支持为其参数获取空值，否则我们需要执行其他重构。

其他类似错误：

  • 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方法将引发异常。 这是必须解决的：

 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（string） 。 这是代码作者的问题。

以下错误更加明显。

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'对象。 检查行：223、228。SyntaxTreeExtensions.cs 223
  • V3095在验证是否为null之前，已使用“文本”对象。 检查行：376、385。MSBuildWorkspace.cs 376
  • V3095在对null进行验证之前，已使用'nameOrMemberAccessExpression'对象。 检查行：206，223。CSharpGenerateTypeService.cs 206
  • V3095在对null进行验证之前，已使用'simpleName'对象。 检查行：83，85。CSharpGenerateMethodService.cs 83
  • V3095在对null进行验证之前，已使用了'option'对象。 检查行：23，28。OptionKey.cs 23

让我们考虑一下V3105错误。 在此，在初始化变量时使用条件空运算符，但是在不检查null的情况下使用该变量。

两个警告指示以下错误：

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

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

可以使用null初始化documentId变量。 结果，创建对象ReferenceLocationDescriptor将导致引发异常。 该代码必须是固定的：

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

开发人员还应考虑传递给构造函数的变量为null的可能性。

代码中的其他类似错误：

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

到此结束我对发现的错误的描述。

结论

尽管我设法找到了很多错误，但就罗斯林项目代码的大小（2,770,000行）而言，它并不过分。 正如Andrey在上一篇文章中所写，我也准备承认该项目的高质量。

我想指出的是，这种偶尔的代码检查与静态分析的方法无关，并且几乎无济于事。 静态分析应定期进行，而不应逐案进行。 这样，许多错误将在最早的阶段得到纠正，因此修复这些错误的成本将减少十倍。 请在本小笔记中更详细地阐述这个想法，请检查一下。

您可以检查自己在该项目中还是在另一个项目中的一些错误。 为此，您只需下载并试用我们的分析仪。

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

More articles:


All Articles