Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Первая статья про проверку C# проекта

Первая статья про проверку C# проекта

13 Ноя 2015

Сейчас команда PVS-Studio активно занимается разработкой статического анализатора C# кода. Мы планируем выпустить первую версию анализатора к концу 2015 года. А пока моя задача - написать несколько статей, чтобы заранее заинтересовать C# программистов инструментом PVS-Studio. Сегодня мне выдали обновлённый инсталлятор. Теперь стало возможным установить PVS-Studio с поддержкой C# и даже что-то проверить. Я не замедлил этим воспользоваться и решил проверить первое, что подвернется под руку. Первым подвернулся проект Umbraco. Конечно, пока текущая версия анализатора умеет мало, но этого мне уже сейчас хватит для написания маленькой статьи.

0357_UmbracoCMS_ru/image1.png

Umbraco

Umbraco - это платформа системы управления контента с открытым кодом, использующаяся для публикации контента во всемирной сети. Она написана на языке C#. С момента выхода 4.5 вся система стала доступной под лицензией MIT.

Проект имеет средний размер. Однако часть, написанная на C#, не очень велика. Большая часть проекта написана на JavaScript. Всего в проекте около 3200 файлов с расширением ".cs", суммарным размером в 15 мегабайт. Количество строк С# кода: 400 KLOC.

О PVS-Studio 6.00

Проверка будет осуществлять с помощью Альфа-версии анализатора PVS-Studio 6.00. В этой версии произойдут два больших изменения:

  • Будет поддержан анализ C# проектов.
  • Анализатор перестанет поддерживать VS2005, VS2008. Небольшому количеству пользователей, работающих с VS2005/2008 мы предлагаем продолжить использовать версию 5.31 или более старшие версии 5.xx, если будут исправляться какие-то ошибки.

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

Почему C#?

На конференциях я нередко говорил, что делать C# анализатор не очень интересно. Множество ошибок, которые присутствуют в C++, в языке C# просто невозможны. Это действительно так. Например, в C# нет таких функций как memset(), соответственно и нет массы проблем (см. примеры, связанные с memset(): V511, V512, V575, V579, V597, V598).

Однако постепенно я изменил своё мнение. Большое количество ошибок, обнаруживаемых PVS-Studio, связано не с какими-то особенностями языка программирования, а с невнимательностью программистов. Я имею ввиду опечатки и неудачное изменение кода после copy-paste. Именно в этом силён анализатор PVS-Studio для C++, и мы решили, что эти наработки можно применить и для C#.

Язык C# не защищает от того, чтобы перепутать имя переменной или от "эффекта последней строки", связанного с утратой внимания.

Ещё одним важным фактором, который подтолкнул решиться на создание C# анализатора, является появление Roslyn. Без него работа по созданию анализатора была бы слишком дорогой для нас.

Roslyn - это открытая платформа компиляции C# и Visual Basic. Roslyn выполняет два основных действия: строит синтаксическое дерево (парсинг) и компилирует его. Дополнительно позволяет анализировать исходный код, рекурсивно обходить его, работать с проектами Visual Studio, выполнять код на лету.

Что нашлось интересного?

Для C++ у меня есть любимая диагностика V501. Теперь есть её аналог и для C#: V3001. Начнем именно с этой диагностики.

Фрагмент кода N1

В коде есть свойство "фокальная точка":

[DataMember(Name = "focalPoint")]
public ImageCropFocalPoint FocalPoint { get; set; }

Данное свойство имеет тип 'ImageCropFocalPoint', определение которого приведено ниже:

public class ImageCropFocalPoint
{
  [DataMember(Name = "left")]
  public decimal Left { get; set; }

  [DataMember(Name = "top")]
  public decimal Top { get; set; }
}

Казалось бы, при работе с таким свойством невозможно ошибиться. Ан нет. В методе HasFocalPoint() допущена досадная опечатка:

public bool HasFocalPoint()
{
  return FocalPoint != null &&
   FocalPoint.Top != 0.5m && FocalPoint.Top != 0.5m;
}

Два раза проверяется 'Top', а про 'Left' забыли.

Соответствующее предупреждение PVS-Studio: V3001 There are identical sub-expressions 'FocalPoint.Top != 0.5m' to the left and to the right of the '&&' operator. ImageCropDataSet.cs 58

Фрагмент кода N2

protected virtual void OnBeforeNodeRender(ref XmlTree sender,
            ref XmlTreeNode node,
            EventArgs e)
{
  if (node != null && node != null)
  {
    if (BeforeNodeRender != null)
      BeforeNodeRender(ref sender, ref node, e);    
  }
}

Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'node != null' to the left and to the right of the '&&' operator. BaseTree.cs 503

Дважды проверяется ссылка 'node'. Скорее всего хотели проверить ещё и ссылку 'sender'.

Фрагмент кода N3

public void Set (ExifTag key, string value)
{
  if (items.ContainsKey (key))
    items.Remove (key);
  if (key == ExifTag.WindowsTitle ||   // <=
      key == ExifTag.WindowsTitle ||   // <=
      key == ExifTag.WindowsComment ||
      key == ExifTag.WindowsAuthor ||
      key == ExifTag.WindowsKeywords ||
      key == ExifTag.WindowsSubject) {
    items.Add (key, new WindowsByteString (key, value));
  ....
}

Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'key == ExifTag.WindowsTitle' to the left and to the right of the '||' operator. ExifPropertyCollection.cs 78

Ключ два раза сравнивается с константой 'ExifTag.WindowsTitle'. Мне сложно судить, насколько серьёзна эта ошибка. Возможно, одна из проверок просто лишняя, и её можно удалить. Но возможно, здесь должно быть сравнение с какой-то другой константой.

Фрагмент кода N4

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

Имеется перечисление с 4 именованными константами:

public enum DBTypes
{
  Integer,
  Date,
  Nvarchar,
  Ntext
}

А вот метод SetProperty() почему-то рассматривает только 3 варианта. Ещё раз повторюсь, я не говорю, что это ошибка. Однако анализатор предлагает обратить внимание на этот фрагмент кода и я с ним полностью согласен.

public static Content SetProperty(....)
{
  ....
  switch (((DefaultData)property.PropertyType.
    DataTypeDefinition.DataType.Data).DatabaseType)
  {
    case DBTypes.Ntext:
    case DBTypes.Nvarchar:
      property.Value = preValue.Id.ToString();
      break;

    case DBTypes.Integer:
      property.Value = preValue.Id;
      break;
  }
  ....
}

Предупреждение PVS-Studio: V3002 The switch statement does not cover all values of the 'DBTypes' enum: Date. ContentExtensions.cs 286

Фрагмент кода N5

public TinyMCE(IData Data, string Configuration)
{
  ....
  if (p.Alias.StartsWith("."))
    styles += p.Text + "=" + p.Alias;
  else
    styles += p.Text + "=" + p.Alias;
  ....
}

Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. TinyMCE.cs 170

Фрагмент кода N6, N7

В начале статьи я говорил, что C# не защищает от "эффекта последней строки". И вот мы добрались до соответствующего примера:

public void SavePassword(IMember member, string password)
{
  ....
  member.RawPasswordValue = result.RawPasswordValue;
  member.LastPasswordChangeDate = result.LastPasswordChangeDate;
  member.UpdateDate = member.UpdateDate;
}

Предупреждение PVS-Studio: V3005 The 'member.UpdateDate' variable is assigned to itself. MemberService.cs 114

Программист копировал члены класса из объекта 'result' в объект 'member'. Но в последний момент человек расслабился и скопировал член 'member.UpdateDate' сам в себя.

Дополнительно настораживает, что метод SavePassword() работает с паролями, а значит, требует повышенного к себе внимания.

Точно такой же фрагмент кода можно наблюдать в файле UserService.cs (см. строка 269). Скорее всего его просто туда скопировали, не проверив.

Фрагмент кода N8

private bool ConvertPropertyValueByDataType(....)
{
  if (string.IsNullOrEmpty(string.Format("{0}", result)))
  {
    result = false;
    return true;
  }
  ....
    return true;
  ....
    return true;
  ....
    return true;
  ....
    return true;
  ....
  ....
  return true;
}

Предупреждение PVS-Studio: V3009 It's odd that this method always returns one and the same value of 'true'. DynamicNode.cs 695

В методе используется много операторов 'if' и много операторов 'return'. Настораживает то, что все операторы 'return' возвращают значение 'true'. Нет-ли в этом ошибки? Быть может всё-таки где-то надо было вернуть 'false'?

Фрагмент кода N9

Предлагаю читателям проверить внимательность и найти ошибку в следующем фрагменте кода. Для этого изучите метод, но не читайте текст ниже. Чтобы не сделать это случайно, я вставил разделитель (единорога :).

public static string GetTreePathFromFilePath(string filePath)
{
  List<string> treePath = new List<string>();
  treePath.Add("-1");
  treePath.Add("init");
  string[] pathPaths = filePath.Split('/');
  pathPaths.Reverse();
  for (int p = 0; p < pathPaths.Length; p++)
  {
    treePath.Add(
      string.Join("/", pathPaths.Take(p + 1).ToArray()));
  }
  string sPath = string.Join(",", treePath.ToArray());
  return sPath;
}
0357_UmbracoCMS_ru/image2.png

Рисунок 1. Отделяет код от пояснения, в чем ошибка.

Предупреждение PVS-Studio: V3010 The return value of function 'Reverse' is required to be utilized. DeepLink.cs 19

Вызывая метод Reverse(), программист планировал изменить массив 'pathPaths'. Возможно его сбило с толку то, что такая операция совершенно корректна, если речь идёт, например, о списках (List<T>.Reverse). Однако применительно к массивам, метод Reverse() не меняет исходный массив. Для массивов этот метод реализуется за счёт метода-расширения Reverse() класса 'Enumerable'. Этот метод не производит перестановку на месте, а возвращает изменённую коллекцию.

Правильно будет написать:

string[] pathPaths = filePath.Split('/');
pathPaths = pathPaths.Reverse().ToArray();

Или даже так:

string[] pathPaths = filePath.Split('/').Reverse().ToArray();

Фрагмент кода N10

Анализатор PVS-Studio выдал несколько предупреждений V3013, о подозрительном совпадении тел некоторых методов. На мой взгляд, все эти предупреждения ложные. Как мне кажется, внимания заслуживает только одно из этих предупреждений:

public void GetAbsolutePathDecoded(string input, string expected)
{
    var source = new Uri(input, UriKind.RelativeOrAbsolute);
    var output = source.GetSafeAbsolutePathDecoded();
    Assert.AreEqual(expected, output);
}
public void GetSafeAbsolutePathDecoded(string input, string expected)
{
    var source = new Uri(input, UriKind.RelativeOrAbsolute);
    var output = source.GetSafeAbsolutePathDecoded();
    Assert.AreEqual(expected, output);
}

Предупреждение PVS-Studio: V3013 It is odd that the body of 'GetAbsolutePathDecoded' function is fully equivalent to the body of 'GetSafeAbsolutePathDecoded' function. UriExtensionsTests.cs 141

Возможно внутри метода GetAbsolutePathDecoded(), нужно использовать не

source.GetSafeAbsolutePathDecoded()

а

source. GetAbsolutePathDecoded()

Я не уверен, что прав, но это место заслуживает проверки.

Ответы на вопросы

Статья рассчитана на новую аудиторию. Поэтому я заранее предвижу ряд вопросов, которые могут возникнуть. Попробую заранее ответить на часть из них.

Вы уведомили разработчиков проекта о найденных дефектах?

Да, мы всегда стараемся это сделать.

Вы используете PVS-Studio для проверки кода самого PVS-Studio?

Да.

PVS-Studio поддерживает Mono?

Нет.

Более подробно ответы на эти и другие вопросы даны в заметке: "Ответы на вопросы читателей статей про PVS-Studio".

Заключение

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

  • Статический анализатор — это инструмент регулярного использования. Его смысл находить ошибки на самом раннем этапе. Нет смысла в разовых проверках проекта. Как правило так выявляются ошибки, не оказывающие существенного влияния на работу программы или расположенные в редко используемых участках кода. Причина в том, что настоящие ошибки всё это время исправлялись потом и кровью. Их находили программисты, часами отлаживая код, их находили тестеры и что ещё хуже - пользователи. Многие из этих ошибок можно было бы сразу исправить, если регулярно использовать статический анализатор кода. Рассматривайте PVS-Studio как расширение предупреждений C# компилятора. Вы ведь, надеюсь, смотрите список предупреждений, выданных компилятором, не раз в год? Более подробно всё это разобрано в статье: "Лев Толстой и статический анализ кода".
  • В статьях мы упоминаем только о тех фрагментах кода, которые показались нам интересными. Как правило мы не пишем о коде, который анализатор совершенно честно посчитал подозрительным, но при этом видно, что настоящей ошибки нет. Мы называем этот "код с запахом". Когда вы используете PVS-Studio, этот код стоит проверить. Но рассказывать про такие места в статье неуместно.
  • Этого пункта нет для C++, но он существует пока для C#. Мы реализовали ещё не так много диагностик, но быстро движемся. Дайте нашему C#-единорогу немного подрасти. Уж он вам тогда покажет!

Спасибо всем за внимание и желаю всем безбажных программ.

Популярные статьи по теме
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

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

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

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

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…

Комментарии (0)

Следующие комментарии

На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять