Топ-10 ошибок, найденных в C# проектах за 2025 год / Хабр

В 2025 году команда PVS-Studio продолжила разбирать код открытых C#-проектов. За год было обнаружено немало дефектов, и из всего этого разнообразия мы выбрали 10 наиболее любопытных. Надеемся, что этот обзор будет не только интересным, но и полезным — приятного чтения!

Как составлялся топ?

Уточню, по каким критериям составлялся топ:

код из Open Source проекта;
проблема в коде обнаружена с помощью PVS-Studio;
с большой вероятностью код содержит в себе ошибку;
проблема интересна для рассмотрения;
ошибки не похожи друг на друга.

Поскольку подобный топ мы составляем регулярно, накопилась внушительная коллекция интересных ошибок. Ознакомиться со статьями, написанными в прошлые года, можно здесь:

Приступим же к самому интересному — просмотру ошибок!

P. S. Ошибки были отобраны и распределены на основе субъективного мнения автора. Если вам покажется, что какую-то ошибку засудили (или наоборот), можете смело писать об этом в комментариях :)

10 место. Попробуй найди

Сегодняшний топ открывает ошибка из статьи о проверке .NET 9. Кажется, что .NET 9 вышел совсем недавно, однако чуть больше месяца назад ему на замену пришёл .NET 10. О наиболее значимых изменениях мы рассказали в статье.

Возвращаемся к разбору:

public static void SetAsIConvertible(this ref ComVariant variant,
                                     IConvertible value)
{
  TypeCode tc = value.GetTypeCode();
  CultureInfo ci = CultureInfo.CurrentCulture;

  switch (tc)
  {
    case TypeCode.Empty: break;
    case TypeCode.Object: 
      variant = ComVariant.CreateRaw(....); break;
    case TypeCode.DBNull: 
      variant = ComVariant.Null; break;
    case TypeCode.Boolean: 
      variant = ComVariant.Create<bool>(....)); break;
    case TypeCode.Char: 
      variant = ComVariant.Create<ushort>(value.ToChar(ci)); break;
    case TypeCode.SByte: 
      variant = ComVariant.Create<sbyte>(value.ToSByte(ci)); break;
    case TypeCode.Byte: 
      variant = ComVariant.Create<byte>(value.ToByte(ci)); break;
    case TypeCode.Int16: 
      variant = ComVariant.Create(value.ToInt16(ci)); break;
    case TypeCode.UInt16: 
      variant = ComVariant.Create(value.ToUInt16(ci)); break;
    case TypeCode.Int32: 
      variant = ComVariant.Create(value.ToInt32(ci)); break;
    case TypeCode.UInt32: 
      variant = ComVariant.Create(value.ToUInt32(ci)); break;
    case TypeCode.Int64: 
      variant = ComVariant.Create(value.ToInt64(ci)); break;
    case TypeCode.UInt64: 
      variant = ComVariant.Create(value.ToInt64(ci)); break;
    case TypeCode.Single: 
      variant = ComVariant.Create(value.ToSingle(ci)); break;
    case TypeCode.Double: 
      variant = ComVariant.Create(value.ToDouble(ci)); break;
    case TypeCode.Decimal: 
      variant = ComVariant.Create(value.ToDecimal(ci)); break;
    case TypeCode.DateTime: 
      variant = ComVariant.Create(value.ToDateTime(ci)); break;
    case TypeCode.String: 
      variant = ComVariant.Create(....); break;

    default:
      throw new NotSupportedException();
  }
}

Видите ошибку? А она есть!

case TypeCode.Int64: 
  variant = ComVariant.Create(value.ToInt64(ci)); break;
case TypeCode.UInt64: 
  variant = ComVariant.Create(value.ToInt64(ci)); break; // <=

Предупреждение PVS-Studio: V3139 Two or more case-branches perform the same actions. DynamicVariantExtensions.cs 68

Надеюсь, вы размяли свои глаза, и зоркость вас не подвела. В case TypeCode.UInt64 вместо выражения value.ToInt64 нужно использовать метод ToUInt64, который существует. Вероятно, эта ошибка произошла из-за Copy Paste.

9 место. Некорректный формат

На девятом месте располагается ошибка из статьи про проверку проектов Neo и NBitcoin:

public override string ToString()
{
  var sb = new StringBuilder();

  sb.AppendFormat("{1:X04} {2,-10}{3}{4}", 
                  Position, 
                  OpCode, 
                  DecodeOperand());

  return sb.ToString();
}

Предупреждение PVS-Studio: V3025 [CWE-685] Incorrect format. A different number of format items is expected while calling 'AppendFormat' function. Format items not used: {3}, {4}. Arguments not used: 1st. VMInstruction.cs 105

Вызов переопределённого метода ToString неизбежно приведёт к исключению. Всё из-за некорректного вызова sb.AppendFormat, в котором допущены сразу две ошибки:

число аргументов для вставки меньше числа плейсхолдеров в строке формата, что и станет причиной исключения;
даже если первая проблема будет исправлена (число аргументов и плейсхолдеров станет равным), этот вызов всё так же будет вызывать исключение, т. к. индексация плейсхолдеров начинается с 0, а не с 1. Из этого следует, что для плейсхолдера с индексом 4 необходим 5-й аргумент, который будет отсутствовать.

8 место. Самокопание

Следующая на очереди ошибка из статьи про проверку трейдерского движка Lean:

public override int GetHashCode()
{
  unchecked
  {
    var hashCode = Definition.GetHashCode();
    var arr = new int[Legs.Count];
    for (int i = 0; i < Legs.Count; i++)
    {
      arr[i] = Legs[i].GetHashCode();
    }

    Array.Sort(arr);

    for (int i = 0; i < arr.Length; i++)
    {
      hashCode = (hashCode * 397) ^ arr[i];
    }

    return hashCode;
  }
}

public override bool Equals(object obj)
{
    ....

    return Equals((OptionStrategyDefinitionMatch) obj);
}

Предупреждение PVS-Studio: V3192 The 'Legs' property is used in the 'GetHashCode' method but is missing from the 'Equals' method. OptionStrategyDefinitionMatch.cs 176

Анализатор межпроцедурно проверил метод Equals, который вызывает переопределённый метод Equals, и обнаружил отсутствие использования свойства Legs, хотя это свойство используется в методе GetHashCode.

Внимательно рассмотрим метод Equals:

public bool Equals(OptionStrategyDefinitionMatch other)
{
  ....

  var positions = other.Legs
                       .ToDictionary(leg => leg.Position, 
                                     leg => leg.Multiplier);
  foreach (var leg in other.Legs)                                   // <=
  {
    int multiplier;
    if (!positions.TryGetValue(leg.Position, out multiplier))
    {
      return false;
    }

    if (leg.Multiplier != multiplier)
    {
      return false;
    }
  }

  return true;
}

Заметим, что в нём есть итерация по other.Legs. Каждый элемент этой коллекции мы пытаемся найти в словаре positions, но этот словарь получается из other.Legs. Таким образом, проверяется наличие элементов коллекции в этой же коллекции.

Исправить код можно, заменив в отмеченном месте other.Legs на Legs.

7 место. Equals с подвохом

На седьмом месте расположилась ошибка, взятая из статьи про проверку ScottPlot:

public class CoordinateRangeMutable : IEquatable<CoordinateRangeMutable>
{
  ....
  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 The 'Max' property is used in the 'GetHashCode' method but is missing from the 'Equals' method. ScottPlot CoordinateRangeMutable.cs 198

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. В сообщении анализатора говорится, что свойство Max используется в методе GetHashCode, но не используется в методе Equals. Посмотрев на переопределённый метод Equals, можно заметить, что в его теле вызывается ещё один Equals. В нём мы видим следующую запись: Equals(Min, other.Min) && Equals(Min, other.Min). А на этот фрагмент указала диагностика V3001.

Очевидно, что один из операндов && должен иметь следующий вид: Equals(Max, other.Max).

Получается, что анализатор прав, в методе Equals действительно не фигурирует Max.

6 место. Битовые фокусы

Завершает первую половину топа дефект, который, как и предыдущий, взят из статьи про проверку ScottPlot:

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 Unreachable code detected. The 'case' value is out of range of the match expression. ScottPlot.WinForms FormsPlotExtensions.cs 106

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

Сперва стоит рассмотреть значения, которые соответствуют "проблемным" элементам перечисления.

[Flags]
[TypeConverter(typeof(KeysConverter))]
[Editor(....)]
public enum Keys
{
  /// <summary>
  ///  The bit mask to extract modifiers from a key value.
  /// </summary>
  Modifiers = unchecked((int)0xFFFF0000),

  ....
  /// <summary>
  ///  The SHIFT modifier key.
  /// </summary>
  Shift = 0x00010000,

  /// <summary>
  ///  The  CTRL modifier key.
  /// </summary>
  Control = 0x00020000,

  /// <summary>
  ///  The ALT modifier key.
  /// </summary>
  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.

5 место. Упаковали

Пятёрку лидеров открывает ошибка из статьи о проверке .NET 9:

struct StackValue
{
  ....
  public override bool Equals(object obj)
  {
    if (Object.ReferenceEquals(this, obj))
      return true;

    if (!(obj is StackValue))
      return false;

    var value = (StackValue)obj;
    return    this.Kind == value.Kind 
           && this.Flags == value.Flags 
           && this.Type == value.Type;
  }
}

Предупреждение PVS-Studio: V3161 Comparing value type variables with 'ReferenceEquals' is incorrect because 'this' will be boxed. ILImporter.StackValue.cs 164

Метод ReferenceEquals принимает параметр типа Object, при передаче значимого типа будет произведена упаковка. Созданная в куче ссылка не будет равна никакой другой.

Так как в качестве первого аргумента передаётся this структуры, упаковка будет производиться при каждом вызове метода Equals. Следовательно, проверка с помощью ReferenceEquals всегда будет возвращать false.

Стоит сказать, что данная проблема не влияет на логику работы метода. Однако проверка с помощью метода ReferenceEquals была сделана, чтобы не выполнять дальнейшее сравнение, если ссылки равны. Т. е. это своего рода оптимизация. На самом же деле мы получаем полностью противоположную ситуацию:

код после проверки будет выполняться всегда;
при каждом вызове Equals будет производиться упаковка.

Забавно, что данную проблему также ищет анализатор, всторенный в .NET (правило CA2013). Однако сами разработчики .NET не смогли её избежать :)

Возможно, на проекте данное правило было отключено. К тому же, по умолчанию CA2013 включено начиная с .NET 5.

4 место. Отписка

На четвёртом месте находится ошибка из статьи про проверку MSBuild:

private static void SubscribeImmutablePathsInitialized()
{
  NotifyOnScopingReadiness?.Invoke();

  FileClassifier.Shared.OnImmutablePathsInitialized -= () =>
    NotifyOnScopingReadiness?.Invoke();
}

Предупреждение PVS-Studio: V3084. Anonymous function is used to unsubscribe from 'OnImmutablePathsInitialized' event. No handlers will be unsubscribed, as a separate delegate instance is created for each anonymous function declaration. CheckScopeClassifier.cs 67

В этом случае отписка от делегата не будет выполнена, поскольку каждый раз, когда объявляется анонимная функция, создаётся новый экземпляр делегата. Попытка отписаться от этого экземпляра не приведёт к желаемому результату. Условно OnImmutablePathsInitialized подписан на делегат 1, а отписывается от делегата 2, что ничего не поменяет.

3 место. Путаница в приоритетах

Вот мы и добрались до тройки лидеров. На почётном третьем месте расположилась ошибка из статьи про проверку проектов Neo и NBitcoin:

public override int Size =>   base.Size
                            + ChangeViewMessages?.Values.GetVarSize() ?? 0
                            + 1 + PrepareRequestMessage?.Size ?? 0
                            + PreparationHash?.Size ?? 0
                            + PreparationMessages?.Values.GetVarSize() ?? 0
                            + CommitMessages?.Values.GetVarSize() ?? 0;

Предупреждение PVS-Studio: V3123 [CWE-783] Perhaps the '??' operator works in a different way than it was expected. Its priority is lower than priority of other operators in its left part. RecoveryMessage.cs 35

На этом коде анализатор выдал сразу несколько предупреждений диагностики V3123, но для компактности я привёл только одно. Оператор ?? имеет меньший приоритет по сравнению с оператором +. Однако форматирование приведённого выражения указывает на то, что разработчик ожидал обратное.

Но есть ли в этом случае разница, какая операция будет выполнена первой? Для ответа на этот вопрос рассмотрим пример одного подвыражения сложения, если ChangeViewMessages равен null:

base.Size + ChangeViewMessages?.Values.GetVarSize() ?? 0

Неважно, какое значение будет у base.Size, результат подвыражения всегда будет 0, ведь в результате сложения base.Size c null получится всё тот же null.

Если поместить ChangeViewMessages?.Values.GetVarSize() ?? 0 в скобки, изменив тем самым порядок вычисления операций, то результатом будет base.Size.

2 место. Коварный паттерн

Второе место забирает ошибка из статьи про проверку файлового менеджера Files:

protected void ChangeMode(OmnibarMode? oldMode, OmnibarMode newMode)
{
  ....
  var modeSeparatorWidth = 
    itemCount is not 0 or 1 
      ? _modesHostGrid.Children[1] is FrameworkElement frameworkElement
        ? frameworkElement.ActualWidth
        : 0
      : 0;
  ....
}

Предупреждение PVS-Studio: V3207 [CWE-670] The 'not 0 or 1' logical pattern may not work as expected. The 'not' pattern is matched only to the first expression from the 'or' pattern. Files.App.Controls Omnibar.cs 149

В этом коде нас интересует часть itemCount is not 0 or 1. Уже догадались? Этот паттерн избыточен. Его вторая часть ни на что не влияет.

Люди привыкли, что когда мы в речи говорим "x это не 0 или 1", то имеем в виду, что x это не 0 и не 1. Но в C# приоритет операций другой. x is not 0 or 1 на самом деле x is (not 0) or 1. Подобные ошибки могут приводить не просто к избыточности, но и к ошибкам типа NullReferenceException: list is not null or list.Count == 0. По этой проблеме даже была встреча, на которой обсуждались возможные пути решения. Исходя из обсуждения, вероятно, в будущем эта ситуация будет отлавливаться или компилятором, или встроенным статическим анализом как предупреждение.

1 место. Как работает LINQ?

Победителем становится ошибка из статьи про проверку трейдерского движка Lean. Именно эта ошибка располагается на первом месте из-за её неочевидности. Не каждый разработчик задумывается о том, какое поведение будет при использовании методов с отложенным выполнением в комбинации с захваченными переменными. Все подробности дальше:

public void FutureMarginModel_MarginEntriesValid(string market)
{
  ....
  var lineNumber = 0;
  var errorMessageTemplate = $"Error encountered in file " + 
                             $"{marginFile.Name} on line ";
  var csv = File.ReadLines(marginFile.FullName)
                .Where(x =>    !x.StartsWithInvariant("#") 
                            && !string.IsNullOrWhiteSpace(x))
                .Skip(1)
                .Select(x =>
  {
    lineNumber++;                                                  // <=

    ....
  });

  lineNumber = 0;                                                  // <=
  foreach (var line in csv)
  {
    lineNumber++;                                                  // <=

    ....
  }
}

Предупреждение PVS-Studio: V3219 The 'lineNumber' variable was changed after it was captured in a LINQ method with deferred execution. The original value will not be used when the method is executed. FutureMarginBuyingPowerModelTests.cs 720

Переменная lineNumber, захватывается и инкрементируется в делегате, который передаётся в LINQ-метод. Так как Select — метод с отложенным выполнением, код делегата будет выполнен при итерировании по получившейся коллекции, а не в момент вызова Select.

Во время итерирования по коллекции csv переменная lineNumber также инкрементируется. Получается, что на каждой итерации значение lineNumber будет увеличиваться на 2 (первый раз, когда отработает код делегата, второй раз — внутри foreach), что выглядит странно.

Обратите внимание на присваивание lineNumber = 0 перед foreach. Скорее всего, ожидалось, что перед циклом значение этой переменной может быть отлично от нуля. Однако это невозможно, так как переменная инициализируется нулём, а единственное изменение её значения перед foreach находится в коде делегата. Как было сказано ранее, код делегата выполнится при итерировании по коллекции. Судя по всему, разработчик ожидал выполнения кода делегата перед циклом.

Заключение

Вот и всё — мы с вами разобрали самые интересные срабатывания по мнению автора :)

Надеюсь, подборка оказалась интересной и дала пищу для размышлений по работе с кодом.

Если вы хотите узнать, встречаются ли подобные проблемы в вашем проекте, самое время проверить его с помощью статического анализатора. Оставляю здесь ссылку для скачивания.

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Nikita Panevin. Top 10 errors found in C# projects in 2025.

Топ-10 ошибок, найденных в C# проектах за 2025 год