orchard cms_扫描Orchard CMS的代码以查找错误

orchard cms

This article reviews the results of a second check of the Orchard project with the PVS-Studio static analyzer. Orchard is an open-source content manager system delivered as part of the ASP.NET Open Source Gallery under the non-profit Outercurve Foundation. Today's check is especially interesting because both the project and the analyzer have come a long way since the first check, and this time we'll be looking at new diagnostic messages and some nice bugs.

本文回顾了使用PVS-Studio静态分析器对Orchard项目进行第二次检查的结果。 Orchard是一个开源内容管理器系统，作为非营利组织Outercurve Foundation的ASP.NET Open Source Gallery的一部分提供。 今天的检查特别有趣，因为自第一次检查以来，项目和分析器都走了很长一段路，这次我们将查看新的诊断消息和一些不错的错误。

关于果园CMS (About Orchard CMS)

We checked Orchard three years ago. PVS-Studio's C# analyzer has greatly evolved since then: we have improved the data flow analysis, added interprocedural analysis and new diagnostics, and fixed a number of false positives. More than that, the second check revealed that the developers of Orchard had fixed all the bugs reported in the first article, which means we had achieved our goal, i.e. had helped them make their code better.

我们三年前检查过果园。 从那时起，PVS-Studio的C＃分析器有了很大的发展：我们改进了数据流分析，增加了过程间分析和新的诊断方法，并修复了一些误报。 不仅如此，第二次检查还显示Orchard的开发人员已修复了第一篇文章中报告的所有错误，这意味着我们已经实现了我们的目标，即帮助他们改善了代码。

I hope they will pay attention to this article as well and make the necessary fixes or, better still, adopt PVS-Studio for use on a regular basis. As a reminder, we provide open-source developers with a free license. By the way, there are other options that proprietary projects may enjoy as well.

我希望他们也注意这篇文章，并进行必要的修复，或者更好的是，定期采用PVS-Studio。 提醒一下，我们为开源开发人员提供了免费许可证。 顺便说一下，专有项目还有其他选择 。

Orchard's source code is available for download here. The complete project description is found here. If you don't have a PVS-Studio copy yet, you can download the trial version. I used PVS-Studio 7.05 Beta and will include some of its warnings in this article. I hope this review will convince you that PVS-Studio is a useful tool. Just keep in mind that it's meant to be used regularly.

Orchard的源代码可在此处下载。 完整的项目描述可在此处找到。 如果您还没有PVS-Studio副本，则可以下载试用版。 我使用了PVS-Studio 7.05 Beta，并将在本文中包含一些警告。 我希望这篇评论能使您相信PVS-Studio是有用的工具。 请记住，它应该定期使用。

分析结果 (Analysis results)

Here are some of the figures from the first check of Orchard so that you don't have to switch between the two articles for comparison.

以下是从Orchard的第一次检查中获得的一些数据，因此您不必在两篇文章之间进行切换即可进行比较。

During the previous check, «we did the analysis of all source code files (3739 items) with the .cs extension. In sum total there were 214,564 lines of code. The result of the check was 137 warnings. To be more precise, there were 39 first (high) level warnings. There were also 60 second (medium) level warnings.»

在上一次检查中，«我们对所有带有.cs扩展名的源代码文件(3739个项目)进行了分析。 总共有214,564行代码。 检查结果为137警告。 更准确地说，有39个第一(高级)警告。 也有60秒(中)级别的警告。»

The current version of Orchard is made up of 2,767 .cs files, i.e. it's about one thousand files smaller. The downsizing and renaming of the repository suggests that the developers have isolated the project's core (commit 966), which is 108,287 LOC long. The analyzer issued 153 warnings: 33 first-level and 70 second-level ones. We don't typically include third-level warnings, and I'm going to stick to the tradition.

Orchard的当前版本由2,767个.cs文件组成，即，它大约小了1,000个文件。 存储库的缩小和重命名表明开发人员已经隔离了项目的核心( 提交966 )，该核心的长度为108287 LOC。 分析仪发出了153条警告：33条第一级警告和70条第二级警告。 我们通常不包含三级警告，而我将坚持这一传统。

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3110 Possible infinite recursion inside 'TryValidateModel' method. PrefixedModuleUpdater.cs 48 V3110'TryValidateModel '方法内可能进行的无限递归。 PrefixedModuleUpdater.cs 48

public bool TryValidateModel(object model, string prefix)
{
  return TryValidateModel(model, Prefix(prefix));
}

Let's start off with an infinite recursion bug, as we did in the first article. This time the exact intentions of the developer aren't clear, but I noticed that the TryValidateModel method had an overloaded version with one parameter:

让我们从无限递归错误开始，就像我们在第一篇文章中所做的那样。 这次开发人员的确切意图尚不清楚，但我注意到TryValidateModel方法具有一个带有一个参数的重载版本：

public bool TryValidateModel(object model)
{
  return _updateModel.TryValidateModel(model);
}

I think that, just like in the case of the overloaded version, the developer intended to call the method through _updateModel. The compiler didn't notice the mistake; _updateModel is of type IUpdateModel, and the current class also implements this interface. Since the method doesn't include any check against StackOverflowException,it was probably never called, though I wouldn't count on that. If my assumption is correct, the fixed version should look like this:

我认为，就像重载版本一样，开发人员打算通过_updateModel调用该方法。 编译器没有注意到错误。 _updateModel的类型为IUpdateModel ，当前类也实现此接口。 由于该方法不包含对StackOverflowException的任何检查，因此可能从未调用过，尽管我不会指望该方法。 如果我的假设是正确的，则固定版本应如下所示：

public bool TryValidateModel(object model, string prefix)
{
  return _updateModel.TryValidateModel(model, Prefix(prefix));
}

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3008 The 'content' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 197, 190. DynamicCacheTagHelper.cs 197 V3008为 “内容”变量连续两次分配值。 也许这是一个错误。 检查行：197、190。DynamicCacheTagHelper.cs 197

public override async Task ProcessAsync(....)
{ 
  ....
  IHtmlContent content;
  ....
  try
  {
    content = await output.GetChildContentAsync();
  }
  finally
  {
    _cacheScopeManager.ExitScope();
  }
  content = await ProcessContentAsync(output, cacheContext);
  ....
}

The analyzer detected two assignments to the local variable content. GetChildContentAsync is a library method that is used too rarely for us to take the trouble to examine and annotate it. So, I'm afraid, neither we, nor the analyzer know anything about the method's return object and side effects. But we know for sure that assigning the return value to content makes no sense if it's not used further in the code. Perhaps it's just a redundant operation rather than a mistake. I can't say how exactly this should be fixed, so I leave it to the developers.

分析器检测到两个对局部变量内容的分配。 GetChildContentAsync是一种图书馆方法，对于我们来说，很少使用它来麻烦检查和注释它。 因此，恐怕我们和分析人员都不了解该方法的返回对象和副作用。 但是我们可以肯定的是，如果在代码中不再使用返回值，则将返回值分配给内容毫无意义。 也许这只是一个多余的操作，而不是一个错误。 我不能说应该如何确切地解决它，所以我将其留给开发人员。

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3080 Possible null dereference. Consider inspecting 'itemTag'. CoreShapes.cs 92 V3080可能为空的取消引用。 考虑检查“ itemTag”。 CoreShapes.cs 92

public async Task<IHtmlContent> List(....string ItemTag....)
{
  ....
  string itemTagName = null;
  if (ItemTag != "-")
  {
    itemTagName = string.IsNullOrEmpty(ItemTag) ? "li" : ItemTag;
  }
  var index = 0;
  foreach (var item in items)
  {
    var itemTag = String.IsNullOrEmpty(itemTagName) ? null : ....;
    ....
    itemTag.InnerHtml.AppendHtml(itemContent);
    listTag.InnerHtml.AppendHtml(itemTag);
    ++index;
  }
  return listTag;
}

The analyzer detected an unsafe dereference of itemTag. This snippet is a good example of how a static analysis tool is different from a human developer doing code review. The method has a parameter named ItemTag and a local variable named itemTag. No need to tell you it makes a huge difference to the compiler! These are two different, even though related, variables. The way they are related is through a third variable, itemTagName. Here's the sequence of steps leading to the possible exception: if the ItemTag argument is equal to "-", no value will be assigned to itemTagName, so it will remain a null reference, and if it's a null reference, then the local variable itemTag will turn into a null reference too. In my opinion, it's better to have an exception thrown following the string check.

分析器检测到对itemTag的不安全取消引用。 此代码段很好地说明了静态分析工具与进行代码审查的人工开发人员有何不同。 该方法有一个名为ItemTag参数，并命名为itemTag一个局部变量。 无需告诉您，它对编译器有很大的影响！ 这是两个不同的(尽管相关)变量。 它们的关联方式是通过第三个变量itemTagName。 这是导致可能的异常的步骤序列：如果ItemTag参数等于“-”，则没有将任何值分配给itemTagName ，因此它将保持为空引用，如果为空引用，则使用局部变量itemTag也会变成空引用。 我认为，最好在字符串检查之后引发异常。

public async Task<IHtmlContent> List(....string ItemTag....)
{
  ....
  string itemTagName = null;
  if (ItemTag != "-")
  {
    itemTagName = string.IsNullOrEmpty(ItemTag) ? "li" : ItemTag;
  }
  var index = 0;
  foreach (var item in items)
  {
    var itemTag = ....;
    if(String.IsNullOrEmpty(itemTag))
      throw ....
    ....
    itemTag.InnerHtml.AppendHtml(itemContent);
    listTag.InnerHtml.AppendHtml(itemTag);
    ++index;
  }
  return listTag;
}

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3095 The 'remoteClient' object was used before it was verified against null. Check lines: 49, 51. ImportRemoteInstanceController.cs 49 V3095在对null进行验证之前，已使用'remoteClient'对象。 检查行：49，51。ImportRemoteInstanceController.cs 49

public async Task<IActionResult> Import(ImportViewModel model)
{
  ....
  var remoteClient = remoteClientList.RemoteClients.FirstOrDefault(....);
  var apiKey = Encoding.UTF8.GetString(....(remoteClient.ProtectedApiKey));
  if (remoteClient == null || ....)
  {
    ....
  }
  ....
}

The analyzer detected a dereference of remoteClient followed by a null check a couple of lines later. This is indeed a potential NullReferenceException as the FirstOrDefault method may return a default value (which is null for reference types). I guess this snippet can be fixed by simply moving the check up so that is precedes the dereference operation:

分析器检测到对remoteClient的取消引用，随后在几行中执行空检查。 实际上，这确实是一个潜在的NullReferenceException，因为FirstOrDefault方法可能会返回一个默认值(对于引用类型，该值为null )。 我猜想可以通过简单地向上移动检查来修复此片段，以便在取消引用操作之前：

public async Task<IActionResult> Import(ImportViewModel model)
{
  ....
  var remoteClient = remoteClientList.RemoteClients.FirstOrDefault(....);
  if (remoteClient != null)
     var apiKey = UTF8.GetString(....remoteClient.ProtectedApiKey);
  else if (....)
  {
    ....
  }
  ....
}

Or perhaps it should be fixed by replacing FirstOrDefault with First and removing the check altogether.

或者也许应该通过将FirstOrDefault替换为First并完全取消检查来解决此问题。

PVS-Studio 7.05 Beta发出的警告： (Warnings by PVS-Studio 7.05 Beta:)

By now, we have annotated all of LINQ's orDefault methods. This information will be used by the new diagnostic we are working on: it detects cases where the values returned by these methods are dereferenced without a prior check. Each orDefault method has a counterpart that throws an exception if no matching element has been found. This exception will be more helpful in tracking down the problem than the abstract NullReferenceException.

到目前为止，我们已经注释了所有LINQ的orDefault方法。 此信息将由我们正在使用的新诊断程序使用：它检测在没有事先检查的情况下取消引用了这些方法返回的值的情况。 每个orDefault方法都有一个对应项，如果找不到匹配的元素，则该异常将引发异常。 与抽象的NullReferenceException相比，此异常将更有助于跟踪问题。

I can't but share the results I've got from this diagnostic on the Orchard project. There are 27 potentially dangerous spots. Here are some of them:

我不能不分享在Orchard项目中从该诊断程序获得的结果。 有27个潜在危险点。 这里是其中的一些：

ContentTypesAdminNodeNavigationBuilder.cs 71:

ContentTypesAdminNodeNavigationBuilder.cs 71：

var treeBuilder = treeNodeBuilders.Where(....).FirstOrDefault();
await treeBuilder.BuildNavigationAsync(childNode, builder, treeNodeBuilders);

ListPartDisplayDriver.cs 217:

ListPartDisplayDriver.cs 217：

var contentTypePartDefinition = ....Parts.FirstOrDefault(....);
return contentTypePartDefinition.Settings....;

ContentTypesAdminNodeNavigationBuilder.cs 113:

ContentTypesAdminNodeNavigationBuilder.cs 113：

var typeEntry = node.ContentTypes.Where(....).FirstOrDefault();
return AddPrefixToClasses(typeEntry.IconClass);

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3080 Possible null dereference of method return value. Consider inspecting: CreateScope(). SetupService.cs 136 V3080方法返回值的空null取消引用。 考虑检查：CreateScope()。 SetupService.cs 136

public async Task<string> SetupInternalAsync(SetupContext context)
{
  ....
  using (var shellContext = await ....)
  {
    await shellContext.CreateScope().UsingAsync(....);
  }
  ....
}

The analyzer mentioned a dereference of the value returned by the CreateScope method. CreateScope is a tiny method, so here's its complete implementation:

分析器提到了对CreateScope方法返回的值的取消引用。 CreateScope是一个微小的方法，因此这是其完整的实现：

public ShellScope CreateScope()
{
  if (_placeHolder)
  {
    return null;
  }
  var scope = new ShellScope(this);
  // A new scope can be only used on a non released shell.
  if (!released)
  {
    return scope;
  }
  scope.Dispose();
  return null;
}

As you can see, there are two cases where it can return null. The analyzer doesn't know which branch the flow of execution will follow, so it plays safe and reports the code as suspicious. If I were to write code like that, I'd write a null check right away.

如您所见，在两种情况下它可以返回null 。 分析器不知道执行流程将遵循哪个分支，因此它可以安全运行并将代码报告为可疑。 如果要编写这样的代码，我会立即编写一个空检查。

Perhaps my opinion is biased, but I believe that every asynchronous method should be protected from NullReferenceException as much as possible because debugging stuff like that is far from enjoyable.

也许我的看法是有偏见的，但我认为应该尽可能避免每个异步方法都受到NullReferenceException的影响 ，因为调试此类东西远非令人愉快。

In this particular case, the CreateScope method is called four times: two of those calls are accompanied by checks and the other two are not. The latter two calls (without checks) seem to be copy-paste clones (same class, same method, same way of dereferencing the result to call UsingAsync). The first of those two calls was shown above, and you may be sure the second one triggered the same warning:

在这种情况下， CreateScope方法被调用了四次：其中两个调用带有检查，而另外两个则没有。 后两个调用(无检查)似乎是复制粘贴克隆(相同的类，相同的方法，相同的解引用结果的方式以调用UsingAsync)。 上面显示了这两个调用中的第一个，您可以确定第二个调用触发了相同的警告：

V3080 Possible null dereference of method return value. Consider inspecting: CreateScope(). SetupService.cs 192 V3080方法返回值的空null取消引用。 考虑检查：CreateScope()。 SetupService.cs 192

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3127 Two similar code fragments were found. Perhaps, this is a typo and 'AccessTokenSecret' variable should be used instead of 'ConsumerSecret' TwitterClientMessageHandler.cs 52 V3127找到两个相似的代码片段。 也许这是一个错字，应该使用“ AccessTokenSecret”变量代替“ ConsumerSecret” TwitterClientMessageHandler.cs 52

public async Task ConfigureOAuthAsync(HttpRequestMessage request)
{
  ....
  if (!string.IsNullOrWhiteSpace(settings.ConsumerSecret))
    settings.ConsumerSecret = 
      protrector.Unprotect(settings.ConsumerSecret);
  if (!string.IsNullOrWhiteSpace(settings.ConsumerSecret))
    settings.AccessTokenSecret = 
      protrector.Unprotect(settings.AccessTokenSecret);
  ....
}

That's a classic copy-paste mistake. ConsumerSecret was checked twice, while AccessTokenSecret was not checked at all. Obviously, this is fixed as follows:

这是一个经典的复制粘贴错误。 ConsumerSecret进行了两次检查，而AccessTokenSecret根本没有进行检查。 显然，此问题固定如下：

public async Task ConfigureOAuthAsync(HttpRequestMessage request)
{
  ....
  if (!string.IsNullOrWhiteSpace(settings.ConsumerSecret))
    settings.ConsumerSecret = 
      protrector.Unprotect(settings.ConsumerSecret);
  if (!string.IsNullOrWhiteSpace(settings.AccessTokenSecret))
    settings.AccessTokenSecret =
      protrector.Unprotect(settings.AccessTokenSecret);
  ....
}

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3139 Two or more case-branches perform the same actions. SerialDocumentExecuter.cs 23 V3139两个或更多案例分支执行相同的操作。 SerialDocumentExecuter.cs 23

Another copy-paste bug. For clarity, here's the complete class implementation (it's small).

另一个复制粘贴错误。 为了清楚起见，这是完整的类实现(很小)。

public class SerialDocumentExecuter : DocumentExecuter
{
  private static IExecutionStrategy ParallelExecutionStrategy 
    = new ParallelExecutionStrategy();
  private static IExecutionStrategy SerialExecutionStrategy
    = new SerialExecutionStrategy();
  private static IExecutionStrategy SubscriptionExecutionStrategy
    = new SubscriptionExecutionStrategy();

  protected override IExecutionStrategy SelectExecutionStrategy(....)
  {
    switch (context.Operation.OperationType)
    {
      case OperationType.Query:
        return SerialExecutionStrategy;

      case OperationType.Mutation:
        return SerialExecutionStrategy;

      case OperationType.Subscription:
        return SubscriptionExecutionStrategy;

      default:
        throw ....;
    }
  }
}

The analyzer didn't like the two identical case branches. Indeed, the class has three entities, while the switch statement returns only two of them. If this behavior is intended and the third entity isn't actually meant to be used, the code can be improved by removing the third branch having merged the two of them as follows:

分析仪不喜欢两个相同的案例分支。 实际上，该类具有三个实体，而switch语句仅返回其中两个。 如果有此行为，而实际上并不打算使用第三个实体，则可以通过删除合并了两个实体的第三个分支来改进代码，如下所示：

switch (context.Operation.OperationType)
{
  case OperationType.Query:
  case OperationType.Mutation:
    return SerialExecutionStrategy;

  case OperationType.Subscription:
    return SubscriptionExecutionStrategy;

  default:
    throw ....;
}

If this is a copy-paste bug, the first of the duplicate return fields should be fixed as follows:

如果这是一个复制粘贴错误，则应按以下步骤修复第一个重复的返回字段：

switch (context.Operation.OperationType)
{
  case OperationType.Query:
    return ParallelExecutionStrategy;

  case OperationType.Mutation:
    return SerialExecutionStrategy;

  case OperationType.Subscription:
    return SubscriptionExecutionStrategy;

  default:
    throw ....;
}

Or it should be the second case branch. I don't know the project's details and therefore can't determine the correlation between the names of the operation types and strategies.

或者它应该是第二种情况分支。 我不知道项目的详细信息，因此无法确定操作类型和策略的名称之间的相关性。

switch (context.Operation.OperationType)
{
  case OperationType.Query:
    return SerialExecutionStrategy; 

  case OperationType.Mutation:
    return ParallelExecutionStrategy;

  case OperationType.Subscription:
    return SubscriptionExecutionStrategy;

  default:
    throw ....;
}

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3080 Possible null dereference. Consider inspecting 'request'. GraphQLMiddleware.cs 148 V3080可能为空的取消引用。 考虑检查“请求”。 GraphQLMiddleware.cs 148

private async Task ExecuteAsync(HttpContext context....)
{
  ....
  GraphQLRequest request = null;
  ....
  if (HttpMethods.IsPost(context.Request.Method))
  {
    ....
  }
  else if (HttpMethods.IsGet(context.Request.Method))
  {
    ....
    request = new GraphQLRequest();
    ....
  }
  var queryToExecute = request.Query;
  ....
}

The request variable is assigned a value different from null several times in the first if block, but each time with nested conditions. Including all those conditions would make the example too long, so we'll just go with the first few ones, which check the type of the http method IsGet or IsPost. The Microsoft.AspNetCore.Http.HttpMethods class has nine static methods for checking the query type. Therefore, passing, for example, a Delete or Set query to the ExecuteAsync method would lead to raising a NullReferenceException. Even if such methods are currently not supported at all, it would still be wise to add an exception-throwing check. After all, the system requirements may change. Here's an example of such a check:

在第一个if块中多次为请求变量分配了一个不同于null的值，但每次都有嵌套条件。 包括所有这些条件会使示例变得太长，因此我们只介绍前几个条件，它们检查http方法IsGet或IsPost的类型。 Microsoft.AspNetCore.Http.HttpMethods类具有九种用于检查查询类型的静态方法。 因此，例如，将Delete或Set查询传递给ExecuteAsync方法将导致引发NullReferenceException 。 即使目前根本不支持这种方法，添加异常抛出检查仍然是明智的。 毕竟，系统要求可能会更改。 这是此类检查的示例：

private async Task ExecuteAsync(HttpContext context....)
{
  ....
  if (request == null)
    throw ....;
  var queryToExecute = request.Query;
  ....
}

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3080 Possible null dereference of method return value. Consider inspecting: Get<ContentPart>(...). ContentPartHandlerCoordinator.cs 190 V3080方法返回值的空null取消引用。 考虑检查：Get <ContentPart>(...)。 ContentPartHandlerCoordinator.cs 190

Most of the V3080 warnings are more convenient to view within the development environment because you need the method navigation, type highlighting, and the friendly atmosphere of the IDE. I'm trying to reduce the text of examples as much as possible to keep them readable. But if I'm not doing it right or if you want to test your programming skill and figure it all out for yourself, I recommend checking out this diagnostic's result on any open-source project or just your own code.

大多数V3080警告在开发环境中更方便查看，因为您需要方法导航，键入突出显示和IDE的友好氛围。 我试图尽量减少示例文本，以使它们易于阅读。 但是，如果我做得不好，或者您想测试自己的编程技能并自己解决所有问题，我建议您在任何开源项目或您自己的代码中检查该诊断程序的结果。

public override async Task LoadingAsync(LoadContentContext context)
{
  ....
  context.ContentItem.Get<ContentPart>(typePartDefinition.Name)
                     .Weld(fieldName, fieldActivator.CreateInstance());
  ....
}

The analyzer reports this line. Let's take a look at the Get method:

分析仪报告此行。 让我们看一下Get方法：

public static TElement Get<TElement>(this ContentElement contentElement....)
        where TElement : ContentElement
{
    return (TElement)contentElement.Get(typeof(TElement), name);
}

It calls its overloaded version. Let's check it too:

它调用其重载版本。 我们也检查一下：

public static ContentElement Get(this ContentElement contentElement....)
{
  ....
  var elementData = contentElement.Data[name] as JObject;
  if (elementData == null)
  {
    return null;
  }
  ....
}

It turns out that if we get an entity of a type incompatible with JObject from Data using the name indexer, the Get method will return null. I don't know for sure how likely that is because these types are from the Newtonsoft.Json library, which I haven't worked much with. But the author of the code was suspecting that the sought element might not exist, so we should keep that in mind when accessing the result of the read operation as well. Personally, I'd have an exception thrown in the very first Get if we believe the node must be present, or add a check before the dereference if the node's non-existence doesn't change the overall logic (for example, we get a default value).

事实证明，如果使用名称索引器从Data中获得与JObject不兼容的类型的实体，则Get方法将返回null 。 我不确定这种可能性有多大，因为这些类型来自Newtonsoft.Json库，但我没有做太多的工作。 但是代码的编写者怀疑所寻找的元素可能不存在，因此在访问读取操作的结果时也应牢记这一点。 就个人而言，如果我们认为该节点必须存在，则会在第一个Get中引发异常，或者如果该节点的不存在不会更改整体逻辑，则在取消引用之前添加一个检查(例如，默认值)。

Solution 1:

解决方案1：

public static ContentElement Get(this ContentElement contentElement....)
{
  ....
  var elementData = contentElement.Data[name] as JObject;
  if (elementData == null)
  {
    throw....
  }
  ....
}

Solution 2:

解决方案2：

public override async Task LoadingAsync(LoadContentContext context)
{
  ....
  context.ContentItem.Get<ContentPart>(typePartDefinition.Name)
                     ?.Weld(fieldName, fieldActivator.CreateInstance());
  ....
}

PVS-Studio诊断消息： (PVS-Studio diagnostic message: )

V3080 Possible null dereference. Consider inspecting 'results'. ContentQueryOrchardRazorHelperExtensions.cs 19 V3080可能为空的取消引用。 考虑检查“结果”。 ContentQueryOrchardRazorHelperExtensions.cs 19

public static async Task<IEnumerable<ContentItem>> ContentQueryAsync(....)
{
  var results = await orchardHelper.QueryAsync(queryName, parameters);
  ....
  foreach (var result in results)
  {
    ....
  }
 ....
}

This is quite a simple example in comparison with the previous one. The analyzer suspects that the QueryAsync method might return a null reference. Here's the method's implementation:

与前一个示例相比，这是一个非常简单的示例。 分析器怀疑QueryAsync方法可能返回空引用。 这是该方法的实现：

public static async Task<IEnumerable> QueryAsync(....)
{
  ....
  var query = await queryManager.GetQueryAsync(queryName);
  if (query == null)
  {
    return null;
  }
  ....
}

Since GetQueryAsync is an interface method, you can't be sure about each implementation, especially if we consider that the project also includes the following version:

由于GetQueryAsync是接口方法，因此您不能确定每个实现，特别是如果我们认为该项目还包括以下版本：

public async Task<Query> GetQueryAsync(string name)
{
  var document = await GetDocumentAsync();
  if(document.Queries.TryGetValue(name, out var query))
  {
    return query;
  }
  return null;
}

The multiple calls to external functions and cache accesses make the analysis of GetDocumentAsync difficult, so let's just say that the check is needed — all the more so because the method is an asynchronous one.

对外部函数和缓存访问的多次调用使对GetDocumentAsync的分析变得困难，所以我们只说需要进行检查-更是如此，因为该方法是异步方法。

public static async Task<IEnumerable<ContentItem>> ContentQueryAsync(....)
{
  var results = await orchardHelper.QueryAsync(queryName, parameters);
  if(results == null)
    throw ....;
  ....
  foreach (var result in results)
  {
    ....
  }
 ....
}

结论 (Conclusion)

I can't but mention the high quality of Orchard's code! True, there were some other defects, which I didn't discuss here, but I showed you all of the most severe bugs. Of course, this is not to say that checking your source code once in three years is enough. You'll get the most out of static analysis if you use it regularly because this is the way you are guaranteed to catch and fix bugs at the earliest development stages, where bug fixing is cheapest and easiest.

我不得不提到Orchard的高质量代码！ 确实，还有一些其他缺陷，我在这里没有讨论，但是我向您展示了所有最严重的错误。 当然，这并不是说三年检查一次源代码就足够了。 如果定期使用静态分析，您将获得最大的收益，因为这样可以保证在最早的开发阶段就可以捕获并修复错误，在这种情况下，错误修复最便宜，最容易。

Even though one-time checks don't help much, I still encourage you to download PVS-Studio and try it on your project: who knows, maybe you'll find some interesting bugs too.

即使一次检查并没有多大帮助，我仍然鼓励您下载PVS-Studio并在您的项目上进行尝试：谁知道，也许您还会发现一些有趣的错误。

翻译自: https://habr.com/en/company/pvs-studio/blog/472360/

orchard cms