补充PVS-Studio团队的程序员已经开始写一篇关于分析开源项目的文章，这已经成为一种传统。 这次，这样一个经过验证的项目将是用于UWP的Telerik UI。
PVS-Studio代码分析器
PVS-Studio是用于检测用C，C ++，C＃和Java编写的程序的源代码中的错误和潜在漏洞的工具。 在Windows，Linux和macOS上运行。
分析器提供了各种使用方案：
- 可以在开发机器上本地使用，并作为插件与Visual Studio或IntelliJ IDEA集成；
- 可以与SonarQube连续质量保证平台集成；
- 可独立使用，集成到装配系统中；
- 可以使用编译监视实用程序；
- 可以与Azure DevOps，Jenkins，TeamCity，Travis CI和类似系统集成；
- 等等。
审核项目
用于UWP的Telerik UI是用于通用Windows平台（UWP）的一组用户界面组件。 该项目的源代码可以
在Github上
找到 。 该集合包含20多个组件，可让您以图表的形式可视化数据，创建列表和表格，使用地图演示与位置关联的数据。
研究分析仪报告时引起关注的片段
PVS-Studio警告 ：
V3013 “ 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); } } }
分析器检测到执行相同操作的两种方法
OnMinValuePropertyChanged和
OnMaxValuePropertyChanged 。 我强烈怀疑该代码中已包含错误。 请注意，
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); }
在代码中从未使用过
RaiseMaximumPropertyChangedEvent方法，但是两次使用
RaiseMinimumPropertyChangedEvent方法。 而且，您知道
OnMaxValuePropertyChanged方法的性能引发了一些问题……我认为应该编写以下内容：
private static void OnMaxValuePropertyChanged( DependencyObject sender, DependencyPropertyChangedEventArgs args) { .... peer.RaiseMaximumPropertyChangedEvent((double)args.OldValue, (double)args.NewValue); .... }
但是即使这样，由于大量重复元素，代码看起来也不是很整洁。 很难理解，重复的行使程序员的注意力减弱，并且在这种情况下进行代码审查变得更加困难。 但是静态分析工具在检查此类代码方面做得非常出色（但这并不意味着您可以不进行重构，尤其是在不减少重复行数的情况下）。
从上面和下面的代码片段中，我们可以假定作者不反对复制粘贴。 但是，像我们所有人一样... :)
PVS-Studio 警告 ：
V3001在'||'的左侧和右侧有相同的子表达式'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); }
分析器检测到错误代码片段，其中“ ||”运算符的右侧和左侧 if
语句使用相同的子表达式。 第二个子表达式显然应该看起来有所不同。 也许代替第二个
RenderSize应该是
DesiredSize 。 否则根本不应该有第二个子表达式。 无论如何，这段代码需要修复。
PVS-Studio警告 ：
V3001在'||'的左侧和右侧有相同的子表达式'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; } .... }
在此，开发人员将在文本框字段中输入的文本写入变量。 然后，将字符串的第一个字符与相同的字符“-”进行两次比较，这是一个可疑的决定。 显然，此功能中的文本验证无法按原计划工作。
PVS-Studio警告 ：
V3001 “ &&”运算符的左侧和右侧有相同的子表达式“ 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)
如果
newValue包含任何值，则表达式
newValue.HasValue返回
true ，并且表达式
newValue！= Null表示相同。 分析器会注意这一点，而要做的是删除其中一个子表达式或将其替换为另一个子表达式（如果应检查其他内容），则由开发人员决定。
PVS-Studio警告 ：
V3125在验证是否为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(); } } }
如果变量
CurrentAttachedMenu最终为
null ，则对表达式
CurrentAttachedMenu.IsOpen求值将
引发异常。 乍一看，这似乎是一个简单的错字，并不意味着要与
null进行比较，而是进行反运算-'！='。 但是，
如果 CurrentAttachedMenu变量为
null ，则
if语句的条件下将发生异常。
在代码中，还有另外
37个相同的警告，其中一些警告似乎指示错误。 但是在一篇文章的框架内描述它们仍然太多，因此我将不加理会。
PVS-Studio警告 ：
V3019使用“ 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; } .... }
作者很有可能混淆了变量。
空不等式不是通过强制转换获得的链接来检查的，而是通过原始的（
dragDropElement ）来检查的。 最有可能应该
检查uiDragDropElement链接。 程序员还使用了
uiDragDropElement而不检查
null的事实也证实了这一推测。
PVS-Studio警告 ：
V3030定期检查。 在行139中已经验证了'！ShowIndicatorWhenNoData'条件。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; } .... }
分析器发现了一段代码，其中在两种情况下，重新检查了相同的
showIndicatorWhenNoData变量。 也许检查只是多余的，但是也有可能重复的子表达式之一应该完全不同。
PVS-Studio警告 ：
V3031过度检查可以简化。 “ ||” 运算符被相反的表达式包围。 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); } }
这段代码并不是形式上错误的。 分析器暗示在这种情况下某些代码冗余。 但是，值得记住的是，额外的代码有时是错误的结果，例如，当多次检查另一个变量而不是一个变量时。
您可以稍微减少这种情况并删除多余的代码。 例如，像这样：
internal class SelectedItemCollection : ObservableCollection<object> { .... private bool CanInsertItem(object item) { return this.suspendLevel == 0 && this.AllowSelect && (this.AllowMultipleSelect || this.Count == 0); } }
其他类似消息：
- V3031过度检查可以简化。 “ ||” 运算符被相反的表达式包围。 SelectedItemCollection.cs 93
- V3031过度检查可以简化。 “ ||” 运算符被相反的表达式包围。 StackVirtualizationStrategy.cs 49
- V3031过度检查可以简化。 “ ||” 运算符被相反的表达式'state == null'和'state！= null'包围。 LocalFieldDescriptionsProviderBase.cs 24
考虑分析器返回以下内容的另一段代码：
PVS-Studio警告 ：
- V3137分配了'leftMargin'变量，但在函数末尾未使用该变量。 DragDrop.cs 87
- V3137分配了“ topMargin”变量，但在函数末尾未使用该变量。 DragDrop.cs 88
internal static class DragDrop { .... double leftMargin = 0d; double topMargin = 0d; if (frameworkElementSource != null) { leftMargin = frameworkElementSource.Margin.Left;
分析器报告已为
leftMargin和
topMargin变量分配了值，但是此后直到方法结束才使用这些变量。 这里可能没有错误，但是这样的代码看起来可疑。 这可能是由于输入错误或重构失败。
在其他地方也发现了相同的问题：V3137分配了“ currentColumnLength”变量，但未在函数末尾使用。 WrapLayout.cs 824
PVS-Studio警告 ：
V3061参数“索引”始终在使用前在方法主体中重写。 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,
FindGroupAndItemIndex方法的
index参数在使用之前会被覆盖。 这很可能表明程序员有错误。
PVS-Studio警告 ：
V3083对事件“已完成”的不安全调用，可能会发生NullReferenceException。 请考虑在调用事件之前将事件分配给局部变量。 ActionBase.cs 32
internal abstract class ActionBase { .... protected virtual void OnCompleted() { this.IsCompleted = true; if (this.Completed != null) { this.Completed(this, EventArgs.Empty); } } }
程序员在此方法中允许对事件处理程序进行可能不安全的调用，这可能导致
NullReferenceException类型的异常。 如果在
null检查和事件处理程序的调用之间不保留此事件，则将引发异常。
代码中还有
49个类似的问题。 观看本文中的所有内容并不会很有趣，并且作者可以使用PVS-Studio轻松地单独找到它们，因此我们将继续探讨其他错误。
PVS-Studio警告 ：
V3145不安全地取消引用弱参考目标，请考虑检查info.Target。 在检查“ IsAlive”和访问“ Target”属性之间可能已被垃圾回收。 FadeAnimation.cs 84
public class RadFadeAnimation : RadAnimation { .... protected internal override void ApplyAnimationValues(PlayAnimationInfo info) { .... if (info.Target.Opacity != opacity)
当访问
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; } } .... }
通常，您深入研究此代码，可以在其中发现更多潜在问题。 让我们看一下也许是最有趣的一个-分析器向其发出警告的对象。 事实是，即使执行遵循
if语句的then-branch，也不能保证将返回非零引用。 不管有关强制类型转换的讨论如何，由于构造函数中对象的初始化，这里我们都考虑所有允许的事情。
这怎么可能？ 事实是，如果在
IsAlive检查和
Target调用之间执行垃圾回收，而
WeakReference引用的对象
属于该垃圾回收，则
this.target.Target将返回
null 。 也就是说，
IsAlive检查不能保证下次访问
目标时该对象仍然可用。
顺便说一句，情况是
返回null； 捕获另一个诊断规则：V3080可能为空的取消引用。 考虑检查“ info.Target”。 FadeAnimation.cs 84
代码中的类似问题又发生了几次：
- V3145不安全地取消引用WeakReference目标，请考虑检查目标。 在访问“目标”属性之前，该对象可能已被垃圾回收。 MoveXAnimation.cs 80
- V3145不安全地取消引用WeakReference目标，请考虑检查目标。 在访问“目标”属性之前，该对象可能已被垃圾回收。 MoveYAnimation.cs 80
- V3145不安全地取消引用WeakReference目标，请考虑检查info.Target。 在访问“目标”属性之前，该对象可能已被垃圾回收。 平面投影动画.cs 244
- V3145对WeakReference目标的不安全取消引用。 在检查“ IsAlive”和访问“ Target”属性之间可能已被垃圾回收。 WeakEventHandler.cs 109
让我们继续下一个示例。
警告PVS-Studio ：
V3066传递给'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);
要了解分析器发出的此警告的含义，请查看
NotifyCollectionChangedEventArgs构造函数
参数 ：
public NotifyCollectionChangedEventArgs( NotifyCollectionChangedAction action, object newItem, object oldItem, int index);
分析器警告表达式中
return new NotifyCollectionChangedEventArgs( action, oldItem, newItem, changeIndex);
交换了变量
oldItem和
newItem 。 在构造函数定义中，它们以不同顺序列出。 无论这是有意识地完成的，还是只能猜测。
PVS-Studio警告 ：
V3102通过循环内的常量索引可疑访问“ 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]))
在循环的每次迭代中，程序员都会比较
x [0]和
y [0]。 但是，该循环在此代码中没有意义，因为仅比较了第一个元素。 最有可能的是，这意味着比较数组的相应元素。 然后正确的代码将如下所示：
for (int i = 0; i < x.Length; i++) { if (!object.Equals(x[i], y[i])) { return false; } }
警告PVS-Studio ：
V3123也许“？：”操作符的工作方式与预期的不同。 其优先级低于其条件下其他运营商的优先级。 EditRowHostPanel.cs 35
protected override Size MeasureOverride(Size availableSize) { .... bool shouldUpdateRowHeight = editorLine == 0 || displayedElement == null ? false : displayedElement.ContainerType != typeof(DataGridGroupHeader); .... }
使用“？：”运算符时会出现警告。 它的优先级比
！=，||，==低。 因此，表达式可能无法按程序员的计划进行求值。 显然，在这种情况下，这是一个误报，并且代码可以正常工作。 但是阅读这样的代码非常困难，而且永远无法确定它是否被正确理解。 感觉就像是开发人员写的那样，没人能理解任何东西：)做到这一点的最佳方法是更具可读性-使用方括号或
if语句 。
PVS-Studio警告 ：
V3078重复调用'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)); } }
该错误与再次为
IOrderedEnumerable类型的集合调用
OrderBy有关。 在这里，集合首先按列排序，然后按行排序。 而且，在按行排序时，以前任何地方都不考虑按列排序。 为了不丢失按列排序并立即按几个条件对集合进行排序，请使用
ThenBy ：
this.MergeCellSelectionRegions(selectedItemsInView .OrderBy(c => c.Column.ItemInfo.LayoutInfo.Line) .ThenBy(c => c.RowItemInfo.LayoutInfo.Line));
PVS-Studio警告 ：
V3008为 '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;
对于分析器而言，似乎对
currentColumnLength变量分配了两次值似乎很可疑。 在分配之间不使用该变量。 无论条件如何，该变量最终将为零。 该代码不正确或多余。
PVS-Studio警告 ：
V3127找到两个相似的代码片段。 也许这是一个错字，应该使用'emptyIconContainer'变量而不是'filledIconContainer'RadRatingItem.cs 240
public class RadRatingItem : RadContentControl { .... protected override void OnApplyTemplate() { .... this.filledIconContainer = this.GetTemplateChild( "FilledIconContainer") as Border; if (this.filledIconContainer == null)
由于有错字，代码中出现了两个相同的条件。 从生成的异常判断，第二个条件应如下所示：
if (this.emptyIconContainer == null) { throw new MissingTemplatePartException( "EmptyIconContainer", typeof(Border)); }
PVS-Studio警告 ：
V3020循环内无条件的“中断”。 节点池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; } } }
分析器发现这里的
break语句不属于
if语句 。 不论
pair.IsFrozen的值如何，都将执行
break 。
因此 ，由于在
foreach中仅执行一次迭代。
到此结束我对警告的考虑。 为了使Telerik开发人员可以进行更彻底的代码分析并修复缺陷，我们准备为他们提供临时许可证。 此外，他们可以利用
免费提供给开源项目作者的
PVS-Studio选项的优势。
结论
尽管用于UWP的Telerik UI的开发人员做了出色的工作，但并非没有错别字，因为它通常会发生:)。 所有这些错误都可以通过静态分析仪轻松找到并纠正。 最重要的是
正确，定期使用分析仪。
如果您想与说英语的读者分享这篇文章，请使用以下链接：Ekaterina Nikiforova。
为UWP检查Telerik UI作为PVS-Studio的入门方法 。