# PVS\-Studio и protobuf\-net: лучшие предупреждения в один клик

Разбираем best practice знакомства с PVS\-Studio\. Покажем быстрый старт работы с анализатором на примере проекта protobuf\-net\.

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

## О чём речь?

Первое знакомство со статическим анализатором может стать тяжёлой задачкой, особенно на крупных проектах\. Отчёт анализатора может содержать сотни, а иногда и тысячи предупреждений\. Просматривать все будет действительно трудно\. Вот именно для таких ситуаций и создавался механизм Best Warnings\. Он поможет в один клик увидеть самые лучшие предупреждения – те, которые вероятнее всего указывают на ошибку\.

Действительно ли в один клик? Действительно\! В окне PVS\-Studio рядом с уровнями предупреждений есть кнопка 'Best'\. Именно она и отвечает за показ лучших, по мнению анализатора, предупреждений\.

Вот так выглядит окно PVS\-Studio 7\.22 в Visual Studio 2022:

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

При активации Best Warnings окно выглядит следующим образом:

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

С версии PVS\-Studio 7\.22 данный механизм реализован и в других IDE:

* Rider;
* CLion;
* IntelliJ IDEA\.

## Демонстрация работы

Показать возможности данного механизма легче всего на примере Open Source проекта\. Для демонстрации Best Warnings был выбран [protobuf\-net](https://github.com/protobuf-net/protobuf-net)\. Если говорить простыми словами, то это сериализатор, который преобразует данные в формат [Protocol Buffers](https://en.wikipedia.org/wiki/Protocol_Buffers)\. Для анализа был взят последний релиз версии [3\.1\.4](https://github.com/protobuf-net/protobuf-net/releases/tag/3.1.4)\.

Стоит сказать, что мы не будем рассматривать все предупреждения, которые были получены в результате фильтрации с помощью Best Warnings\. Поглядим только на самые интересные и явно ошибочные места в коде\. Вы же можете попробовать на своём проекте данный функционал в полном объёме\. Потребуется всего лишь взять пробный ключ [здесь](https://pvs-studio.ru/ru/pvs-studio/try-free/)\.

Кстати, для всех рассмотренных фрагментов кода был создан [issue](https://github.com/protobuf-net/protobuf-net/issues/972) на GitHub\. Разработчик проекта ответил на него и подтвердил, что описанные ошибки нужно исправить\.

Перейдём ближе к коду и ошибкам\.

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

### Нужно больше проверок

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/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? Используй\!

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'method'\. Helpers\.cs 74

В данном случае допущена ошибка в логике условного выражения\. Получается, что разработчик проверяет значение локальной переменной _method_ на равенство _null_, после чего производит обращение к нему\. В итоге это может привести к исключению _NullReferenceException_\.

Взглянем на название рассматриваемого метода\. В нём можно заметить слово Get\. Логично предположить, что должен быть и Set\. Метод _GetSetMethod_ действительно существует\. Причём в нём есть такая же ошибка\. Кстати, в Best Warnings есть предупреждение и о ней\. Эх, Copy\-Paste\. 

### Преждевременное использование

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v3095/) The 'enumType' object was used before it was verified against null\. Check lines: 13, 14\. EnumMemberSerializer\.cs 13

Стоит ли проверять параметр на равенство _null_? Может, и стоит, но не после его использования :\)\. В данном кейсе разработчик хотел выбросить исключение, если _enumType_ окажется равен _null_\. Исключение действительно будет выброшено, но совсем не то, которое ожидал разработчик\.

## Итоги

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

Стоит помнить, что это механизм, который предназначен в первую очередь для быстрого и лёгкого знакомства с возможностями анализатора\. Best Warnings не является заменой обычной работы с отчётом\. Приятного использования :\)\.