В эпоху монолитных UI-фреймворков существовала легенда о земле, где один код правил всеми платформами. Сегодня мы ступаем на берега этого забытого материка зовущегося Avalonia.

Материк Avalonia разделен на шесть регионов: королевство Windows, вольные территории Linux, горные цитадели macOS, мобильные княжества Android, побережные владения iOS, туманный архипелаг WebAssembly. Вооружимся компасом статического анализа и будем вести дневник наблюдений.

Немного о проекте

Avalonia — это открытый кроссплатформенный UI-фреймворк, который позволяет разработчикам создавать приложения с использованием .NET для Windows, macOS, Linux, iOS, Android и WebAssembly.

Avalonia объединяет десктопные, мобильные и веб-платформы с помощью уникального подхода, который отличается от традиционных кроссплатформенных фреймворков. Фреймворк использует собственный кроссплатформенный движок рендеринга для отображения UI-элементов, что обеспечивает одинаковый внешний вид и поведение на всех поддерживаемых платформах. Это означает, что разработчики могут делиться кодом пользовательского интерфейса и сохранять единый стиль и функциональность независимо от целевой платформы.

Примечание. Этот же проект мы проверяли ещё в 2019 году. Прочитать статью можно здесь.

Разведка Avalonia

Для проверки Avalonia использован плагин для VS Code. Прочитать, как с ним работать, можно в этой документации.

Тайные знания материка

Опытные проводники материка Avalonia знают хитрости, позволяющие сократить путь. Здесь мы соберём знания об использовании паттернов в C#.

В данном случае речь пойдёт о конструкции is not null or.

Issue 1

public PanelContainerGenerator(ItemsPresenter presenter)
{
  Debug.Assert(presenter.ItemsControl is not null);
  Debug.Assert(presenter.Panel is not null or VirtualizingPanel);
  
  ....
}

Предупреждение PVS-Studio: V3207 The 'not null or VirtualizingPanel' logical pattern may not work as expected. The 'not' pattern is matched only to the first expression from the 'or' pattern. PanelContainerGenerator.cs 22

Проблема возникает, если не учитывается, что оператор not имеет приоритет выше, чем or. В этой конструкции второе подвыражение оператора or оказывается бессмысленным. Например, в выражении presenter.Panel is not null or VirtualizingPanel, если presenter.Panel — null, при проверке на null будет получен результат true. Получается, что вторая часть выражения не будет влиять на конечный результат.

Для корректной работы выражение нужно переписать так:

public PanelContainerGenerator(ItemsPresenter presenter)
{
  Debug.Assert(presenter.ItemsControl is not null);
  Debug.Assert(presenter.Panel is not (null or VirtualizingPanel));

  ....
}

Миражи или случаи, когда параметры искажаются

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

Issue 2

protected override Size MeasureOverride(Size availableSize)
{
  availableSize = new Size(double.PositiveInfinity,
                           double.PositiveInfinity);

  foreach (Control child in Children)
  {
    child.Measure(availableSize);
  }

  return new Size();
}

Предупреждение PVS-Studio: V3061 Parameter 'availableSize' is always rewritten in method body before being used. Canvas.cs 149

В этом примере автор передаёт параметр в метод и сразу же его перезаписывает. Судя по коду, метод не нуждается в передаваемом параметре, либо нарушена логика метода.

Issue 3

public static async Task<string> Load(string generatedCodeResourceName)
{
  var assembly = typeof(XamlXNameResolverTests).Assembly;
  var fullResourceName = assembly
      .GetManifestResourceNames()
      .First(name => name.Contains("InitializeComponent")
                  && name.Contains("GeneratedInitializeComponent") 
                  && name.EndsWith(generatedCodeResourceName));

  ....
}

Предупреждение PVS-Studio: V3053 An excessive expression. Examine the substrings 'InitializeComponent' and 'GeneratedInitializeComponent'. InitializeComponentCode.cs 27

Во фрагменте кода, на который указывает анализатор, в метод Contains передаются две строки: InitializeComponent и GeneratedInitializeComponent. Но первая строка является подстрокой второй. Поэтому вызов Contains со второй строкой не влияет на результат условия.

Ловушки на тропах выбора

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

Issue 4

public override void Promote(FrugalListBase<T> oldList)
{
  for (var index = 0; index < oldList.Count; ++index)
  {
    if (FrugalListStoreState.Success == Add(oldList.EntryAt(index)))
    {
      continue;
    }

    // this list is smaller than oldList
    throw new ArgumentException($"Cannot promote from '{oldList}' 
      to '{ToString()}'    because the target map is too small.",
      nameof(oldList));
  }
}

Предупреждение PVS-Studio: V3022 Expression 'FrugalListStoreState.Success == Add(oldList.EntryAt(index))' is always true. FrugalList.cs 1473

Анализатор сообщает о том, что условие FrugalListStoreState.Success == Add(oldList.EntryAt(index)) всегда истинно. Чтобы разобраться, в чём проблема, давайте рассмотрим содержимое метода Add:

public override FrugalListStoreState Add(T value)
{
  ....
  return FrugalListStoreState.Success;
}

Вот и ошибка. Метод всегда возвращает одно и то же значение — FrugalListStoreState.Success. Так как в if происходит сравнение именно с этим значением, условие всегда истинно.

Issue 5

private void NotifyingDataSource_CollectionChanged(
object sender,NotifyCollectionChangedEventArgs e)
{
  ....
  if (ShouldAutoGenerateColumns)
  {
     // The columns are also affected (not just rows) 
    //  in this case so we need to reset everything
    _owner.InitializeElements(false /*recycleRows*/);
  }
  ....
}

Предупреждение PVS-Studio: V3022 Expression 'ShouldAutoGenerateColumns' is always false. DataGridDataConnection.cs 641

Здесь ошибка похожа не предыдущую. Если рассмотреть свойство ShouldAutoGenerateColumns, оно всегда false, поэтому код в этом блоке if никогда не выполнится.

public bool ShouldAutoGenerateColumns
{
  get
  {
    return false;
  }
}

Призраки NullReference

В самых тёмных уголках материка Avalonia обитают невидимые духи — призраки NullReference. Они прячутся в тенях неинициализированных объектов и выскакивают неожиданно, прерывая путешествие в самый неподходящий момент.

Issue 6

private void InitPicker()
{
  ....
  _hourSelector!.MaximumValue = clock12 ? 12 : 23;
  _hourSelector.MinimumValue = clock12 ? 1 : 0;
  _hourSelector.ItemFormat = "%h";
  var hr = Time.Hours;
  _hourSelector.SelectedValue = !clock12 ? hr : hr > 12 ? hr - 12 
                                              : hr == 0 ? 12 : hr;
  ....
  _hourSelector?.Focus(NavigationMethod.Pointer);
}

Предупреждение PVS-Studio: V3095 The '_hourSelector' object was used before it was verified against null. Check lines: 260, 283. TimePickerPresenter.cs 260

В данном фрагменте несколько раз встречается переменная _hourSelector. Автор даже использует оператор !, демонстрируя уверенность в том, что переменная точно не принимает значение null. И только в конце автор решает проверить, не null ли это.

Опечатки в коде

Одна неверная буква, и указанный путь ведёт не в долину, а в непроходимые болота. Рассмотрим типичную ошибку опечатки в тексте, и сколько таких ошибок нашёл анализатор PVS-Studio.

Issue 7

public ImmutableRadialGradientBrush(RadialGradientBrush source) : base(source)
{
  Center = source.Center;
  GradientOrigin = source.GradientOrigin;
  RadiusX = source.RadiusX;
  RadiusY = source.RadiusX;
}

Предупреждение PVS-Studio: V3056 Consider reviewing the correctness of 'RadiusX' item's usage. ImmutableRadialGradientBrush.cs 74

Из-за опечатки автор записывает в свойство RadiusY значение из source.RadiusX. Это же значение записывается в свойство RadiusX.

Кажется, что здесь нарушена логика кода. Судя по коду, если мы используем эту кисть, градиент будет всегда круглый и никогда не овальный. Этот код может привести к неожиданному поведению при работе с фреймворком.

Спящие стражи материка

Тесты — это стражи, охраняющие стабильность материка. Но что происходит, когда они оказываются ненадёжны? В этом разделе рассмотрим еще одну проблему, которую нашел анализатор.

Issue 8

[Fact]
public void Content_Can_Be_TopLeft_Aligned()
{
  Border content;
  var target = new ContentPresenter
  {
    Content = content = new Border
    {
      MinWidth = 16,
      MinHeight = 16,
      HorizontalAlignment = HorizontalAlignment.Right,
    },
  };

  target.UpdateChild();
  target.Measure(new Size(100, 100));
  target.Arrange(new Rect(0, 0, 100, 100));

  Assert.Equal(new Rect(84, 0, 16, 16), content.Bounds);
}

[Fact]
public void Content_Can_Be_TopRight_Aligned()
{
  Border content;
  var target = new ContentPresenter
  {
    Content = content = new Border
    {
      MinWidth = 16,
      MinHeight = 16,
      HorizontalAlignment = HorizontalAlignment.Right,
    },
  };

  target.UpdateChild();
  target.Measure(new Size(100, 100));
  target.Arrange(new Rect(0, 0, 100, 100));

  Assert.Equal(new Rect(84, 0, 16, 16), content.Bounds);
}

Предупреждение PVS-Studio: V3013 It is odd that the body of 'Content_Can_Be_TopLeft_Aligned' function is fully equivalent to the body of 'Content_Can_Be_TopRight_Aligned' function. ContentPresenterTests_Layout.cs 169

В этом фрагменте два одинаковых тестовых метода. Зачем использовать два метода, если они тестируют одно и то же? Судя по названию, автор хотел проверять отступы слева и справа. Но так как тесты одинаковые, проверяются отступы только с одной стороны.

Законы Чистого Кода: от хаоса к порядку

В сердце континента Avalonia хранятся книги с законами Чистого Кода — сводом правил. Они помогают путникам не сбиться с пути.

В этом разделе мы рассмотрим предупреждение анализатора, которое может привести к неправильному поведению программы. Чтобы этого избежать, нужно поддерживать качественный, чистый код.

Issue 9

private static RuleResult LB25(ReadOnlySpan<char> text, LineBreakState state)
{
  switch (state.Next(text).LineBreakClass)
  {
    ....
    // [25.03] NU(SY|IS)* CL × PR
    case LineBreakClass.ClosePunctuation:
    {
      switch (state.Previous.LineBreakClass)
      {
        case LineBreakClass.Numeric:  // <=
        {
          return RuleResult.NoBreak;
        }
        case LineBreakClass.BreakSymbols:
        case LineBreakClass.InfixNumeric:
        {
          if (state.Previous.LineBreakClass == LineBreakClass.Numeric)  // <=
          {
            return RuleResult.NoBreak;
          }

          break;
        }
      }

      break;
    }
    ....
  }
}

Предупреждение PVS-Studio: V3022 Expression 'state.Previous.LineBreakClass == LineBreakClass.Numeric' is always false. LineBreakEnumerator.cs 963

В данном методе 10 конструкций switch-case, некоторые вложены друг в друга до 4 раз! Не удивительно, что в такой громоздкой конструкции очень легко ошибиться. Что тут и получилось. Автор дважды в одном блоке switch пытается сравнить значение с LineBreakClass.Numeric. Первый раз это получится, но во второй он не зайдет в блок if, так как уже был в блоке case выше.

Это классический пример необходимости рефакторинга кода, также об этом мы писали в другой статье. Мало того, что здесь огромный и запутанный блок switch-case, еще и в блоках case выполняются одни и те же действия. Так как ожидается, что при LineBreakClass.Numeric вернётся RuleResult.NoBreak, как и в блоке выше.

Итоги путешествия

По сравнению с прошлой проверкой проект сильно улучшился. Данный фреймворк пользуется популярностью среди пользователей и за прошедшее время он стал только надежнее, стабильнее.

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

Чтобы проверить свой проект на наличие ошибок, вы можете воспользоваться нашим анализатором PVS-Studio.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Nikita Sviridov. Expedition into Avalonia project.