﻿# Есть ли в вашей IDE баги? Проверка AvalonStudio с помощью PVS\-Studio

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

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

## Введение

Жизнь современного разработчика невозможна без различных инструментов программирования\. К таковым относятся IDE \(Integrated development environment\)\. Они содержат в себе целый набор инструментов, упрощающих жизнь\. Современная среда разработки включает в себя текстовый редактор, компилятор или интерпретатор, отладчик и так далее\.

В данной статье я проверю одну из IDE с открытым исходным кодом\. Для проверки проекта я буду использовать статический анализатор кода PVS\-Studio\. Он может искать ошибки и потенциальные уязвимости в проектах на языках программирования C, C\+\+, C\# и Java\. 

А теперь представим проект, который мы будем анализировать\.

AvalonStudio – это кроссплатформенная IDE, написанная на C\#\. Эта среда в первую очередь ориентирована на разработку под Embedded C, C\+\+, \.NET Core, Avalonia и TypeScript\. AvalonStudio построена с использованием фреймворка [Avalonia UI](https://avaloniaui.net/), который мы, кстати, тоже [проверяли](https://pvs-studio.ru/ru/blog/posts/csharp/0701/)\.

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

Проверить исходный код AvalonStudio было несложно, так как он доступен в [репозитории на GitHub](https://github.com/VitalElement/AvalonStudio/tree/2ee2adbf1772c54531f0d5b0bc96a4789d6b826f)\. Среди срабатываний анализатора я отобрал для вас самые интересные :\)\. Приятного чтения\!

## Проверка

Каждое предупреждение анализатора в статье содержит код и описание проблемы с примесью мнения автора\. Если с какими\-нибудь моментами статьи вы будете не согласны, то я с удовольствием обсудил бы это в комментариях\.

**Issue 1**

```cpp
private async void Timer_Tick(object sender, EventArgs e)
{
  ....  
  if (AssociatedObject.IsPointerOver)
  {
    var mouseDevice = (popup.GetVisualRoot() as IInputRoot)?.MouseDevice;
    lastPoint = mouseDevice.GetPosition(AssociatedObject);
    popup.Open();
  }
}
```

[V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'mouseDevice' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. TooltipBehavior\.cs 173

Анализатор заметил подозрительную ситуацию в коде\. Результат преобразования через оператор _as_ проверяют на равенство _null_ с помощью null\-условного оператора\. Это значит, что разработчик предполагал, что преобразование через _as_ может вернуть _null_\. Оператором '?\.' он защитил себя от _NullReferenceException_ один раз, но вот дальше исключение вполне может быть выброшено\.

**Issue 2**

А вот следующее срабатывание не указывает на столь явную ошибку, но всё же фрагмент кода выглядит подозрительно:

```cpp
private static Signature BuildSignature(IMethodSymbol symbol)
{
  ....
  var returnTypeInfo = CheckForStaticExtension.GetReturnType(symbol);
  if(returnTypeInfo.HasValue)
  {
    if(returnTypeInfo.Value.inbuilt)
    {
      signature.BuiltInReturnType = returnTypeInfo.Value.name;
    }
    else
    {
      signature.ReturnType = returnTypeInfo.Value.name;
    }
  }
  signature.Parameters = symbol.Parameters.Select(parameter =>
  {
    var info = CheckForStaticExtension.GetReturnType(parameter);
    ....
    if(info.HasValue)
    {
      if(info.Value.inbuilt)
      {
        result.BuiltInType = info.Value.name;   // <=
      }
      else
      {
        result.BuiltInType = info.Value.name;   // <=
      }
    }
    ....
  }).ToList();
  ....
}
```

[V3004](https://pvs-studio.ru/ru/docs/warnings/v3004/) The 'then' statement is equivalent to the 'else' statement\. InvocationContext\.cs 400

PVS\-Studio обнаружил, что then и else ветви оператора _if_ эквивалентны\. При этом, если мы поднимемся чуть выше по коду, то заметим, что имеется похожий фрагмент, но ветви разные\. Возможно, что одна из ветвей просто лишняя\. Но не стоит исключать ситуацию, что на месте свойства _BuiltInType_ должно быть другое\. Например, похожее свойство _Type_, которое действительно существует\. Разработчик мог просто воспользоваться авто дополнением и не заметить, что оно оказалось неправильным\. Кстати, мне VS2022 тоже предложила написать неправильно\. Как говорится: "Доверяй, но проверяй"\.

**Issue 3**

Следующий код я отформатировал, так как он был не структурирован и плохо читаем, но суть оставил без изменений\.

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

Вот так код выглядит в самом редакторе\. Если же код чуть лучше отформатировать, то ошибка становится гораздо виднее\.

```cpp
private bool CursorIsValidDeclaration(ClangCursor c)
{
  var result = false;

  if (  (c.Kind == NClang.CursorKind.FunctionDeclaration)  // <=
      || c.Kind == NClang.CursorKind.CXXMethod 
      || c.Kind == NClang.CursorKind.Constructor 
      || c.Kind == NClang.CursorKind.Destructor 
      || c.Kind == NClang.CursorKind.FunctionDeclaration)  // <=
  {
    result = true;
  }
  return result;
}
```

[V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'c\.Kind \=\= NClang\.CursorKind\.FunctionDeclaration' to the left and to the right of the '\|\|' operator\. CPlusPlusLanguageService\.cs 1275

Подобные ошибки допускаются по невнимательности\. Возможно, одно из сравнений просто лишнее\. А может быть и так, что вместо одного из _FunctionDeclaration_ должно быть что\-то другое :\)\.

**Issue 4**

```cpp
public override void Render(DrawingContext context)
{
  ....
  foreach (var breakPoint in _manager?.OfType<Breakpoint>().Where(....))
  {
    ....
  }
  ....
}
```

[V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. BreakPointMargin\.cs 46

Анализатор обнаружил, что результат работы оператора '?\.' сразу же разыменовывается\. Использование такой проверки на _null_ внутри _foreach_ не имеет никакого смысла\. _NullReferenceException_ всё равно настигнет при попытке обхода нулевой ссылки :\)\.

Вообще, такое поведение неочевидно для многих программистов\. Мы даже написали на эту тему целую статью: "[Использование оператора ?\. в foreach: защита от NullReferenceException, которая не работает](https://pvs-studio.ru/ru/blog/posts/csharp/0832/)"\.

![0966_AvalonStudio_ru/image4.png](https://import.viva64.com/docx/blog/0966_AvalonStudio_ru/image4.png)

**Issue 5**

```cpp
public async Task<(....)> LoadProject(....)
{
  ....
  return await Task.Run(() =>
  {
    ....
    if (   loadData.CscCommandLine != null                            // <=
        && loadData.CscCommandLine.Count > 0)
    {
      ....
      return (projectInfo, projectReferences, loadData.TargetPath);
    }
    else
    {
      ....
      return (projectInfo, projectReferences, loadData?.TargetPath);  // <=
    }
  });
}
```

[V3095](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'loadData' object was used before it was verified against null\. Check lines: 233, 262\. MSBuildHost\.cs 233

Кажется, что долго объяснять ошибку тут не стоит\. Если переменная _loadData_ потенциально равна _null _в ветви else, то и в условии этого же _if_ она может быть равна _null_\. Вот только в условии нет проверки _loadData_ на _null_, а значит может быть сгенерировано исключение\. И да, в else _loadData_ никак не меняется\. Скорее всего разработчик пропустил оператор '?\.' в условии _if_\. Хотя, возможно, что данный оператор в _return_ лишний и его стоит убрать, чтобы не путать разработчиков\.

**Issue 6**

```cpp
public override void Render(DrawingContext context)
{
  if (_lastFrame != null && !(   _lastFrame.Width == 0 
                              || _lastFrame.Width == 0))
  {
    ....
  }
  base.Render(context);
}
```

[V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions '\_lastFrame\.Width \=\= 0' to the left and to the right of the '\|\|' operator\. RemoteWidget\.cs 308

Анализатор обнаружил два одинаковых подвыражения слева и справа от оператора '\|\|'\. Думаю, что исправление подобной оплошности довольно лёгкое: нужно изменить одно из свойств _Width_ на _Height_\. Это свойство существует и используется далее в методе\.

**Issue 7**

```cpp
public GlobalRunSpec(....)
{
  ....
  if (specialEntry.Value != null)
  {
    ....
  }
  RunSpec spec = new(specialOps,
                     specialVariables ?? variables,
                     specialEntry.Value.VariableSetup.FallbackFormat);
  ....
}
```

[V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'specialEntry\.Value' object was used after it was verified against null\. Check lines: 92, 86\. GlobalRunSpec\.cs 92

PVS\-Studio обнаружил подозрительную ситуацию\. _specialEntry\.Value_ проверяется на равенство _null_, а после используется без соответствующей проверки\. При этом внутри условной конструкции _specialEntry\.Value_ не меняет своё значение\.

**Issue 8**

```cpp
private static StyledText InfoTextFromCursor(ClangCursor cursor)
{
  ....
  if (cursor.ResultType != null)
  {
    result.Append(cursor.ResultType.Spelling + " ",
                  IsBuiltInType(cursor.ResultType) ? theme.Keyword 
                                                   : theme.Type);
  }
  else if (cursor.CursorType != null)
  {
    switch (kind)
    {
      ....
    }
    result.Append(cursor.CursorType.Spelling + " ",
                  IsBuiltInType(cursor.ResultType) ? theme.Keyword   // <=
                                                   : theme.Type);
  }
  ....
}
```

[V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'IsBuiltInType\(cursor\.ResultType\)' is always false\. CPlusPlusLanguageService\.cs 1105

Анализатор считает, что _IsBuiltInType\(cursor\.ResultType\)_ в указанном месте всегда вернёт _false_\. Метод _IsBuiltInType_ вызывается в else\-ветке, значит _cursor\.ResultType_ имеет значение _null_\. 

Внутри метода _IsBuiltInType_ есть проверка, что если параметр равен _null_, то возвращается _false_\.

```cpp
private static bool IsBuiltInType(ClangType cursor)
{
  var result = false;
  if (cursor != null && ....)
  {
    return true;
  }
  return result;
}
```

Получается, что _IsBuiltInType\(cursor\.ResultType\)_ всегда будет возвращать _false_\.

Хм, почему же так произошло? Тут явная проблема copy\-paste\. Разработчик просто скопировал похожий фрагмент выше\. Вот только один раз _cursor\.ResultType_ сменить на _cursor\.CursorType_ забыл\.

**Issue 9**

```cpp
private int BuildCompletionsForMarkupExtension(....)
{
  ....
  if (t.SupportCtorArgument == MetadataTypeCtorArgument.HintValues)
  {
    ....
  }
  else if (attribName.Contains("."))
  {
    if (t.SupportCtorArgument != MetadataTypeCtorArgument.Type)
    {
      ....
      if (   mType != null 
          && t.SupportCtorArgument ==
             MetadataTypeCtorArgument.HintValues)        // <=
      {
        var hints = FilterHintValues(....);
        completions.AddRange(hints.Select(.....));
      }
      ....
    }  
  }
}
```

[V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'mType \!\= null && t\.SupportCtorArgument \=\= MetadataTypeCtorArgument\.HintValues' is always false\. CompletionEngine\.cs 601

Свойство _SupportCtorArgument_ сравнивается со значением перечисления\. Это выражение находится в else\-ветке условной конструкции\. _SupportCtorArgument_ является обычным автосвойством\. Если бы свойство было равно _MetadataTypeCtorArgument\.HintValues_, то до этой else\-ветки исполнение кода не дошло бы\. Соответственно, выражение в условии всегда будет ложным, а код, который находится в then\-ветке, никогда не будет выполнен\.

**Issue 10**

```cpp
private void RegisterLanguageService(ISourceFile sourceFile)
{
  ....
  var languageServiceProvider = IoC.Get<IStudio>()
                                   .LanguageServiceProviders
                                   .FirstOrDefault(....)?.Value;

  LanguageService = languageServiceProvider.CreateLanguageService();  // <=

  if (LanguageService != null)
  {
    ....
  }
}
```

[V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'languageServiceProvider' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. CodeEditorViewModel\.cs 309

Анализатор указывает, что происходит разыменование переменной, значение которой получено с помощью null\-условного оператора\. И как мы видим, оператор '?\.' действительно используется при вычислении значения переменной _languageServiceProvider_\. На следующей строчке происходит разыменование этой переменной без проверки\. Подобный код может генерировать исключение _NullReferenceException_\. Возможно, стоит использовать '?\.' при вызове _CreateLanguageService_\. 

**Issue 11**

```cpp
public void SetCursorPosition(int column, int row)
{
  ....
  if (LeftAndRightMarginEnabled)
  {
    if (CursorState.OriginMode && CursorState.CurrentColumn < LeftMargin)
      CursorState.CurrentColumn = LeftMargin;
    if (CursorState.CurrentColumn >= RightMargin)
      RightMargin = RightMargin;                   // <=
  }
  ....
}
```

[V3005](https://pvs-studio.ru/ru/docs/warnings/v3005/) The 'RightMargin' variable is assigned to itself\. VirtualTerminalController\.cs 1446

Переменная _RightMargin_ присваивается сама себе\. Код, несомненно, подозрительный\. И, пожалуй, только разработчик данного проекта знает, как должно быть на самом деле\. Мы же можем только догадываться, что следует присваивать _RightMargin_\.

**Issue 12**

Следующее срабатывание довольно интересное и неочевидное\.

```cpp
public class ColorScheme
{
  private static List<ColorScheme> s_colorSchemes =
    new List<ColorScheme>();
  private static Dictionary<string, ColorScheme> s_colorSchemeIDs =
    new Dictionary<string, ColorScheme>();
  private static readonly ColorScheme DefaultColorScheme = 
    ColorScheme.SolarizedLight;
  ....
  public static readonly ColorScheme SolarizedLight = new ColorScheme
  {
    ....
  };
}
```

Попробуйте найти среди этих строк ошибку\. Задача максимально упрощена, ведь за "\.\.\.\." я скрыл почти 200 строк кода, которые не нужны для обнаружения ошибки\. Догадались?

Уберём ещё немного кода и добавим сообщение анализатора\.

```cpp
public class ColorScheme
{
  private static readonly ColorScheme DefaultColorScheme = 
    ColorScheme.SolarizedLight;
  ....
  public static readonly ColorScheme SolarizedLight = new ColorScheme
  {
    ....
  };
}
```

[V3070](https://pvs-studio.ru/ru/docs/warnings/v3070/) Uninitialized variable 'SolarizedLight' is used when initializing the 'DefaultColorScheme' variable\. ColorScheme\.cs 32

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

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

Проверка AvalonStudio позволила найти несколько интересных ошибок\. Я надеюсь, что эта статья внесёт свой маленький вклад в улучшения данного проекта\. Стоит сказать, что IDE с открытым исходным кодом сейчас довольно мало\. Может быть в будущем их станет больше\. Так каждый разработчик сможет выбрать среду разработки себе по душе\. И даже попробовать самостоятельно улучшить инструмент, которым пользуется\.

Вы также можете лёгким и бесплатным способом улучшить свой проект\. Для этого нужно всего лишь проверить свой код с помощью PVS\-Studio\. А в этом вам поможет пробный ключ, который можно взять на нашем [сайте](https://pvs-studio.ru/ru/pvs-studio/try-free/) :\)\.