Код в одну строку или проверка Nethermind с помощью PVS-Studio C# для Linux

• Введение
• PVS-Studio C# для Linux/macOS
• Немного негодования
• Результаты проверки
  • Условия, которые не любят 0
  • Ошибка или недоделанная оптимизация?
  • Разыменования нулевой ссылки
  • Наш любимый Copy-Paste
  • Путающая проверка
  • Приоритет ??
• Заключение

Данная статья приурочена к старту бета-теста PVS-Studio C# для Linux, а также плагина для Rider. По такому прекрасному поводу с помощью данных инструментов была проведена проверка исходного кода продукта Nethermind и в данной статье мы посмотрим на интересные, а иногда и забавные ошибки.

Nethermind — это быстрый клиент для .NET Core Ethereum для Linux, Windows, macOS. Он может быть использован в проектах при настройке частных сетей Ethereum или dApps. Nethermind имеет открытый исходный код расположенный на GitHub. Проект основан в 2017 году и постоянно развивается.

Введение

Любите ли вы ручной труд? Например, такой как поиск ошибок в коде программы. Согласитесь, это довольно утомительно читать и анализировать написанный вами участок или целый проект в поиске какого-то каверзного бага. Ладно если проект небольшой, ну скажем 5000 строчек, а если его размер уже перевалил за сто тысяч или за миллион строк? При этом написан он разными программистами и иногда не в очень удобоваримом виде. Что делать в этом случае? Неужели придется где-то недоспать, где-то недоесть и 100% своего времени осознавать все эти нескончаемые строки, чтобы понять, где же эта мерзкая ошибка? Сомневаюсь, что вам хотелось бы заниматься подобным. Так что же делать, может быть есть современные средства чтобы это как-то автоматизировать?

И тут вступает в игру такой инструмент, как статический анализатор кода. Статический анализатор – это инструмент для выявления дефектов в исходном коде программ. Преимущество данного инструмента над ручной проверкой заключается в следующем:

• почти не тратит ваше время на поиск места нахождения ошибки (по крайней мере это точно быстрее, чем искать неудачный copy-paste глазами);
• не устает, в отличии от человека, которому после некоторого времени поиска потребуется отдых;
• знает множество шаблонов ошибок, о которых человек может даже и не догадываться;
• использует такие технологии как: анализ потока данных, символьное выполнение, сопоставление с шаблоном и другие;
• позволяет регулярно и в любое время проводить анализ;
• и т.д.

Конечно, использование статического анализатора кода не заменяет и не отменяет обзоры кода. Зато обзоры кода становятся более продуктивными и полезными. Можно сосредоточиться на поиске высокоуровневых ошибок, на передаче знаний, а не утомительно вычитывать код в поисках опечатки.

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

PVS-Studio C# для Linux/macOS

В данный момент ведется портирование нашего C# анализатора под .NET Core, а также идет активная разработка плагина для IDE Rider.

Если вам это интересно, то записаться на бета-тест можно, заполнив форму на этой странице. На указанную вами почту придет инструкция по установке (не пугайтесь, она очень простая), а также лицензия на использование анализатора.

Вот так выглядит Rider с плагином PVS-Studio:

Немного негодования

Хочу сказать, что местами было очень сложно читать исходный код Nethermind, потому что в нем вполне нормальным являются строчки длиной по 300-500 символов. Да-да именно весь код без форматирования в 1 строку. И в этих строках, например, содержатся и несколько тернарных операторов, и логических операторов, и что там только нет. В общем наслаждение как от последнего сезона игры престолов.

Немного поясню для понимания масштабов. У меня UltraWide монитор с длиной около 82 сантиметров. Открывая на нем IDE на весь экран, помещается около 340 символов, то есть строки, о которых я говорю даже не влезают. Если захотите посмотреть, как это выглядит, я оставил ссылки файлы на GitHub:

Пример 1

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    string authorString = (block.Author == null ? null : "sealed by " +
(KnownAddresses.GoerliValidators.ContainsKey(block.Author) ?
KnownAddresses.GoerliValidators[block.Author] : block.Author?.ToString())) ??
(block.Beneficiary == null ? string.Empty : "mined by " +
(KnownAddresses.KnownMiners.ContainsKey(block.Beneficiary) ?
KnownAddresses.KnownMiners[block.Beneficiary] : block.Beneficiary?.ToString()));
    if (_logger.IsInfo)
    {
        if (_logger.IsInfo) _logger.Info($"Discovered a new block
{string.Empty.PadLeft(9 - block.Number.ToString().Length, '
')}{block.ToString(Block.Format.HashNumberAndTx)} {authorString}, sent by
{syncPeer:s}");
    }
}

Ссылка на файл.

Пример 2

private void BuildTransitions()
{
    ...
    releaseSpec.IsEip1283Enabled = (_chainSpec.Parameters.Eip1283Transition ??
long.MaxValue) <= releaseStartBlock &&
((_chainSpec.Parameters.Eip1283DisableTransition ?? long.MaxValue) 
> releaseStartBlock || (_chainSpec.Parameters.Eip1283ReenableTransition ??
long.MaxValue) <= releaseStartBlock);           
    ...
}

Ссылка на файл.

public void 
Will_not_reject_block_with_bad_total_diff_but_will_reset_diff_to_null()
{
    ...
    _syncServer = new SyncServer(new StateDb(), new StateDb(), localBlockTree,
NullReceiptStorage.Instance, new BlockValidator(Always.Valid, new
HeaderValidator(localBlockTree, Always.Valid, MainnetSpecProvider.Instance,
LimboLogs.Instance), Always.Valid, MainnetSpecProvider.Instance, 
LimboLogs.Instance), Always.Valid, _peerPool, StaticSelector.Full, 
new SyncConfig(), LimboLogs.Instance);
    ...     
}

Ссылка на файл.

А теперь представьте, что в таком участке случится ошибка, приятно ли будет ее искать? Уверен, что нет, и все прекрасно понимают, что так писать нельзя. И кстати подобное место с ошибкой в данном проекте есть.

Результаты проверки

Условия, которые не любят 0

Условие 1

public ReceiptsMessage Deserialize(byte[] bytes)
{
    if (bytes.Length == 0 && bytes[0] == Rlp.OfEmptySequence[0])
        return new ReceiptsMessage(null);
    ...
}

Предупреждение PVS-Studio: V3106 Possibly index is out of bound. The '0' index is pointing beyond 'bytes' bound. Nethermind.Network ReceiptsMessageSerializer.cs 50

Чтобы заметить ошибку рассмотрим случай, когда число элементов в массиве будет равно 0. Тогда условие bytes.Length == 0 будет истинным и при обращении к 0 элементу массива возникнет исключение типа IndexOutOfRangeException.

Из данного метода хотели выходить сразу, если массив пустой или 0 элемент равен определенному значению, но похоже случайно перепутали "||" с "&&". Предлагаю исправить данную проблему следующим образом:

public ReceiptsMessage Deserialize(byte[] bytes)
{
    if (bytes.Length == 0 || bytes[0] == Rlp.OfEmptySequence[0])
        return new ReceiptsMessage(null);
    ...
}

Условие 2

public void DiscoverAll()
{
    ...
    Type? GetStepType(Type[] typesInGroup)
    {
        Type? GetStepTypeRecursive(Type? contextType)
        {
            ...
        }
        ...
        return typesInGroup.Length == 0 ? typesInGroup[0] :
               GetStepTypeRecursive(_context.GetType());
    }
    ...
}

Предупреждение PVS-Studio: V3106 Possibly index is out of bound. The '0' index is pointing beyond 'typesInGroup' bound. Nethermind.Runner EthereumStepsManager.cs 70

Происходит ситуация подобная той, что описана выше. Если количество элементов в typesInGroup будет равно 0, то при обращении к 0 элементу возникнет исключение типа IndexOutOfRangeException.

Только в этом случае я не понимаю, что хотел разработчик. Скорее всего вместо typesInGroup[0] нужно написать null.

Ошибка или недоделанная оптимизация?

private void DeleteBlocks(Keccak deletePointer)
{
   ...
   if (currentLevel.BlockInfos.Length == 1)
   {
      shouldRemoveLevel = true;
   }
   else
   {
      for (int i = 0; i < currentLevel.BlockInfos.Length; i++)
      {
         if (currentLevel.BlockInfos[0].BlockHash == currentHash) // <=
         {
            currentLevel.BlockInfos = currentLevel.BlockInfos
                                      .Where(bi => bi.BlockHash != currentHash)
                                      .ToArray();
            break;
         }
      }
   }
   ...
}

Предупреждение PVS-Studio: V3102 Suspicious access to element of 'currentLevel.BlockInfos' object by a constant index inside a loop. Nethermind.Blockchain BlockTree.cs 895

На первый взгляд ошибка явная – цикл нацелен для перебора элементов currentLevel.BlockInfos, но при обращении вместо currentLevel.BlockInfos[i] написали currentLevel.BlockInfos[0]. Исправляем 0 на i и миссия выполнена. Но не спешите, давайте разберемся.

Сейчас мы Length раз обращаемся к BlockHash нулевого элемента. Eсли он равен currentHash, то берем из currentLevel.BlockInfos все элементы которые не равны currentHash, записываем в него же и выходим из цикла. Получается, что цикл лишний.

Я думаю, что раньше тут был алгоритм, который решили изменить/оптимизировать с помощью linq, но что-то пошло не так. Теперь в случае, когда условие будет ложно, мы получим бессмысленные итерации.

Кстати, если бы разработчик, который это писал, использовал режим инкрементального анализа, то он сразу понял, что что-то не так и быстренько все бы поправил. На данный момент я бы переписал код вот так:

private void DeleteBlocks(Keccak deletePointer)
{
    ...
    if (currentLevel.BlockInfos.Length == 1)
    {
        shouldRemoveLevel = true;
    }
    else
    {
        currentLevel.BlockInfos = currentLevel.BlockInfos
                                  .Where(bi => bi.BlockHash != currentHash)
                                  .ToArray();
    }
    ...
}

Разыменования нулевой ссылки

Разыменование 1

public void Sign(Transaction tx, int chainId)
{
    if (_logger.IsDebug)
        _logger?.Debug($"Signing transaction: {tx.Value} to {tx.To}");
    IBasicWallet.Sign(this, tx, chainId);
}

Предупреждение PVS-Studio: V3095 The '_logger' object was used before it was verified against null. Check lines: 118, 118. Nethermind.Wallet DevKeyStoreWallet.cs 118

Ошибка в неправильной последовательности. Сначала идет обращение к _logger.IsDebug и только после этого идет проверка _logger на null. Соответственно в случае, когда _logger равен null, мы получим исключение типа NullReferenceException.

Разыменование 2

private void BuildNodeInfo()
{
    _nodeInfo = new NodeInfo();
    _nodeInfo.Name = ClientVersion.Description;
    _nodeInfo.Enode = _enode.Info;                           // <=
    byte[] publicKeyBytes = _enode?.PublicKey?.Bytes;        // <=
    _nodeInfo.Id = (publicKeyBytes == null ? Keccak.Zero :
                   Keccak.Compute(publicKeyBytes)).ToString(false);
    _nodeInfo.Ip = _enode?.HostIp?.ToString();
    _nodeInfo.ListenAddress = $"{_enode.HostIp}:{_enode.Port}";
    _nodeInfo.Ports.Discovery = _networkConfig.DiscoveryPort;
    _nodeInfo.Ports.Listener = _networkConfig.P2PPort;
    UpdateEthProtocolInfo();
}

Предупреждение PVS-Studio: V3095 The '_enode' object was used before it was verified against null. Check lines: 55, 56. Nethermind.JsonRpc AdminModule.cs 55

Ошибка полностью аналогично описанной выше, только на этот раз виновником является _enode.

Хочу добавить, что если вы забыли проверить на null, то вспомните об этом только тогда, когда ваша программа уже упадет. Анализатор же вам напомнит об этом и все будет хорошо.

Наш любимый Copy-Paste

Случай 1

public static bool Equals(ref UInt256 a, ref UInt256 b)
{
    return a.s0 == b.s0 && a.s1 == b.s1 && a.s2 == b.s2 && a.s2 == b.s2;
}

Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'a.s2 == b.s2' to the left and to the right of the '&&' operator. Nethermind.Dirichlet.Numerics UInt256.cs 1154

Здесь 2 раза проверяется одно и тоже условие:

a.s2 == b.s2

Так как у параметров a и b имеется поле s3, то предполагаю, что при копировании просто забыли изменить s2 на s3.

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

Кстати, ошибки в функциях сравнения это вообще классика. Видимо программисты, считая такие функции простыми, относятся к их написанию очень небрежно и невнимательно. Proof. Зная это, будьте теперь бдительны :)!

Случай 2

public async Task<ApiResponse> 
PublishBlockAsync(SignedBeaconBlock signedBlock,
                  CancellationToken cancellationToken)
{
    bool acceptedLocally = false;
    ...
    if (acceptedLocally)
    {
        return new ApiResponse(StatusCode.Success);
    }
    else
    {
        return new ApiResponse(StatusCode.Success);
    }
    ...
}

Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. Nethermind.BeaconNode BeaconNodeFacade.cs 177

При любом значении переменной acceptedLocally метод возвращает одно и тоже. Сложно сказать ошибка это или нет. Допустим программист скопировал строчку и забыл поменять StatusCode.Success на что-то другое, то тогда это самая настоящая ошибка. Тем более что у StatusCode имеется InternalError и InvalidRequest. Но возможно виной всему рефакторинг кода и нам уже все равно на значение acceptedLocally, в этом случае условие становится тем местом, которое заставляет сидеть и думать ошибка это или нет. Так что при любом раскладе данный случай крайне неприятный.

Случай 3

public void TearDown()
{
    ...
    foreach (var testResult in _results)
    {
         string message = $"{testResult.Order}. {testResult.Name} has " 
               + $"{(testResult.Passed ? "passed [+]" : "failed [-]")}";
         if (testResult.Passed)
         {
               TestContext.WriteLine(message);
         }
         else
         {
               TestContext.WriteLine(message);
         }
     }
}

Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. Nethermind.Overseer.Test TestBuilder.cs 46

И снова нам все равно на проверку, ведь в результате получаем одно и тоже. И снова сидим и мучаемся, думая, а что же тут хотел написать разработчик. Бессмысленная трата времени, которую можно было избежать, используя статический анализ и сразу исправляя такой неоднозначный код.

Случай 4

public void Setup()
{
    if (_decoderBuffer.ReadableBytes > 0)
    {
        throw new Exception("decoder buffer");
    }

    if (_decoderBuffer.ReadableBytes > 0)
    {
        throw new Exception("decoder buffer");
    }
    ...
}

Предупреждение PVS-Studio: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless Nethermind.Network.Benchmark InFlowBenchmarks.cs 55

Просто лишний раз нажали Ctrl+V. Удаляем лишнюю проверку и все хорошо. Уверен, что если бы тут было важно еще какое-то условие, то все бы написали в одном if через логический оператор И.

Случай 5

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    if (_logger.IsInfo)
    {
        if (_logger.IsInfo)
        {
            ...
        }
    }
}

Предупреждение PVS-Studio: V3030 Recurring check. The '_logger.IsInfo' condition was already verified in line 242. Nethermind.Synchronization SyncServer.cs 244

Так же, как и в четвертом случае совершается лишняя проверка. Однако отличие в том, что у _logger есть не только одно свойство, а еще например 'bool IsError { get; }'. Поэтому код, вероятно, должен выглядеть так:

private void LogBlockAuthorNicely(Block block, ISyncPeer syncPeer)
{
    if (_logger.IsInfo)
    {
        if (!_logger.IsError) // <=
        {
            ...
        }
    }
}

Ну или всему виной рефакторинг кода и просто одна проверка больше не нужна.

Случай 6

if (missingParamsCount != 0)
{
    bool incorrectParametersCount = missingParamsCount != 0; // <=
    if (missingParamsCount > 0)
    {
        ...
    }
    ...
}

Предупреждение PVS-Studio: V3022 Expression 'missingParamsCount != 0' is always true. Nethermind.JsonRpc JsonRpcService.cs 127

Проверяем условие (missingParamsCount != 0) и если оно истинно, то опять вычисляем его и присваиваем результат переменной. Согласитесь, что это достаточно оригинальный способ написать true.

Путающая проверка

public async Task<long> 
DownloadHeaders(PeerInfo bestPeer, 
                BlocksRequest blocksRequest, 
                CancellationToken cancellation)
{
  ...
  for (int i = 1; i < headers.Length; i++)
  {
    ...
    BlockHeader currentHeader = headers[i];
    ...
    bool isValid = i > 1 ? 
        _blockValidator.ValidateHeader(currentHeader, headers[i - 1], false):
        _blockValidator.ValidateHeader(currentHeader, false);
    ...
    if (HandleAddResult(bestPeer, 
                        currentHeader, 
                        i == 0,                              // <=
                        _blockTree.Insert(currentHeader))) 
    {
       headersSynced++;
    }

    ...
  }
  ...
}

Предупреждение PVS-Studio: V3022 Expression 'i == 0' is always false. Nethermind.Synchronization BlockDownloader.cs 192

Начнем по порядку. При инициализации переменной i присваивается значение 1. Далее переменная только инкрементируется, следовательно, в функцию всегда будет передаваться значение false.

Теперь давайте посмотрим на HandleAddResult:

private bool HandleAddResult(PeerInfo peerInfo, 
                             BlockHeader block,
                             bool isFirstInBatch, 
                             AddBlockResult addResult)
{
    ...
    if (isFirstInBatch)
    {
        ...
    }
    else
    {
        ...
    }
    ...
}

Здесь нас интересует isFirstInBatch. Если судить по имени этого параметра, то он отвечает за то, является ли что-то первым в партии. Хм, первым. Посмотрим-ка опять выше и увидим, что имеется 2 обращения с использованием i:

BlockHeader currentHeader = headers[i];
_blockValidator.ValidateHeader(currentHeader, headers[i - 1], false)

Не забываем, что отсчет в данном случае идет с 1. Получается, что у нас 2 варианта: либо под "первым" подразумевается элемент под индексом 1, либо под индексом 0. Но в любом случае при этом i будет равно 1.

Получается, что вызов функции должен выглядеть вот так:

HandleAddResult(bestPeer, currentHeader, 
                i == 1, _blockTree.Insert(currentHeader))

Или вот так:

HandleAddResult(bestPeer, currentHeader, 
                i - 1 == 0, _blockTree.Insert(currentHeader))

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

Приоритет ??

Ситуация 1

public int MemorySize
{
  get
  {
    int unaligned = (Keccak == null ? MemorySizes.RefSize : 
        MemorySizes.RefSize + Keccak.MemorySize) 
        + (MemorySizes.RefSize + FullRlp?.Length 
                                 ?? MemorySizes.ArrayOverhead)   // <=
        + (MemorySizes.RefSize + _rlpStream?.MemorySize 
                                 ?? MemorySizes.RefSize)         // <=
        + MemorySizes.RefSize + (MemorySizes.ArrayOverhead + _data?.Length 
        * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 
        + MemorySizes.SmallObjectOverhead + (Key?.MemorySize ?? 0);
    return MemorySizes.Align(unaligned);
  }
}

Предупреждения PVS-Studio:

• V3123 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. Nethermind.Trie TrieNode.cs 43
• V3123 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. Nethermind.Trie TrieNode.cs 44

Анализатор советует проверить то, как мы используем операторы "??", и, чтобы понять в чем проблема, предлагаю рассмотреть следующую ситуацию. Смотрим вот на эту строчку:

(MemorySizes.RefSize + FullRlp?.Length ?? MemorySizes.ArrayOverhead)

MemorySizes.RefSize и MemorySizes.ArrayOverhead являются константами:

public static class MemorySizes
{
    ...
    public const int RefSize = 8;
    public const int ArrayOverhead = 20;
    ...
}

Поэтому для наглядности предлагаю переписать строку, подставив их значения:

(8 + FullRlp?.Length ?? 20)

Теперь допустим, что FullRlp будет равным null. Тогда (8+null) будет равно null. Далее получаем выражение (null ?? 20), которое вернет 20.

Получается, что при условии, когда FullRlp равно null, всегда будет возвращаться значение из MemorySizes.ArrayOverhead вне зависимости, что хранится в MemorySizes.RefSize. Фрагмент строчкой ниже аналогичен.

Но вопрос в том, этого ли поведения хотел разработчик? Посмотрим на следующую строчку:

MemorySizes.RefSize + (MemorySizes.ArrayOverhead 
    + _data?.Length * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead)

Тут, как и в рассматриваемых выше участках MemorySizes.RefSize складывается с выражением, но

обратите внимания, что после первого оператора "+" стоит скобка. Получается, что именно к MemorySizes.RefSize мы должны прибавлять какое-то выражение, а если оно равно null, то прибавлять уже другое. Значит код должен выглядеть вот так:

public int MemorySize
{
  get
  {
    int unaligned = (Keccak == null ? MemorySizes.RefSize : 
       MemorySizes.RefSize + Keccak.MemorySize) 
       + (MemorySizes.RefSize + (FullRlp?.Length 
                                 ?? MemorySizes.ArrayOverhead))    // <=
       + (MemorySizes.RefSize + (_rlpStream?.MemorySize 
                                 ?? MemorySizes.RefSize))          // <=
       + MemorySizes.RefSize + (MemorySizes.ArrayOverhead + _data?.Length 
       * MemorySizes.RefSize ?? MemorySizes.ArrayOverhead) 
       + MemorySizes.SmallObjectOverhead + (Key?.MemorySize ?? 0);
    return MemorySizes.Align(unaligned);
  }
}

Опять же это только предположение, однако, если бы разработчик хотел другого поведения, тогда следовало явно это указать:

((MemorySizes.RefSize + FullRlp?.Length) ?? MemorySizes.ArrayOverhead)

И тогда, тому кто читает этот код не пришлось бы долго вникать, что же тут происходит, и чего же хотел программист.

Ситуация 2

private async Task<JsonRpcResponse> 
ExecuteAsync(JsonRpcRequest request, 
             string methodName,
             (MethodInfo Info, bool ReadOnly) method)
{
    var expectedParameters = method.Info.GetParameters();
    var providedParameters = request.Params;
    ...
    int missingParamsCount = expectedParameters.Length 
            - (providedParameters?.Length ?? 0) 
            + providedParameters?.Count(string.IsNullOrWhiteSpace) ?? 0; // <=
    if (missingParamsCount != 0)
    {
        ...
    }
    ...
}

Предупреждение PVS-Studio: V3123 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. Nethermind.JsonRpc JsonRpcService.cs 123

И опять приоритет операции "??", поэтому, как и в прошлый раз рассмотрим ситуацию. Смотрим вот на эту строчку:

expectedParameters.Length 
            - (providedParameters?.Length ?? 0) 
            + providedParameters?.Count(string.IsNullOrWhiteSpace) ?? 0;

Допустим, что providedParameters равно null, тогда давайте для наглядности сразу заменим все что связано с providedParameters на null, а вместо expectedParameters.Length подставим случайное значение:

100 - (null ?? 0) + null ?? 0;

Теперь сразу заметно, что имеется две аналогичные проверки, только в одном случае скобки есть, а в другом нет. Давайте выполним данный пример. Сначала получаем, что (null ?? 0) вернет 0, далее из 100 вычитаем 0 и получаем 100:

100 + null ?? 0;

Теперь, вместо того, чтобы как прежде сначала выполнить "null ?? 0" и в итоге получить (100 + 0), мы получим совершенно иное.

Сначала выполнится (100 + null) и мы получаем null. Затем проверяется (null ?? 0), что приводит к тому, что значение переменой missingParamsCount будет равно 0.

Так как дальше идет условие проверяющее неравно ли missingParamsCount нулю, можно предположить, что именно такого поведения разработчик и добивался. А я скажу почему бы тогда не поставить скобки и явно выразить свои мысли? Вообще возможно, что эта проверка возникла из-за непонимания почему иногда возвращается 0, и это ничто иное как костыль.

И снова мы тратим время, хотя могли бы этого не делать, используй разработчик при написании кода режим инкрементального анализа.

Заключение

В заключении я надеюсь, что смог донести до вас то, что статический анализатор — это ваш друг, а не злой надзиратель, который только и ждет чтобы вы ошиблись.

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

Простая истина в том, что все совершают ошибки и это нормально. Все мы учимся на ошибках, но только на тех, которые смогли заметить и понять. Поэтому пользуйтесь современными средствами для поиска этих самых ошибок, например - PVS-Studio. Спасибо за внимание.