Самые быстрые отчёты на диком западе. И горстка багов в придачу…
Не только Microsoft в последнее время выкладывает код собственных проектов в открытый доступ — другие компании тоже следуют этой тенденции. Для нас же — разработчиков PVS-Studio — это отличный способ ещё раз протестировать анализатор, посмотреть, что интересного он сможет найти и сообщить об этом авторам проекта. Сегодня заглядываем внутрь проекта компании Fast Reports.
Что проверяли?
FastReport — генератор отчётов, разрабатываемый компанией Fast Reports. Написан на C# и совместим с .Net Standard 2.0+. Исходный код проекта недавно выложили на GitHub, откуда он и был загружен для последующего анализа.
Отчёты поддерживают использование текста, изображений, линий, фигур, диаграмм, таблиц, штрихкодов и пр. Могут быть одностраничными и многостраничными, в том числе содержать помимо данных обложку и заднюю страницу. В качестве источников данных могут выступать XML, CSV, Json, MS SQL, MySql, Oracle, Postgres, MongoDB, Couchbase, RavenDB, SQLite.
Существуют различные способы создания шаблонов отчётов: из кода, как XML-файла, с использованием онлайн дизайнера или FastReport Designer Community Edition.
При необходимости библиотеки можно загрузить как NuGet пакеты.
Более полно о возможностях продукта можно почитать на GitHub-странице проекта.
Со сборкой проекта никаких проблем не возникло — собирал его из Visual Studio 2017, откуда в дальнейшем и проверил с помощью плагина PVS-Studio.
PVS-Studio — статический анализатор, который ищет ошибки в коде на C, C++, C#, Java. При анализе C# кода можно использовать анализатор c IDE Visual Studio, используя для этого плагин PVS-Studio, или же можно проверять проекты из командной строки, для чего используется command-line утилита PVS-Studio_Cmd.exe. При желании можно настроить анализ на сборочном сервере или подцепить результаты анализа к SonarQube.
Что ж, давайте посмотрим, что интересного нашлось на этот раз.
Так как проект небольшой, на множество опечаток и подозрительных мест рассчитывать не стоит. Посмотрим на то, что нашлось, а что-то даже попробуем воспроизвести на практике.
Очепятки и не только
Встретился следующий метод:
public override string ToString() {
if (_value == null) return null;
return this.String;
}
Предупреждение PVS-Studio: V3108 It is not recommended to return 'null' from 'ToSting ()' method. Variant.cs 1519
Да, возвращение null из переопределённого метода ToString () само по себе не является ошибкой, но всё же это — плохой стиль. Это указано, в том числе, в документации от Microsoft: Your ToString () override should not return Empty or a null string. Разработчики, не ожидающие возврата null в качестве возвращаемого значения ToString (), могут быть неприятно удивлены, когда в ходе исполнения представленного ниже кода будет сгенерировано исключение ArgumentNullException (при условии, что вызывается метод расширения для IEnumerable
Variant varObj = new Variant();
varObj.ToString().Contains(character);
Можете придраться к тому, что пример синтетический, но суть от этого не меняется.
Более того, данный код прокомментирован следующим образом:
///
/// Returns
///
Упс. Вместо » возвращает null.
Продолжим.
В библиотеке есть класс FastString. Описание: Fast alternative of StringBuilder. Собственно, этот класс содержит поле типа StringBuilder. Конструкторы класса FastString вызывают метод Init, который выполняет инициализацию соответствующего поля.
Код одного из конструкторов:
public FastString()
{
Init(initCapacity);
}
И код метода Init:
private void Init(int iniCapacity)
{
sb = new StringBuilder(iniCapacity);
//chars = new char[iniCapacity];
//capacity = iniCapacity;
}
При желании можно обратиться к полю sb через свойство StringBuilder:
public StringBuilder StringBuilder
{
get { return sb; }
}
Всего FastString имеет 3 конструктора:
public FastString();
public FastString(int iniCapacity);
public FastString(string initValue);
Тело первого конструктора я уже показал, что делают два остальных догадаться, думаю, тоже несложно. А теперь, внимание. Угадайте, что выведет следующий код:
FastString fs = new FastString(256);
Console.WriteLine(fs.StringBuilder.Capacity);
Внимание, ответ:
Неожиданно? Давайте посмотрим на тело соответствующего конструктора:
public FastString(int iniCapacity)
{
Init(initCapacity);
}
У постоянных читателей наших статей уже должен быть намётан глаз, чтобы найти здесь проблему. У анализатора глаз (нюх, логика, называйте как хотите) точно намётан, и проблему он нашёл: V3117 Constructor parameter 'iniCapacity' is not used. FastString.cs 434
Какое совпадение, что в коде класса содержится константное поле initCapacity, которое и передаётся в качестве аргумента методу Init вместо параметра конструктора iniCapacity…
private const int initCapacity = 32;
При использовании схожих имён нужно быть очень, очень внимательным. Где только ни встречались ошибки, связанные с использованием похожих имён — проекты на C, C++, C#, Java — везде были опечатки подобного рода…
Кстати, про опечатки.
Сделаем следующий простенький пример и посмотрим, как он работает:
static void Main(string[] args)
{
TextObject textObj = new TextObject();
textObj.ParagraphFormat = null;
Console.WriteLine("Ok");
}
Как вы уже догадались, вывод будет другим, нежели строка «Ok» :)
Каким? Таким, например:
Проблема кроется в свойстве ParagraphFormat и в использовании схожих имён:
public ParagraphFormat ParagraphFormat
{
get { return paragraphFormat; }
set { ParagraphFormat = value; }
}
Предупреждение PVS-Studio: V3110 Possible infinite recursion inside 'ParagraphFormat' property. TextObject.cs 281
Свойство ParagraphFormat является обёрткой над полем paragraphFormat. Причём его get property accessor написан правильно, а вот set property accessor содержит досадную опечатку: вместо поля запись происходит в это же свойство, из-за чего возникает рекурсия. Опять ошибка, связанная со схожими именами.
Рассмотрим следующий фрагмент кода.
public override Run Split(float availableWidth, out Run secondPart)
{
....
if (r.Width > availableWidth)
{
List list = new List();
for (int i = point; i < size; i++)
list.Add(chars[i]);
secondPart = new RunText(renderer, word, style, list,
left + r.Width, charIndex);
list.Clear();
for (int i = 0; i < point; i++)
list.Add(chars[i]);
r = new RunText(renderer, word, style, list, left, charIndex);
return r;
}
else
{
List list = new List();
for (int i = point; i < size; i++)
list.Add(chars[i]);
secondPart = new RunText(renderer, word, style, list,
left + r.Width, charIndex);
list.Clear();
for (int i = 0; i < point; i++)
list.Add(chars[i]);
r = new RunText(renderer, word, style, list, left, charIndex);
return r;
}
....
}
Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. HtmlTextRenderer.cs 2092
Немного копипаста, и теперь вне зависимости от значения выражения r.Width > availableWidth будут выполняться одни и те же действия. Стоит или убрать оператор if, или изменить логику в одной из веток.
public static string GetExpression(FindTextArgs args,
bool skipStrings)
{
while (args.StartIndex < args.Text.Length)
{
if (!FindMatchingBrackets(args, skipStrings))
break;
return args.FoundText;
}
return "";
}
Предупреждение анализатора: V3020 An unconditional 'return' within a loop. CodeUtils.cs 262
Из-за безусловного оператора return для приведённого выше цикла выполнится не более одной итерации. Возможно, такой код получился после рефакторинга, а может, это просто необычный способ сделать то, что можно было бы сделать без цикла.
private int FindBarItem(string c)
{
for (int i = 0; i < tabelle_cb.Length; i++)
{
if (c == tabelle_cb[i].c)
return i;
}
return -1;
}
internal override string GetPattern()
{
string result = tabelle_cb[FindBarItem("A")].data + "0";
foreach (char c in text)
{
int idx = FindBarItem(c.ToString());
result += tabelle_cb[idx].data + "0";
}
result += tabelle_cb[FindBarItem("B")].data;
return result;
}
Предупреждение PVS-Studio: V3106 Possible negative index value. The value of 'idx' index could reach -1. BarcodeCodabar.cs 70
Потенциально опасный код. Метод FindBarItem может вернуть значение -1, если не найдёт элемента, переданного в качестве параметра. В вызывающем же коде (метод GetPattern) это значение записывается в переменную idx и используется в качестве индекса массива tabelle_cb без предварительной проверки. При обращении по индексу -1 будет сгенерировано исключение типа IndexOutOfRangeException.
Продолжим.
protected override void Finish()
{
....
if (saveStreams)
{
FinishSaveStreams();
}
else
{
if (singlePage)
{
if (saveStreams)
{
int fileIndex = GeneratedFiles.IndexOf(singlePageFileName);
DoPageEnd(generatedStreams[fileIndex]);
}
else { .... }
....
}
....
}
....
}
Предупреждение PVS-Studio: V3022 Expression 'saveStreams' is always false. HTMLExport.cs 849
Приведённый код с получением значения fileIndex и вызовом метода DoPageEnd никогда не выполнится, так как результат второго приведённого в коде выражения saveStreams всегда будет иметь значение false.
Из наиболее интересного это, пожалуй, всё (вы же не ждали статьи в духе анализа Mono?). Были и другие предупреждения анализатора, но они не показались мне достаточно интересными, чтобы включать их в статью (часть всегда остаётся за кадром).
Для их анализа будет полезным знание проекта, так что в идеале авторам следовало бы посмотреть на эти предупреждения самостоятельно. Это такие предупреждения, как V3083 (потенциально опасный вызов обработчиков события), V3022 (условие всегда true / false (в данном случае часто из-за методов, которые возвращают одно значение)), V3072, V3073 (работа с IDisposable) и прочие.
Если что-то из этого неактуально, можно:
Заключение
Несмотря на то, что статья вышла небольшой, мне доставило удовольствие 'пощупать' предупреждения анализатора руками — посмотреть, как то, на что ругается анализатор, проявляется на практике.
Авторам же проекта желаю успехов, исправления обнаруженных проблем и хочу похвалить за шаг навстречу open-source community!
Остальным советую попробовать анализатор на своём коде и посмотреть, что интересного удастся найти.
Всех благ!
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev. The Fastest Reports in the Wild West — and a Handful of Bugs…
