Любишь ли ты '?.
' оператор? А кто это не любит? Многим нравятся эти краткие проверки на ноль.
Однако сегодня мы поговорим о случае, когда знак '?.
' оператор лишь создает иллюзию безопасности.
Мы поговорим об использовании его в цикле foreach. Предлагаю начать с небольшой задачи для самопроверки.
Взгляните на следующий код:
Как, по вашему мнению, будет работать каждый из циклов, если коллекция — нулевой ? Кажется, что вариант использования оператора ?.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); }
более безопасный.
Но так ли это на самом деле? Название статьи уже должно было вселить в вашу душу сомнения.
В любом случае, мы попробуем разобраться во всем этом ниже.
И мы вернемся к вышеуказанной проблеме в конце статьи, когда у нас будет больше информации.
Прежде чем мы начнем погружение, давайте определимся немного с терминологией.
Давайте посмотрим Спецификация С# , раздел «Оператор foreach».
Выражение, через которое будет проходить цикл, обозначается просто как «выражение».
Однако использовать прямой перевод слова – выражения – не кажется лучшей идеей, так как непременно приведет к путанице.
Поэтому в дальнейшем я буду использовать термин «перечисляемое выражение», имея в виду «выражение» из примера выше.
Просто запомни это.
Почему опасно использовать знак '?' оператор? в перечислимом выражении цикла foreach
Здесь стоит вспомнить, как работает оператор ?.Мы закончим это быстро.
var b = a?.
Foo();
Так:
- Если а == ноль , б == ноль ;
- Если а != ноль , б == а.
Foo()
.
Не будем забывать, что это всего лишь куча синтаксического сахара.
void Foo1(IEnumerable<String> collection)
{
foreach (var item in collection)
Console.WriteLine(item);
}
Если посмотреть IL-код, то приведенный выше фрагмент можно переписать на C# без использования для каждого .
Это будет выглядеть примерно так: 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();
}
}
}
Примечание .
В некоторых случаях для каждого может расширяться в другой код. Например, в коде, идентичном коду, сгенерированному для цикла для .
Однако проблема по-прежнему остается актуальной.
Я думаю, что возможна оптимизация цикла для каждого Более подробно мы рассмотрим это в отдельной статье.
Ключевым моментом здесь для нас является выражение коллекция.
GetEnumerator() .
В черно-белом коде (хотя это зависит от вашей цветовой схемы) код говорит, что ссылка разыменовывается при вызове метода.
GetEnumerator .
Если эта ссылка равна нулю, мы получим исключение типа NullReferenceException .
Теперь посмотрим, что происходит при использовании оператора ?.
в перечислимом выражении для каждого : static void Foo3(Wrapper wrapper)
{
foreach (var item in wrapper?.
Strings)
Console.WriteLine(item);
}
Этот код можно переписать примерно так: 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 ( строки.
GetEnumerator ).
Однако обратите внимание, что значение струны может иметь значение нулевой , Если обертка — нулевой .
В принципе это ожидаемо, если вспомнить как работает оператор ?.
(об этом мы говорили выше).
В этом случае при попытке вызвать метод строка.
GetEnumerator() мы получаем исключение типа NullReferenceException .
Вот почему использование оператора ?.
в перечислимом выражении для каждого не защищает от разыменования нулевых ссылок, а лишь создает иллюзию безопасности.
Как вам пришла в голову идея улучшить анализатор?
Ко мне приходит коллега и говорит — вот код, не можем отловить ошибку.Я удивлен.
Я точно помню, как я сам предлагал для улучшения случай, когда в перечислимом выражении для каждого может быть смысл нулевой .
Я проверяю.
Действительно, в приведенном ниже коде анализатор не работает. void Test1(IEnumerable<String> collection,
Func<String, bool> predicate)
{
foreach (var item in collection?.
Where(predicate))
Console.WriteLine(item);
}
И на этом тоже.
void Test2(IEnumerable<String> collection,
Func<String, bool> predicate)
{
var query = collection?.
Where(predicate);
foreach (var item in query)
Console.WriteLine(item);
}
Но в этом фрагменте кода уже есть предупреждение.
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 : В3080 Возможное нулевое разыменование.
Рассмотрите возможность проверки «запроса».
Следующий код также выдаст предупреждение.
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 : В3080 Возможно нулевое разыменование возвращаемого значения метода.
Рассмотрите возможность проверки: GetPotentialNull(.
).
Почему в методах Тест3 И Тест4 выдается предупреждение и Тест1 И Тест2 - Нет? Дело в том, что анализатор разделяет эти ситуации:
- анализатор не выдавал предупреждение, если результат работы оператора записывался в переменную ?.
; - если использовалось выражение, в котором значение где-то было явно записано нулевой , было вынесено предупреждение.
- выдается более точное предупреждение;
- возможность отдельно обрабатывать эти случаи (повышать/понижать уровень, подавлять/не подавлять и т.д.);
- отдельная документация, более точно объясняющая суть проблемы.
Какая диагностика была улучшена?
По результатам были доработаны 2 диагностических правила: В3105 И В3153 .Примечание .
Документация по этим диагностикам будет обновлена после выхода PVS-Studio 7.13, в которую войдут соответствующие изменения.
В3105 теперь обнаруживает подозрительные фрагменты кода, когда результат оператора записывается в переменную ?.
, а затем эта переменная используется как перечислимое выражение для каждого .
void Test(IEnumerable<String> collection,
Func<String, bool> predicate)
{
var query = collection?.
Where(predicate);
foreach (var item in query)
Console.WriteLine(item);
}
Внимание PVS-Studio : В3105 Переменная «запрос» использовалась после того, как она была присвоена с помощью условного оператора с нулевым значением.
Возможно исключение NullReferenceException. В3153 теперь обнаруживает случаи, когда оператор ?.
используется непосредственно в перечисляемом выражении для каждого .
void Test(IEnumerable<String> collection,
Func<String, bool> predicate)
{
foreach (var item in collection?.
Where(predicate))
Console.WriteLine(item);
}
Внимание PVS-Studio : В3153 Перечисление результата оператора условного доступа со значением NULL может привести к исключению NullReferenceException. Рассмотрите возможность проверки: коллекция?.
Где(предикат).
Примеры найденных проблем
Самое приятное в модификациях анализатора – это когда ты видишь от них прибыль.Как мы уже неоднократно говорили, мы тестируем анализаторы, в том числе на основе проектов с открытым исходным кодом.
И после правок В3105 И В3153 удалось найти несколько новых позитивов! Примечание .
Приведенные фрагменты кода относятся к базе кода, которая была актуальной на момент добавления проектов в наши тесты.
То есть этих фрагментов кода может не быть в текущей базе кода или они могут находиться в других строках.
RavenDB
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);
}
}
Я специально включил весь фрагмент кода.
Согласитесь, проблема не так заметна.
Конечно, если вы знаете, что ищете, тогда все пойдет проще.
;) Если код упростить, проблема становится более очевидной.
private void HandleInternalReplication(DatabaseRecord newRecord,
List<IDisposable> instancesToDispose)
{
var newInternalDestinations = newRecord.Topology?.
GetDestinations(.
); .
foreach (var item in newInternalDestinations) .
}
К переменной новыеВнутренние назначения фиксировал результат работы оператора ?.
.
Если newRecord.Topology — нулевой , новыеВнутренниеНазначения также будет иметь значение нулевой .
При попытке выполнить оператор для каждого будет выдано исключение типа NullReferenceException .
Внимание PVS-Studio : В3105 Переменная newInternalDestinations использовалась после того, как она была присвоена с помощью условного оператора с нулевым значением.
Возможно исключение NullReferenceException. РепликацияLoader.cs 828 Еще интересно то, что эта же переменная – новыеВнутренниеНазначения - передается методу База данныхTopology.FindChanges , где проверяется нулевой (параметр новыеНаправления ): 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
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 : В3153 Перечисление результата оператора условного доступа со значением NULL может привести к исключению NullReferenceException. Рассмотрите возможность проверки: свойства?.
Ключи.
MockEngine.cs 159 Здесь оператор ?.
используется непосредственно в перечисляемом выражении для каждого .
Возможно, разработчик посчитал, что так будет безопаснее.
Но мы с тобой знаем, что безопаснее не будет. ;)
Пустоты
Пример аналогичен предыдущему.
public NLogLogger(.
) { .
foreach (FileTarget target in global::NLog.LogManager .
Configuration ?.
AllTargets .
OfType<FileTarget>()) { .
} .
}
Внимание PVS-Studio : В3153 Перечисление результата оператора условного доступа со значением NULL может привести к исключению NullReferenceException. Нлоглоггер.
cs 50 Разработчики также решили «защитить себя» от NullReferenceException и использовал оператор ?.
непосредственно в перечислимом выражении для каждого .
Возможно, им повезет и у них будет недвижимость.
Конфигурация никогда не вернется нулевой .
В противном случае придет время, когда этот код сработает.
Рослин
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 : В3153 Перечисление результата оператора условного доступа со значением NULL может привести к исключению NullReferenceException. ЗавершениеSource.cs 482 Ну, это здорово, не так ли? Мне нравится, когда PVS-Studio находит интересные места в компиляторах или других анализаторах.
PVS-Студия
А теперь пора сказать, что на самом деле мы сами не без греха и тоже прошли через те же грабли.
:) 
Мы регулярно проверяем PVS-Studio с помощью PVS-Studio. Алгоритм работы примерно такой:
- Ночью на сборочном сервере собирается новая версия дистрибутива анализатора.
Содержит правки, которые были добавлены в основную ветку в течение дня;
- новая версия проверяет код различных проектов, в том числе и самого PVS-Studio;
- информация о предупреждениях, выдаваемых анализатором, отправляется разработчикам и менеджерам с помощью Утилиты BlameNotifier ;
- обнаруженные предупреждения исправлены.
Действительно, существовало и прямое использование оператора ?.
в перечислимом выражении для каждого , и косвенный (с записью в переменную).
Нам повезло, что код в этих местах не зависал.
В любом случае предупреждения уже учтены и соответствующие места исправлены.
;) Пример кода, выдавшего предупреждение: public override void
VisitAnonymousObjectCreationExpression(
AnonymousObjectCreationExpressionSyntax node)
{
foreach (var initializer in node?.
Initializers) initializer?.
Expression?.
Accept(this);
}
Да, операторы ?.
здесь от души - найди того, кто стреляет тебе в ногу.
Казалось бы, максимальная безопасность (читай в голосе нанокостюма из Crysis), но на самом деле это не так.
Можно ли использовать '?.
' оператор? безопасно ли это в перечислимом выражении цикла foreach? Конечно вы можете.
И мы видели такие примеры кода.
Например, на помощь может прийти оператор ?? .
Следующий код опасен и рискует вызвать потенциальное исключение типа.
NullReferenceException : static void Test(IEnumerable<String> collection,
Func<String, bool> predicate)
{
foreach (var item in collection?.
Where(predicate))
Console.WriteLine(item);
}
Но в этой версии кода исключение больше не будет выдаваться: static void Test(IEnumerable<String> collection,
Func<String, bool> predicate)
{
foreach (var item in collection?.
Where(predicate)
?? Enumerable.Empty<String>())
{
Console.WriteLine(item);
}
}
Если оператор ?.
в результате своей работы вернет значение нулевой , то результат оператора ?? будет выражение Перечисляемый.
Пустой () - следовательно, исключение не будет выброшено.
Но, конечно, стоит задуматься, не проще ли было бы добавить явную проверку на нулевой .
static void Test(IEnumerable<String> collection,
Func<String, bool> predicate)
{
if (collection != null)
{
foreach (var item in collection.Where(predicate))
Console.WriteLine(item);
}
}
Выглядит это, конечно, не так модно, но, может быть, даже более очевидно и понятно.
Разберем проблему из начала статьи
Напомню, что мы анализировали следующий код. 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 - при звонке коллекция.
NotNullItems() .
Но это не факт! Давайте представим, что NotNullItems — метод расширения, имеющий следующее тело: 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);
}
Как видим, метод включает проверку на случай, когда коллекция имеет значение нулевой .
Поскольку в этом случае возвращаемое значение равно Перечисляемый.
Пустой () , при попытке пройти его в цикле для каждого никакое исключение не будет выбрано.
То есть цикл №1 будет работать успешно, даже если коллекция — нулевой .
Но второй цикл был и остается опасным.
Если коллекция — нулевой , метод NotNullItems не будет вызван.
Следовательно, встроенная в него защита не будет работать в том случае, когда коллекция — нулевой .
В результате мы получаем ту же ситуацию, которую неоднократно рассматривали: попытка вызова метода ПолучитьЭнумератор() через нулевую ссылку.
Интересный момент возникает, когда явный вызов метода коллекция.
NotNullItems() защищает от исключений NullReferenceException и «безопасный вызов» коллекция?.
NotNullItems() - не защищает.
Заключение
Здесь есть несколько выводов:- не используйте оператор ?.
в перечислимом выражении для каждого прямо или косвенно – это лишь иллюзия безопасности; - регулярно используйте статический анализатор.
Обсуждаемые изменения вошли в релиз PVS-Studio 7.13. Интересно посмотреть, пользуется ли кто-нибудь этим оператором ?.
в перечислимых выражениях в вашей кодовой базе? Тогда я приглашаю вас скачать дистрибутив анализатора с сайта и проверьте интересующий код. И, как обычно, приглашаю вас подписаться на мою рассылку.
Если вы хотите поделиться этой статьей с англоязычной аудиторией, воспользуйтесь ссылкой для перевода: Сергей Васильев.
'?.
' Оператор в foreach не защитит от исключения NullReferenceException .
Теги: #C++ #.
NET #статический анализ #ForEach
-
Открыть Вебинар C#: Асинхронность И Ожидание
19 Oct, 24 -
Делаем «Самое Бесполезное Устройство» Сами
19 Oct, 24 -
Вышла Первая Бета-Версия Fireflies
19 Oct, 24
