# Эволюция PVS\-Studio: анализ потока данных для связанных переменных

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

![0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image1.png](https://import.viva64.com/docx/blog/0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image1.png)

## О чём статья?

Команда разработки PVS\-Studio постоянно работает над повышением качества анализа\. Но мало просто добавить в продукт улучшение – о нём, конечно же, надо рассказать\! И сегодня мы решили поведать о том, как связи между переменными мешают статическому анализатору и как C\# анализатор PVS\-Studio старается их учитывать\. Приятного чтения\!

## Немного об анализе потока данных

Начнём немного издалека\. Одним из важнейших механизмов, используемых C\# анализатором PVS\-Studio, является анализ потока данных \([data flow analysis](https://pvs-studio.ru/ru/blog/terms/7004/)\)\. Если не вдаваться в детали, это технология, позволяющая анализатору отслеживать возможные значения переменных\. В PVS\-Studio анализ потока данных тесно взаимодействует с другими технологиями, о которых можно прочитать [здесь](https://pvs-studio.ru/ru/blog/posts/0908/)\.

### Числовые и логический типы

Возможности анализа потока данных проще всего продемонстрировать на примере числовых и логических переменных:

```cpp
int a = 5;
int b = 3;
bool flag = a > b;

if (flag) // always true
{
  ....
}
```

Анализ потока данных позволяет PVS\-Studio рассчитать точное значение _flag_ и сообщить о том, что проверка не имеет смысла, ведь _a_ всегда больше _b_\.

Во многих случаях выражения и, в частности, переменные могут иметь любое значение из некоторого множества\. Например:

```cpp
void MyFunc(bool flag)
{
  int a = flag ? 1 : 10;
  bool greater = a > 5;

  if (greater)
    Console.WriteLine("a > 5");

  if (a == 5) 
    Console.WriteLine("a = 5");
}
```

В зависимости от значения, которое будет передано в параметр _flag_, переменная _a_ будет равна 1 или 10\. Следовательно, значение переменной _greater_ может быть как _true_, так и _false_\. Поэтому анализатор не будет считать проверку значения _greater_ бессмысленной\.

С другой стороны, PVS\-Studio точно знает, что _a_ никогда не равно 5\. Именно поэтому анализатор выдаст предупреждение:

[V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'a \=\= 5' is always false\.

В некоторых случаях "лишние" проверки появляются из\-за опечаток или логических ошибок\. К примеру, иногда программист проверяет значение не той переменной, которой нужно\.

### Null\-state анализ

С переменными ссылочных типов дела обстоят несколько иначе\. Здесь анализатор отслеживает возможность равенства _null_, то есть производит null\-state анализ\. Каждая переменная ссылочного типа с точки зрения PVS\-Studio может быть в одном из 4 состояний:

* Unknown – когда нет информации о том, может переменная быть равна _null_ или нет\. Данное состояние все переменные ссылочных типов имеют по умолчанию;
* Null – когда переменная точно равна _null_;
* NotNull – когда переменная точно не равна _null_;
* PotentialNull – когда в некоторых случаях переменная точно равна _null_\.

Пример:

```cpp
void TestReferences(bool flag)
{
  string potentialNullStr = flag ? "not null" : null;
    
  _ = potentialNullStr.GetHashCode();
}
```

На момент вызова _GetHashCode_ переменная _potentialNullStr_ может быть равна или не равна _null_\. Разыменование ссылки, потенциально имеющей значение _null_, может привести к выбрасыванию исключения, поэтому анализатор генерирует соответствующее предупреждение:

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'potentialNullStr'\.

Что же делать? Самое простое – всё\-таки проверить, что переменная не равна _null_:

```cpp
void TestReferences(bool flag)
{
  string potentialNullStr = flag ? "not null" : null;
    
  if (potentialNullStr != null)
    _ = potentialNullStr.GetHashCode();
}
```

Анализатор сможет легко вычислить, что в теле оператора _if_ переменная_ potentialNullStr _точно не равна _null_, а значит, вызов _GetHashCode_ не приведёт к выбрасыванию исключения\.

## Связанные переменные

Всё бы хорошо, но иногда в реальном коде проверки на _null_ производятся более изощрённым способом\. И нет, речь не про [_null_\-условный оператор](https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/member-access-operators#null-conditional-operators--and-) – его поддержка является более\-менее тривиальной задачей\. В самом простом случае нам достаточно просто не ругаться, если доступ к члену осуществляется через "?\."\. Реальной же сложностью является именно проверка на _null_ с помощью связанной переменной\.

Чтобы лучше понять тему, давайте вернёмся к ранее приведённому примеру:

```cpp
public void TestReferences(bool flag)
{
  string potentialNull = flag ? "not null" : null;

  if (potentialNull != null)
    _ = potentialNull.GetHashCode();
}
```

В переменную _potentialNull_ может быть записан _null_\. Однако перед разыменованием есть проверка, и анализ потока данных это учитывает\. Но что если проверка на _null_ производится неявно?

```cpp
public void TestReferences(bool flag)
{
  string potentialNull = flag ? "not null" : null;

  if (flag)
    _ = potentialNull.GetHashCode();
}
```

С точки зрения статического анализатора значение _flag_ неизвестно, а значит, в _potentialNull_ может быть записан _null_\. Дальнейшая проверка не даёт никакой информации о _potentialNull_, ведь эта переменная в условии даже не используется\. Соответственно, анализатор предупредит о потенциальном разыменовании нулевой ссылки\.

На самом же деле если _flag \= true_, то и в _potentialNull_ записана строка\. Проверки на равенство _null_ как бы нет, но никакого разыменования нулевой ссылки тут быть не может\.

Связи могут быть построены множеством способов\. Ранее мы рассмотрели пример с переменными логического и ссылочного типов\. Однако в общем случае что угодно может зависеть от чего угодно\. К примеру, ниже представлена связь двух переменных ссылочных типов:

```cpp
public void RelatedVariables2(string param)
{
  string? potentialNull = param != null ? "not null" : null;

  if (param != null)
  {
    _ = potentialNull.GetHashCode();
  }
}
```

Переменная _potentialNull_ равна _null_ только в случае, если _param_ равен _null_\. Иначе говоря, либо обе переменные равны _null_, либо обе не равны\. Соответственно, вызов _GetHashCode_ здесь никогда не приведёт к выбросу исключения\.

Но что\-то я зациклился на переменных ссылочных типов\. Давайте рассмотрим другой пример:

```cpp
public void RelatedVariables3(int a, int[] array)
{
  int b = 0;
  int index = -1;

  if (a == 0)
  {
    b = 10;
    index = 1;
  }

  if (b > 0)
  {
    _ = array[index];
  }
}
```

Взгляните на этот код и скажите – может ли здесь произойти попытка обращения к элементу с индексом \-1?

![0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image2.png](https://import.viva64.com/docx/blog/0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image2.png)

Пожалуй, в таком примере даже человек не сразу разберётся\. Переменная _index_ **не** может быть равна \-1, если _b \> 0_\. Ведь _b_ \> _0,_ только если _a_ \= _0_, а если _a \= 0_, то и _index_ \= _1_\. Надеюсь, вы не запутались :\)\.

Приведённые примеры являются синтетическими, и на реальном коде такое встречается не так часто\. Тем не менее, пользователи анализатора время от времени сообщают о [false positive срабатываниях](https://pvs-studio.ru/ru/blog/terms/6461/), причиной которых является связанность переменных\. К примеру, недавно нам написали о проблеме обработки кода следующего вида:

```cpp
public void Test()
{
  var a = GetPotentialNull();
  bool z = a != null;

  if (z)
  {
    _ = a.GetHashCode(); // <=
  }
}
```

Увы и ах, **раньше** анализатор совершенно бессовестно лгал о возможном разыменовании нулевой ссылки\!

![0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image3.png](https://import.viva64.com/docx/blog/0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image3.png)

Это, конечно, не катастрофа\. Ложные срабатывания в общем случае неизбежны, но анализатор предоставляет различные возможности для работы с ними\. Самое простое, что можно сделать – отметить предупреждение как ложное, чтобы оно не мозолило глаза\. Подробнее об этом можно прочитать [здесь](https://pvs-studio.ru/ru/docs/manual/0017/)\.

Тем не менее, мы ведём беспрестанную борьбу с ложными срабатываниями, чтобы пользователи не тратили своё время на их изучение\. Эта тема, кстати, хорошо раскрыта в статье "[Как и почему статические анализаторы борются с ложными срабатываниями](https://pvs-studio.ru/ru/blog/posts/cpp/0488/)"\. Рекомендую к прочтению :\)\.

## Ты не туда воюешь\!

Возможно, вам покажется, что мне не стоило всё это рассказывать\. Ведь я говорю о слабостях статического анализа\! Выглядит так, будто я играю не за ту команду :\)\.

![0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image4.png](https://import.viva64.com/docx/blog/0942_PVS_evolution_Data-Flow_analysis_related_variables_ru/image4.png)

Вот только на самом деле это совсем не так\. Подобные статьи в первую очередь посвящены развитию анализатора, рассказу о том, как мы смогли сделать продукт лучше\. А любое развитие начинается именно с признания проблем\. Анализатор работает неидеально? Да\. Иной раз не ругается там, где нужно, а иной раз выдаёт ложные срабатывания? Бывает\. Но мы действительно работаем над этим\. Клиенты пишут нам о проблемах, которые их волнуют больше всего, и мы делаем всё, чтобы сделать PVS\-Studio лучше\.

Статьи же подобного плана позволяют нам рассказать миру о достигнутых успехах :\)\. Кстати, о них\.\.\.

## PVS\-Studio и связанные переменные

Разнообразие возможных связей переменных поражает, и их поддержка – задача весьма нетривиальная\. Однако бороться с ложными срабатываниями нужно, поэтому мы решили постепенно покрывать наиболее распространённые способы связей\.

_Перед тем как начнём, небольшое отступление\._

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

Как разработчики PVS\-Studio, мы очень благодарны пользователям за то, что они пишут нам о проблемах, с которыми столкнулись \(в том числе и о ложных срабатываниях\)\. Но ещё больше мы рады, когда нам присылают именно такие простые примеры кода, на которых некорректное поведение легко воспроизводится\. Это невероятно ускоряет процесс внесения нужных исправлений :\)\.

### Эвристический алгоритм

Первым решением, позволившим избавиться от большого количества ложных срабатываний, стал специальным образом подобранный алгоритм\. Он частично исключает предупреждения, появляющиеся из\-за неявных связей различных значений с переменными ссылочных типов\.

Изучая ложные срабатывания, мы заметили интересную закономерность\. Если разыменование производится в теле условной конструкции, то null\-state соответствующей переменной, скорее всего, связан с выражением в условии\. Иначе говоря, разыменование, производимое под условием, обычно было безопасным, так как соответствующая ссылка неявно проверялась с помощью связанной переменной\.

Приведём пример:

```cpp
void Test(bool condition)
{
  object a;
  if (condition)
    a = new object();
  else
    a = null;

  ....

  if (condition)
    _ = a.ToString();
}
```

Так как разыменование переменной _a_ производится в теле условной конструкции, анализатор как бы предполагает наличие связи между _a_ и условием\. За счёт этого PVS\-Studio не будет выдавать предупреждение\. В данном случае срабатывание на вызов _ToString_ действительно было бы ложным, так как если _condition \= true_, то _a_ не равно _null\._ 

В таком виде алгоритм отсекал слишком много хороших срабатываний, поэтому мы стали думать над доработками\. Наилучших результатов удалось добиться, добавив дополнительное условие исключения: установка _null_ должна быть в том же методе, в котором производится разыменование\. Именно в этих случаях null\-state обычно связан с условием\.

Приведём пример, в котором _null_ может быть получен из другого метода:

```cpp
bool _flag;

object GetPotentialNull() => _flag ? "not null" : null;

void Test(bool condition)
{
  object a = GetPotentialNull();

  if (condition)
    _ = a.ToString();
}
```

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

Долгое время данный алгоритм был основным средством против связанных переменных\. С его помощью и сейчас получается убирать значительную часть ложных срабатываний на коде реальных проектов\. И всё же результаты работы такого исключения неидеальны: иной раз отсекаются хорошие срабатывания, а иногда напротив – "пропускаются" ложные\. И если потеря пары хороших срабатываний является не столь критичной проблемой, то с ложными нужно что\-то делать\.

### Не такое уж и бессмысленное присваивание

Вообще клиенты обычно не пишут о том, что нам стоит "поддержать связанные переменные"\. Это даже звучит очень абстрактно\! Как пользователи, они заинтересованы именно в качественном выводе PVS\-Studio, а как и что там внутри работает – это не так важно\. Поэтому и пишут нам о конкретных ложных срабатываниях, выданных анализатором\. Мы же уже разбираемся, в чём именно проблема, и стараемся её решить\.

И вот в один прекрасный день пишет нам пользователь о предупреждении, выданном на код следующего вида:

```cpp
static void Foo()
{
  Holder h = new Holder();
  Parameter p = h.GetParam();

  p.Text = "ABC"; // <=
  h.f();
  p.Text = "XYZ"; // <=
  h.f();
}
```

[V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'p\.Text' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 35, 33\.

Предупреждение говорит о бессмысленности первого присваивания – значение "ABC" никак не используется\. Что\-то тут не так, код нужно изучить и поправить\.\.\.

А вот и нет\! Присваивание вовсе не бессмысленное\. Но как же так? Первая мысль, которая может возникнуть, – нужно посмотреть, что это за свойство такое – _Text_\. Может быть, присваивание этому свойству на что\-то влияет? Ничего подобного:

```cpp
class Parameter
{
  internal string Text { get; set; }
}
```

Обычное автосвойство\. Присваивание ему значения не приводит к выполнению каких\-то особых действий\. Соответственно, присваивать ему значение 2 раза подряд\.\.\. Несколько странно\. Однако срабатывание всё же ложное\.

Чтобы наконец стало понятно, в чём же тут дело, следует взглянуть на класс _Holder_:

```cpp
class Holder
{
  private Parameter param;
  internal Parameter GetParam() 
  {
    return param;
  }
  
  internal Holder() 
  {
    param = new Parameter();
    param.Text = "";
  }
  
  internal void f()
  {
    Console.WriteLine("Holder: {0}", param.Text);
  }
}
```

Оказывается, метод _f_ использует значение свойства _param\.Text_\. Зная это, давайте вернёмся к исходному примеру:

```cpp
static void Foo()
{
  Holder h = new Holder();
  Parameter p = h.GetParam();

  p.Text = "ABC";
  h.f();
  p.Text = "XYZ";
  h.f();
}
```

Фактически в переменную _p_ записывается ссылка на поле _param _объекта _h_\. При вызове метода _f_ это поле используется – точнее, используется его свойство _Text_\. При первом вызове _f_ в _Text_ записано "ABC", а при втором – "XYZ"\. Таким образом, каждое присваивание сыграло свою роль и никакой ошибки тут нет\.

В данном случае ложное срабатывание вызвано наличием достаточно необычной связи между свойством _p\.Text_ и переменной _h_\. Вызов _h\.f\(\) _использует значение, записанное в _p\.Text_, и диагностике нужно это как\-то учитывать\.

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

```cpp
void Test()
{
  int a, x;
  a = 10;
  x = a; // a is used
  a = 20;
}
```

Анализатор не срабатывает на таком коде, так как переменная _a_ была использована между присваиваниями ей значений\. В отличие от предыдущего случая, здесь переменная _a_ используется явно, поэтому и исключить срабатывание здесь легко\. Но что делать, когда использование присвоенного значения производится неявно, при вызове метода? Давайте разбираться:

```cpp
static void Foo()
{
  Holder h = new Holder();
  Parameter p = h.GetParam();

  p.Text = "ABC";
  h.f();        // p.Text is used here
  p.Text = "XYZ";
  h.f();        // and here
}
```

Чтобы решить проблему, мы решили доработать правило [V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/)\. Теперь при исследовании кода эта диагностика сохраняет пары потенциально связанных переменных\. Если одна из них каким\-либо способом используется, то использованной считается и другая\. _p_ считается потенциально связанной с _h_, так как её значение получено при вызове _h\.GetParam\(\)_\. В то же время вызов _h\.f\(\)_ означает не только использование _h_, но и потенциальное использование связанной с ней _p_ и её свойств\. Поэтому и срабатывание на "лишнее присваивание" _p\.Text_ больше не генерируется\.

### Реальный пример связи

Синтетика – это, конечно, хорошо, но кому она интересна? Да, круто, что анализатор стал лучше работать на выдуманных примерах\. Правда толку от этого немного, если люди попросту не пишут код, на котором улучшение заметно\. Про оценку анализаторов на синтетических примерах, кстати, есть [достаточно яркая заметка](https://pvs-studio.ru/ru/blog/posts/cpp/0471/)\. Там про C\+\+, но суть не меняется\.

У нас же совсем другой случай\. Во\-первых, правка вносилась в первую очередь по просьбе клиента, то есть как минимум на коде в его проекте от ложных срабатываний мы избавились\. Во\-вторых, улучшения работы анализатора хорошо заметны и на других реальных проектах\. К примеру, давайте взглянем на код из [RavenDB](https://github.com/ravendb/ravendb), который мы используем для тестирования PVS\-Studio:

```cpp
[Fact]
public void CRUD_Operations_With_Array_In_Object_2()
{
  ....
  var family = new Family()
  {
    Names = new[] { "Hibernating Rhinos", "RavenDB" }
  };
  newSession.Store(family, "family/1");
  newSession.SaveChanges();

  var newFamily = newSession.Load<Family>("family/1");

  newFamily.Names = new[] {"Hibernating Rhinos", "RavenDB"};   // <=
  Assert.Equal(newSession.Advanced.WhatChanged().Count, 0);

  newFamily.Names = new[] { "RavenDB", "Hibernating Rhinos" }; // <=
  Assert.Equal(newSession.Advanced.WhatChanged().Count, 1);

  newSession.SaveChanges();
  ....
}
```

[V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'newFamily\.Names' variable is assigned values twice successively\. Perhaps this is a mistake\.

Итак, анализатор сообщал, что в _newFamily\.Names_ дважды присваивается значение, при этом первое значение никак не используется\. И действительно, по коду явного использования не видно\. Но что если взглянуть получше?

Объект класса _Family_ сохраняется в сессию\. На этот момент он содержит имена "Hibernating Rhinos" и "RavenDB"\. Затем этот же объект \(или, как минимум, объект, содержащий те же значения\) из сессии загружается\. После этого в него записываются **те же самые имена**\. А далее производится вызов:

```cpp
Assert.Equal(newSession.Advanced.WhatChanged().Count, 0);
```

Очевидно, что эта проверка учитывает записанное ранее значение\. Данный тест проверяет, что изменений нет – ведь имена записаны те же самые\. Чуть ниже по коду имена меняются местами и опять производится подобная проверка\. Там уже ожидается именно наличие изменений\. Очевидна связь между вызовами _newSession\.Advanced\.WhatChanged\(\)_ и _newFamily\.Names_\.

Получается, выдавать тут предупреждения про "ненужное" присваивание не стоит\. И знаете что? Теперь PVS\-Studio не будет делать этого :\)\. Следовательно, разработчики не будут тратить время на исследование лишних срабатываний\.

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

### Связи через оператор as

Не успели мы порадоваться победе над ложными срабатываниями, сообщавшими о "лишних" присваиваниях, как другой клиент прислал нам новый пример:

```cpp
void Test(object obj)
{
  if (obj != null)
    Console.WriteLine("obj is not null");

  string str = obj as string;

  if (str != null)
    Console.WriteLine(obj.GetHashCode()); // <=
}
```

[V3125](https://pvs-studio.ru/ru/docs/warnings/v3125/) The 'obj' object was used after it was verified against null\.

Ну, давайте разбираться\.

Вначале параметр _obj_ проверяется на равенство _null_\. Получается, что метод предполагает: **в _obj_ может быть передана нулевая ссылка**\. Затем c помощью оператора _as_ производится преобразование _obj_ к типу _String_, а результат записывается в переменную _str_\.

Самое интересное происходит дальше\. Если _str_ не равна _null_, то производится обращение к методу _GetHashCode_\. Однако вызывается он не у _str_, а у _obj_\! Получается, что тут проверили не ту переменную\. Даже если _str_ не равна _null_, то _obj_ всё ещё остаётся потенциальным _null_\-значением\.

По крайней мере так может показаться\. На самом деле, если _str \!\= null_, то и _obj \!\= null_\. Почему? 

Допустим, _obj_ действительно равен _null_\. Тогда первая проверка даст _false_ – ну и ладно\. Далее происходит вычисление значения для _str_\. Так как переменная _obj_ равна _null_, то и в _str_ точно будет _null_\. Из этого следует обратный вывод: **если в _str_ не _null_, то и в _obj_ тоже не _null_**\.

Круто, конечно, что мы это поняли, но надо бы и анализатор тоже научить\. В этом нам помогает существующая реализация Data Flow\. Для подходящих выражений из анализируемого кода PVS\-Studio создаёт специальные объекты, которые хранят информацию о возможных значениях\. Такие объекты мы называем виртуальными значениями\. В них также содержатся и вспомогательные данные, широко используемые диагностиками\. К примеру, Data Flow отслеживает, является ли значение переменной:

* результатом вызова _FirstOrDefault_;
* потенциально заражённым \(подробнее [тут](https://pvs-studio.ru/ru/blog/terms/6496/)\);
* результатом приведения через оператор _as;_
* и так далее\.

Чтобы стало понятно, как именно анализатор начал учитывать связи через оператор _as_, вернёмся к примеру:

```cpp
void Test(object obj)
{
  if (obj != null)
    Console.WriteLine("obj is not null");

  string str = obj as string;

  if (str != null)
    Console.WriteLine(obj.GetHashCode());
}
```

В переменную _str_ записывается результат приведения _obj_ через оператор _as_\. Data Flow запишет в соответствующее виртуальное значение информацию об этом\. Данный функционал уже был реализован и активно использовался некоторыми правилами\. Одно из них — [V3149](https://pvs-studio.ru/ru/docs/warnings/v3149/)\.

В момент обработки условия _str \!\= null_ анализатор вычисляет, что если это выражение истинно, то _str_ точно не равна _null_\. При этом анализатору уже известно, что значение _str_ получено в результате приведения _obj_ через оператор _as_\. Получается, что и значение _obj_ также вполне справедливо может считаться не равным _null_\.

### Реальные примеры связей через оператор as

Честно говоря, мы даже сами не ожидали такого результата, но пропала целая куча ложных срабатываний\. Кто бы мог подумать, что такая вот проверка на _null_ через оператор _as_ встречается столь часто?

**Issue 1**

В качестве первого примера приведу фрагмент кода из проекта [SpaceEngineers](https://github.com/KeenSoftwareHouse/SpaceEngineers):

```cpp
void Toolbar_ItemChanged(MyToolbar self, MyToolbar.IndexArgs index)
{
  Debug.Assert(self == Toolbar);
    
  var tItem = ToolbarItem.FromItem(self.GetItemAtIndex(index.ItemIndex));
  ....
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of method return value when it is passed to method as its 1st argument\.

Итак, срабатывание говорило о том, что в метод _ToolbalItem\.FromItem_ может передаваться _null_, что приведёт к выбрасыванию исключения\. Так ли это?

В первую очередь стоит взглянуть на метод _GetItemAtIndex_:

```cpp
public MyToolbarItem GetItemAtIndex(int index)
{
  if (!IsValidIndex(index)) 
    return null;

  return this[index];
}
```

Анализ потока данных позволил анализатору выяснить, что вызов этого метода в некоторых случаях возвращает _null_\. Но приведёт ли это к проблемам? Давайте теперь перейдём к определению метода _FromItem_:

```cpp
public static ToolbarItem FromItem(MyToolbarItem item)
{
  var tItem = new ToolbarItem();
  tItem.EntityID = 0;
  var terminalItem = item as MyToolbarItemTerminalBlock;
  if (terminalItem != null)
  {
    var block = item.GetObjectBuilder() as ....; // <=
    ....
  }
  ....
  return tItem;
}
```

Ранее мы видели, что в параметре _item_ может быть записан _null_\. Здесь же производится разыменование, а сам _item_ перед этим не проверяется\. Зато проверяется _terminalItem_\! А если _terminalItem_ не равен _null_, то и _item_ точно не равен _null_\.

**Issue 2**

Похожий пример мы обнаружили в проекте [SharpDevelop](https://github.com/icsharpcode/SharpDevelop):

```cpp
DocumentScript GetScript(string fileName)
{
  ....
  var formattingOptions
       = CSharpFormattingPolicies.Instance
                                 .GetProjectOptions(compilation.GetProject());
  ....
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference of 'compilation\.GetProject\(\)' method return value at 'project\.FileName' when it is passed to method as its 1st argument\.

Итак, анализатор предупреждал о возможном разыменовании нулевой ссылки внутри метода _GetProjectOptions_\. Причиной этого является передача _compilation\.GetProject\(\)_ в качестве первого аргумента\. Давайте разбираться\.

Межпроцедурный анализ позволил выяснить, что_ GetProject_ действительно иногда возвращает _null_\. Что же насчёт _GetProjectOptions_? Давайте взглянем:

```cpp
public CSharpFormattingPolicy GetProjectOptions(IProject project)
{
  if (!initialized)
    return GlobalOptions;

  var csproject = project as CSharpProject;
  if (csproject != null) {
    string key = project.FileName;            // <=
    ....
  }

  return SolutionOptions ?? GlobalOptions;
}
```

Да, здесь производится обращение к свойству первого аргумента\. Однако лишь в том случае, если он не равен _null_\! Просто проверяется не сам _project_, а результат его приведения через _as_\.

**Issue 3**

Ещё одно пропавшее ложное срабатывание выдавалось на код проекта [ILSpy](https://github.com/icsharpcode/ILSpy):

```cpp
protected override Expression DoResolve (ResolveContext ec)
{
  var res = expr.Resolve(ec);
  var constant = res as Constant;

  if (constant != null && constant.IsLiteral)
  {
    return Constant.CreateConstantFromValue(res.Type,           // <=
                                            constant.GetValue(),
                                            expr.Location);
  }

  return res;
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'res'\.

_res_ получает своё значение из вызова _expr\.Resolve\(ec\)_\. В некоторых случаях он действительно возвращает _null_\. Но в момент обращения к свойству _Type_ переменная уже точно не равна _null_\. Как и ранее, проверка произведена неявно, и если _constant \!\= null_, то и _res_ тоже\.

Были и многие другие ложные срабатывания, которые "ушли" благодаря поддержке связей через оператор _as_\. Но все они так или иначе напоминают те, что мы здесь уже привели\. Если у вас есть желание самостоятельно проверить, как PVS\-Studio обрабатывает случаи такого вида, то вы можете бесплатно сделать это, загрузив анализатор по [ссылке](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. Have fun\!

### Типичные связанные переменные

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

Ранее я уже приводил пример, демонстрирующий такую связь:

```cpp
public void Test()
{
  var a = GetPotentialNull();
  bool flag = a != null;

  if (flag)
  {
    _ = a.GetHashCode(); // <=
  }
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'a'\.

Если _flag_ _\= true_, то переменная _a_ не может быть равна _null_\. Таким образом неявная проверка защищает от проблем\.

Чтобы научить анализатор учитывать такие связи, мы вновь решили внести доработки в наш Data Flow\. Однако данный случай был несколько сложнее\.

В отличие от случая с оператором _as_, здесь понадобилось добавление нового типа информации о переменной\. В частности – данных о связи с другой переменной\. Обрабатывая объявление _flag_, анализатор вычисляет возможные значения переменных из выражения в случаях:

* если выражение \(а следовательно, и _flag_\) равно _true_;
* если выражение равно _false_\.

Обработав объявление _flag_, анализатор добавляет в соответствующее виртуальное значение 2 правила:

* если _flag \=\= true_, то _a \!\= null_;
* если _flag \=\= false_, то _a \=\= null_\.

Теперь, когда для _flag_ имеются нужные данные, осталось лишь воспользоваться ими при обработке условия _if \(flag\)_\. Здесь Data Flow вычисляет возможные значения переменных в then\-ветке\. Соответственно, _flag_ в ней всегда равна _true_, а связанная с этой переменной _a_ точно не равна _null_\.

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

На синтетике всё это выглядит здорово, но давайте теперь посмотрим, что там в коде реальных проектов\.

### Типичные связи в реальном коде

Тут результат даже круче, чем со связями через оператор _as_\. Что любопытно, правка позволила не только избавиться от ложных срабатываний, но и добавить несколько "истинных"\.

**Issue 1**

Для начала рассмотрим довольно простое ложное срабатывание на коде проекта [BouncyCastle](https://github.com/bcgit/bc-csharp)\.

```cpp
public static Stream ReplaceSigners(....)
{
  ....

  CmsTypedStream signedContent = parser.GetSignedContent();
  bool encapsulate = (signedContent != null);
  Stream contentOut = gen.Open(outStr,
                               parser.SignedContentType.Id,
                               encapsulate);
  if (encapsulate)
  {
    Streams.PipeAll(signedContent.ContentStream, contentOut); // <=
  }

  ....
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'signedContent'\.

Пропавшее ложное срабатывание говорило о возможном разыменовании нулевой ссылки\. Если _signedContent_ будет равно _null_, то обращение к свойству _ContentStream_ приведёт к выбрасыванию исключения\.

Но обратите внимание на проверку значения _encapsulate_\. Именно она неявно защищает от разыменования нулевой ссылки, ведь _encapsulate \= true_ только тогда, когда _signedContent \!\= null_\. Последние правки "научили" PVS\-Studio учитывать такие связи, поэтому ложное срабатывание и пропало\.

**Issue 2**

Следующий пример взят из проекта [ccnet](https://github.com/ccnet/CruiseControl.NET):

```cpp
public bool Authenticate(LoginRequest credentials)
{
  // Check that both the user name and the password match
  string userName = GetUserName(credentials);
  string password = NameValuePair.FindNamedValue(....);
  
  bool isValid =    !string.IsNullOrEmpty(userName)
                 && !string.IsNullOrEmpty(password);

  if (isValid)
  {
    isValid =    SecurityHelpers.IsWildCardMatch(userName,     // <=
                                                 this.userName)
              && ....;
  }

  return isValid;
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference inside method at 'wildCard\.Replace'\. Consider inspecting the 1st argument: userName\.

Это предупреждение говорило о том, что в метод _IsWildCardMatch_ в качестве первого аргумента потенциально передаётся нулевая ссылка, а также о том, что внутри может произойти её разыменование\. Соответственно, возможна ситуация, когда будет выброшено исключение типа _NullReferenceException_\. Но так ли это на самом деле?

Значение первого аргумента – _userName_ — приходит из вызова_ GetUserName_\. И оттуда действительно может приходить _null_, что и обнаружил анализатор\. Да и в методе _IsWildCardMatch_ действительно производится разыменование первого аргумента:

```cpp
public static bool IsWildCardMatch(string wildCard, string value)
{
  Regex wildCardRegex = new Regex(wildCard.Replace("*",
                                                   "[a-zA-Z0-9_.@-]*"),
                                  RegexOptions.IgnoreCase);

  return wildCardRegex.IsMatch(value);
}
```

Вот только _null_ туда не передаётся\! Наверняка вы уже заметили проверку значения _isValid_ здесь:

```cpp
bool isValid =    !string.IsNullOrEmpty(userName)
               && !string.IsNullOrEmpty(password);

if (isValid)
{
  isValid =    SecurityHelpers.IsWildCardMatch(userName,
                                               this.userName)
            && ....;
}
```

Если _isValid_ _\= true_, то и _userName_ никак не может быть _null_\. Благодаря поддержке связей теперь и анализатор знает об этом\.

**Issue 3**

Ещё одно интересное ложное срабатывание выдавалось на код из проекта [FlashDevelop](https://github.com/fdorg/flashdevelop):

```cpp
public void HandleEvent(Object sender, NotifyEvent e, HandlingPriority priority)
{
  ....
  features = enabledLanguages.ContainsKey(ext) ? enabledLanguages[ext] : null;
  
  if (completion == null)
    completion = new Completion(config, settingObject);

  completion.OnFileChanged(features);                      // <=

  if (features != null && features.Syntax != null)
    ....
  ....
}
```

[V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) Possible null dereference inside method at 'features\.Mode'\. Consider inspecting the 1st argument: features\.

Срабатывание говорило о том, что переменная _features_, потенциально имеющая значение _null_, передаётся в метод _OnFileChanged_, что может привести к разыменованию нулевой ссылки\.

По коду хорошо видно, что в _features_ в некоторых случаях записывается _null_, а ниже присутствует и соответствующее условие\. Однако перед передачей в метод _OnFIleChanged_ переменная никак не проверяется – нет даже какой\-нибудь неявной проверки через связь\.

Так почему же поддержка связей привела к исчезновению этого срабатывания? Ответ находится в коде метода _OnFileChanged_:

```cpp
internal void OnFileChanged(CssFeatures features)
{
  if (features == this.features) return;
  this.features = features;
  enabled = features != null;               // <=

  if (enabled)
  {
    wordChars = lang.characterclass.Characters;
    if (features.Mode != "CSS") wordChars += features.Trigger;
    InitBlockLevel();
  }
}
```

А вот и связанные переменные\! Разыменование _features_ производится только в случае, если _enabled \= true_, а это возможно лишь в случае, когда _features \!\= null_\. Таким образом, срабатывание действительно было ложным\.

**Issue 4**

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

Например, можно рассмотреть следующий фрагмент кода из [Roslyn](https://github.com/dotnet/roslyn):

```cpp
public override object GetFunctionExtender(string name,
                                           SyntaxNode node,
                                           ISymbol symbol)
{
  ....
  
  var methodSymbol = (IMethodSymbol)symbol;
  isDeclaration = methodSymbol.PartialDefinitionPart == null;
  hasOtherPart = isDeclaration
                    ? methodSymbol.PartialImplementationPart != null
                    : methodSymbol.PartialDefinitionPart != null;    // <=
    
  ....
}
```

[V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'methodSymbol\.PartialDefinitionPart \!\= null' is always true\.

Итак, научившись отслеживать связи соответствующего типа, PVS\-Studio сформировал предупреждение о наличии в коде логического выражения, которое всегда возвращает _true_\. Откуда взялось такое мнение?

Как и раньше, логика здесь проста\. _isDeclaration_ будет иметь значение _true_ только в том случае, если _methodSymbol\.PartialDefinitionPart_ будет равным _null_\. С другой стороны, если _isDeclaration_ равен _false_, то и _methodSymbol\.PartialDefinitionPart_ точно не равно _null_\.

Таким образом, последнее выражение тернарного оператора всегда будет иметь значение _true_\. Иной раз always\-true выражения представляют собой безобидный избыточный код, в других же случаях — свидетельствуют об ошибках\. Иногда подобный код и правда пишут для улучшения читаемости\. Какой именно случай здесь – сказать сложно\.

Если ошибки тут нет, то код, в принципе, можно было бы написать чуть проще:

```cpp
hasOtherPart =    !isDeclaration
               || methodSymbol.PartialImplementationPart != null;
```

С другой стороны, это лишь моё мнение и для кого\-то проще будет именно тот код, который есть сейчас\.

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

Переменные могут быть связаны огромным количеством способов, поэтому и поддержать их все достаточно проблематично, если вообще возможно\. Такие связи встречаются не так уж часто, но всё\-таки время от времени они приводят к появлению ложных срабатываний\. Разработчики PVS\-Studio постоянно работают над улучшением анализатора, и поддержка связанных переменных – одно из направлений нашей деятельности\. Конечно, наиболее важны для нас именно пожелания наших клиентов\. Тем не менее, нам ценна любая обратная связь\. Поэтому предлагаю и вам, дорогие читатели, [бесплатно попробовать](https://pvs-studio.ru/ru/pvs-studio/try-free/) статический анализатор на своих проектах\. Уверен, вы не будете разочарованы :\)\.

А какие случаи связанности переменных встречали вы? Пишите в комментарии – посмотрим, сколько разных случаев мы сможем собрать\.

До новых встреч\!