# Использование оператора ?\. в foreach: защита от NullReferenceException, которая не работает

Любите оператор '?\.' ? А кто же не любит? Эти лаконичные проверки на null нравятся многим\. Однако сегодня мы поговорим о случае, когда оператор ?\. только создаёт иллюзию безопасности\. Речь пойдёт о его использовании в цикле foreach\.

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

Начать предлагаю с небольшой задачки для самопроверки\. Взгляните на следующий код:

```cpp
void ForeachTest(IEnumerable<String> collection)
{
  // #1
  foreach (var item in collection.NotNullItems())
    Console.WriteLine(item);

  // #2
  foreach (var item in collection?.NotNullItems())
    Console.WriteLine(item);
}
```

Как вы думаете, как отработает каждый из циклов, если _collection_ — _null_? Кажется, что вариант с использованием оператора _?\._ более безопасный\. Но так ли это на самом деле? Название статьи уже должно было заложить в вашу душу сомнения\.

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

Перед тем как начнём погружение, давайте немного определимся с терминологией\. Посмотрим [спецификацию C\#](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/statements#the-foreach-statement), раздел "The foreach statement"\.

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

Выражение, по которому будет идти цикл, обозначено просто как "expression"\. Однако использовать прямой перевод слова — выражение — кажется не лучшей идеей, так как наверняка приведёт к путанице\. Поэтому далее я буду использовать термин "перечислимое выражение", имея в виду "expression" из примера выше\. Просто помните об этом\.

## Почему опасно использовать оператор ?\. в перечислимом выражении цикла foreach

Здесь стоит вспомнить, как работает оператор _?\._

С ним закончим быстро\.

```cpp
var b = a?.Foo();
```

Итак:

* если _a \=\= null_, _b \=\= null_;
* если _a \!\= null_, _b \=\= a\.Foo\(\)_\.

Теперь посмотрим на цикл _foreach_\. Не будем забывать, что это всего лишь горка синтаксического сахара\.

```cpp
void Foo1(IEnumerable<String> collection)
{
  foreach (var item in collection)
    Console.WriteLine(item);
}
```

Если посмотреть [IL](https://pvs-studio.ru/ru/blog/terms/7006/) код, то приведённый выше фрагмент можно переписать на C\#, не используя _foreach_\. Выглядеть он будет примерно следующим образом:

```cpp
void Foo2(IEnumerable<String> collection)
{
  var enumerator = collection.GetEnumerator();
  try
  {
    while (enumerator.MoveNext())
    {
      var item = enumerator.Current;
      Console.WriteLine(item);
    }
  }
  finally
  {
    if (enumerator != null)
    {
      enumerator.Dispose();
    }
  }
}
```

**Примечание**\. В некоторых случаях _foreach_ может раскрываться в другой код\. Например, в код, идентичный тому, что генерируется для цикла _for_\. Однако проблема всё также остаётся актуальной\. Я думаю, возможные оптимизации цикла _foreach_ мы более подробно рассмотрим в отдельной статье\.

Ключевой момент здесь для нас — выражение _collection\.GetEnumerator\(\)_\. Чёрным по белому \(хотя зависит от вашей цветовой схемы\) в коде написано, что происходит разыменование ссылки при вызове метода _GetEnumerator_\. Если эта ссылка будет нулевой, получим исключение типа _NullReferenceException_\.

Теперь рассмотрим, что же происходит при использовании оператора _?\._ в перечислимом выражении _foreach_:

```cpp
static void Foo3(Wrapper wrapper)
{
  foreach (var item in wrapper?.Strings)
    Console.WriteLine(item);
}
```

Этот код можно переписать примерно следующим образом:

```cpp
static void Foo4(Wrapper wrapper)
{
  IEnumerable<String> strings;
  if (wrapper == null)
  {
    strings = null;
  }
  else
  {
    strings = wrapper.Strings;
  }

  var enumerator = strings.GetEnumerator();
  try
  {
    while (enumerator.MoveNext())
    {
      var item = enumerator.Current;
      Console.WriteLine(item);
    }
  }
  finally
  {
    if (enumerator != null)
    {
      enumerator.Dispose();
    }
  }
}
```

Здесь, как и в прошлом случае, вызывается метод _GetEnumerator_ \(_strings\.GetEnumerator_\)\. Однако обратите внимание на то, что значение _strings_ может иметь значение _null_, если _wrapper_ — _null_\. В принципе, это ожидаемо, если вспомнить, как работает оператор _?\._ \(мы обсуждали это выше\)\. В таком случае при попытке вызова метода _string\.GetEnumerator\(\)_ получим исключение типа _NullReferenceException_\.

Именно поэтому использование оператора _?\._ в перечислимом выражении _foreach_ не защищает от разыменования нулевых ссылок, а только создаёт иллюзию безопасности\.

## Как пришла идея доработки анализатора?

Приходит ко мне коллега и говорит — вот код, ошибку не ловим\. Я в удивлении\. Точно помню, как я сам предлагал на доработку случай, когда в перечислимом выражении _foreach_ может быть значение _null_\. Проверяю\. Действительно, на представленном ниже коде анализатор не срабатывает\.

```cpp
void Test1(IEnumerable<String> collection, 
          Func<String, bool> predicate)
{
  foreach (var item in collection?.Where(predicate))
    Console.WriteLine(item);
}
```

И на таком — тоже\.

```cpp
void Test2(IEnumerable<String> collection, 
          Func<String, bool> predicate)
{
  var query = collection?.Where(predicate);
  foreach (var item in query)
    Console.WriteLine(item);
}
```

А вот на таком фрагменте кода предупреждение уже есть\.

```cpp
void Test3(IEnumerable<String> collection, 
          Func<String, bool> predicate,
          bool flag)
{
  var query = collection != null ? collection.Where(predicate) : null;
  foreach (var item in query)
    Console.WriteLine(item);
}
```

**Предупреждение PVS\-Studio**: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'query'\.

На следующий код предупреждение также будет выдано\.

```cpp
IEnumerable<String> GetPotentialNull(IEnumerable<String> collection,
                                     Func<String, bool> predicate,
                                     bool flag)
{
  return collection != null ? collection.Where(predicate) : null;
}

void Test4(IEnumerable<String> collection, 
          Func<String, bool> predicate,
          bool flag)
{
  foreach (var item in GetPotentialNull(collection, predicate, flag))
    Console.WriteLine(item);
}
```

**Предупреждение PVS\-Studio**: [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetPotentialNull\(\.\.\.\)\.

Почему же в методах _Test3_ и _Test4_ предупреждение выдаётся, а в _Test1_ и _Test2_ — нет? Дело в том, что анализатор разделяет эти ситуации:

* анализатор не выдавал предупреждение, если в переменную записывался результат работы оператора _?\._;
* если же использовалось выражение, в которое где\-то явно записывалось значение _null_, предупреждение выдавалось\.

Сделано это разделение для того, чтобы анализатор смог более точно обработать каждую ситуацию:

* выдаётся более точное предупреждение;
* возможность отдельно обрабатывать эти случаи \(повышать / понижать уровень, подавлять / не подавлять и т\.п\.\);
* отдельная документация, более точно объясняющая суть проблемы\.

## Какие диагностики были улучшены?

По результатам были доработаны 2 диагностических правила: [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) и [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/)\.

[V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) теперь обнаруживает подозрительные фрагменты кода, когда в переменную записывается результат работы оператора _?\._, а далее эта переменная используется в качестве перечислимого выражения _foreach_\.

```cpp
void Test(IEnumerable<String> collection, 
          Func<String, bool> predicate)
{
  var query = collection?.Where(predicate);
  foreach (var item in query)
    Console.WriteLine(item);
}
```

**Предупреждение PVS\-Studio**: [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'query' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\.

[V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) теперь обнаруживает случаи, когда оператор _?\._ используется прямо в перечислимом выражении _foreach_\.

```cpp
void Test(IEnumerable<String> collection, 
          Func<String, bool> predicate)
{
  foreach (var item in collection?.Where(predicate))
    Console.WriteLine(item);
}
```

**Предупреждение PVS\-Studio**: [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. Consider inspecting: collection?\.Where\(predicate\)\.

## Примеры обнаруженных проблем

Самое приятное в доработках анализатора — когда видишь профит от них\. Как мы неоднократно говорили, мы тестируем анализатор, в том числе и на базе open source проектов\. И после правок [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) и [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) удалось найти несколько новых срабатываний\!

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

### RavenDB

```cpp
private void HandleInternalReplication(DatabaseRecord newRecord, 
                                       List<IDisposable> instancesToDispose)
{
  var newInternalDestinations =
        newRecord.Topology?.GetDestinations(_server.NodeTag,
                                            Database.Name,
                                            newRecord.DeletionInProgress,
                                            _clusterTopology,
                                            _server.Engine.CurrentState);
  var internalConnections 
        = DatabaseTopology.FindChanges(_internalDestinations, 
                                       newInternalDestinations);

  if (internalConnections.RemovedDestiantions.Count > 0)
  {
    var removed = internalConnections.RemovedDestiantions
                                     .Select(r => new InternalReplication
      {
        NodeTag = _clusterTopology.TryGetNodeTagByUrl(r).NodeTag,
        Url = r,
        Database = Database.Name
      });

    DropOutgoingConnections(removed, instancesToDispose);
  }
  if (internalConnections.AddedDestinations.Count > 0)
  {
    var added = internalConnections.AddedDestinations
                                   .Select(r => new InternalReplication
    {
      NodeTag = _clusterTopology.TryGetNodeTagByUrl(r).NodeTag,
      Url = r,
      Database = Database.Name
    });
    StartOutgoingConnections(added.ToList());
  }
  _internalDestinations.Clear();
  foreach (var item in newInternalDestinations)
  {
    _internalDestinations.Add(item);
  }
}
```

Специально привёл весь фрагмент кода целиком\. Согласитесь, проблема не то чтобы очень заметна\. Конечно, если знать, что ищешь, то дело идёт проще\. ;\) 

Если код упростить, проблема становится более очевидной\.

```cpp
private void HandleInternalReplication(DatabaseRecord newRecord, 
                                       List<IDisposable> instancesToDispose)
{
  var newInternalDestinations = newRecord.Topology?.GetDestinations(....);
  ....
  foreach (var item in newInternalDestinations)
    ....
}
```

В переменную _newInternalDestinations_ записали результат работы оператора _?\._\. Если _newRecord\.Topology_ — _null_, _newInternalDestinations_ также будет иметь значение _null_\. При попытке выполнения оператора _foreach_ будет выброшено исключение типа _NullReferenceException_\.

**Предупреждение PVS\-Studio**: [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/) The 'newInternalDestinations' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. ReplicationLoader\.cs 828

Что ещё интересно, эта же переменная — _newInternalDestinations_ — передаётся в метод _DatabaseTopology\.FindChanges_, где она проверяется на _null_ \(параметр _newDestinations_\):

```cpp
internal static 
(HashSet<string> AddedDestinations, HashSet<string> RemovedDestiantions)
FindChanges(IEnumerable<ReplicationNode> oldDestinations, 
            List<ReplicationNode> newDestinations)
{
  ....
  if (newDestinations != null)
  {
    newList.AddRange(newDestinations.Select(s => s.Url));
  }
  ....
}
```

### MSBuild

```cpp
public void LogTelemetry(string eventName, 
                         IDictionary<string, string> properties)
{
  string message 
           = $"Received telemetry event '{eventName}'{Environment.NewLine}";

  foreach (string key in properties?.Keys)
  {
    message += $"  Property '{key}' = '{properties[key]}'{Environment.NewLine}";
  }
  ....
}
```

**Предупреждение PVS\-Studio**: [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. Consider inspecting: properties?\.Keys\. MockEngine\.cs 159

Здесь оператор _?\._ используется напрямую в перечислимом выражении _foreach_\. Возможно, разработчик думал, что так будет безопаснее\. Но мы\-то с вами знаем, что безопаснее не будет\. ;\)

### Nethermind

Пример похож на предыдущий\. 

```cpp
public NLogLogger(....)
{
  ....

  foreach (FileTarget target in global::NLog.LogManager
                                            .Configuration
                                           ?.AllTargets
                                            .OfType<FileTarget>())
  {
    ....
  }
  ....
}
```

**Предупреждение PVS\-Studio**: [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. NLogLogger\.cs 50

Разработчики также решили "обезопаситься" от _NullReferenceException_ и использовали оператор _?\._ прямо в перечислимом выражении _foreach_\. Возможно, им повезёт и свойство _Configuration_ никогда не вернёт _null_\. Иначе настанет час, когда этот код "выстрелит"\.

### Roslyn

```cpp
private ImmutableArray<char>
GetExcludedCommitCharacters(ImmutableArray<RoslynCompletionItem> roslynItems)
{
  var hashSet = new HashSet<char>();
  foreach (var roslynItem in roslynItems)
  {
    foreach (var rule in roslynItem.Rules?.FilterCharacterRules)
    {
      if (rule.Kind == CharacterSetModificationKind.Add)
      {
        foreach (var c in rule.Characters)
        {
          hashSet.Add(c);
        }
      }
    }
  }

  return hashSet.ToImmutableArray();
}
```

**Предупреждение PVS\-Studio**: [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. CompletionSource\.cs 482

Ну здорово же, а? Люблю, когда PVS\-Studio находит интересные места в компиляторах или других анализаторах\.

### PVS\-Studio

А теперь самое время сказать, что вообще\-то мы и сами не без греха, и тоже прошлись по этим же граблям\. :\) 

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

Мы регулярно проверяем PVS\-Studio с помощью PVS\-Studio\. Алгоритм работы примерно такой:

* ночью на сборочном сервере собирается новая версия дистрибутива анализатора\. В неё попадают правки, которые были заложены в основную ветку в течение дня;
* этой новой версией проверяется код различных проектов, в том числе — самого PVS\-Studio;
* информация о выданных анализатором предупреждениях рассылается разработчикам и руководителям с помощью [утилиты BlameNotifier](https://pvs-studio.ru/ru/docs/manual/0050/);
* найденные предупреждения исправляются\.

И вот, после того как были доработаны [V3153](https://pvs-studio.ru/ru/docs/warnings/v3153/) и [V3105](https://pvs-studio.ru/ru/docs/warnings/v3105/), мы обнаружили, что и на нашем коде появилось несколько предупреждений\. Действительно, нашлись и прямые использования оператора _?\._ в перечислимом выражении _foreach_, и косвенные \(с записью в переменную\)\. Повезло нам, что код в этих местах не падал\. В любом случае, предупреждения уже учтены, а соответствующие места исправлены\. ;\)

Пример кода, на который было выдано предупреждение:

```cpp
public override void
VisitAnonymousObjectCreationExpression(
  AnonymousObjectCreationExpressionSyntax node)
{
  foreach (var initializer in node?.Initializers)
    initializer?.Expression?.Accept(this);
}
```

Да, операторов _?\._ здесь от души — найдите тот, который прострелит ногу\. Казалось бы, максимум безопасности \(прочитать голосом озвучки нанокостюма из Crysis\), а на деле — нет\.

## Можно ли использовать оператор ?\. в перечислимом выражении цикла foreach безопасно?

Конечно, можно\. И мы встречали такие примеры кода\. Например, на помощь может прийти оператор _??_\.

Следующий код опасен и грозит потенциальным исключением типа _NullReferenceException_:

```cpp
static void Test(IEnumerable<String> collection,
                 Func<String, bool> predicate)
{
  foreach (var item in collection?.Where(predicate))
    Console.WriteLine(item);
}
```

А на таком варианте кода исключение уже выброшено не будет:

```cpp
static void Test(IEnumerable<String> collection,
                 Func<String, bool> predicate)
{
  foreach (var item in    collection?.Where(predicate) 
                       ?? Enumerable.Empty<String>())
  {
    Console.WriteLine(item);
  }
}
```

Если оператор _?\._ в результате своей работы вернёт значение _null_, то результатом работы оператора _??_ будет выражение _Enumerable\.Empty<String\>\(\)_ — следовательно, исключение выброшено не будет\. Но, конечно, стоит подумать, не проще ли будет добавить явную проверку на _null_\.

```cpp
static void Test(IEnumerable<String> collection,
                 Func<String, bool> predicate)
{
  if (collection != null)
  {
    foreach (var item in collection.Where(predicate))
      Console.WriteLine(item);
  }
}
```

Выглядит не так модно, конечно, но, может, даже более очевидно и понятно\.

## Разбираем задачку из начала статьи

Напоминаю, что мы анализировали следующий код\.

```cpp
void ForeachTest(IEnumerable<String> collection)
{
  // #1
  foreach (var item in collection.NotNullItems())
    Console.WriteLine(item);

  // #2
  foreach (var item in collection?.NotNullItems())
    Console.WriteLine(item);
}
```

Теперь вы знаете, что вариант \#2 совсем не безопасный и не защищает от _NullReferenceException_\. А что же с вариантом \#1? На первый взгляд, кажется, что здесь также будет выброшено исключение _NullReferenceException_ — при вызове _collection\.NotNullItems\(\)_\. А вот и не факт\! Предположим, что _NotNullItems_ — метод расширения, имеющий следующее тело:

```cpp
public static IEnumerable<T>
NotNullItems<T>(this IEnumerable<T> collection) where T : class
{
  if (collection == null)
    return Enumerable.Empty<T>();

  return collection.Where(item => item != null);
}
```

Как мы видим, в метод зашита проверка на случай, когда _collection_ имеет значение _null_\. Так как в этом случае возвращается значение _Enumerable\.Empty<T\>\(\)_, при попытке его обхода в цикле _foreach_ никакого исключения выброшено не будет\. То есть цикл \#1 отработает успешно, даже если _collection_ — _null_\.

А вот второй цикл как был опасным, так и остался\. Если _collection_ — _null_, метод _NotNullItems_ вызван не будет\. Следовательно, не сработает и заложенная в него защита от случая, когда _collection_ — _null_\. В итоге, получаем всё ту же ситуацию, которую мы неоднократно рассматривали: попытка вызова метода _GetEnumerator\(\)_ через нулевую ссылку\.

Такой вот интересный момент выходит, когда явный вызов метода _collection\.NotNullItems\(\)_ защищает от исключения _NullReferenceException_, а 'безопасный вызов' — _collection?\.NotNullItems\(\)_ — не защищает\. 

## Заключение

Выводов здесь несколько:

* не используйте оператор _?\._ в перечислимом выражении _foreach_ прямо или косвенно — это только иллюзия безопасности;
* используйте статический анализатор регулярно\.

Для себя же мы ещё раз отметили, что полезно не только разрабатывать новые диагностики, но и дорабатывать текущие\.

Обсуждаемые правки вошли в релиз PVS\-Studio 7\.13\. Интересно посмотреть, не использует ли кто\-нибудь оператор _?\._ в перечислимых выражениях на вашей кодовой базе? Тогда приглашаю [загрузить дистрибутив анализатора с сайта](https://pvs-studio.ru/ru/pvs-studio/download/) и проверить интересующий код\.

И, как обычно, приглашаю подписываться на мой [Twitter\-аккаунт](https://twitter.com/_SergVasiliev_)\.