# Графический переполох: что таит в себе ScottPlot?
ScottPlot — библиотека под \.NET для построения графиков\. Из\-за специфики подобных проектов их код нередко бывает запутанным\. Сегодня мы попробуем распутать его и отыскать проблемные места с помощью статического анализатора\.
Обо всех возможностях ScottPlot можно узнать на [сайте проекта](https://scottplot.net/)\. На нём же представлен график, построенный с помощью библиотеки:
Как было сказано ранее, ScottPlot — библиотека\. При разработке подобных решений стоит уделять особое внимание качеству кода\. Ведь ошибки в таких проектах влияют не только на непосредственных пользователей библиотеки, но и на тех, кто использует приложения, написанные с её помощью\. Это стало одной из причин, по которой мы решили проверить ScottPlot с помощью статического анализатора [PVS\-Studio](https://pvs-studio.ru)\.
Проверяемый код соответствует этому [коммиту](https://github.com/ScottPlot/ScottPlot/tree/fff3fca297c0e05c85e5dc19033504a3454605cf)\.
Приступим к разбору подозрительных мест\.
## Битовые фокусы
**Фрагмент кода 1**
```cpp
public static Interactivity.Key GetKey(this Keys keys)
{
Keys keyCode = keys & ~Keys.Modifiers; // <=
Interactivity.Key key = keyCode switch
{
Keys.Alt => Interactivity.StandardKeys.Alt, // <=
Keys.Menu => Interactivity.StandardKeys.Alt,
Keys.Shift => Interactivity.StandardKeys.Shift, // <=
Keys.ShiftKey => Interactivity.StandardKeys.Shift,
Keys.LShiftKey => Interactivity.StandardKeys.Shift,
Keys.RShiftKey => Interactivity.StandardKeys.Shift,
Keys.Control => Interactivity.StandardKeys.Control, // <=
Keys.ControlKey => Interactivity.StandardKeys.Control,
Keys.Down => Interactivity.StandardKeys.Down,
Keys.Up => Interactivity.StandardKeys.Up,
Keys.Left => Interactivity.StandardKeys.Left,
Keys.Right => Interactivity.StandardKeys.Right,
_ => Interactivity.StandardKeys.Unknown,
};
....
}
```
Предупреждение PVS\-Studio: [V3202](https://pvs-studio.ru/ru/docs/warnings/v3202/) Unreachable code detected\. The 'case' value is out of range of the match expression\. ScottPlot\.WinForms FormsPlotExtensions\.cs 106
Несколько значений паттернов внутри `switch` невозможны в текущем контексте\. Давайте попробуем разобраться, в чём тут дело\.
Сперва стоит рассмотреть значения, которые соответствуют "проблемным" элементам перечисления\.
```cpp
[Flags]
[TypeConverter(typeof(KeysConverter))]
[Editor(....)]
public enum Keys
{
///
/// The bit mask to extract modifiers from a key value.
///
Modifiers = unchecked((int)0xFFFF0000),
....
///
/// The SHIFT modifier key.
///
Shift = 0x00010000,
///
/// The CTRL modifier key.
///
Control = 0x00020000,
///
/// The ALT modifier key.
///
Alt = 0x00040000
}
```
Далее переведём их в двоичную систему:
|Имя|Значение \\\(16\\\-ая система\\\)|Бинарное представление \\\(2\\\-ая система\\\)|
|---|---|---|
|Modifiers|0xFFFF0000|1111 1111 1111 1111 0000 0000 0000 0000|
|Shift|0x00010000|0000 0000 0000 0001 0000 0000 0000 0000|
|Control|0x00020000|0000 0000 0000 0010 0000 0000 0000 0000|
|Alt|0x00040000|0000 0000 0000 0100 0000 0000 0000 0000|
Теперь становится понятно, что `Modifiers` включает в себя каждый из "проблемных" членов перечисления\.
Значение, передаваемое в `switch`, получается из выражения `keys & ~Keys.Modifiers`\. Это выражение исключает из `keys` значение `Keys.Modifiers`\. Однако, помимо `Keys.Modifiers`, также будут исключены `Shift`, `Control` и `Alt`, так как `Modifiers` уже включает в себя эти значения \(исходя из того, что для каждого ненулевого бита "проблемных" элементов перечисления у `Modifiers` есть ненулевой бит\)\.
Из всего этого мы можем сделать вывод, что нет такой комбинации битов, которая позволит получить `Shift`, `Control` или `Alt` для операции `keys & ~Keys.Modifiers`\.
Возможно, проблема заключается не в значениях перечисления, а в реализации `switch`\.
## Подозрительные свойства
**Фрагмент кода 2**
```cpp
internal double BackAngleSweep
{
get
{
double maxBackAngle = CircularBackground ? 360 : MaximumSizeAngle;
if (!Clockwise) maxBackAngle = -maxBackAngle;
return maxBackAngle;
}
private set { BackAngleSweep = value; }
}
```
Предупреждение PVS\-Studio: [V3110](https://pvs-studio.ru/ru/docs/warnings/v3110/) Possible infinite recursion inside 'BackAngleSweep' property\. ScottPlot RadialGauge\.cs 45
Весьма необычная реализация setter'а свойства `BackAngleSweep`\. Если приглядеться, то можно заметить, что в setter'е свойство присваивается само себе\. Это приведёт к возникновению исключения "Stack Overflow"\.
Посмотрев на использования этого свойства, я обнаружил, что нет мест, где в него присваивается значение\. Из этого следует, что setter вообще не отрабатывает\. Если разработчик захочет что\-то записать в `BackAngleSweep`, то неминуемо получит "Stack Overflow"\.
**Фрагмент кода 3**
```cpp
public bool Rounded
{
get => StrokeCap == SKStrokeCap.Round;
set { StrokeCap = SKStrokeCap.Round; StrokeJoin = SKStrokeJoin.Round; }
}
```
Предупреждение PVS\-Studio: [V3077](https://pvs-studio.ru/ru/docs/warnings/v3077/) The setter of 'Rounded' property does not utilize its 'value' parameter\. ScottPlot LineStyle\.cs 36
Опять что\-то неочевидное творится в setter'е свойства\. При присваивании ему любого значения оно будет потеряно\. Всё потому, что значение не записывается ни в `value`, ни куда\-либо ещё\.
## Сможем сравнить?
**Фрагмент кода 4**
```cpp
public class CoordinateRangeMutable : IEquatable
{
....
public bool Equals(CoordinateRangeMutable? other)
{
if (other is null)
return false;
return Equals(Min, other.Min) && Equals(Min, other.Min); // <=
}
public override bool Equals(object? obj)
{
if (obj is null)
return false;
if (obj is CoordinateRangeMutable other)
return Equals(other);
return false;
}
public override int GetHashCode()
{
return Min.GetHashCode() ^ Max.GetHashCode(); // <=
}
}
```
Предупреждения PVS\-Studio:
[V3192](https://pvs-studio.ru/ru/docs/warnings/v3192/) The 'Max' property is used in the 'GetHashCode' method but is missing from the 'Equals' method\. ScottPlot CoordinateRangeMutable\.cs 198
[V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'Equals\(Min, other\.Min\)' to the left and to the right of the '&&' operator\. ScottPlot CoordinateRangeMutable\.cs 172
На этот фрагмент кода анализатор выдал сразу два срабатывания\. Давайте разберёмся, почему так получилось\.
Начнём со срабатывания диагностики [V3192](https://pvs-studio.ru/ru/docs/warnings/v3192/)\. В сообщении анализатора говорится, что свойство `Max` используется в методе `GetHashCode`, но не используется в методе `Equals`\. Посмотрев на переопределённый метод `Equals`, можно заметить, что в его теле вызывается ещё один `Equals`\. В нём мы видим следующую запись: `Equals(Min, other.Min) && Equals(Min, other.Min)`\. А на этот фрагмент указала диагностика [V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/)\.
Очевидно, что один из операндов `&&` должен иметь следующий вид: `Equals(Max, other.Max)`\.
Получается, что анализатор прав, в методе `Equals` действительно не фигурирует `Max`\.
**Фрагмент кода 5**
```cpp
public class CoordinateRangeMutable : IEquatable
{
....
public static bool operator ==(CoordinateRangeMutable a,
CoordinateRangeMutable b)
{
return a.Equals(b);
}
public static bool operator !=(CoordinateRangeMutable a,
CoordinateRangeMutable b)
{
return !a.Equals(b);
}
}
```
Предупреждение PVS\-Studio:
[V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to '\=\=' operator should not result in 'NullReferenceException'\. ScottPlot CoordinateRangeMutable\.cs 188
[V3115](https://pvs-studio.ru/ru/docs/warnings/v3115/) Passing 'null' to '\!\=' operator should not result in 'NullReferenceException'\. ScottPlot CoordinateRangeMutable\.cs 193
Анализатор не отпускает класс `CoordinateRangeMutable`\. Следующие предупреждения указывают на то, что при использовании перегруженных операторов `==` и `!=` возможно выбрасывание исключения `NullReferenceException`\. И это действительно так: если при сравнении объектов типа `CoordinateRangeMutable` левый операнд будет `null`, то мы обязательно получим `NullReferenceException`\.
Подобная ситуация возникает из\-за того, что в реализациях перегрузок операторов сравнение вызывается `Equals` без проверки на `null`\.
## Всегда да
**Фрагмент кода 6**
```cpp
private double GetIdealTickSpacing(CoordinateRange range,
PixelLength axisLength,
PixelLength maxLabelLength)
{
int targetTickCount = (int)(axisLength.Length / maxLabelLength.Length) + 1;
int radix = 10; // <=
int exponent = (int)Math.Log(range.Length, radix) + 1;
double initialSpace = Math.Pow(radix, exponent);
List tickSpacings = [initialSpace];
double[] divBy;
if (radix == 10) // <=
divBy = [2, 2, 2.5]; // 10, 5, 2.5, 1
else if (radix == 16)
divBy = [2, 2, 2, 2]; // 16, 8, 4, 2, 1
else
throw new ArgumentException($"radix {radix} is not supported");
....
}
```
Предупреждение PVS\-Studio: [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'radix \=\= 10' is always true\. ScottPlot DecimalTickSpacingCalculator\.cs 42
Анализатор сообщает, что выражение `radix == 10` всегда истинно\. Не надо далеко ходить, чтобы убедиться в этом\. Переменной `radix` единожды присваивается значение в самом начале метода\. Следовательно, на момент проверки условия `radix == 10` она всегда будет иметь значение 10\.
Скорее всего, раньше значение этой переменной менялось при каком\-то условии, но после внесения правок эта опция пропала\. Стоит обращать внимание на такие моменты и своевременно подчищать код, который никак не влияет на работу программы\.
## Можно быстрее
**Фрагмент кода 7**
```cpp
public void Apply(RenderPack rp, bool beforeLayout)
{
....
if (isPanning & Math.Abs(newLimits.Max - oldRight) < tickDelta / 2)
{
newRight = oldRight;
newLeft = oldLeft;
}
....
}
```
Предупреждение PVS\-Studio: [V3093](https://pvs-studio.ru/ru/docs/warnings/v3093/) The '&' operator evaluates both operands\. Perhaps a short\-circuit '&&' operator should be used instead\. ScottPlot SnapToTicksX\.cs 94
Анализатор предлагает заменить `&` на `&&`, так как вычисление правой части логического выражения бессмысленно, если левый операнд `false`\.
Подобное исправление могло бы изменить логику работы программы, например, если выполнение кода правого операнда изменяет что\-то в глобальном скоупе \(поле, свойство\)\. Однако в нашем случае метод, находящийся в правом операнде, ничего не меняет, а лишь возвращает значение\.
## Отрицательный размер
**Фрагмент кода 8**
```cpp
public class DraggableRows() : IMultiplotLayout
{
readonly List PlotHeights = [];
....
float[] GetDividerPositions()
{
if (PlotHeights.Count == 1)
return [PlotHeights[0]];
float[] positions = new float[PlotHeights.Count - 1]; // <=
positions[0] = PlotHeights[0];
for (int i = 1; i < positions.Length; i++)
{
positions[i] = positions[i - 1] + PlotHeights[i];
}
return positions;
}
....
}
```
Предупреждение PVS\-Studio: [V3171](https://pvs-studio.ru/ru/docs/warnings/v3171/) The value used as the size of an array could reach \-1\. Consider inspecting: PlotHeights\.Count \- 1\. ScottPlot DraggableRows\.cs 42
Обратите внимание на создание массива `positions`\. В качестве его размера используется `PlotHeights.Count - 1`\. Однако нет гарантии, что `PlotHeights.Count` будет больше 0\. Единственная проверка размера `PlotHeights` гарантирует, что на момент создания массива коллекция **не** будет содержать один элемент\. Стоит отметить, что `PlotHeights` инициализируется пустой коллекцией\. Это повышает вероятность использования отрицательного значения при указании размера массива\.
## Неиспользуемый параметр
**Фрагмент кода 9**
```cpp
private void RenderColorbarAxis(RenderPack rp,
PixelRect colormapRect,
float size,
float offset)
{
GenerateTicks(rp.DataRect);
float size2 = Edge switch
{
Edge.Left => size - colormapRect.Width,
Edge.Right => size - colormapRect.Width,
Edge.Bottom => size - colormapRect.Height,
Edge.Top => size - colormapRect.Height,
_ => throw new NotImplementedException(),
};
float offset2 = Edge switch
{
Edge.Left => rp.DataRect.Left - colormapRect.Left,
Edge.Right => colormapRect.Right - rp.DataRect.Right,
Edge.Bottom => colormapRect.Bottom - rp.DataRect.Bottom,
Edge.Top => rp.DataRect.Top - colormapRect.Top,
_ => throw new NotImplementedException(),
};
Axis.Render(rp, size2, offset2);
}
```
Предупреждение PVS\-Studio: [V3196](https://pvs-studio.ru/ru/docs/warnings/v3196/) The 'offset' parameter is not utilized inside the method body, but an identifier with a similar name is used inside the same method\. ScottPlot ColorBar\.cs 133
Достаточно тривиальная проблема, но безобиднее от этого она не становится\. В методе не используется параметр `offset`\. Основная проблема заключается в том, что вызывающий код высчитывает значения для передачи его в качестве аргумента \(соответствующему параметру\), что не совсем целесообразно\.
При использовании метода `RenderColorbarAxis` разработчик может не обратить внимание на его реализацию\. Соответственно, он будет думать, что `offset`, переданный в качестве аргумента, повлияет на логику работы метода\.
## Беспорядок
**Фрагмент кода 10**
```cpp
public Rectangle Rectangle(CoordinateRect rect)
{
return Rectangle(rect.Left, rect.Right, rect.Top, rect.Bottom);
}
public Rectangle Rectangle(double left, double right, double bottom, double top)
{
Color color = GetNextColor();
Rectangle rp = new()
{
X1 = left,
X2 = right,
Y1 = bottom,
Y2 = top,
LineColor = color,
FillColor = color.WithAlpha(.5),
};
Plot.PlottableList.Add(rp);
return rp;
}
```
Предупреждение PVS\-Studio: [V3066](https://pvs-studio.ru/ru/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'Rectangle' method: 'rect\.Top' and 'rect\.Bottom'\. ScottPlot PlottableAdder\.cs 1078
Анализатор сообщает, что при передаче аргументов в метод `Rectangle` был перепутан их порядок\. И это действительно так: аргумент `rect.Top` соответствует параметру `bottom`, а аргумент `rect.Bottom` — параметру `top`\.
Сложно однозначно сказать, допущена здесь ошибка или нет, но выглядит подозрительно\. Даже если ошибка есть, то кажется, что она не является критичной, но это не отменяет факт её наличия :\)
## Заключение
На самом деле код проекта оказался достаточны чистым: анализатор выдал чуть более 50 предупреждений среднего и высокого уровня\. Однако даже из их числа удалось выделить 10, как мне кажется, достойных для рассмотрения\.
Интересной особенностью является то, что на проекте практически нет предупреждений, связанных с потенциальным разыменованием нулевой ссылки\. А ведь именно такие ошибки обычно встречаются чаще всего при анализе проектов\. В качестве примера предлагаю ознакомиться с другой [проверкой проекта](https://pvs-studio.ru/ru/blog/posts/csharp/1109/)\.
А если хотите проверить свой проект с помощью PVS\-Studio, то попробовать анализатор можете по [ссылке](https://pvs-studio.ru/ru/pvs-studio/try-free/)\!