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

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

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

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

Ваше сообщение отправлено.

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


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

>
>
>
Повторная проверка SharpDevelop: что но…

Повторная проверка SharpDevelop: что нового?

30 Янв 2017

Инструмент PVS-Studio постоянно совершенствуется. При этом наиболее динамично в настоящее время развивается анализатор C# кода: в 2016 году в него было добавлено девяносто новых диагностических правил. Ну а лучшим показателем качества работы анализатора являются обнаруживаемые им ошибки. Всегда интересно, а также достаточно полезно, проводить повторные проверки больших открытых проектов, сравнивая результаты. Сегодня я остановлюсь на повторной проверке проекта SharpDevelop.

0469_SharpDevelop_ru/image1.png

Введение

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

Для проверки я загрузил актуальную версию исходного кода SharpDevelop с портала GitHub. Проект содержит около миллиона строк кода на языке C#. В процессе работы анализатор выдал 809 предупреждений. Из них первого уровня - 74, второго - 508, третьего - 227:

0469_SharpDevelop_ru/image2.png

Не будем рассматривать предупреждения на уровне Low: статистически там велик процент ложных срабатываний. Анализ предупреждений на уровнях Meduim и High (582 предупреждения) выявил наличие около 40% ошибочных, либо крайне подозрительных конструкций. Это составляет 233 предупреждения. Другими словами, анализатор PVS-Studio в среднем нашел 0.23 ошибки на 1000 строк кода. Это говорит о высоком качестве кода проекта SharpDevelop. На многих других проектах всё выглядит куда более печально.

В результате повторной проверки была обнаружена часть ошибок, описанных ранее Андреем в его статье. Но большая часть найденных ошибок - новые. Далее я приведу наиболее интересные из них.

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

Эталонная ошибка Copy-Paste

Ошибка, которая по праву может занять почетное место в "палате мер и весов". Яркая иллюстрация полезности использования статического анализа кода и опасности Copy-Paste.

Предупреждение анализатора PVS-Studio: V3102 Suspicious access to element of 'method.SequencePoints' object by a constant index inside a loop. CodeCoverageMethodTreeNode.cs 52

public override void ActivateItem()
{
  if (method != null && method.SequencePoints.Count > 0) {
    CodeCoverageSequencePoint firstSequencePoint =  
      method.SequencePoints[0];
    ....
    for (int i = 1; i < method.SequencePoints.Count; ++i) {
      CodeCoverageSequencePoint sequencePoint = 
        method.SequencePoints[0];  // <=
      ....
    }
    ....
  }
  ....
}

На каждой итерации цикла for используют доступ к нулевому элементу коллекции. Я умышленно привел дополнительный фрагмент кода, находящийся сразу после условия if. Легко заметить, откуда была скопирована строка, помещенная внутрь цикла. Имя переменной firstSequencePoint заменили на sequencePoint, а вот выражение доступа по индексу изменить забыли. Правильный вариант данной конструкции имеет вид:

public override void ActivateItem()
{
  if (method != null && method.SequencePoints.Count > 0) {
    CodeCoverageSequencePoint firstSequencePoint =  
      method.SequencePoints[0];
    ....
    for (int i = 1; i < method.SequencePoints.Count; ++i) {
      CodeCoverageSequencePoint sequencePoint = 
        method.SequencePoints[i];
      ....
    }
    ....
  }
  ....
}

"Найдите 10 отличий", или снова Copy-Paste

Предупреждение анализатора PVS-Studio: V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless NamespaceTreeNode.cs 87

public int Compare(SharpTreeNode x, SharpTreeNode y)
{
  ....
  if (typeNameComparison == 0) {
    if (x.Text.ToString().Length < y.Text.ToString().Length)  // <=
      return -1;
    if (x.Text.ToString().Length < y.Text.ToString().Length)  // <=
      return 1;
  }  
  ....
}

Оба блока if содержат одинаковое условие. В данном случае трудно судить о том, как должен выглядеть правильный вариант программы. Необходимо изучение фрагмента кода его автором.

Несвоевременная проверка на равенство null

Предупреждение анализатора PVS-Studio: V3095 The 'position' object was used before it was verified against null. Check lines: 204, 206. Task.cs 204

public void JumpToPosition()
{
  if (hasLocation && !position.IsDeleted)  // <=
    ....
  else if (position != null)
    ....
}

Переменную position используют, не проверив на равенство null. Проверка производится в другом условии, в блоке кода else. Правильный вариант кода мог бы иметь вид:

public void JumpToPosition()
{
  if (hasLocation && position != null && !position.IsDeleted)
    ....
  else if (position != null)
    ....
}

Пропущенная проверка на равенство null

Предупреждение анализатора PVS-Studio: V3125 The 'mainAssemblyList' object was used after it was verified against null. Check lines: 304, 291. ClassBrowserPad.cs 304

void UpdateActiveWorkspace()
{
  var mainAssemblyList = SD.ClassBrowser.MainAssemblyList;
  if ((mainAssemblyList != null) && (activeWorkspace != null)) {
    ....
  }
  ....
  mainAssemblyList.Assemblies.Clear();  // <=
  ....
}

Переменную mainAssemblyList используют без предварительной проверки на равенство null. При этом другой фрагмент кода содержит такую проверку. Корректный вариант кода:

void UpdateActiveWorkspace()
{
  var mainAssemblyList = SD.ClassBrowser.MainAssemblyList;
  if ((mainAssemblyList != null) && (activeWorkspace != null)) {
    ....
  }
  ....
  if (mainAssemblyList != null) {
    mainAssemblyList.Assemblies.Clear();
  }  
  ....
}

Неожиданный результат сортировки

Предупреждение анализатора PVS-Studio: V3078 Original sorting order will be lost after repetitive call to 'OrderBy' method. Use 'ThenBy' method to preserve the original sorting. CodeCoverageMethodElement.cs 124

void Init()
{
  ....
  this.SequencePoints.OrderBy(item => item.Line)
                     .OrderBy(item => item.Column);  // <=
}

Результатом работы данного фрагмента кода будет сортировка коллекции SequencePoints только по полю Column. По всей видимости, это не совсем то, что ожидал автор кода. Проблема заключается в том, что повторный вызов метода OrderBy выполнит сортировку коллекции без учета результата предыдущей сортировки. Для исправления ситуации необходимо использовать ThenBy вместо повторного вызова OrderBy:

void Init()
{
  ....
  this.SequencePoints.OrderBy(item => item.Line)
                     .ThenBy(item => item.Column);
}

Потенциальная возможность деления на ноль

Предупреждение анализатора PVS-Studio: V3064 Potential division by zero. Consider inspecting denominator 'workAmount'. XamlSymbolSearch.cs 60

public XamlSymbolSearch(IProject project, ISymbol entity)
{
  ....
  interestingFileNames = new List<FileName>();
  ....
  foreach (var item in ....)
    interestingFileNames.Add(item.FileName);
  ....
  workAmount = interestingFileNames.Count;
  workAmountInverse = 1.0 / workAmount;  // <=
}

В случае, если коллекция interestingFileNames окажется пустой, произойдет деление на ноль. Здесь достаточно трудно предложить подходящий вариант исправления ошибки. Но, в любом случае, требуется доработка алгоритма вычисления значения переменной workAmountInverse в ситуации, когда переменная workAmount будет равна нулю.

Повторное присваивание

Предупреждение анализатора PVS-Studio: V3008 The 'ignoreDialogIdSelectedInTextEditor' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 204, 201. WixDialogDesigner.cs 204

void OpenDesigner()
{
  try {
    ignoreDialogIdSelectedInTextEditor = true;  // <=
    WorkbenchWindow.ActiveViewContent = this;
  } finally {
    ignoreDialogIdSelectedInTextEditor = false;  // <=
  }
}

Переменная ignoreDialogIdSelectedInTextEditor получит значение false независимо от результата выполнения блока try. Для исключения вероятности наличия "подводных камней" ознакомимся с объявлением используемых переменных. Объявление ignoreDialogIdSelectedInTextEditor имеет вид:

bool ignoreDialogIdSelectedInTextEditor;

Объявление IWorkbenchWindow и ActiveViewContent:

public IWorkbenchWindow WorkbenchWindow {
  get { return workbenchWindow; }
}
IViewContent ActiveViewContent {
  get;
  set;
}

Как видим, нет никаких очевидных причин для повторного присвоения переменной ignoreDialogIdSelectedInTextEditor значения. Рискну предположить, что корректный вариант приведенной конструкции мог бы отличаться от оригинала использованием ключевого слова catch вместо finally:

void OpenDesigner()
{
  try {
    ignoreDialogIdSelectedInTextEditor = true;
    WorkbenchWindow.ActiveViewContent = this;
  } catch {
    ignoreDialogIdSelectedInTextEditor = false;
  }
}

Ошибочный поиск подстроки

Предупреждение анализатора PVS-Studio: V3053 An excessive expression. Examine the substrings '/debug' and '/debugport'. NDebugger.cs 287

public bool IsKernelDebuggerEnabled {
  get {
    ....
    if (systemStartOptions.Contains("/debug") ||
     systemStartOptions.Contains("/crashdebug") ||
     systemStartOptions.Contains("/debugport") ||  // <=
     systemStartOptions.Contains("/baudrate")) {
      return true;
    }
    ....
  }
}

В строке systemStartOptions производится последовательный поиск одной из подстрок "/debug" или "/debugport". Проблема заключается в том, что строка "/debug" сама является подстрокой для "/debugport". Таким образом, нахождение подстроки "/debug" делает дальнейший поиск подстроки "/debugport" бессмысленным. Пожалуй, это не ошибка, но код можно упростить:

public bool IsKernelDebuggerEnabled {
  get {
    ....
    if (systemStartOptions.Contains("/debug") ||
     systemStartOptions.Contains("/crashdebug") ||
     systemStartOptions.Contains("/baudrate")) {
      return true;
    }
    ....
  }
}

Ошибка обработки исключения

Предупреждение анализатора PVS-Studio: V3052 The original exception object 'ex' was swallowed. Stack of original exception could be lost. ReferenceFolderNodeCommands.cs 130

DiscoveryClientProtocol DiscoverWebServices(....)
{
  try {
    ....
  } catch (WebException ex) {
    if (....) {
      ....
    } else {
      throw ex;  // <=
    }
  }
  ....
}

В данном случае вызов throw ex приведет к "затиранию" стека оригинального исключения, так как перехваченное исключение будет сгенерировано повторно. Исправленный вариант:

DiscoveryClientProtocol DiscoverWebServices(....)
{
  try {
    ....
  } catch (WebException ex) {
    if (....) {
      ....
    } else {
      throw;
    }
  }
  ....
}

Использование неинициализированного поля в конструкторе класса

Предупреждение анализатора PVS-Studio: V3128 The 'contentPanel' field is used before it is initialized in constructor. SearchResultsPad.cs 66

Grid contentPanel;
public SearchResultsPad()
{
  ....
  defaultToolbarItems = ToolBarService
    .CreateToolBarItems(contentPanel, ....);  // <=
  ....
  contentPanel = new Grid {....};
  ....
}

Поле contentPanel передается в качестве одного из параметров в метод CreateToolBarItems в конструкторе класса SearchResultsPad. При этом инициализация данного поля производится уже после использования. Возможно, в данном случае ошибки нет, так как в теле метода CreateToolBarItems и далее по стеку может быть учтена возможность равенства null переменной contentPanel. Но код выглядит подозрительно и требует проверки автором.

Ещё раз подчеркну, что я описал в статье далеко не все ошибки, которые обнаружил анализатор PVS-Studio, а только те, что показались мне интересными. Авторы проекта могут обратиться к нам и получить временный лицензионный ключ, чтобы выполнить более тщательную проверку проекта.

Заключение

И вновь PVS-Studio не подвел: в ходе повторной проверки проекта SharpDevelop были найдены новые интересные ошибки. А это значит, что анализатор отлично справляется со своей работой, позволяя делать мир вокруг нас чуть более совершенным.

Напоминаю, что и вы можете в любое время включиться в данный процесс, воспользовавшись возможностью бесплатного использования статического анализатора PVS-Studio для проверки собственных проектов.

Скачать и попробовать PVS-Studio: /ru/pvs-studio/

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

Популярные статьи по теме
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

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

Дата: 14 Апр 2016

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

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

Дата: 19 Май 2017

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

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

Дата: 22 Дек 2018

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

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

Дата: 30 Янв 2019

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

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

Дата: 16 Окт 2017

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

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

Дата: 31 Июл 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 21 Ноя 2018

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

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

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

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