Форум программистов [Powered by Invision Power Board]

Версия для печати темы
Нажмите сюда для просмотра этой темы в оригинальном формате

Форум программистов > Общие вопросы по .NET и C# > Проверка Open Live Writer

Автор: LittleGrumblerDron 25.12.2015, 14:26

Я студент и на данный момент прохожу производственную практику в ООО "СиПроВер". Попал я сюда очень вовремя, как раз к выходу нового релиза статического анализатора кода PVS-Studio 6.00. Так у меня появилась возможность стать одним из первых, кому удалось попробовать новые возможности PVS-Studio. Теперь помимо анализа кода, написанного на языках С и С++, добавилась возможность осуществлять проверку кода на языке С#. Вы тоже можете протестировать обновленный анализатор, перейдя по http://www.viva64.com/ru/pvs-studio-download/ и скачав пробную версию. Проектом, который я взял для проверки с помощью новой версии анализатора, стал редактор блогов Open Live Writer.

Open Live Writer

Разработчики описывают Open Live Writer, как лёгкий и мощный редактор блогов, позволяющий создавать свои посты, добавлять туда фото и видео и размещать всё это на своём веб-сайте. Open Live Writer работает со многими популярными сервисами, такими как: WordPress, Blogger, TypePad, DasBlog и др. Хотелось бы отметить, что несколько лет назад мир знал этот редактор под именем Windows Live Writer.

Редактор блогов Windows Live Writer (WLW) компания Microsoft выпустила в 2007 г. Ключевая идея приложения — возможность быстрого создания полноценной записи (с включением графики, фото и пр.) в автономном режиме, а потом прямой публикации на интернет-ресурсе без использования его панели управления. Редактор поддерживает широкий спектр блог-движков, включая Windows Live Spaces, SharePoint, Blogger, LiveJournal, WordPress. Приложение распространялось бесплатно, в том числе в рамках набора Windows Essentials, и довольно быстро стало весьма популярным среди блогеров. Три года назад Microsoft объявила о прекращении развития этого ПО. Однако несколько месяцев назад группа сотрудников компании заявила о намерении дать этому приложению вторую жизнь, и это начинание получило поддержку со стороны департамента Microsoft .NET Foundation. Сейчас команда представила результаты своей добровольной работы в виде открытого продукта http://openlivewriter.org/, исходный код которого был найден в соответствующем https://github.com/OpenLiveWriter/OpenLiveWriter.

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

Не могу не отметить высокое качество данного проекта, так как для такого большого количества кода ошибок нашлось крайне мало, но и их можно было бы избежать, регулярно проводя статический анализ в процессе разработки. Хочу заметить, что в проектах, написанных на C#, ошибок меньше, чем в проектах на C++. Это и понятно, язык более безопасен для разработчиков, нежели С++. Ниже будут рассмотрены некоторые наиболее интересные ошибки, обнаруженные в данном проекте.

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0381/ There are identical sub-expressions '_editMode != null' to the left and to the right of the '&&' operator. ContentEditorProxy.cs 377

Код


private EditingMode? _editMode;
public enum EditingMode
{
  Wysiwyg = 1,
  Preview = 2,
  Source = 4,
  PlainText = 8
}
void blogPostHtmlEditor_DocumentComplete(object sender, EventArgs e)
{
  ....
  if (_editMode != null &amp;&amp; _editMode.HasValue)
  ....
}

В данном отрезке кода используются оператор сравнения и свойство, выполняющие одну и ту же проверку для переменной '_editMode'. То есть вполне достаточно было использовать что-то одно, хотя не исключено, что разработчики пользуются поговоркой: "одна проверка - хорошо, а две - лучше".

Идём далее.

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0402/ The 'then' statement is equivalent to the 'else' statement. MiniTab.cs 147, 149

Код

protected override void OnPaint(PaintEventArgs e)
{
  ....
  Rectangle textBounds = tabRectangle;
  if (!selected)
    textBounds.Y += (int)DisplayHelper.ScaleX(3);
  else
    textBounds.Y += (int)DisplayHelper.ScaleX(3);
  ....
}

Тут всё видно невооруженным глазом. Огромное количество найденных нами ошибок было совершено по невнимательности, при использовании Copy-Paste. Однако в больших проектах, насчитывающих более тысячи файлов, это обычное явление, и такие ошибки разработчикам трудно отследить без анализатора.

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0406/ The return value of function 'Replace' is required to be utilized. DiagnosticsConsole.cs 699

Код

private void LoadEntries()
{
  ....
  text.Replace('\r', ' ');
  ....
}

Здесь анализатор обнаружил вызов метода, возвращаемое значение которого нигде не учитывается. Такие участки кода лишь занимают память и увеличивают время работы программы. Возможно, когда-то этот код использовался в каком-то методе, но разработчик удалил его, забыв про этот участок.

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

Код

private void LoadEntries()
{
  ....
  text = text.Replace('\r', ' ');
  ....
}

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0383/ The '?:' operator, regardless of its conditional expression, always returns one and the same value: ctx.Font. MiniTab.cs 156

Код

protected override void OnPaint(PaintEventArgs e)
{
  ....
  g.DrawText(Text, selected ? ctx.Font : ctx.Font, .... );
  ....
}

Здесь видно, что независимо от значения переменной 'selected' результат будет один и тот же. Опять же, это, скорее всего, следствие невнимательного использования Copy-Paste. Подобные ошибки встречаются еще в нескольких местах, которые перечислены ниже:

V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: Colorize(Color.FromArgb(229, 238, 248)). ColorizedResources.cs 264
V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: Color.FromArgb(99, 101, 99). ColorizedResources.cs 450
V3012 The '?:' operator, regardless of its conditional expression, always returns one and the same value: -PAD + 1. TabLightweightControl.cs 749

Во всех вышеперечисленных отрывках кода оператор '?:' возвращает одно и то же значение. То есть использование данного оператора нецелесообразно. Скорее всего, проблема кроется в том, что, как и в большинстве случаев, описанных здесь, разработчики использовали Copy-Paste и не изменили имена используемых переменных.

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0389/ It is odd that the body of 'workspaceColumnPaneUpper_VisibleChanged' function is fully equivalent to the body of 'workspaceColumnPaneLower_VisibleChanged' function. WorkspaceColumnLightweightControl.cs 1165

Код

private void workspaceColumnPaneUpper_VisibleChanged(
  object sender, EventArgs e)
{
  PerformLayout();
  Invalidate();
}
private void workspaceColumnPaneLower_VisibleChanged(
  object sender, EventArgs e)
{
  PerformLayout();
  Invalidate();
}

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

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0388/ Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'selection', 'htmlSelection'. HtmlEditorSidebarHost.cs 247

Код

private SidebarControl GetSidebarForCurrentSelection(object selection)
{
  IHtmlEditorSelection htmlSelection = selection as  
    IHtmlEditorSelection;
  if (selection == null || (!htmlSelection.IsValid &amp;&amp;     
      !InlineEditField.IsEditField(selection)))
    return _defaultSidebarControl;
  ....
}

Анализатор обнаружил потенциальную ошибку, которая может привести к доступу по нулевой ссылке. Сначала объект 'selection' приводится к интерфейсу 'IHtmlEditorSelection' с помощью оператора 'as'. А затем этот же объект проверяется на значение null, хотя в этом случае скорее всего предполагалось проверить на null объект 'htmlSelection'. Если же 'htmlSelection' будет равным null, а 'selection' - нет, будет сгенерировано исключение типа 'NullReferenceException'.

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0410/ An unconditional 'break' within a loop. LightWeightHTMLMetaData.cs 268

Код


public string Charset
{
  get
  {
    ....
    LightWeightTag[] metaTags =      
      BaseHTMLDocument.GetTagsByName("META");
      foreach (LightWeightTag metaTag in metaTags)
        {
          Attr charset = metaTag.BeginTag.GetAttribute(CHARSET);
          if (charset != null)
            m_charset = charset.Value;
          break;
        }
    ....
  }
  set
  {
    m_charset = value;
  }
}
private string m_charset;

В данном отрывке кода видно неаккуратное использование оператора 'break' в цикле. При таком использовании цикл завершится сразу после первой итерации. Для корректной работы тело цикла, возможно, стоит переписать следующим образом:

Код

foreach (LightWeightTag metaTag in metaTags)
{
  Attr charset = metaTag.BeginTag.GetAttribute(CHARSET);
  if (charset != null)
  {
    m_charset = charset.Value;
    break;
  }
}

Предупреждение PVS-Studio: http://www.viva64.com/ru/d/0411/ Consider inspecting this expression. The expression is excessive or contains a misprint. BlogAccountDetector.cs 126

Код

public enum MessageId
{
  None,
  ....
  WeblogAuthenticationError,
  ....
}
public bool UserAuthorizationError
{
  get { return (ErrorMessageType != MessageId.None) &amp;&amp;
      (ErrorMessageType == MessageId.WeblogAuthenticationError); }

В данном случае проверка свойства 'ErrorMessageType' на неравенство является избыточной, так как если это свойство будет равно одному из элементов перечисления, то автоматически будет неравно всем остальным. Но и тут разработчики решили перестраховаться и добавить еще одну проверку. Данный код можно упростить:

Код

get { return ErrorMessageType == MessageId.WeblogAuthenticationError;}

Заключение

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

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

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

Автор: chupachups 28.12.2015, 12:53

Иди на базар мандарины продавать. Совсем охренели - ужу в IT область со своей долбаной рекламой лезут, даже не удосужившись почистить ее от лоховских разводок:

Цитата

Я студент и на данный момент прохожу производственную практику

Цитата

Мы и дальше будем искать интересные проекты и проверять их нашим анализатором

Маркетинг - это по-старому (лет 20 назад) "развод лохов" !!!

©•Деточка, а вам не кажется, что ваше место возле параши?

Этот ответ добавлен с нового Винграда - http://ru.vingrad.com/Proverka-Open-Live-Writer-id567d2806ae201566238b4568#findElement_E7045_568106b5ae20157944d295c8_0

Автор: GrumblerDron 4.1.2016, 12:27

Солнышко, а что тебя здесь не устраивает? Я действительно студент. Проходил практику в этой фирме. Писать статьи было частью моей работы. Развода здесь никакого нет. Всё честно, проверил проект, выписал ошибки. Кто-то почитает и таких ошибок у себя не сделает, а кто-то, возможно, укажет мне на мои))) К чему вся эта желчь, мне не понятно. Есть какие то придирки к технической составляющей - пиши, если нет - то

Цитата

Этот ответ добавлен с нового Винграда - http://ru.vingrad.com/Proverka-Open-Live-Writer-id567d2806ae201566238b4568#findElement_E7045_568a3ae9ae2015f137fb050e_0