# The ?\. operator in foreach will not protect from NullReferenceException

Do you like the ?\. operator? Well, who doesn't? Many people like these concise null checks\. However, today's article shows that the ?\. operator may be tricky sometimes\. That is, it can create an illusion of safety when used in the foreach loop\.

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

Let's start with a small task\. Take a look at the following code:

```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);
}
```

Suppose the _collection_ is _null_\. Have you got any ideas on how each of the loops will run? Case \#2 with _?\._ seems to be safer\. But is that really so? The article's title should have already planted a seed of doubt in your mind\.

In any case, we'll try to figure this out below\. We'll return to this task at the end of the article when we have more information\.

**Note**\. The [C\# Specification](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/statements) uses the term "expression" to denote the following entity\. In this article we use the term "enumerable expression"\. This may help to avoid confusion when we talk about different expressions\.

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

## Why is it dangerous to use the ?\. operator in the foreach loop's enumerable expression?

First, let's recall what the _?\._ operator is\.

It won't take long\.

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

So:

* if _a \=\= null_,_b \=\= null_;
* if _a \!\= null_, _b \=\= a\.Foo\(\)_\.

Now let's take a look at the _foreach_ loop\.

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

[IL](https://pvs-studio.com/en/blog/terms/7006/) code suggests that you can rewrite the above code fragment in C\# without _foreach_\. It would look something like this:

```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();
    }
  }
}
```

**Note**\. In some cases, _foreach _loop's IL code may become identical to the code for the _for _loop\. However, the problem still persists\. I think we'll have another article about the possible optimizations of the _foreach_ loop\.

The _collection\.GetEnumerator\(\)_ is the key element here\. In black and white \(although it depends on your color scheme\), the code says that when the _GetEnumerator_ method is called, the reference is dereferenced\. If this reference is _null_, we get _NullReferenceException_\.

Now let's take a look at what happens in the _foreach_ loop's enumerable expression with the _?\._ operator:

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

We may rewrite this code as follows:

```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();
    }
  }
}
```

As in the previous case, the _GetEnumerator_ \(_strings\.GetEnumerator_\) call occurs\. However, note that the _strings_ value can be _null_ if _wrapper _is _null_\. Well, that's to be expected with the _?\._ operator \(we discussed it earlier\)\. In this case, when trying to call the _string\.GetEnumerator\(\)_ method, we get a _NullReferenceException_\.

That's why the _?\._ operator in the _foreach_ loop's enumerable expression does not protect against null dereference\. It only creates an illusion of safety\.

## What prompted us to improve the analyzer?

Once my colleague came to me and said — here's the code, we can't find the error\. I was surprised\. I recall exactly how I offered to work on the case that involved the _foreach_ loop's enumerable expression having the _null_ value\. Checked it out\. Indeed, the analyzer was not issuing warnings on the code below\.

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

The same was with this code\.

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

However, the analyzer issued a warning on the following code fragment\.

```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 warning**: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'query'\.

The analyzer would also issue a warning on the following code\.

```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 warning**: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetPotentialNull\(\.\.\.\)\.

Why did the analyzer issue warnings for _Test3_ and _Test4_, but not for _Test1_ and _Test2_? The point is that the analyzer sees these cases as different:

* the analyzer did not issue a warning if a variable received the _?\._ operator's result;
* an expression may have the _null_ value\. For example, if a variable directly received _null_ or if a method returned _null_\. In this case, the analyzer issued a warning\.

This differentiation helps the analyzer to handle each situation thoroughly\. So, as a result, the analyzer:

* issues a more accurate warning;
* has the ability to handle these cases separately \(to raise / lower the warning level, to suppress / not suppress, etc\.\);
* has documentation for each case\.

## What diagnostics we refined

As a result, we improved 2 diagnostic rules: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) and [V3153](https://pvs-studio.com/en/docs/warnings/v3153/)\.

[V3105](https://pvs-studio.com/en/docs/warnings/v3105/) now detects suspicious code fragments when a variable contains the result of the _?\._ operator\. Then, the enumerable expression _foreach_ uses this variable\.

```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 warning**: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'query' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\.

[V3153](https://pvs-studio.com/en/docs/warnings/v3153/) now detects cases where the _foreach loop's_ enumerable expression directly uses the _?\._ operator\.

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

**PVS\-Studio warning**: [V3153](https://pvs-studio.com/en/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. Consider inspecting: collection?\.Where\(predicate\)\.

## The improved analyzer detects more issues

It's a great feeling to see the analyzer works better\! As I've already said, we regularly test the analyzer on open\-source projects\. So, after we improved [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) and [V3153](https://pvs-studio.com/en/docs/warnings/v3153/), we managed to find some new triggerings\!

**Note**\. This code was up to date when we added the projects to our tests\. By now the code could have changed and may not contain these code fragments\.

### 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);
  }
}
```

I intentionally listed the entire code fragment\. You will probably agree this issue is not very obvious\. And of course, it's easier to find something if you know what you're looking for\. ;\)

If you simplify the code, the issue becomes more obvious\.

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

The _newInternalDestinations_ variable takes the _?\._ operator's result\. If _newRecord\.Topology_ is _null_, _newInternalDestinations_ will also be _null_\. When the execution flow reaches the _foreach_ loop, the _NullReferenceException_ exception will be thrown\.

**PVS\-Studio warning**: [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'newInternalDestinations' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. ReplicationLoader\.cs 828

What's more interesting, the _DatabaseTopology\.FindChanges_ method takes the _newInternalDestinations_ variable as the _newDestinations_ parameter and checks it for _null_\.

```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 warning**: [V3153](https://pvs-studio.com/en/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. Consider inspecting: properties?\.Keys\. MockEngine\.cs 159

Here the _foreach_ directly contains the _?\._ operator\. Perhaps the developer thought the _?\._ operator would protect from _NullReferenceException_\. But we know that it's not safer\. ;\)

### Nethermind

This example is similar to the previous one\. 

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

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

**PVS\-Studio warning**: [V3153](https://pvs-studio.com/en/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. NLogLogger\.cs 50

Also, the developers used the _?\._ operator directly in the _foreach_ loop's enumerable expression to avoid _NullReferenceException_\. Maybe they'll get lucky, and the _Configuration_ property will never return _null_\. Otherwise, some time later this code may play a trick on you\.

### 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 warning**: [V3153](https://pvs-studio.com/en/docs/warnings/v3153/) Enumerating the result of null\-conditional access operator can lead to NullReferenceException\. CompletionSource\.cs 482

That's great, isn't it? I love it when PVS\-Studio finds interesting cases in compilers or other analyzers\.

### PVS\-Studio

And now it's time to admit that we are not perfect either\. We've made the same mistakes\. :\) 

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

We regularly check PVS\-Studio with PVS\-Studio\. This is how it works:

* at night, we build a new version of the analyzer distribution\. It includes changes we committed to the main branch during the day;
* this new version checks various projects, including PVS\-Studio itself;
* the [BlameNotifier](https://pvs-studio.com/en/docs/manual/0050/) utility notifies developers and managers about the warnings the analyzer issued;
* then, we fix the warnings found\.

And so, after we've improved [V3153](https://pvs-studio.com/en/docs/warnings/v3153/) and [V3105](https://pvs-studio.com/en/docs/warnings/v3105/), the analyzer issued several warnings on our code\. Indeed, the analyzer detected cases when the _foreach_ loop's enumerable expression contained the _?\._ operator\. Also, we found indirect cases \(when a variable takes a value\)\. We were lucky that we hadn't been getting an exception\. In any case, we've already taken the warnings into account and fixed the corresponding cases\. ;\)

Here's a code fragment that triggered a warning:

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

Yeah, there's a bunch of _?\._ here\. Try to find the one that will shoot you in the foot\. It seems like _?\._ operators provide maximum safety \(use the Crysis nanosuit voice effect while reading\) for your code, but in fact, that's not true\.

## Is it possible to use the ?\. operator in the enumerable expression without exceptions?

Of course, you can do that\. And we've seen such code examples\. For example, the _??_ operator can come to the rescue\.

The following code is dangerous and may lead to _NullReferenceException_:

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

And this code will not throw an exception:

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

While the _?\._ operator returns a _null_ value, the _??_ operator results in _Enumerable\.Empty<String\>\(\)_\. Therefore, there will be no exception\. However, adding an explicit _null_ check instead could be a good idea\.

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

Obviously, it looks not so modern but clear and easy to read\.

## Let's solve the task discussed in the beginning

As you may remember, we started the article with the following task:

```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);
}
```

Now you know that option \#2 is not safe at all\. It won't help you avoid _NullReferenceException_\. And what about option \#1? At first glance, it seems that we'll have _NullReferenceException_ when calling _collection\.NotNullItems\(\)_\. But that's not necessarily true\! Suppose _NotNullItems_ is an extension method with the following body:

```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);
}
```

As we can see, the method checks _collection_ for _null_\. Since in this case the method returns the _Enumerable\.Empty<T\>\(\)_ value, there will be no exception\. That is, loop \#1 works successfully, even if _collection_ is _null_\.

But the second loop remains dangerous\. If _collection_ is _null_, the _NotNullItems_ method is not called\. Therefore, check for _null_ does not work\. As a result, we have the same situation which we kept seeing time and again — an attempt to call the _GetEnumerator\(\)_ method for a null reference\.

That's one interesting case we have\! Calling the _collection\.NotNullItems\(\)_ method explicitly prevents _NullReferenceException_, but a "safe" call — _collection?\.NotNullItems\(\)_ — does not\.

## Conclusion

We have several conclusions here:

* do not use the _?\._ operator in the _foreach_ loop's enumerable expression directly or indirectly\. It only creates an illusion of safety;
* use a static analyzer regularly\.

We, as developers once again realized that it's important not only to develop new diagnostics but also to refine the existing ones\.

PVS\-Studio 7\.13 includes the updates we discussed in this article\. Do you want to know whether anyone uses the _?\._ operator in the enumerable expression in your code base? Then, feel free to [download the analyzer from the website](https://pvs-studio.com/en/pvs-studio/download/) and check the code\.

As usual, feel free to follow me on [Twitter](https://twitter.com/_SergVasiliev_)\.