# Проверяем Microsoft Code Contracts

Мы успешно создали и продолжаем развивать анализатор PVS\-Studio для языка C/C\+\+\. Со временем стало понятно, что многие из реализованных диагностик никак не связаны с конкретным языком программирования, и тогда мы решили попробовать применить свой опыт к другому языку программирования, к C\#\. В данной статье будет рассказано о проверке проекта Code Contracts от Microsoft с помощью нового C\# анализатора\.

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

## О проекте MS Code Contracts

[Code Contracts](https://www.microsoft.com/en-us/research/project/code-contracts/?from=http%3a%2f%2fresearch.microsoft.com%2fen-us%2fprojects%2fcontracts%2f) предоставляют метод, с помощью которого выражаются предположения о коде программы в \.NET приложениях\. Контракты принимают форму предусловий, постусловий и инвариантов объекта, и выступают в роли проверенной документации ваших внешних и внутренних API\. Контракты используются для того, чтобы улучшить процесс тестирования с помощью проверки во время выполнения программы, позволяя проводить статическую верификацию контрактов и генерацию документации\.

Это проект среднего размера \(около \~4000 исходных файлов\), который активно развивается: содержит много недописанного и некорректно написанного кода\. Именно на этом этапе необходимо внедрять статический анализ в процесс разработки\.

## О новом анализаторе C\# кода

Анализатор в скором времени перестанет быть экспериментальным\. Мы планируем выпустить первую релизную версию PVS\-Studio с поддержкой С\# уже 22\.12\.2015\. При этом анализатор меняет старший номер версии на 6\.0\.

Ценовая политика не претерпит никаких изменений\. Раньше PVS\-Studio позволял проверять программы на языках C, C\+\+, C\+\+/CLI, C\+\+/CX\. Теперь к этому списку просто добавляется C\#\. 

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

Подготавливая статью о проверке [открытого проекта](https://pvs-studio.ru/ru/blog/inspections/), мы приводим в ней информацию далеко не о всех предупреждениях, которые выдаёт статический анализатор\. Поэтому мы рекомендуем авторам проекта самостоятельно выполнить анализ проекта и изучить все выдаваемые анализатором сообщения\.

### Наиболее опасные найденные места

[V3025](https://pvs-studio.ru/ru/docs/warnings/v3025/) Incorrect format\. A different number of actual arguments is expected while calling 'Format' function\. Expected: 3\. Present: 2\. VSServiceProvider\.cs 515

```cpp
void AskToReportError(Exception exn) {
  ....
  var emailBody = new StringBuilder();
  emailBody.AppendLine("Hi Code Contracts user,");
  emailBody.AppendLine();
  ....
  emailBody.AppendLine(
    String.Format(".... {0} {1} Visual Studio {2} Bug Report",
      typeof(VSServiceProvider).Assembly.GetName().Version,
#if DEBUG
                                                 "Debug"
#else
                                                 "Release"
#endif
                                           ));
  ....
}
```

Функция String\.Format\(\) ожидает 3 аргумента, а передано всего 2 аргумента\. В этом случае возникает [FormatException](https://msdn.microsoft.com/en-us/library/system.formatexception%28v=vs.110%29.aspx)\.

[V3014](https://pvs-studio.ru/ru/docs/warnings/v3014/) It is likely that a wrong variable is being incremented inside the 'for' operator\. Consider reviewing 'i'\. SparseArray\.cs 1956

```cpp
override public string ToString()
{
  StringBuilder str = new StringBuilder();

  for (int i = 0; i < data.Length; i++)
  {
    if (data[i] != null)
    {
      for (int j = 0; j < lastElement[i]; i++)  // <=
      {
        str.AppendFormat("({0},{1})", data[i][j].Index,
                                      data[i][j].Value);
      }
    }
  }

  return str.ToString();
}
```

Во вложенном цикле не изменяется переменная\-счётчик 'j', т\.к\. вместо 'j\+\+' выполняется изменение счётчика внешнего цикла 'i\+\+'\.

Ещё несколько подобных мест:

* V3014 It is likely that a wrong variable is being incremented inside the 'for' operator\. Consider reviewing 'k'\. Writer\.cs 3984
* V3014 It is likely that a wrong variable is being incremented inside the 'for' operator\. Consider reviewing 'count\_d'\. Octagons\.cs 509

[V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 203, 207\. WeakestPreconditionProver\.csToSMT2\.cs 203

```cpp
private BoxedExpression DeclareVariable(....)
{
  var tmp = original.ToString().Replace(' ', '_');
  this.Info.AddDeclaration(string.Format("....", tmp, type));
  this.ResultValue = tmp;

  if (type == FLOAT32)       // <=
  {
    types[original] = FloatType.Float32;
  }
  else if (type == FLOAT64)  // <=
  {
    types[original] = FloatType.Float64;
  }

  return original;
}
```

Анализатор обнаружил два одинаковых условных выражения, из\-за чего операторы во втором условии никогда не получат управления\. Хотя, на первый взгляд, это не так, мы перейдём к определению констант FLOAT32 и FLOAT64, и увидим следующий код:

```cpp
private const string FLOAT32 = "(_ FP 11 53)"; // To change!!!
private const string FLOAT64 = "(_ FP 11 53)";
```

Константы действительно равны\! Хотя здесь и есть комментарий, что необходимо произвести замену значения константы FLOAT32, это место легко пропустить в будущем\. В развивающихся проектах важно помечать такие места как TODO и регулярно просматривать результаты статического анализа кода\.

[V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 1200, 1210\. OutputPrettyCS\.cs 1200

```cpp
public enum TypeConstraint
{
  NONE,
  CLASS,     // <=
  STRUCT,    // <=
  BASECLASS,
}

public void Output(OutputHelper oh)
{
  Contract.Requires(oh != null);

  oh.Output("where ", false);
  mParent.OutputName(oh);
  oh.Output(" : ", false);
  //** base class
  bool comma = false;
  if (mTypeConstraint == TypeConstraint.CLASS)       // <=???
  {
    oh.Output("class", false);
    comma = true;
  }
  else if (mTypeConstraint == TypeConstraint.STRUCT)
  {
    oh.Output("struct", false);
    comma = true;
  }
  else if (mTypeConstraint == TypeConstraint.CLASS)  // <=???
  {
    oh.Output(mClassConstraint, false);
    comma = true;
  }
}
```

В этом фрагменте кода одинаковые условия более очевидны\. Скорее всего, в одном из условий переменную 'mTypeConstraint' хотели сравнить с константой TypeConstraint\.BASECLASS, вместо TypeConstraint\.CLASS\.

[V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) Expression 'c \> '\\xFFFF'' is always false\. Output\.cs 685

```cpp
private static string Encode(string s)
{
  ....
  foreach( char c in s ) {
    if (c == splitC || c == '\n' || c == '\\') {
      specialCount++;
    }
    else if (c > '\x7F') {
      if (c > '\xFFFF') specialCount += 9;
      else specialCount += 5;
    }
  }
  ....
}
```

Выражение "c \> '\\xFFFF'" никогда не будет истинным и оператор "specialCount \+\= 9" никогда не выполнится\. Переменная 'c' имеет тип Char, максимальное значение которого и есть "\\xFFFF'\. Не очень понятно, как должен работать этот код и как его надо исправить\. Возможно мы имеем дело с опечаткой или фрагментом кода, позаимствованным из приложения на другом языке\. Например, в языке Си/Си\+\+ иногда используются символы разрядностью 32\-бита\. И там как раз "играют" со значением 0xFFFF\. Пример подобного кода:

```cpp
/* putUTF8 -- write a character to stdout in UTF8 encoding */
static void putUTF8(long c)
{
  if (c <= 0x7F) {         /* Leave ASCII encoded */
    printf("&#%ld;", c);
  } else if (c <= 0x07FF) {     /* 110xxxxx 10xxxxxx */
    putchar(0xC0 | (c >> 6));
    putchar(0x80 | (c & 0x3F));
  } else if (c <= 0xFFFF) {     /* 1110xxxx + 2 */
    putchar(0xE0 | (c >> 12));
    putchar(0x80 | ((c >> 6) & 0x3F));
    putchar(0x80 | (c & 0x3F));
  } else if (c <= 0x1FFFFF) {    /* 11110xxx + 3 */
    putchar(0xF0 | (c >> 18));
    putchar(0x80 | ((c >> 12) & 0x3F));
    putchar(0x80 | ((c >> 6) & 0x3F));
    putchar(0x80 | (c & 0x3F));
  } else if (c <= 0x3FFFFFF) {    /* 111110xx + 4 */
    putchar(0xF8 | (c >> 24));
    putchar(0x80 | ((c >> 18) & 0x3F));
    putchar(0x80 | ((c >> 12) & 0x3F));
    putchar(0x80 | ((c >> 6) & 0x3F));
    putchar(0x80 | (c & 0x3F));
  } else if (c <= 0x7FFFFFFF) {    /* 1111110x + 5 */
    putchar(0xFC | (c >> 30));
    putchar(0x80 | ((c >> 24) & 0x3F));
    putchar(0x80 | ((c >> 18) & 0x3F));
    putchar(0x80 | ((c >> 12) & 0x3F));
    putchar(0x80 | ((c >> 6) & 0x3F));
    putchar(0x80 | (c & 0x3F));
  } else {          /* Not a valid character... */
    printf("&#%ld;", c);
  } 
}
```

[V3008](https://pvs-studio.ru/ru/docs/warnings/v3008/) The 'this\.InsideMonitor' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 751, 749\. AssertionCrawlerAnalysis\.cs 751

```cpp
private Data(Data state, Variable v)
{
  this.IsReached = state.IsReached;
  this.InsideMonitor = state.InsideMonitor;  // <=
  this.symbols = new List<Variable>(state.symbols) { v };
  this.InsideMonitor = false;                // <=???
}
```

Очень подозрительно, что некая функция изменяет состояние объекта с помощью значений, переданных через параметры функции и в последним момент перезаписывает значение поля "this\.InsideMonitor" константой 'false'\. Ранее уже было выполнено присваивание "this\.InsideMonitor \= state\.InsideMonitor"\.

[V3009](https://pvs-studio.ru/ru/docs/warnings/v3009/) It's odd that this method always returns one and the same value of 'true'\. LinearEqualities\.cs 5262

```cpp
public bool TryGetFirstAvailableDimension(out int dim)
{
  for (var i = 0; i < map.Length; i++)
  {
    if (!map[i])
    {
      dim = i;
      return true;
    }
  }

  map.Length++;

  dim = map.Length;

  return true;
}
```

Анализатор обнаружил функцию, которая всегда возвращает одно значение "true"\. Можно предположить, что при выполнении условия "\!map\[i\]" функция должна вернуть одно значение, а если условие ни разу не было истинным, то вернуть другое значение\. Возможно, здесь ошибка\.

### Остальные предупреждения

[V3025](https://pvs-studio.ru/ru/docs/warnings/v3025/) Incorrect format\. A different number of actual arguments is expected while calling 'Format' function\. Expected: 1\. Present: 2\. Output\.cs 68

```cpp
public override void WriteLine(string value)
{
  output.WriteLine(string.Format("{1}", DateTime.Now,
    value.Replace("{", "{{").Replace("}","}}")));
  //output.WriteLine(string.Format("[{0}] {1}",
    //DateTime.Now., value));
}
```

Раньше функция String\.Format\(\) принимала и печатала 2 значения: дату и некоторое значение\. Потом этот код закомментировали и написали другой вариант, в котором аргумент с индексом 0 не используется, т\.е\. дата теперь не печатается\.

Другие примеры вызова функций форматирования с неиспользованными аргументами:

* V3025 Incorrect format\. A different number of actual arguments is expected while calling 'Format' function\. Expected: 6\. Present: 7\. CacheModelExtensions\.cs 46
* V3025 Incorrect format\. A different number of actual arguments is expected while calling 'Format' function\. Expected: 1\. Present: 2\. CodeFixesInference\.cs 1608
* V3025 Incorrect format\. A different number of actual arguments is expected while calling 'Format' function\. Expected: 2\. Present: 3\. ExpressionManipulation\.cs 442

[V3004](https://pvs-studio.ru/ru/docs/warnings/v3004/) The 'then' statement is equivalent to the 'else' statement\. Metadata\.cs 2602

```cpp
private void SerializeFieldRvaTable(....)
{
  ....
  switch (row.TargetSection){
    case PESection.SData:
    case PESection.TLS:
      Fixup fixup = new Fixup();
      fixup.fixupLocation = writer.BaseStream.Position;
      fixup.addressOfNextInstruction = row.RVA;
      if (row.TargetSection == PESection.SData){
        sdataFixup.nextFixUp = fixup;   // <=
        sdataFixup = fixup;             // <=
      }else{
        sdataFixup.nextFixUp = fixup;   // <=
        sdataFixup = fixup;             // <=
      }
      writer.Write((int)0);
      break;
  ....
}
```

Анализатор обнаружил идентичные блоки кода в условном операторе\. Это может быть лишним кодом или один блок кода забыли изменить после копирования\. [Copy\-Paste](https://pvs-studio.ru/ru/blog/terms/0068/) не щадит и C\# программистов\.

Весь список подобных мест:

* V3004 The 'then' statement is equivalent to the 'else' statement\. Nodes\.cs 6698
* V3004 The 'then' statement is equivalent to the 'else' statement\. Nodes\.cs 6713
* V3004 The 'then' statement is equivalent to the 'else' statement\. WarningSuggestionLinkOutput\.cs 108
* V3004 The 'then' statement is equivalent to the 'else' statement\. NonNullAnalyzer\.cs 1753

[V3001](https://pvs-studio.ru/ru/docs/warnings/v3001/) There are identical sub\-expressions 'semanticType\.Name \=\= null' to the left and to the right of the '\|\|' operator\. ContractsProvider\.cs 694

```cpp
public bool TryGetTypeReference(....)
{
  ....
  if (semanticType.Name == null || semanticType.Name == null)
    goto ReturnFalse;
  cciType = new Microsoft.Cci.MutableCodeModel.NamespaceTypeR....
  {
    ContainingUnitNamespace = cciNamespace,
    GenericParameterCount = (ushort) (....),
    InternFactory = Host.InternFactory,
    IsValueType = semanticType.IsValueType,
    IsEnum = semanticType.TypeKind == TypeKind.Enum,
    Name = Host.NameTable.GetNameFor(semanticType.Name),
    TypeCode=CSharpToCCIHelper.GetPrimitiveTypeCode(semanticType)
  };
  goto ReturnTrue;'
  ....
}
```

Условие "semanticType\.Name \=\= null" проверяют 2 раза\. Либо проверка избыточна и её можно упростить, либо забыли проверить другое поле у объекта\.

Ещё одно предупреждение на эту тему:

* V3001 There are identical sub\-expressions 'semanticType\.Name \=\= null' to the left and to the right of the '\|\|' operator\. ContractsProvider\.cs 714

[V3019](https://pvs-studio.ru/ru/docs/warnings/v3019/) Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'other', 'right'\. CallerInvariant\.cs 189

```cpp
public override Predicate JoinWith(Predicate other)
{
  var right = other as PredicateNullness;
  if (other != null)
  {
    if (this.value == right.value)
    {
      return this;
    }
  }

  return PredicateTop.Value;
}
```

Анализатор обнаружил потенциальную ошибку, которая может привести к доступу по нулевой ссылке\. Необходимо сравнивать с 'null' результат выполнения оператора 'as'\.

Если возникнет ситуация, когда объект 'other' не будет нулевым, но его не удастся привести к типу 'PredicateNullness', то возникнет доступ по нулевой ссылке при обращении к "right\.value"\.

Таких сравнений в проекте найдено много, вот весь список:

* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'facts', 'moreRefinedFacts'\. SimplePostconditionDispatcher\.cs 319
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'objProvenance', 'provenance'\. AssertionCrawlerAnalysis\.cs 816
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'prev', 'other'\. NonRelationalValueAbstraction\.cs 1063
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'prev', 'castedPrev'\. GenericDomains\.cs 1657
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'a', 'right'\. LinearEqualitiesForSubpolyhedra\.cs 859
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'a', 'other'\. NonRelationalValueAbstraction\.cs 1047
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'a', 'other'\. NonRelationalValueAbstraction\.cs 1055
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'a', 'right'\. LinearEqualities\.cs 849
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'a', 'right'\. LinearEqualities\.cs 973
* V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'a', 'right'\. LinearEqualities\.cs 1119

[V3030](https://pvs-studio.ru/ru/docs/warnings/v3030/) Recurring check\. The 'this\.lineOffsets \=\= null' condition was already verified in line 612\. Nodes\.cs 613

```cpp
public virtual void InsertOrDeleteLines(....)
{
  ....
  if (this.lineOffsets == null)
    if (this.lineOffsets == null) this.ComputeLineOffsets();
  if (lineCount < 0)
    this.DeleteLines(offset, -lineCount);
  else
    this.InsertLines(offset, lineCount);
  ....
}
```

Идущие подряд две одинаковые проверки "this\.lineOffsets \=\= null"\. Такой код не имеет смысла\. Возможно, хотели проверить что\-то ещё\.

[V3002](https://pvs-studio.ru/ru/docs/warnings/v3002/) The switch statement does not cover all values of the 'UnaryOperator' enum: Conv\_dec\. WeakestPreconditionProver\.csToSMT2\.cs 453

```cpp
private string Combine(UnaryOperator unaryOperator, string arg)
{
  Contract.Requires(arg != null);

  var format = "({0} {1})";
  string op = null;

  switch (unaryOperator)
  {
    case UnaryOperator.Neg:
    case UnaryOperator.Not:
    case UnaryOperator.Not:
      {
        op = "not";
      }
      break;

    case UnaryOperator.WritableBytes:
    case UnaryOperator.Conv_i:
    case UnaryOperator.Conv_i1:
    case UnaryOperator.Conv_i2:
    case UnaryOperator.Conv_i4:
    case UnaryOperator.Conv_i8:
    case UnaryOperator.Conv_r_un:
    case UnaryOperator.Conv_r4:
    case UnaryOperator.Conv_r8:
    case UnaryOperator.Conv_u:
    case UnaryOperator.Conv_u1:
    case UnaryOperator.Conv_u2:
    case UnaryOperator.Conv_u4:
    case UnaryOperator.Conv_u8:
      {
        return null;
     }
  }

  return string.Format(format, op, arg);
}
```

Анализатор обнаружил оператор 'switch', в котором выбор варианта осуществляется по переменной enum\-типа\. При этом в операторе 'switch' пропущен 1 элемент "UnaryOperator\. Conv\_dec"\. Это очень подозрительно\.

Ниже приведено определение перечисления "UnaryOperator":

```cpp
public enum UnaryOperator
{
  ....
  Conv_u8,
  Conv_r_un,
  Neg,
  Not,
  WritableBytes,
  Conv_dec,      // <=
}
```

Возможная ошибка заключается в том, что данная функция реализована таким образом, чтобы для значения "UnaryOperator\.Not" возвращать форматированную строку, а в остальных случаях значение 'null'\. Но так как элемент "UnaryOperator\. Conv\_dec" пропущен, то для него значение переменной 'op' останется равно 'null' и оно попадёт в форматированную строку, которую вернёт функция\. 

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

Надеемся вам понравилась эта статья\. Обещаем и дальше радовать наших читателей проверками интересных открытых проектов\.

Как уже упоминалось выше, первый релиз намечен на 22\.12\.2015\. Как правило в конце года принимаются решения о будущих закупках\. Поэтому, всем заинтересовавшимся, предлагаем, не откладывая уже сейчас связаться с нами по вопросам [приобретения](https://pvs-studio.ru/ru/order/) PVS\-Studio\. И мы будем рады видеть вашу компанию среди наших довольных [клиентов](https://pvs-studio.ru/ru/about/customers/)\.

Спасибо за внимание\. И безбажного вам кода\!