# Нужно ли проверять библиотеки перед их использованием? Разберём на примере MudBlazor

В нашей компании возникла потребность использования библиотеки для Blazor компонентов\. Мы остановились на MudBlazor и перед внедрением проверили качество её кода\. В результате нашли ряд странностей и даже воспроизводящееся падение, о чём и расскажем в статье\.

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

[MudBlazor](https://mudblazor.com/) — одна из самых популярных библиотек для приложений под Blazor\. Во многом наш выбор пал на неё из\-за количества компонентов и красоты их реализации\. Её исходный код написан на C\#, поэтому перед непосредственным использованием мы решили проанализировать его\. Для анализа взяли [исходники версии v6\.1\.7](https://github.com/MudBlazor/MudBlazor/tree/v6.1.7)\. Дальше речь пойдёт о найденных ошибках\.

## Сомнительный вызов

**Issue 1**

```cpp
public EndSlopeSpline(....):base(....)
{
  m = new Matrix(n);
  gauss = new MatrixSolver(n, m);
  a = new double[n];
  b = new double[n];
  c = new double[n];
  d = new double[n];
  h = new double[n];

  CalcParameters(firstSlopeDegrees, lastSlopeDegrees);
  Integrate();                                            // <=
  Interpolate();
}
```

Предупреждение PVS\-Studio: [V3010](https://pvs-studio.ru/ru/docs/warnings/v3010/) The return value of function 'Integrate' is required to be utilized\. EndSlopeSpline\.cs 26

Чтобы разобраться в сути данного срабатывания, необходимо обратиться к реализации метода _Integrate_:

```cpp
public double Integrate()
{
  double integral = 0;
  for (int i = 0; i < h.Length; i++)
  {
    double termA = a[i] * h[i];
    double termB = b[i] * Math.Pow(h[i], 2) / 2.0;
    double termC = c[i] * Math.Pow(h[i], 3) / 3.0;
    double termD = d[i] * Math.Pow(h[i], 4) / 4.0;
    integral += termA + termB + termC + termD;
  }
  return integral;
}
```

В методе не изменяется чьё\-либо состояние \(например, поля или свойства\)\. Исходя из этого, можно сделать вывод, что единственное назначение этого метода — расчёт с последующим возвращением значения переменной _integral_\. Это делает бессмысленным вызов _Integrate_ без использования возвращаемого значения\.

Стоит отметить, что методы _CalcParameters_ и _Interpolate_ \(вызванные рядом с _Integrate_\) как раз изменяют состояния ряда свойств и полей\.

**Issue 2**

```cpp
internal void DateValueChanged(DateTime? value)
{
  _valueDate = value;

  if (value != null)
  {
    var date = value.Value.Date;

    // get the time component and add it to the date.
    if (_valueTime != null)
    {
      date.Add(_valueTime.Value);                    // <=
    }

    _filterDefinition.Value = date;
  }
  else
    _filterDefinition.Value = value;

  _dataGrid.GroupItems();
}
```

Предупреждение PVS\-Studio: [V3010](https://pvs-studio.ru/ru/docs/warnings/v3010/) The return value of function 'Add' is required to be utilized\. Filter\.cs 140

По своей сути срабатывание похоже на предыдущее\. Здесь не было использовано возвращаемое значение метода _Add_, вызванного у переменной типа _DateTime_\. Скорее всего, разработчик хотел добавить значение _\_valueTime\.Value_ к _date_\. Однако он не учёл, что метод _Add_ для объекта типа _DateTime_ возвращает результат добавления, а не изменяет исходный объект\. В итоге имеем бесполезный вызов, который по задумке наверняка должен был на что\-то влиять\. 

## Неожиданный NullReferenceException

**Issue 3**

```cpp
public static bool operator ==(ResizeOptions l, ResizeOptions r) 
                                                       => l.Equals(r);

public static bool operator !=(ResizeOptions l, ResizeOptions r) 
                                                       => !l.Equals(r);
```

Предупреждения PVS\-Studio:

* [V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to '\=\=' operator should not result in 'NullReferenceException'\. ResizeOptions\.cs 34
* [V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to '\!\=' operator should not result in 'NullReferenceException'\. ResizeOptions\.cs 35

При перегрузке операторов равенства и неравенства стоит учитывать возможность того, что один из операндов может иметь значение _null_\. Подобные рекомендации даны в [документации Microsoft](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/statements-expressions-operators/how-to-define-value-equality-for-a-type)\.



Однако в реализации перегрузки операторов '\=\=' и '\!\=' левый операнд не проверяется на _null_\. Если значение параметра _l_ – _null_, то при вызове метода _Equals_ будет выброшено исключение типа _NullReferenceException_\. Аргумент метода _Equals_, кстати, так же разыменовывается без проверки:

```cpp
public bool Equals(ResizeOptions other)
{
  if (ReportRate != other.ReportRate || ....)     // <=
  {
    return false;
  }
  ....
}
```

Такое поведение может стать весьма неприятным сюрпризом для пользователя, если класс публичный\. Именно таким и является _ResizeOptions_\.

Чтобы убедиться в наличии проблемы, попробуем сравнить объект типа _ResizeOptions_ с _null_\.

![1032_MudBlazor_ru/image2.png](https://import.viva64.com/docx/blog/1032_MudBlazor_ru/image2.png)

Вполне ожидаемо получаем исключение типа _NullReferenceException_\. Если правый операнд – _null_, поведение будет аналогичным\.

![1032_MudBlazor_ru/image3.png](https://import.viva64.com/docx/blog/1032_MudBlazor_ru/image3.png)

## Необоснованное присваивание

**Issue 4**

```cpp
protected override void OnParametersSet()
{
  if (....)
  {
    ....
    foreach (....)
    {
      if (firstTime)
      {
        ....
        gridValueY = verticalStartSpace;                   // <=
      }
      else
      {
        ....
        gridValueY = verticalStartSpace;                   // <=
      }
      gridValueY = yValue;                                 // <=
      ....
    }
  }
  else
  {
    ....
  }
  ....
}
```

Предупреждение PVS\-Studio: [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'gridValueY' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 191, 189\. Line\.razor\.cs 191

Рассмотрим вложенный _if_\. В обеих ветвях этого оператора переменной _gridValueY_ присваивается некоторое значение\. Стоит отметить, что оно одинаково как в первом, так и во втором случае\. Получается, что такое присваивание стоило вообще вынести за _if_\. Однако основная проблема заключается не в этом: значение _gridValueY_ изменяется сразу после _if_, хотя оно не использовалось\.

Блок _else_ оператора _if_ верхнего уровня схож с блоком _then_:

```cpp
foreach (....)
{
  if (firstTime)
  {
    ....
    gridValueY = verticalStartSpace;
  }
  else
  {
    ....
    gridValueY = verticalStartSpace;
  }
  ....
  gridValueY = boundHeight - (gridValueY + gridValue);   // <=
  ....
}
```

В данном случае значение _gridValueY_ тоже перезаписывается сразу после _if_, однако старое значение при этом используется\. 

## Неправильно расставленные приоритеты

**Issue 5**

```cpp
internal async Task<bool> StartResizeColumn(....)
{
  ....
  // In case resize mode is column, we have to find any column right
  // of the current one that can also be resized and is not hidden.

  var nextResizableColumn = _columns.Skip(_....) + 1)
                                    .FirstOrDefault(c =>
                                        c.Resizable ?? true && !c.Hidden); // <=
  ....
}
```

Предупреждение PVS\-Studio: [V3177](https://pvs-studio.ru/ru/docs/warnings/v3177/) The 'true' literal belongs to the '&&' operator with a higher priority\. It is possible the literal was intended to belong to '??' operator instead\. DataGridColumnResizeService\.cs 52

Рассмотрим выражение _c\.Resizable ?? true && \!c\.Hidden_\. Стоит сразу сказать, что приоритет '??' ниже, чем у '&&'\. Исходя из этого, если _c\.Resizable_ — _null_, то оператор null\-coalescing вернёт результат операции _true && \!c\.Hidden_\. Операция "&&" с литералом _true_ бессмысленна\. Скорее всего, правильный вариант выглядит так:

```cpp
(c.Resizable ?? true) && !c.Hidden
```

Разница этих записей проявит себя, если значения _c\.Resizable_ и _c\.Hidden_ будут равны _true_\.

Есть ещё один момент, указывающий на то, что была допущена ошибка\. Строчкой выше написан комментарий о том, что столбец должен иметь возможность изменения размера \(информация об этом содержится в _c\.Resizable_\) и не быть скрытым \(это можно узнать из _c\.Hidden_\)\. Если _c\.Resizable_ — _true_, то логика работы будет нарушена, так как значение _c\.Hidden_ никак не повлияет на результат\.

## Подводя итог

Как мне кажется, каждое из разобранных срабатываний может указывать на ошибку, меняющую логику работы программы\. В одном из случаев это удалось доказать\. Перед написанием статьи я создал [issue](https://github.com/MudBlazor/MudBlazor/issues/6128) на GitHub\. Однако, ответ от разработчиков пока не был получен\. Надеюсь, что они рассмотрят перечисленные проблемы и смогут повысить качество кода своего проекта :\)\.

Можно попробовать использованный в статье анализатор, скачав его [отсюда](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. Кстати, недавно в PVS\-Studio появилась возможность анализа блоков C\# кода в Blazor\-компонентах\. Некоторые найденные в таких компонентах ошибки описаны в [этой статье](https://pvs-studio.ru/ru/blog/posts/csharp/1023/)\.