It has become a tradition for newly employed developers on the PVS-Studio team to start off by writing an article reviewing bugs found by the analyzer in some open-source project. Telerik UI for UWP is the project picked for today's review.

开始撰写PVS-Studio团队的开发人员的传统是，撰写一篇文章，回顾分析仪在某些开源项目中发现的错误，以此作为开篇。 用于UWP的Telerik UI是为今天的回顾而挑选的项目。

PVS-Studio代码分析器 (PVS-Studio code analyzer)

PVS-Studio is a tool for detecting bugs and potential vulnerabilities in the source code of programs written in C, C++, C#, and Java. The analyzer runs on Windows, Linux, and macOS.PVS-Studio是用于检测用C，C ++，C＃和Java编写的程序的源代码中的错误和潜在漏洞的工具。 该分析仪可在Windows，Linux和macOS上运行。

PVS-Studio can be run in a number of ways:

PVS-Studio可以通过多种方式运行：

• as a plugin for Visual Studio or IntelliJ IDEA locally on developers' individual computers;作为开发人员的本地计算机上Visual Studio或IntelliJ IDEA的插件；
• by integrating with SonarQube: the continuous code quality inspection platform;通过与SonarQube集成：连续代码质量检查平台；
• as a standalone application for integrating into a build system;作为集成到构建系统中的独立应用程序；
• by running in combination with a special compilation monitoring utility;通过与特殊的编译监视实用程序结合运行；
• by integrating with Azure DevOps, Jenkins, TeamCity, Travis CI, and other similar systems;通过与Azure DevOps，Jenkins，TeamCity，Travis CI和其他类似系统集成；
• etc.等等

正在分析的项目 (The project under analysis)

Telerik UI for UWP is a set of UI controls for the Universal Windows Platform (UWP). The project's source code is available at Github. The set includes over 20 components allowing users to visualize data in chart form, create lists and tables, and use a map to display content in a geographical context.

Telerik UI for UWP是通用Windows平台(UWP)的一组UI控件。 该项目的源代码可在Github上获得 。 该集合包括20多个组件，允许用户以图表形式可视化数据，创建列表和表格，以及使用地图在地理环境中显示内容。

分析器报告的有趣代码段 (Interesting code snippets reported by the analyzer)

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

V3013 It is odd that the body of 'OnMinValuePropertyChanged' function is fully equivalent to the body of 'OnMaxValuePropertyChanged' function. RadGauge.cs 446V3013奇怪的是，“ OnMinValuePropertyChanged”函数的主体与“ OnMaxValuePropertyChanged”函数的主体完全等效。 RadGauge.cs 446

private static void OnMinValuePropertyChanged(DependencyObject sender,DependencyPropertyChangedEventArgs args)
{double newVal = (double)args.NewValue;ValidateValue(newVal);RadGauge gauge = sender as RadGauge;if (gauge.panel != null){gauge.panel.UpdateOnMinMaxValueChange();} if(AutomationPeer.ListenerExists(AutomationEvents.PropertyChanged)){var peer = FrameworkElementAutomationPeer.FromElement(gauge) as RadGaugeAutomationPeer;if (peer != null){peer.RaiseMinimumPropertyChangedEvent((double)args.OldValue, (double)args.NewValue);}}
}private static void OnMaxValuePropertyChanged(DependencyObject sender, DependencyPropertyChangedEventArgs args)
{double newVal = (double)args.NewValue;ValidateValue(newVal);RadGauge gauge = sender as RadGauge;if (gauge.panel != null){gauge.panel.UpdateOnMinMaxValueChange();}if (AutomationPeer.ListenerExists(AutomationEvents.PropertyChanged)){var peer = FrameworkElementAutomationPeer.FromElement(gauge) as RadGaugeAutomationPeer;if (peer != null){peer.RaiseMinimumPropertyChangedEvent((double)args.OldValue, (double)args.NewValue);}}
}

Two methods, OnMinValuePropertyChanged and OnMaxValuePropertyChanged, perform the same actions. I strongly suspect there's a bug here. Note that both methods call the same method, RaiseMinimumPropertyChangedEvent, while the RadGaugeAutomationPeer class implements individual methods for «Minimum» and «Maximum»:

OnMinValuePropertyChanged和OnMaxValuePropertyChanged这两种方法执行相同的操作。 我强烈怀疑这里有一个错误。 请注意，这两个方法都调用相同的方法RaiseMinimumPropertyChangedEvent ，而RadGaugeAutomationPeer类为«Minimum»和«Maximum»实现单独的方法：

internal void RaiseMaximumPropertyChangedEvent(double oldValue, double newValue)
{this.RaisePropertyChangedEvent(RangeValuePatternIdentifiers.MaximumProperty, oldValue, newValue);
}internal void RaiseMinimumPropertyChangedEvent(double oldValue, double newValue)
{this.RaisePropertyChangedEvent(RangeValuePatternIdentifiers.MinimumProperty,oldValue,newValue);
}

The RaiseMinimumPropertyChangedEvent method is used twice, while the RaiseMaximumPropertyChangedEvent method is not used at all. This makes me doubt the OnMaxValuePropertyChanged method works well… I guess it was meant to look like this:

RaiseMinimumPropertyChangedEvent方法使用两次，而RaiseMaximumPropertyChangedEvent方法则完全不使用。 这使我怀疑OnMaxValuePropertyChanged方法是否运行良好……我想它看起来像这样：

private static void OnMaxValuePropertyChanged(DependencyObject sender,DependencyPropertyChangedEventArgs args)
{....peer.RaiseMaximumPropertyChangedEvent((double)args.OldValue,(double)args.NewValue);....
}

But even with this fix, the code doesn't look neat because of the numerous duplicate elements. It's difficult to read, and the similarly looking lines dull your attention, which makes code review a difficult job. Static analysis tools, on the contrary, can easily handle it (which doesn't mean you shouldn't refactor your code and especially eliminate duplicate lines).

但是即使进行了此修复，由于存在大量重复元素，因此代码看起来也不整洁。 很难阅读，外观相似的线条使您的注意力减弱，这使代码审查变得困难。 相反，静态分析工具可以轻松地处理它(这并不意味着您不应该重构代码，尤其是消除重复的行)。

Looking at this fragment and the next one, I suspect that the project authors indulge in copy-paste every now and then. Well, we all do… :)

查看此片段和下一个片段，我怀疑项目作者不时沉迷于复制粘贴。 好吧，我们都做... :)

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

There are identical sub-expressions 'element.RenderSize == emptySize' to the left and to the right of the '||' operator. TiltInteractionEffect.cs 181

在“ ||”的左侧和右侧有相同的子表达式“ element.RenderSize == emptySize” 操作员。 TiltInteractionEffect.cs 181

private static bool IsPointInElementBounds(FrameworkElement element, Point position)
{Size emptySize = new Size(0, 0);if (element.RenderSize == emptySize || element.RenderSize == emptySize){return false;}return new Rect(....).Contains(position);
}

Both operands of the '||' operator in the if statement's conditional expression are represented by identical subexpressions. Obviously, the second subexpression should be different. Maybe the second RenderSize was meant to be DesiredSize or maybe the second subexpression shouldn't be there at all. In any case, this code needs fixing.

“ ||”的两个操作数 if语句的条件表达式中的运算符由相同的子表达式表示。 显然，第二个子表达式应该不同。 也许第二个RenderSize应该是DesiredSize，或者第二个子表达式根本不应该存在。 无论如何，此代码都需要修复。

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

V3001 There are identical sub-expressions 'text[0] == '-'' to the left and to the right of the '||' operator. RadNumericBox.cs 1057V3001在'||'的左侧和右侧有相同的子表达式'text [0] =='-' 操作员。 RadNumericBox.cs 1057

private void ValidateText()
{string text = this.textBox.Text;....if (text.Length == 1 && (text[0] == '-' || text[0] == '-')){if (this.isNegative){this.isNegative = false;}else{this.SetText(string.Empty);}return;}....
}

The text entered into the textbox field is read into a variable. The string's first character is then compared twice with the character '-', which doesn't look right. Obviously, this function doesn't perform text validation as intended.

输入到文本框字段中的文本将读入变量。 然后将字符串的第一个字符与字符“-”进行两次比较，该字符看起来不太正确。 显然，此功能未按预期执行文本验证。

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

V3001 There are identical sub-expressions 'newValue.HasValue' to the left and to the right of the '&&' operator. DateTimePicker.cs 576V3001在'&&'运算符的左侧和右侧有相同的子表达式'newValue.HasValue'。 DateTimePicker.cs 576

private static void OnValueChanged(object sender, DependencyPropertyChangedEventArgs args)
{DateTimePicker picker = sender as DateTimePicker;var newValue = (DateTime?)args.NewValue;if (newValue.HasValue && newValue != null)                     // <=....
}

Both conditional expressions, newValue.HasValue and newValue != null, return true if newValue has a value. The analyzer points this out, but whether this bug should be fixed by removing one of the subexpressions or by replacing it with another one (in case there was something else to be checked) is something that only the authors of this code can figure out.

这两个条件表达式newValue.HasValue和newValue！= null ，如果newValue有值，则返回true 。 分析器指出了这一点，但是是应该通过删除其中一个子表达式还是将其替换为另一个(如果要检查其他内容)来解决此错误，只有该代码的作者才能确定。

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

V3125 The 'CurrentAttachedMenu' object was used after it was verified against null. Check lines: 98, 96. PopupService.cs 98V3125在验证是否为null之后使用了“ CurrentAttachedMenu”对象。 检查行：98，96。PopupService.cs 98

internal static class PopupService
{....private static void Overlay_PointerPressed(....){if (CurrentAttachedMenu == null ||!CurrentAttachedMenu.hitTestService.HitTest(e.GetCurrentPoint(CurrentAttachedMenu).Position).Any()){CurrentAttachedMenu.IsOpen = false;HideOverlay();}}
}

If the CurrentAttachedMenu variable happens to be equal to null, evaluating the CurrentAttachedMenu.IsOpen expression will result in raising an exception. It looks as if it's just a typo and the developers actually meant the opposite operation, '!=', rather than the null check, but if that is the case, the condition of the if statement will throw an exception if the CurrentAttachedMenu variable is equal to null.

如果CurrentAttachedMenu变量恰好等于null ，则评估CurrentAttachedMenu.IsOpen表达式将导致引发异常。 看起来好像只是一个错字，而开发人员实际上是在说相反的操作'！='，而不是null检查，但是如果是这种情况，如果CurrentAttachedMenu变量为，则if语句的条件将引发异常。等于null 。

There were

曾经有

37 (37 )

more warnings of this type, some of which apparently point to genuine bugs. But that's a bit too many warnings for one article, so I'll skip them.

更多此类警告，其中某些警告显然指向真正的错误。 但是，对于一篇文章来说，警告太多了，因此我将跳过它们。

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

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'dragDropElement', 'uiDragDropElement'. DragDrop.cs 91V3019可能在使用'as'关键字进行类型转换后将不正确的变量与null进行比较。 检查变量“ dragDropElement”，“ uiDragDropElement”。 DragDrop.cs 91

internal static void StartDrag(....)
{var dragDropElement = sender as IDragDropElement;....UIElement uiDragDropElement = dragDropElement as UIElement;....if (dragDropElement == null ||!dragDropElement.CanStartDrag(trigger, initializeContext)){return;}....
}

The programmer must have confused one variable for another. The null check is done on the source reference, dragDropElement, rather than the reference resulting from the cast, uiDragDropElement, which is the one that was actually meant to be checked. This assumption is supported by the fact that uiDragDropElement is used further without any null checks.

程序员必须将一个变量与另一个变量混淆。 空检查在源参考，dragDropElement，而不是从演员阵容，uiDragDropElement，这是实际上意味着要检查的一个产生的参考实现。 uiDragDropElement在没有任何空检查的情况下被进一步使用的事实支持了这种假设。

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

V3030 Recurring check. The '!showIndicatorWhenNoData' condition was already verified in line 139. RadDataBoundListBox.PullToRefresh.cs 141V3030定期检查。 '！showIndicatorWhenNoData'条件已在第139行中得到验证。RadDataBoundListBox.PullToRefresh.cs 141

internal void HandlePullToRefreshItemStateChanged(object item, ItemState state)
{....bool showIndicatorWhenNoData = this.ShowPullToRefreshWhenNoData;if (this.realizedItems.Count == 0 && !showIndicatorWhenNoData){if (state == ItemState.Recycled && !showIndicatorWhenNoData){this.StopPullToRefreshLoading(false);this.HidePullToRefreshIndicator();}return;}....
}

Two conditions check the same variable showIndicatorWhenNoData. The second check might be redundant, but it's also possible that one of the duplicate subexpressions should be entirely something else.

两个条件检查同一变量showIndicatorWhenNoData 。 第二次检查可能是多余的，但重复的子表达式之一也可能完全是其他东西。

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

V3031 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions. SelectedItemCollection.cs 77V3031过度检查可以简化。 “ ||” 运算符被相反的表达式包围。 SelectedItemCollection.cs 77

internal class SelectedItemCollection : ObservableCollection<object>
{....private bool CanInsertItem(object item){return this.suspendLevel == 0 && this.AllowSelect && ((!this.AllowMultipleSelect && this.Count == 0) || this.AllowMultipleSelect);}
}

Technically speaking, this snippet is correct; the analyzer just points out certain redundancy in the condition. But keep in mind that redundant code is often a sign of a programming mistake such as checking one variable more times than necessary instead of some other variable.

从技术上讲，此代码段是正确的； 分析仪仅指出情况下的某些冗余。 但是请记住，冗余代码通常是编程错误的标志，例如检查一个变量比检查其他变量多于必要次数。

The condition can be simplified a bit by removing unnecessary code as follows:

通过删除不必要的代码，可以稍微简化条件，如下所示：

internal class SelectedItemCollection : ObservableCollection<object>
{....private bool CanInsertItem(object item){return this.suspendLevel == 0 && this.AllowSelect &&(this.AllowMultipleSelect || this.Count == 0);}
}

Other similar warnings:

其他类似的警告：

• V3031 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions. SelectedItemCollection.cs 93V3031过度检查可以简化。 “ ||” 运算符被相反的表达式包围。 SelectedItemCollection.cs 93
• V3031 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions. StackVirtualizationStrategy.cs 49V3031过度检查可以简化。 “ ||” 运算符被相反的表达式包围。 StackVirtualizationStrategy.cs 49
• V3031 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 'state == null' and 'state != null'. LocalFieldDescriptionsProviderBase.cs 24V3031过度检查可以简化。 “ ||” 运算符被相反的表达式'state == null'和'state！= null'包围。 LocalFieldDescriptionsProviderBase.cs 24

Let's consider another piece of code, to which the analyzer issued the following:

让我们考虑分析器向其发送以下代码的另一段代码：

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

• V3137 The 'leftMargin' variable is assigned but is not used by the end of the function. DragDrop.cs 87V3137分配了'leftMargin'变量，但未在函数末尾使用。 DragDrop.cs 87
• V3137 The 'topMargin' variable is assigned but is not used by the end of the function. DragDrop.cs 88V3137分配了“ topMargin”变量，但在函数末尾未使用该变量。 DragDrop.cs 88

internal static class DragDrop
{....double leftMargin = 0d;double topMargin = 0d;if (frameworkElementSource != null){leftMargin = frameworkElementSource.Margin.Left;    // <= topMargin = frameworkElementSource.Margin.Top;      // <=}if (dragDropElement == null || !dragDropElement.CanStartDrag(trigger, initializeContext)){return;}var context = dragDropElement.DragStarting(trigger, initializeContext);if (context == null){return;}var startDragPosition = e.GetCurrentPoint(context.DragSurface.RootElement).Position;var relativeStartDragPosition = e.GetCurrentPoint(uiDragDropElement).Position;var dragPositionMode = DragDrop.GetDragPositionMode(uiDragDropElement);AddOperation(new DragDropOperation(context, dragDropElement, dragPositionMode, e.Pointer, startDragPosition, relativeStartDragPosition));
}

The variables leftMargin and topMargin areassigned some values but never used after that. It's not necessarily a bug, but code like that still looks suspicious. It could be a sign of a typo or bad refactoring.

变量leftMargin和topMargin区域分配了一些值，但此后从未使用过。 它不一定是错误，但类似的代码仍然令人怀疑。 这可能是拼写错误或重构错误的标志。

There was another warning of this type: V3137 The 'currentColumnLength' variable is assigned but is not used by the end of the function. WrapLayout.cs 824

此类型的另一个警告：V3137分配了“ currentColumnLength”变量，但未在函数末尾使用。 WrapLayout.cs 824

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

V3061 Parameter 'index' is always rewritten in method body before being used. DataEngine.cs 1443V3061参数'index'总是在使用前在方法主体中重写。 DataEngine.cs 1443

private static Tuple<Group, int> FindGroupAndItemIndex(.... int index, ....)
{if (exhaustiveSearch){....}else{var aggregateRowGroup = rowRootGroup;var rowGroupNames = valueProvider.GetRowGroupNames(item);foreach (var groupName in rowGroupNames){Group group;if (aggregateRowGroup.TryGetGroup(groupName, out group)){aggregateRowGroup = group;}}index = aggregateRowGroup.IndexOf(item,            // <=valueProvider.GetSortComparer());     return Tuple.Create(aggregateRowGroup, index);}
}

The index parameter of the FindGroupAndItemIndex method is overwritten before use. Most likely, this indicates a programmer error.

在使用前， FindGroupAndItemIndex方法的index参数将被覆盖。 这很可能表明程序员有错误。

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

Unsafe invocation of event 'Completed', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. ActionBase.cs 32

对事件'Completed'的不安全调用，可能会发生NullReferenceException。 请考虑在调用事件之前将事件分配给局部变量。 ActionBase.cs 32

internal abstract class ActionBase
{....protected virtual void OnCompleted(){this.IsCompleted = true;if (this.Completed != null){this.Completed(this, EventArgs.Empty);}}
}

The event handler is called in a potentially unsafe way, at the risk of raising a NullReferenceException. This will happen if the event has no subscribers left between the null check and the call of the event handler.

以可能不安全的方式调用事件处理程序，但有引发NullReferenceException的风险。 如果事件在null检查和事件处理程序的调用之间没有订阅者，则会发生这种情况。

The report points out

报告指出

49 (49 )

more problems of this type. They are not much interesting to discuss here, and after all, the project authors can easily find them with PVS-Studio on their own, so let's skip over to the next examples.

这种类型的更多问题。 在这里讨论它们并不是很有趣，毕竟，项目作者可以使用PVS-Studio轻松地找到它们，因此让我们跳到下一个示例。

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

V3145 Unsafe dereference of a WeakReference target, consider inspecting info.Target. The object could have been garbage collected between checking 'IsAlive' and accessing the 'Target' property. FadeAnimation.cs 84V3145不安全地取消引用WeakReference目标，请考虑检查info.Target。 在检查“ IsAlive”和访问“ Target”属性之间可能已被垃圾回收。 FadeAnimation.cs 84

public class RadFadeAnimation : RadAnimation
{....protected internal override voidApplyAnimationValues(PlayAnimationInfo info){....if (info.Target.Opacity != opacity) // <={info.Target.Opacity = opacity;}....}....
}

A NullReferenceException may be raised when addressing the info.Target.Opacity property. To better understand what the problem is about, we need to take a look at certain blocks of the PlayAnimationInfo class, particularly the Target property.

解决info.Target.Opacity属性时，可能会引发NullReferenceException 。 为了更好地了解问题所在，我们需要看一下PlayAnimationInfo类的某些块，尤其是Target属性。

public class PlayAnimationInfo
{....private WeakReference target;....public PlayAnimationInfo(Storyboard storyboard, RadAnimation animation, UIElement target){....this.target = new WeakReference(target);....}....public UIElement Target{get{if (this.target.IsAlive){return this.target.Target as UIElement;}return null;}}....
}

Actually, the deeper you dig into this code, the more potential issues you unearth. Let's take a look at the most interesting one – the one that triggered the warning. The problem is that even if execution follows the else branch of the if statement, it doesn't guarantee returning a non-null reference even if we're not taking the effects of type conversion into account (the object is initialized by the constructor).

实际上，您深入研究此代码，就会发现更多潜在问题。 让我们看一下最有趣的一个-触发警告的那个。 问题在于，即使执行遵循if语句的else分支，即使我们不考虑类型转换的影响(对象由构造函数初始化)，也不能保证返回非null引用。 。

How is that possible? You see, if the object referred to by WeakReference is garbage-collected between the IsAlive check and the call to Target, this.target.Target will return null. That is, the IsAlive check does not guarantee that the object will be still available the next time you call Target.

那怎么可能？ 您会看到，如果WeakReference引用的对象在IsAlive检查和对Target的调用之间被垃圾回收，则this.target.Target将返回null 。 也就是说， IsAlive检查不能保证对象在您下次调用Target时仍然可用。

By the way, the return null; issue is detected by another diagnostic: V3080 Possible null dereference. Consider inspecting 'info.Target'. FadeAnimation.cs 84

顺便说一句， 返回null； 另一个诊断程序检测到该问题：V3080可能为空的取消引用。 考虑检查“ info.Target”。 FadeAnimation.cs 84

There were a few more defects like that:

还有更多类似的缺陷：

• V3145 Unsafe dereference of a WeakReference target, consider inspecting target. The object could have been garbage collected before the 'Target' property was accessed. MoveXAnimation.cs 80V3145不安全地取消引用WeakReference目标，请考虑检查目标。 在访问“目标”属性之前，该对象可能已被垃圾回收。 MoveXAnimation.cs 80
• V3145 Unsafe dereference of a WeakReference target, consider inspecting target. The object could have been garbage collected before the 'Target' property was accessed. MoveYAnimation.cs 80V3145不安全地取消引用WeakReference目标，请考虑检查目标。 在访问“目标”属性之前，该对象可能已被垃圾回收。 MoveYAnimation.cs 80
• V3145 Unsafe dereference of a WeakReference target, consider inspecting info.Target. The object could have been garbage collected before the 'Target' property was accessed. PlaneProjectionAnimation.cs 244V3145不安全地取消引用WeakReference目标，请考虑检查info.Target。 在访问“目标”属性之前，该对象可能已被垃圾回收。 平面投影动画.cs 244
• V3145 Unsafe dereference of a WeakReference target. The object could have been garbage collected between checking 'IsAlive' and accessing the 'Target' property. WeakEventHandler.cs 109V3145不安全地取消引用WeakReference目标。 在检查“ IsAlive”和访问“ Target”属性之间可能已被垃圾回收。 WeakEventHandler.cs 109

Let's move on to the next example.

让我们继续下一个示例。

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

V3066 Possible incorrect order of arguments passed to 'NotifyCollectionChangedEventArgs' constructor: 'oldItem' and 'newItem'. CheckedItemsCollection.cs 470V3066传递给'NotifyCollectionChangedEventArgs'构造函数的参数的可能错误顺序：'oldItem'和'newItem'。 CheckedItemsCollection.cs 470

public class CheckedItemsCollection<T> : IList<T>,INotifyCollectionChanged
{....private NotifyCollectionChangedEventArgs GenerateArgs(....){switch (action){case NotifyCollectionChangedAction.Add:....case NotifyCollectionChangedAction.Remove:....case NotifyCollectionChangedAction.Replace:return new NotifyCollectionChangedEventArgs(action, oldItem, newItem, changeIndex);     // <=default:return new NotifyCollectionChangedEventArgs(action);}}
}

To figure out the meaning of this warning, we need to look at the NotifyCollectionChangedEventArgs constructor's parameters:

为了弄清楚此警告的含义，我们需要查看NotifyCollectionChangedEventArgs构造函数的参数：

public NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction action,object newItem,object oldItem,int index);

The analyzer tells us that the variables oldItem and newItem are swapped in the following expression:

分析器告诉我们，变量oldItem和newItem在以下表达式中交换：

return new NotifyCollectionChangedEventArgs(action,oldItem,newItem,changeIndex);

However, the constructor's implementation has those variables listed in the opposite order. You can only wonder whether or not this was done on purpose.

但是，构造函数的实现具有按相反顺序列出的那些变量。 您只能想知道是否故意这样做。

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

Suspicious access to element of 'x' object by a constant index inside a loop. DataEngine.cs 1718

循环内的常量索引可疑访问“ x”对象的元素。 DataEngine.cs 1718

private class ObjectArrayComparer : IEqualityComparer<object[]>
{public bool Equals(object[] x, object[] y){....for (int i = 0; i < x.Length; i++){if (!object.Equals(x[0], y[0]))    // <={return false;}}return true;}....
}

The elements x[0] and y[0] are compared at each loop iteration. But since only the first elements are compared, the loop doesn't make sense. The developers probably intended to compare the arrays' respective elements instead. In that case, the correct version would look like this:

在每次循环迭代中比较元素x [0]和y [0] 。 但是由于只比较了第一个元素，所以循环没有意义。 开发人员可能打算比较数组的各个元素。 在这种情况下，正确的版本应如下所示：

for (int i = 0; i < x.Length; i++)
{if (!object.Equals(x[i], y[i])){return false;}
}

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

V3123 Perhaps the '?:' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its condition. EditRowHostPanel.cs 35V3123也许'？：'运算符的工作方式与预期的不同。 在此条件下，它的优先级低于其他运营商的优先级。 EditRowHostPanel.cs 35

protected override Size MeasureOverride(Size availableSize)
{....bool shouldUpdateRowHeight = editorLine == 0 || displayedElement == null ? false :displayedElement.ContainerType != typeof(DataGridGroupHeader);....
}

This warning deals with the use of the '?:' operator. Its precedence is lower than that of !=, ||, and ==, which means the order of evaluating the expression above may be different from the expected one. This particular case seems to be a false positive, with the code actually working as intended. But code like that is very difficult to read, and you can never be sure if you understood it correctly. It looks as if it was written that way deliberately so that no one could figure it out :) The best way to make it easier to read is to use parentheses or an if statement.

此警告涉及使用'？：'运算符。 它的优先级低于！=，||和==的优先级，这意味着上述表达式的求值顺序可能与预期的不同。 这种特殊情况似乎是错误的肯定，因为代码实际上按预期工作。 但是，这样的代码很难阅读，因此无法确定您是否正确理解它。 看起来好像是故意用这种方式编写的，所以没有人能弄清楚：)使其更易于阅读的最佳方法是使用括号或if语句。

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

Original sorting order will be lost after repetitive call to 'OrderBy' method. Use 'ThenBy' method to preserve the original sorting. GridModel.Selection.cs 107

重复调用“ OrderBy”方法后，原始排序顺序将丢失。 使用“ ThenBy”方法保留原始排序。 GridModel.Selection.cs 107

internal partial class GridModel
{private void BuildCellSelectionRegions(....){....this.MergeCellSelectionRegions(selectedItemsInView.OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line).OrderBy(c => c.RowItemInfo.LayoutInfo.Line));}
}

This bug has to do with a recurring call of the OrderBy method on a collection of type IOrderedEnumerable. The collection is first sorted by columns and then by rows. The problem is that the result of the first sort – by columns – is not stored anywhere and it will be lost when the sort by rows starts. If you want to keep the result of the column-wise sort and do multiple-criteria sort, use the ThenBy method:

此错误与对IOrderedEnumerable类型的集合的OrderBy方法的反复调用有关。 集合首先按列排序，然后按行排序。 问题在于，第一个按列排序的结果没有存储在任何地方，当按行开始排序时，它将丢失。 如果要保留按列排序的结果并执行多条件排序，请使用ThenBy方法：

this.MergeCellSelectionRegions(selectedItemsInView.OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line).ThenBy(c => c.RowItemInfo.LayoutInfo.Line));

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

V3008 The 'currentColumnLength' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 791, 785. WrapLayout.cs 791V3008为 'currentColumnLength'变量连续两次分配了值。 也许这是一个错误。 检查行：791、785。WrapLayout.cs 791

private void OnAvailableLengthChanged(double oldValue, double newValue)
{....if (....){if (currentColumnLength > 0){var paddingValue = Math.Max(0, newValue - currentColumnLength);this.paddingRenderInfo.Add(paddingValue);currentColumnLength = 0;                     // <=slotCount++;}this.ColumnSlotsRenderInfo.Update(i, newValue);this.paddingRenderInfo.Add(0);currentColumnLength = 0;                       // <=slotCount++;continue;}else{....}....
}

The analyzer found it strange that the currentColumnLength variable is assigned a value twice while not being used anywhere between these two assignments. No matter the condition, the variable will eventually end up as null. This code is either faulty or redundant.

分析器发现奇怪的是， currentColumnLength变量被分配了两次值，而在这两个分配之间的任何地方都没有使用。 无论条件如何，该变量最终都会以null结束。 该代码错误或冗余。

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

Two similar code fragments were found. Perhaps, this is a typo and 'emptyIconContainer' variable should be used instead of 'filledIconContainer' RadRatingItem.cs 240

找到了两个类似的代码片段。 也许这是一个错字，应该使用'emptyIconContainer'变量而不是'filledIconContainer'RadRatingItem.cs 240

public class RadRatingItem : RadContentControl
{....protected override void OnApplyTemplate(){....this.filledIconContainer = this.GetTemplateChild("FilledIconContainer") as Border;if (this.filledIconContainer == null)                       // <={throw new MissingTemplatePartException("FilledIconContainer", typeof(Border));}this.emptyIconContainer = this.GetTemplateChild("EmptyIconContainer") as Border;if (this.filledIconContainer == null)                        // <={throw new MissingTemplatePartException("EmptyIconContainer", typeof(Border));}this.Initialize();}....
}

The two identical conditions above appeared as a result of a typo. The exception thrown by this code suggests that the second condition should look like this:

上面两个相同的条件是由于输入错误而出现的。 此代码引发的异常表明第二个条件应如下所示：

if (this.emptyIconContainer == null)
{throw new MissingTemplatePartException("EmptyIconContainer", typeof(Border));
}

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

V3020 An unconditional 'break' within a loop. NodePool.cs 189V3020循环内无条件的“中断”。 节点池189

public IEnumerable<KeyValuePair<int, List<T>>>GetUnfrozenDisplayedElements()
{foreach (var item in this.generatedContainers){foreach (var pair in item.Value){if (!pair.IsFrozen){yield return item;}break;}}
}

The break statement is not part of the if statement. It will execute no matter what value is stored in pair.IsFrozen, so the foreach loop will iterate only once.

break语句不是if语句的一部分。 无论对pair.IsFrozen中存储了什么值，它都将执行，因此foreach循环将仅迭代一次。

That's all for my review of bugs found in Telerik. We are ready to provide the developers with a free temporary license so that they can do a more thorough analysis and fix the defects. They can also make use of the free PVS-Studio licensing options available to open-source developers.

以上就是我对Telerik中发现的错误的回顾。 我们准备为开发人员提供免费的临时许可证，以便他们可以进行更彻底的分析并修复缺陷。 他们还可以使用开源开发人员可以使用的免费PVS-Studio许可选项 。

结论 (Conclusion)

Though the authors of Telerik UI for UWP have done a big job developing their project, they still let a number of typos creep in, as it typically happens with us :). All those bugs could have been easily caught and fixed using a static analyzer, but the crucial thing to remember about static analysis is that it should be used in the right way and on a regular basis.

尽管用于UWP的Telerik UI的作者在开发他们的项目方面做得很出色，但他们仍然让很多错别字悄悄出现，就像我们通常会遇到的那样:)。 使用静态分析器可以很容易地捕获并修复所有这些错误，但是要记住有关静态分析的关键是要以正确的方式定期使用它。

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