Разбираем best practice знакомства с PVS-Studio. Покажем быстрый старт работы с анализатором на примере проекта protobuf-net.
![](https://habrastorage.org/getpro/habr/upload_files/3c0/5cb/dfc/3c05cbdfc0e76b0b441c918eded258b6.png)
О чём речь?
Первое знакомство со статическим анализатором может стать тяжёлой задачкой, особенно на крупных проектах. Отчёт анализатора может содержать сотни, а иногда и тысячи предупреждений. Просматривать все будет действительно трудно. Вот именно для таких ситуаций и создавался механизм Best Warnings. Он поможет в один клик увидеть самые лучшие предупреждения – те, которые вероятнее всего указывают на ошибку.
Действительно ли в один клик? Действительно! В окне PVS-Studio рядом с уровнями предупреждений есть кнопка ‘Best’. Именно она и отвечает за показ лучших, по мнению анализатора, предупреждений.
Вот так выглядит окно PVS-Studio 7.22 в Visual Studio 2022:
![](https://habrastorage.org/getpro/habr/upload_files/26b/7a6/e0e/26b7a6e0e3785b57385a01287f7675ca.png)
При активации Best Warnings окно выглядит следующим образом:
![](https://habrastorage.org/getpro/habr/upload_files/53c/8d8/47d/53c8d847d9d67747fee56170769e55bc.png)
С версии PVS-Studio 7.22 данный механизм реализован и в других IDE:
-
Rider;
-
CLion;
-
IntelliJ IDEA.
Демонстрация работы
Показать возможности данного механизма легче всего на примере Open Source проекта. Для демонстрации Best Warnings был выбран protobuf-net. Если говорить простыми словами, то это сериализатор, который преобразует данные в формат Protocol Buffers. Для анализа был взят последний релиз версии 3.1.4.
Стоит сказать, что мы не будем рассматривать все предупреждения, которые были получены в результате фильтрации с помощью Best Warnings. Поглядим только на самые интересные и явно ошибочные места в коде. Вы же можете попробовать на своём проекте данный функционал в полном объёме. Потребуется всего лишь взять пробный ключ здесь.
Кстати, для всех рассмотренных фрагментов кода был создан issue на GitHub. Разработчик проекта ответил на него и подтвердил, что описанные ошибки нужно исправить.
Перейдём ближе к коду и ошибкам.
![](https://habrastorage.org/getpro/habr/upload_files/c0d/c37/136/c0dc37136e911d9f8def2084a215b45a.png)
Нужно больше проверок
public virtual string GetName(FileDescriptorProto definition) { var ns = definition?.Options?.GetOptions()?.Namespace; if (!string.IsNullOrWhiteSpace(ns)) return ns; ns = definition.Options?.CsharpNamespace; if (string.IsNullOrWhiteSpace(ns)) ns = GetName(definition.Package); // <= if (string.IsNullOrEmpty(ns)) ns = definition?.DefaultPackage; // <= return string.IsNullOrWhiteSpace(ns) ? null : ns; }
V3095 The ‘definition’ object was used before it was verified against null. Check lines: 139, 141. NameNormalizer.cs 139
В данном случае параметр definition сначала разыменовывается без проверки на null, а потом на null всё-таки проверяется. Стоит обратить внимание, что ошибка с отсутствием проверки даст о себе знать ещё раньше.
Давайте разбираться.
Если значение definition окажется равно null, то в переменную ns будет записан null. После этого следует проверка ns на null с помощью метода IsNullOrWhiteSpace. Если ns всё-таки равен null, то выхода из метода не произойдёт и уже на следующей строчке будет сгенерировано исключение NullReferenceException.
Если честно, то подобное форматирование кода крайне неудачно и тяжело читаемо. Может, из-за этого в методе и оказалась ошибка.
null? Используй!
internal static MethodInfo GetGetMethod(....) { if (property is null) return null; MethodInfo method = property.GetGetMethod(nonPublic); if (method is null && !nonPublic && allowInternal) { method = property.GetGetMethod(true); if (method is null && !( method.IsAssembly // <= || method.IsFamilyOrAssembly)) { method = null; } } return method; }
V3080 Possible null dereference. Consider inspecting ‘method’. Helpers.cs 74
В данном случае допущена ошибка в логике условного выражения. Получается, что разработчик проверяет значение локальной переменной method на равенство null, после чего производит обращение к нему. В итоге это может привести к исключению NullReferenceException.
Взглянем на название рассматриваемого метода. В нём можно заметить слово Get. Логично предположить, что должен быть и Set. Метод GetSetMethod действительно существует. Причём в нём есть такая же ошибка. Кстати, в Best Warnings есть предупреждение и о ней. Эх, Copy-Paste.
Преждевременное использование
public EnumMemberSerializer(Type enumType) { if (!enumType.IsEnum) // <= ThrowHelper.ThrowInvalidOperationException("...." + enumType.NormalizeName()); ExpectedType = enumType ?? // <= throw new ArgumentNullException(nameof(enumType)); _tail = Type.GetTypeCode(enumType) switch { .... }; if (_tail is null) ThrowHelper.ThrowInvalidOperationException("...." + enumType.NormalizeName()); }
V3095 The ‘enumType’ object was used before it was verified against null. Check lines: 13, 14. EnumMemberSerializer.cs 13
Стоит ли проверять параметр на равенство null? Может, и стоит, но не после его использования :). В данном кейсе разработчик хотел выбросить исключение, если enumType окажется равен null. Исключение действительно будет выброшено, но совсем не то, которое ожидал разработчик.
Итоги
Выходит, что знакомство с анализатором может быть вполне приятным :). При этом найти ошибки можно и без просмотра всего отчёта анализатора. Пользователям очень важно оценить продукт с точки зрения его полезности и лёгкости использования. Именно поэтому мы и сделали механизм фильтрации предупреждений, который ещё и работает в один клик.
Стоит помнить, что это механизм, который предназначен в первую очередь для быстрого и лёгкого знакомства с возможностями анализатора. Best Warnings не является заменой обычной работы с отчётом. Приятного использования :).
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Artem Rovenskii. PVS-Studio and protobuf-net: best warnings are one click away.
ссылка на оригинал статьи https://habr.com/ru/articles/706822/
Добавить комментарий