Зачем нужен статический анализ? Разбираем на примере ошибки из Akka.NET

«Статический анализ нужно использовать регулярно, а не только перед релизами… Чем раньше найдена ошибка, тем дешевле её исправление…» – вы уже слышали это 100 раз. Сегодня ещё раз ответим на вопрос «зачем?». Поможет нам ошибка из проекта Akka.NET.

Ошибка

Начнём с задания. Нужно найти дефект в этом фрагменте кода:

protected override bool ReceiveRecover(object message) {   switch (message)   {     case ShardId shardId:       _shards.Add(shardId);       return true;     case SnapshotOffer offer when (offer.Snapshot is                                     ShardCoordinator.CoordinatorState state):       _shards.UnionWith(state.Shards.Keys.Union(state.UnallocatedShards));       return true;     case SnapshotOffer offer when (offer.Snapshot is State state):       _shards.Union(state.Shards);       _writtenMarker = state.WrittenMigrationMarker;       return true;     case RecoveryCompleted _:       Log.Debug("Recovery complete. Current shards [{0}]. Written Marker {1}",                  string.Join(", ", _shards),                  _writtenMarker);        if (!_writtenMarker)       {         Persist(MigrationMarker.Instance, _ =>         {           Log.Debug("Written migration marker");           _writtenMarker = true;         });       }       return true;     case MigrationMarker _:       _writtenMarker = true;       return true;   }   .... }

Разберём, в чём тут дело.

Тип переменной _shards – HashSet<ShardId>. В приведённом выше коде вызываются несколько методов, которые изменяют состояние этого множества.

HashSet<T>.Add:

_shards.Add(shardId);

HashSet<T>.UnionWith:

_shards.UnionWith(state.Shards.Keys.Union(state.UnallocatedShards));

Однако один из вызовов – неправильный:

_shards.Union(state.Shards);

Он не меняет состояние объекта _shards. Enumerable.Union – метод-расширения из LINQ, который возвращает изменённую коллекцию, а не меняет исходную. Значит, результат вызова метода должен быть или куда-то сохранён, или как-то использован. Этого в коде тоже нет.

Анализатор PVS-Studio выдал такое предупреждение: V3010 The return value of function ‘Union’ is required to be utilized. Akka.Cluster.Sharding EventSourcedRememberEntitiesCoordinatorStore.cs 123

Исправленный код, кстати, выглядит так:

_shards.UnionWith(state.Shards);

Как мы нашли ошибку или в 101-ый раз о пользе статического анализа

У нас на сервере каждую ночь запускается анализ нескольких Open Source проектов. В их числе – Akka.NET. Это помогает:

• дополнительно тестировать анализатор;
• рассказывать о пользе стат. анализа в подобных заметках.

Подробнее про систему писали здесь.

А теперь немного хронологии появления и исправления проблемы.

20.04.2022:

• в dev-ветку проекта Akka.NET попадает код с ошибкой (ссылка на конкретную строку);

21.04.2022:

• код анализируется у нас на сервере, и мне приходит письмо с информацией о предупреждениях;
• я изучаю проблему и открываю issue на GitHub;
• разработчики исправляют ошибку. Ссылка на коммит.

Считаю, что отработали слаженно и оперативно. Разработчикам уважение за быстрый фикс.

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

Что делать?

• Использовать статический анализатор. Загрузить можно здесь. Если применить промокод pvs_akka, триал будет работать 30 дней, а не 7.
• Подписаться на меня в Twitter, если интересует подобный контент.

P.S. Если хотите поделиться статьёй с англоязычной аудиторией, вот ссылка на англ. версию.