Приветствуем всех любителей багов. Уже скоро наступит Новый год, так что самое время подвести итоги года уходящего. По традиции - рейтинг самых интересных ошибок, которые были обнаружены командой PVS-Studio в открытых проектах C# за 2019 год. Готовы? Тогда приступим.
Десятое место: "Запутать всех"
V3066 Possible incorrect order of arguments passed to 'AdjustCellBorderStyle' method: 'isFirstDisplayedRow' and 'isFirstDisplayedColumn'. DataGridViewComboBoxCell.cs 1934
Ошибка из статьи "WinForms: ошибки, Холмс". Анализатор указывает, что при передаче аргументов в метод два последних аргумента были перепутаны местами. Посмотрим на объявление AdjustCellBorderStyle:
Похоже, что анализатор прав. Часто некоторые аргументы умышленно передают в обратном порядке, например, чтобы обменять местами какие-то переменные. Но не похоже, что это именно тот случай. Во-первых, перепутаны переменные типа bool. Во-вторых, названия методов также обычные: никаких "Swap" или "Reverse". К тому же, так ошибиться не так уж и сложно: люди часто по-разному воспринимают порядок следования пары "строка/столбец".
Девятое место: "Бесконечное рядом"
V3110 Possible infinite recursion inside 'TryValidateModel' method. PrefixedModuleUpdater.cs 48
Ошибка из статьи "Ищем и анализируем ошибки в коде Orchard CMS". Допущена ошибка, приводящая к возникновению бесконечной рекурсии. Для понимания того, как могла быть допущена эта ошибка, необходимо взглянуть на перегрузку метода TryValidateModel:
Вероятно, что и в первом случае должен использоваться вызов вида:
Код компилировался, так как _updateModel имеет тип IUpdateModel, а текущий класс также реализует интерфейс IUpdateModel.
Восьмое место: "Попробуй, найди"
V3091 Empirical analysis. It is possible that a typo is present inside the string literal: "Management Group Id". The 'Id' word is suspicious. Constants.cs 36
Ошибка из статьи "Azure PowerShell: "в основном безвреден". Анализатор заподозрил, что константу GroupName инициализировали ошибочной строкой. Вероятно, там должно быть что-то наподобие "Management Group Name". Трудно судить о критичности этой ошибки, но она однозначно редкая и трудно обнаруживаемая.
Седьмое место: "Просто недоглядели"
V3078 Original sorting order will be lost after repetitive call to 'OrderBy' method. Use 'ThenBy' method to preserve the original sorting. GridModel.Selection.cs 107
Ошибка из статьи "Проверка Telerik UI for UWP для знакомства с PVS-Studio". Вследствие повторного использования OrderBy для уже отсортированной коллекции, результат предыдущей сортировки будет потерян. Необходимо использовать ThenBy:
Такие ошибки допускают по невнимательности или незнанию. Я думаю, что виноват copy-paste.
Шестое место: "Код документирован", – говорили они
V3009 It's odd that this method always returns one and the same value of 'true'. MaskedTextProvider.cs 1529
Ошибка из статьи "Проверка исходного кода библиотек .NET Core статическим анализатором PVS-Studio". Метод всегда вернёт true. Да, это ошибка, но любопытно другое. К методу прилагается развернутый комментарий:
Removes the last character from the formatted string. (Remove last character in virtual string). On exit the out param contains the position where the operation was actually performed. This position is relative to the test string. The MaskedTextResultHint out param gives more information about the operation result. Returns true on success, false otherwise.
Фокус на последнее предложение. Но кто вообще читает эти комментарии? А если серьёзно, то такая ошибка легко вкрадывается, например, при рефакторинге или отладке. Захотели проверить вариант, когда результат метода будет всегда true, ну и позабыли вернуть всё как было обратно.
Пятое место: "Индексируй меня, немедленно!"
V3102 Suspicious access to element of 'seq' object by a constant index inside a loop. XmlQueryRuntime.cs 738
Ошибка из статьи "Проверка исходного кода библиотек .NET Core статическим анализатором PVS-Studio". При обходе коллекции seq в цикле for ошибочно используют доступ только к её первому элементу на всех итерациях (индекс 0 вместо i).
Четвёртое место: "Всего-то доллара и не хватило"
V3138 String literal contains potential interpolated expression. Consider inspecting: e. SSPIHandleCache.cs 42
Ошибка из статьи "Проверка исходного кода библиотек .NET Core статическим анализатором PVS-Studio". Строка "Attempted to throw: {e}", по всей видимости, должна быть интерполированной. Из-за пропущенного символа $ строковое представление исключения e не будет подставлено в строку. В результате строка будет использована "как есть".
Третье место: "Выхода нет"
V3008 [CWE-563] The 'this.linker.s3.region' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 116, 114. AWSSDK.DynamoDBv2.Net45 S3Link.cs 116
Ошибка из статьи "Ищем ошибки в исходном коде Amazon Web Services SDK для .NET". В теле блока if был пропущен return. В результате переменная this.linker.s3.region всегда получит значение value, включая пустую строку и null.
Второе место: "По порядку становись!"
V3070 Uninitialized variable 'LANG_USER_DEFAULT' is used when initializing the 'LOCALE_USER_DEFAULT' variable. NativeMethods.cs 890
Ошибка из статьи "WinForms: ошибки, Холмс". Перепутан порядок инициализации полей класса. Для вычисления значения поля LOCALE_USER_DEFAULT используют поле LANG_USER_DEFAULT, которое в данный момент ещё не инициализировано и имеет значение 0. Переменная LANG_USER_DEFAULT далее в коде нигде не используется. Для понимания, к чему приводит такая ошибка, была написана тестовая программа, содержащая методы из кода WinForms. Для простоты, вместо некоторых используемых констант были подставлены их фактические значения:
В результате запуска на консоль будет выведено: 0. Теперь исправим ошибку, поменяв местами объявление полей LOCALE_USER_DEFAULT и LANG_USER_DEFAULT. Результат выполнения программы в таком виде: 1024.
Первое место: "Доверяй, но проверяй"
С первым местом всегда непросто. Здесь должно быть что-то экстраординарное и очень интересное. Изначально для этой статьи я отобрал более двадцати интересных ошибок, но ничего достойного первого места среди них не было. И тут я вспомнил о статье моего коллеги Сергея Васильева, посвященной одной единственной ошибке. Прелесть этой ошибки в том, что она напрямую влияла на работу нашего анализатора. Как? Это понятно уже из названия статьи "История о том, как PVS-Studio нашёл ошибку в библиотеке, используемой в... PVS-Studio". Опять же, лень никто не отменял. :) А я здесь совершенно с чистой совестью заленюсь и не стану приводить описание ошибки, а предложу проследовать по ссылке на статью и узнать подробности. Гарантирую – это того стоит. Да и статья короткая.
Заключение
Надеюсь, ошибки были интересными, а статья не утомила. Напоминаю, что вы всегда можете скачать анализатор PVS-Studio, чтобы самим поискать ошибки в своих и сторонних проектах, порадовать себя, друзей и коллег. И пусть ошибок будет поменьше, а времени на самосовершенствование - побольше! :)
Дочитали? Поздравляю с переходом на следующий уровень! В следующих статьях нашего блога - лучшие баги 2019 года в проектах на Java и C++.
Первоисточник: Блог компании PVS-Studio.
