Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Небольшой вклад в борьбе Avalonia UI пр…

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

23 Дек 2019

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

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 (кроссплатформенная IDE для разработки на C# и C/C++) и Core2D (редактор 2D-схем и диаграмм). В качестве коммерческого проекта можно привести Wasabi Wallet (биткойн-кошелек).

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

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

Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'controlledFlags' to the left and to the right of the '^' operator. WindowImpl.cs 975TwitterClientMessageHandler.cs 52

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

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

выражение

1100 0011 | 1111 0000 ^ 1111 0000

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

1100 0011 | 0000 0000

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

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

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

Предупреждение PVS-Studio: V3080 Possible null dereference of method return value. Consider inspecting: 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.

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

  • V3080 Possible null dereference. Consider inspecting 'p'. VisualExtensions.cs 35
  • V3080 Possible null dereference. Consider inspecting '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 Possible null dereference of method return value. Consider inspecting: 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 Possible null dereference. Consider inspecting 'transform'. MouseDevice.cs 80

Предупреждение PVS-Studio: V3022 Expression is always true. Probably the '&&' operator should be used here. NavigationDirection.cs 89

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

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

Предупреждение PVS-Studio: V3066 Possible incorrect order of arguments passed to 'SelectionChangedEventArgs' constructor: 'removedSelectedItems' and 'addedSelectedItems'. DataGridSelectedItemsCollection.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 Possible incorrect order of arguments passed to 'SelectionChangedEventArgs' constructor: 'removed' and 'added'. AutoCompleteBox.cs 707

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

Тот же конструктор SelectionChangedEventArgs.

Предупреждения PVS-Studio 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

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

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 The 'then' statement is equivalent to the 'else' statement. DataGridSortDescription.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. В интерфейсе IEnumerable, от которого наследуется аргумент seq, есть метод 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 Possible negative index value. The value of 'index' index could reach -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]; 
    }
    ....
  }
  ....
}

Анализатор полагает, что 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.");
}

Как мы видим, в цикле проверяется условие и возвращается предыдущее значение итератора. Условие довольно трудно проверить, и я не могу точно сказать, чему равен CueValue, но по описанию он принимает значение от 0.0 до 1.0. Можем кое-что сказать про time, это animationTime из вызывающего метода, он точно больше нуля и меньше единицы. Иначе выполнение программы пошло бы по другой ветке. Если это методы, вызываемые для отрисовки анимации, то ситуация выглядит как хороший плавающий баг. Я бы добавил проверку результата 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 Constructor parameter 'phones' is not used. 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;
}

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

Предупреждение PVS-Studio: V3080 Possible null dereference. Consider inspecting '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;
  }
}

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

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

Предупреждение PVS-Studio: V3142 Unreachable code detected. It is possible that an error is present. 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 коде.

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

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

Предупреждение PVS-Studio: V3009 It's odd that this method always returns one and the same value of '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 – обычно один из самых нестабильных контролов, и мне кажется, тут стоит подумать, а надо ли подтверждать скролл, если он не удовлетворяет условиям?

Заключение

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

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

Обязательно надо отметить высокое качество кода проекта! Надеюсь, разработчики сохранят темп и уровень качества кода. К сожалению, чем больше проект, тем больше багов в нём. Один из возможных путей уменьшения багов – грамотная настройка CI\CD, с подключением статического и динамического анализов. А если вы хотите упростить работу с большими проектами и уменьшить количество времени, необходимого для отладки - скачайте и попробуйте PVS-Studio!

Популярные статьи по теме
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

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

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

Возможно, читатели помнят мою статью под названием "Эффект последней строки". В ней идёт речь о замеченной мной закономерности: ошибка чаще всего допускается в последней строке однотипных блоков текс…
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…

Комментарии (0)

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