Geekly articles weekly

我们对Avalonia UI争取更少平台的贡献

图2

本文是对使用静态分析仪PVS-Studio在Avalonia UI项目中发现的错误的回顾。 Avalonia UI是一个基于XAML的开源跨平台UI框架。 这是.NET历史上最具技术意义的项目之一，因为它使开发人员能够基于WPF系统创建跨平台接口。 我们希望该项目的作者会发现本文有助于修复某些错误，并具有说服力，可以将静态分析纳入其开发过程。

关于Avalonia UI


Avalonia UI（以前称为Perspex）允许开发人员创建可以在Windows，Linux和MacOS上运行的用户界面。 作为一项实验性功能，它还提供了对Android和iOS的支持。 Avalonia UI并不是其他包装器的包装器，例如Xamarin Forms，它包装Xamarin包装器，但直接访问本机API。 在观看其中一个演示视频时，我惊讶地发现您可以将控件输出到Debian控制台。 此外，由于使用了XAML标记语言，与其他UI构造函数相比，Avalonia UI提供了更多的设计和布局功能。

仅举几个例子， Avalonia UI用于AvalonStudio （用于C＃和C / C ++软件开发的跨平台IDE）和Core2D （2D图编辑器）。 Wasabi钱包 （比特币钱包）是使用Avalonia UI的商业软件的示例。

与创建跨平台应用程序时保留大量库的必要性进行斗争非常重要。 我们想要帮助Avalonia UI的作者，所以我下载了项目的源代码，并使用我们的分析器对其进行了检查。 我希望他们能够看到本文并提出建议的修复方法，甚至开始在他们的开发过程中定期使用静态分析。 借助开放源代码开发人员可用的PVS-Studio免费许可选项，可以轻松完成此操作。 定期使用静态分析有助于避免许多问题，并使错误检测和修复便宜得多。

分析结果


PVS-Studio诊断消息： V3001在'^'运算符的左侧和右侧有相同的子表达式' controlFlags '。 WindowImpl.cs 975TwitterClientMessageHandler.cs 52

private void UpdateWMStyles(Action change) { .... var style = (WindowStyles)GetWindowLong(....); .... style = style | controlledFlags ^ controlledFlags; .... }

为了增加一些象征意义，让我们从第一个C＃诊断开始。 分析器使用按位OR运算符检测到一个奇怪的表达式。 让我用数字解释一下：

表达

 1100 0011 | 1111 0000 ^ 1111 0000

相当于

 1100 0011 | 0000 0000

异或（“ ^”）的优先级高于按位或（“ |”）的优先级。 程序员可能并没有打算这样做。 可以通过将第一个表达式放在括号中来固定代码：

 private void UpdateWMStyles(Action change) { .... style = (style | controlledFlags) ^ controlledFlags; .... }

至于接下来的两个警告，我必须承认：这些都是误报。 您会看到，开发人员正在使用TransformToVisual方法的公共API。 在这种情况下， VisualRoot始终是visual的父元素。 检查警告时，我不明白这一点。 直到我写完这篇文章后，项目的一位作者才告诉我。 因此，以下建议的修复程序实际上旨在保护代码免受可能破坏此逻辑的修改，而不是实际的崩溃。

PVS-Studio诊断消息： V3080方法返回值可能为空取消引用。 考虑检查：TranslatePoint（...）。 VisualExtensions.cs 23

 public static Point PointToClient(this IVisual visual, PixelPoint point) { var rootPoint = visual.VisualRoot.PointToClient(point); return visual.VisualRoot.TranslatePoint(rootPoint, visual).Value; }

这种方法很小。 分析器认为取消对TranslatePoint调用返回的值的引用是不安全的。 让我们看一下这种方法：

 public static Point? TranslatePoint(this IVisual visual, Point point, IVisual relativeTo) { var transform = visual.TransformToVisual(relativeTo); if (transform.HasValue) { return point.Transform(transform.Value); } return null; }

确实，它可以返回null 。

该方法被调用六次：三遍检查返回值，另外三遍不检查，从而触发有关潜在取消引用的警告。 第一个是上面的一个，下面是另外两个：

  • V3080可能为空的取消引用。 考虑检查“ p”。 VisualExtensions.cs 35
  • V3080可能为空的取消引用。 考虑检查“ controlPoint”。 Scene.cs 176

我建议按照安全版本中使用的模式修复这些错误，即通过在PointToClient方法内部添加Nullable <Struct> .HasValue检查来解决这些错误：

 public static Point PointToClient(this IVisual visual, PixelPoint point) { var rootPoint = visual.VisualRoot.PointToClient(point); if (rootPoint.HasValue) return visual.VisualRoot.TranslatePoint(rootPoint, visual).Value; else throw ....; }

PVS-Studio诊断消息： V3080方法返回值可能为空取消引用。 考虑检查：TransformToVisual（...）。 ViewportManager.cs 381

此错误与上一个错误非常相似：

 private void OnEffectiveViewportChanged(TransformedBounds? bounds) { .... var transform = _owner.GetVisualRoot().TransformToVisual(_owner).Value; .... }

这是TransformToVisual方法的代码：

 public static Matrix? TransformToVisual(this IVisual from, IVisual to) { var common = from.FindCommonVisualAncestor(to); if (common != null) { .... } return null; }

顺便说一句， FindCommonVisualAncestor方法确实可以返回null作为引用类型的默认值：

 public static IVisual FindCommonVisualAncestor(this IVisual visual, IVisual target) { Contract.Requires<ArgumentNullException>(visual != null); return ....FirstOrDefault(); }

TransformToVisual方法被调用了九次，只有七个检查。 具有不安全取消引用的第一个调用是上面的调用，这是第二个：

V3080可能为空的取消引用。 考虑检查“转换”。 MouseDevice.cs 80

PVS-Studio诊断消息： V3022表达式始终为true。 可能应在此处使用“ &&”运算符。 NavigationDirection.cs 89

 public static bool IsDirectional(this NavigationDirection direction) { return direction > NavigationDirection.Previous || direction <= NavigationDirection.PageDown; }

这张支票很奇怪。 NavigationDirection枚举包含9种类型，最后一种是PageDown类型。 可能并非总是这样，或者这可以防止突然添加新的方向选项。 我认为，第一次检查就足够了。 无论如何，让我们由作者来决定。

PVS-Studio诊断消息： V3066传递给“ SelectionChangedEventArgs”构造函数的参数的可能错误顺序：“ removedSelectedItems”和“ addedSelectedItems”。 数据网格SelectedItemsCollection.cs 338

 internal SelectionChangedEventArgs GetSelectionChangedEventArgs() { .... return new SelectionChangedEventArgs (DataGrid.SelectionChangedEvent, removedSelectedItems, addedSelectedItems) { Source = OwningGrid }; }

分析器警告构造函数的第二个和第三个参数的顺序错误。 让我们看一下该构造函数：

 public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) : base(routedEvent) { AddedItems = addedItems; RemovedItems = removedItems; }

它使用两个IList类型的容器作为参数，这使得以错误的顺序编写它们变得非常容易。 在课程开始时的注释表明，这是从Microsoft借来并修改用于Avalonia的控件代码中的错误。 但是我仍然坚持要固定参数顺序，即使只是为了避免获得错误报告并浪费时间在自己的代码中查找错误。

此类型还有另外三个错误：

PVS-Studio诊断消息： V3066传递给“ SelectionChangedEventArgs”构造函数的参数的可能错误顺序：“已删除”和“已添加”。 AutoCompleteBox.cs 707

 OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added));

构造函数SelectionChangedEventArgs相同。

PVS-Studio诊断消息 V3066 ：
  • 传递给“ ItemsRepeaterElementIndexChangedEventArgs”构造函数的参数的可能错误顺序：“ oldIndex”和“ newIndex”。 ItemsRepeater.cs 532
  • 传递给“更新”方法的参数的可能错误顺序：“ oldIndex”和“ newIndex”。 ItemsRepeater.cs 536

在一个事件调用方法上有两个警告。

 internal void OnElementIndexChanged(IControl element, int oldIndex, int newIndex) { if (ElementIndexChanged != null) { if (_elementIndexChangedArgs == null) { _elementIndexChangedArgs = new ItemsRepeaterElementIndexChangedEventArgs(element, oldIndex, newIndex); } else { _elementIndexChangedArgs.Update(element, oldIndex, newIndex); } ..... } }

分析器注意到，在两个方法ItemsRepeaterElementIndexChangedEventArgs和Update中，参数oldIndex和newIndex的写入顺序不同：

 internal ItemsRepeaterElementIndexChangedEventArgs( IControl element, int newIndex, int oldIndex) { Element = element; NewIndex = newIndex; OldIndex = oldIndex; } internal void Update(IControl element, int newIndex, int oldIndex) { Element = element; NewIndex = newIndex; OldIndex = oldIndex; }

也许这段代码是由不同的程序员编写的，其中一个对过去更感兴趣，而另一个对未来更感兴趣：)

就像上一期一样，本期不需要立即修复。 尚未确定此代码是否确实有错误。

PVS-Studio诊断消息： V3004'then '语句等效于'else'语句。 数据网格排序描述.cs 235

 public override IOrderedEnumerable<object> ThenBy(IOrderedEnumerable<object> seq) { if (_descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } else { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } }

这是ThenBy方法的一个很好奇的实现。 继承自seq参数的IEnumerable接口包含方法ThenBy ，该方法显然打算通过以下方式使用：

 public override IOrderedEnumerable<object> ThenBy(IOrderedEnumerable<object> seq) { if (_descending) { return seq.ThenByDescending(o => GetValue(o), InternalComparer); } else { return seq.ThenBy(o => GetValue(o), InternalComparer); } }

PVS-Studio诊断消息： V3106可能的负索引值。 “索引”索引的值可能达到-1。 Animator.cs 68

 protected T InterpolationHandler(double animationTime, T neutralValue) { .... if (kvCount > 2) { if (animationTime <= 0.0) { .... } else if (animationTime >= 1.0) { .... } else { int index = FindClosestBeforeKeyFrame(animationTime); firstKeyframe = _convertedKeyframes[index]; } .... } .... }

分析器确保索引变量可以以值-1结尾。 为该变量分配了FindClosestBeforeKeyFrame方法返回的值，让我们看一下：

 private int FindClosestBeforeKeyFrame(double time) { for (int i = 0; i < _convertedKeyframes.Count; i++) if (_convertedKeyframes[i].Cue.CueValue > time) return i - 1; throw new Exception("Index time is out of keyframe time range."); }

如您所见，该循环包含一个条件，后跟一个return语句，该语句返回迭代器的先前值。 很难检查此条件是否为真，并且我无法确定CueValue将具有什么值，但是说明表明该值的取值范围是0.0到1.0。 但是我们仍然可以说一些关于时间的信息 ：它是传递给调用方法的animationTime变量，并且绝对大于零且小于一。 如果不是这样，则执行将遵循不同的分支。 如果将这些方法用于动画制作，则这种情况看起来很像一个不错的Heisenbug。 如果这种情况需要特殊处理，我建议检查FindClosestBeforeKeyFrame返回的值；如果不满足其他条件，则从循环中删除第一个元素。 我不知道这一切到底应该如何工作，所以我以第二种解决方案为例：

 private int FindClosestBeforeKeyFrame(double time) { for (int i = 1; i < _convertedKeyframes.Count; i++) if (_convertedKeyframes[i].Cue.CueValue > time) return i - 1; throw new Exception("Index time is out of keyframe time range."); }

PVS-Studio诊断消息： V3117未使用构造函数参数“ phones”。 Country.cs 25

 public Country(string name, string region, int population, int area, double density, double coast, double? migration, double? infantMorality, int gdp, double? literacy, double? phones, double? birth, double? death) { Name = name; Region = region; Population = population; Area = area; PopulationDensity = density; CoastLine = coast; NetMigration = migration; InfantMortality = infantMorality; GDP = gdp; LiteracyPercent = literacy; BirthRate = birth; DeathRate = death; }

这是一个很好的例子，说明静态分析比代码审查更好。 构造函数用13个参数调用，其中不使用其中一个。 实际上，Visual Studio也可以检测到它，但是只能借助第三级诊断程序（通常关闭）。 我们肯定在这里处理一个错误，因为该类还包含13个属性（每个参数一个），但是没有分配给Phones变量。 既然解决方法很明显，我就不会说出来。

PVS-Studio诊断消息： V3080可能为空的取消引用。 考虑检查“ tabItem”。 TabItemContainerGenerator.cs 22

 protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); tabItem.ParentTabControl = Owner; .... }

分析器认为对由CreateContainer方法返回的值的取消引用是不安全的。 让我们看一下这种方法：

 protected override IControl CreateContainer(object item) { var container = item as T; if (item == null) { return null; } else if (container != null) { return container; } else { .... return result; } }

即使通过一连串的五十种方法，PVS-Studio仍可以跟踪null的分配，但是无法确定执行是否会遵循该分支。 就此而言，我也不可能...调用在覆盖的方法和虚拟方法之间丢失，因此我只建议编写另一条检查以防万一：

 protected override IControl CreateContainer(object item) { var tabItem = (TabItem)base.CreateContainer(item); if(tabItem == null) return; tabItem.ParentTabControl = Owner; .... }

PVS-Studio诊断消息： V3142检测到无法访问的代码。 可能存在错误。 DevTools.xaml.cs 91

引用过多的代码来保持悬念是没有用的； 我马上就告诉您：此警告是误报。 分析器检测到该方法的调用引发了无条件异常：

 public static void Load(object obj) { throw new XamlLoadException($"No precompiled XAML found for {obj.GetType()}, make sure to specify x:Class and include your XAML file as AvaloniaResource"); }

调用此方法后，出现三十五（！）条有关无法到达代码的警告，太多了，不能忽略，因此我问一位开发人员，这是怎么回事。 他告诉我，他们使用了一种技术，您可以使用Mono.Cecil库将对一个方法的调用替换为对其他方法的调用 。 该库允许您替换IL代码中的调用。

我们的分析仪不支持该库，因此存在大量误报。 这意味着在检查Avalonia UI时应关闭此诊断。 感觉有些尴尬，但是我不得不承认是由我来做出此诊断的。但是，像其他任何工具一样，静态分析器需要一些微调。

例如，我们目前正在研究诊断不安全的类型转换。 它会在游戏项目中产生大约一千个误报，而游戏项目是在引擎方面进行类型检查的。

PVS-Studio诊断消息： V3009奇怪的是，此方法始终返回一个相同的“ true”值。 DataGridRows.cs 412

 internal bool ScrollSlotIntoView(int slot, bool scrolledHorizontally) { if (....) { .... if (DisplayData.FirstScrollingSlot < slot && DisplayData.LastScrollingSlot > slot) { return true; } else if (DisplayData.FirstScrollingSlot == slot && slot != -1) { .... return true; } .... } .... return true; }

该方法始终返回true 。 自首次编写以来，它的目的也许已经改变，但它看起来更像是个错误。 从课程开始时的评论来看，这是从Microsoft借来的另一个控件类。 如果您问我， DataGrid是最不稳定的控件之一，因此在不满足条件时确认滚动可能不是一个好主意。

结论


上面描述的一些错误是从WPF控件复制的代码所借来的，Avalonia UI的作者与它们无关。 但这对用户没有任何影响：崩溃或出现故障的界面都会给程序的整体质量留下不好的印象。

我提到了微调分析仪的必要性：由于静态分析算法背后的工作原理，误报是不可避免的。 那些熟悉暂停问题的人都知道，在处理一段代码与另一段代码时存在数学约束。 但是，在这种情况下，我们正在谈论禁用几乎一百零五种诊断。 因此，在静态分析的情况下，不会失去意义。 此外，该诊断程序还可以发出警告，指出真正的错误，但是在大量误报中很难注意到这些警告。

我必须提到Avalonia UI项目的卓越品质！ 我希望开发人员能够保持这种方式。 不幸的是，错误的数量不可避免地随着程序的大小而增加。 对CI \ CD系统进行明智的微调，并带有静态和动态分析，这是防止错误发生的一种方法。 如果您想简化大型项目的开发并减少调试时间，请下载并尝试使用 PVS-Studio！

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

More articles:


All Articles