# Небольшой вклад в борьбе Avalonia UI против зоопарка платформ

Эта статья – результат проверки проекта Avalonia UI с помощью статического анализатора PVS\-Studio\. Avalonia UI – это кроссплатформенная платформа пользовательского интерфейса на основе XAML, с открытым исходным кодом\. Это один из технологически значимых проектов в истории \.NET, так как позволяет создавать кроссплатформенные интерфейсы на основе WPF системы\. Надеюсь, эта статья поможет авторам исправить некоторые ошибки и убедит использовать статические анализаторы в будущем\.

![0701_avalonia_ru/image1.png](https://import.viva64.com/docx/blog/0701_avalonia_ru/image1.png)

## О проекте Avalonia UI

Проект Avalonia UI \(ранее назывался Perspex\) предоставляет возможность для создания пользовательских интерфейсов, работающих на Windows, Linux и macOS\. Также есть экспериментальная на данный момент поддержка Android и iOS\. Avalonia UI не является обёрткой над обёртками, а обращается к нативному API\. В отличие от Xamarin Forms, оборачивающего Xamarin обёртки\. В одном из демонстрационных видео меня поразила возможность вывести контрол в консоль Debian\. Кроме того, проект предлагает больше возможностей для вёрстки и дизайна, чем у обычных конструкторов интерфейсов, благодаря использованию XAML разметки\.

В качестве проектов, уже использующих Avalonia UI, можно привести [AvalonStudio](https://github.com/VitalElement/AvalonStudio) \(кроссплатформенная IDE для разработки на C\# и C/C\+\+\) и [Core2D](https://github.com/wieslawsoltes/Core2D) \(редактор 2D\-схем и диаграмм\)\. В качестве коммерческого проекта можно привести [Wasabi Wallet](https://wasabiwallet.io/) \(биткойн\-кошелек\)\.

Борьба с необходимостью иметь по несколько различных библиотек при создании кроссплатформенного приложения имеет огромную важность\. Нам хотелось помочь проекту, и я скачал проект и проверил нашим анализатором\. Надеюсь, что авторы обратят внимание на данную статью и внесут необходимые правки в код, а может и внедрят регулярный статический анализ в процесс разработки\. Для этого они могут воспользоваться вариантом бесплатного лицензирования PVS\-Studio для открытых проектов\. Регулярное использование статического анализатора помогает избежать множества проблем и сократит стоимость обнаружения и исправления многих ошибок\.

## Результаты проверки

**Предупреждение PVS\-Studio:** [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'controlledFlags' to the left and to the right of the '^' operator\. WindowImpl\.cs 975TwitterClientMessageHandler\.cs 52

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

Символично начну с нашей первой C\# диагностики\. Анализатор обнаружил странное применение оператора побитового ИЛИ\. Поясню на цифрах, что тут происходит: 

выражение

```cpp
1100 0011 | 1111 0000 ^ 1111 0000
```

аналогично вот такому:

```cpp
1100 0011 | 0000 0000
```

Исключающее ИЛИ \("^"\) имеет более высокий приоритет, чем побитовое ИЛИ \("\|"\)\. Скорее всего, тут подразумевался другой порядок операций\. В данном случае следует взять первое выражение в скобки:

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

Перед следующими двумя предупреждениями должен признаться: срабатывания ложные\. Это связано с использованием публичного API, метода _TransformToVisual_\. В нашем случае _VisualRoot_ всегда является родительским элементом для _visual_\. Я не понял этого при анализе срабатывания, мне сказал об этом автор проекта уже после написания статьи\. Так что предложенные в статье правки не для защиты от фактического падения, а от потенциальной правки, ломающей эту логику\.

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: TranslatePoint\(\.\.\.\)\. VisualExtensions\.cs 23

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

Небольшой метод\. Анализатор считает, что разыменование результата вызова TranslatePoint небезопасно\. Посмотрим на этот метод:

```cpp
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_\. 

У этого метода 6 вызовов\. В трёх случаях значение проверяется, а в остальных PVS\-Studio обнаруживает потенциальное разыменование и выдаёт предупреждения\. Первое я привёл выше, а другие два предупреждения находятся тут:

* [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'p'\. VisualExtensions\.cs 35
* [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'controlPoint'\. Scene\.cs 176

Исправлять предлагаю по аналогии, добавив внутри метода _PointToClient_ проверку _Nullable<Struct\>\.HasValue:_

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: TransformToVisual\(\.\.\.\)\. ViewportManager\.cs 381

Случай, очень похожий на предыдущий пример:

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

Метод _TransformToVisual_ выглядит так:

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

Кстати, метод _FindCommonVisualAncestor_ действительно может вернуть _null_, как значение по умолчанию для ссылочных типов:

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

Метод _TransformToVisual_ используется в девяти местах, проверки есть в семи\. Первое предупреждение на использование без проверки находится выше, а последнее здесь:

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'transform'\. MouseDevice\.cs 80

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression is always true\. Probably the '&&' operator should be used here\. NavigationDirection\.cs 89

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

Странная проверка\. В перечислении _NavigationDirection_ 9 типов и _PageDown_ является последним из них\. Возможно, так было не всегда, или это страховка от ВНЕЗАПНОГО появления новых вариантов направления\. Мне кажется, здесь достаточно первой проверки\. Оставлю решение авторам проекта\.

**Предупреждение PVS\-Studio:** [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'SelectionChangedEventArgs' constructor: 'removedSelectedItems' and 'addedSelectedItems'\. DataGridSelectedItemsCollection\.cs 338

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

В данном случае анализатор предположил, что второй и третий аргументы конструктора перепутаны местами\. Посмотрим на вызываемый конструктор:

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

Принимаются два контейнера типа _IList_, перепутать легко\. Судя по комментарию в начале класса, это ошибка в коде контрола, скопированного у Microsoft, и доработанного под Avalonia\. Но мне кажется, стоит поправить порядок аргументов метода, хотя бы чтобы не искать возможную ошибку у себя, когда придёт баг репорт\.

Анализатор нашел ещё три подобных ошибки:

**Предупреждение PVS\-Studio:** [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'SelectionChangedEventArgs' constructor: 'removed' and 'added'\. AutoCompleteBox\.cs 707

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

Тот же конструктор _SelectionChangedEventArgs\._ 

**Предупреждения PVS\-Studio** [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/):

* Possible incorrect order of arguments passed to 'ItemsRepeaterElementIndexChangedEventArgs' constructor: 'oldIndex' and 'newIndex'\. ItemsRepeater\.cs 532
* Possible incorrect order of arguments passed to 'Update' method: 'oldIndex' and 'newIndex'\. ItemsRepeater\.cs 536

Два срабатывания в одном методе вызова события\.

```cpp
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_ имеют другой порядок:

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3004/) The 'then' statement is equivalent to the 'else' statement\. DataGridSortDescription\.cs 235

```cpp
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_\. В интерфейсе_ IEnumerable_, от которого наследуется аргумент _seq,_ есть метод_ ThenBy_; полагаю, подразумевалось его использование\. Вот так:

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3106/) Possible negative index value\. The value of 'index' index could reach \-1\. Animator\.cs 68

```cpp
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]; 
    }
    ....
  }
  ....
}
```

Анализатор полагает, что _index_ может иметь значение: \-1\. Переменная получается из метода _FindClosestBeforeKeyFrame_, посмотрим на него:

```cpp
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.");
}
```

Как мы видим, в цикле проверяется условие и возвращается предыдущее значение итератора\. Условие довольно трудно проверить, и я не могу точно сказать, чему равен _CueValue_, но по описанию он принимает значение от 0\.0 до 1\.0\. Можем кое\-что сказать про _time_, это _animationTime_ из вызывающего метода, он точно больше нуля и меньше единицы\. Иначе выполнение программы пошло бы по другой ветке\. Если это методы, вызываемые для отрисовки анимации, то ситуация выглядит как хороший плавающий баг\. Я бы добавил проверку результата _FindClosestBeforeKeyFrame_, если в этом случае нужна особенная обработка\. Или – если первый элемент не удовлетворяет каким\-то другим условиям – убрать его из цикла\. Не зная, как всё это должно работать, в качестве примера исправления выберу второй вариант:

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3117/) Constructor parameter 'phones' is not used\. Country\.cs 25

```cpp
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;
}
```

Хороший пример отличия работы анализатора от ручного обзора кода\. Тринадцать аргументов конструктора, один не используется\. На самом деле Visual Studio тоже отмечает неиспользуемый аргумент, но на третьем уровне предупреждений \(они часто бывают отключены\)\. В данном случае это явная ошибка, потому что в классе также есть тринадцать свойств на каждый аргумент, и значение в _Phones_ нигде не присваивается\. Правка очевидна, не буду её приводить\.

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'tabItem'\. TabItemContainerGenerator\.cs 22

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

Анализатор посчитал опасным разыменование результата вызова _CreateContainer_\. Посмотрим на этот метод:

```cpp
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;
  }
}
```

Анализатор может увидеть присвоение _null_ в переменную даже при передаче значения через цепочку из пятидесяти методов\. Но не может сказать, пойдёт ли выполнение по этой ветке хоть раз\. Да и я тоже не смог, в общем\-то\.\.\. Вызовы метода теряются среди переопределённых и виртуальных методов\. Так что предлагаю просто подстраховаться дополнительной проверкой:

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

**Предупреждение PVS\-Studio:** [V3142](https://pvs-studio.ru/ru/docs/warnings/v3142/) Unreachable code detected\. It is possible that an error is present\. DevTools\.xaml\.cs 91

Не буду писать тут много кода для создания интриги, скажу сразу: предупреждение ложное\. Анализатор увидел вызов метода, выбрасывающего безусловное исключение\. Вот он:

```cpp
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](https://www.mono-project.com/docs/)\. Она позволяет заменять вызовы прямо в IL коде\. 

Анализатор не поддерживает эту библиотеку, следовательно, у нас есть много ложных срабатываний, значит эту диагностику на данном проекте лучше отключить\. Немного неловко признаваться, именно я делал эту диагностику\.\.\. но, как и любой инструмент, статический анализ нуждается в настройке\. 

Например, в настоящий момент мы разрабатываем диагностику о небезопасном приведении типов\. И она даёт чуть меньше тысячи срабатываний на проекте игры, в которой контроль типизации осуществляется на стороне движка\. 

**Предупреждение PVS\-Studio:** [V3009](https://pvs-studio.ru/ru/docs/warnings/v3009/) It's odd that this method always returns one and the same value of 'true'\. DataGridRows\.cs 412

```cpp
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_ – обычно один из самых нестабильных контролов, и мне кажется, тут стоит подумать, а надо ли подтверждать скролл, если он не удовлетворяет условиям?

## Заключение

Некоторые из приведённых ошибок внесены в проект не самими разработчиками Avalonia UI, а кодом, скопированным из контролов WPF\. Однако для пользователя интерфейса источник ошибки обычно не играет роли\. Упавший или некорректно работающий интерфейс портит мнение обо всей программе\. 

В статье я упомянул необходимость настройки анализатора, есть ложно позитивные срабатывания, которые неизбежны в силу принципов работы алгоритмов статического анализа\. Кто знаком с [проблемой остановки](https://en.wikipedia.org/wiki/Halting_problem) знают о математических ограничениях при работе кода с другим кодом\. Но в данном случае мы говорим об отключении одной диагностики из почти полутора сотен\. Так что речь о потере смысла в статическом анализе не идет \(либо вопрос не стоит\)\. Кроме того, у этой диагностики могли быть и хорошие срабатывания, просто их тяжелее найти в массе ложно позитивных\. 

Обязательно надо отметить высокое качество кода проекта\! Надеюсь, разработчики сохранят темп и уровень качества кода\. К сожалению, чем больше проект, тем больше багов в нём\. Один из возможных путей уменьшения багов – грамотная настройка CI\\CD, с подключением статического и динамического анализов\. А если вы хотите упростить работу с большими проектами и уменьшить количество времени, необходимого для отладки \- [скачайте и попробуйте](https://pvs-studio.ru/ru/pvs-studio/download/) PVS\-Studio\!