# Первая статья про проверку C\# проекта Сейчас команда PVS\-Studio активно занимается разработкой статического анализатора C\# кода\. Мы планируем выпустить первую версию анализатора к концу 2015 года\. А пока моя задача \- написать несколько статей, чтобы заранее заинтересовать C\# программистов инструментом PVS\-Studio\. Сегодня мне выдали обновлённый инсталлятор\. Теперь стало возможным установить PVS\-Studio с поддержкой C\# и даже что\-то проверить\. Я не замедлил этим воспользоваться и решил проверить первое, что подвернется под руку\. Первым подвернулся проект Umbraco\. Конечно, пока текущая версия анализатора умеет мало, но этого мне уже сейчас хватит для написания маленькой статьи\. ![0357_UmbracoCMS_ru/image1.png](https://import.viva64.com/docx/blog/0357_UmbracoCMS_ru/image1.png) ## Umbraco [Umbraco](https://umbraco.com/) \- это платформа системы управления контента с открытым кодом, использующаяся для публикации контента во всемирной сети\. Она написана на языке C\#\. С момента выхода 4\.5 вся система стала доступной под лицензией MIT\. Проект имеет средний размер\. Однако часть, написанная на C\#, не очень велика\. Большая часть проекта написана на JavaScript\. Всего в проекте около 3200 файлов с расширением "\.cs", суммарным размером в 15 мегабайт\. Количество строк С\# кода: 400 KLOC\. ## О PVS\-Studio 6\.00 Проверка будет осуществлять с помощью Альфа\-версии анализатора PVS\-Studio 6\.00\. В этой версии произойдут два больших изменения: 1. Будет поддержан анализ C\# проектов\. 1. Анализатор перестанет поддерживать VS2005, VS2008\. Небольшому количеству пользователей, работающих с VS2005/2008 мы предлагаем продолжить использовать версию 5\.31 или более старшие версии 5\.xx, если будут исправляться какие\-то ошибки\. Ценовая политика останется прежней\. Мы не делаем новый продукт, а расширяем возможности существующего\. Мы просто поддержим ещё один язык программирования\. Раньше можно было купить PVS\-Studio и использовать его для проверки проектов, написанных на языках: C, C\+\+, [C\+\+/CLI](https://ru.wikipedia.org/wiki/C%2B%2B/CLI), [C\+\+/CX](https://ru.wikipedia.org/wiki/C%2B%2B/CX)\. Теперь дополнительно появляется возможность проверять C\# проекты\. На цену это никак не повлияет\. Те, кто уже приобрел анализатор для C\+\+, смогут заодно проверять и C\# код\. ## Почему C\#? На конференциях я нередко говорил, что делать C\# анализатор не очень интересно\. Множество ошибок, которые присутствуют в C\+\+, в языке C\# просто невозможны\. Это действительно так\. Например, в C\# нет таких функций как memset\(\), соответственно и нет массы проблем \(см\. примеры, связанные с memset\(\): [V511](https://pvs-studio.ru/ru/docs/warnings/v511/), [V512](https://pvs-studio.ru/ru/docs/warnings/v512/), [V575](https://pvs-studio.ru/ru/docs/warnings/v575/), [V579](https://pvs-studio.ru/ru/docs/warnings/v579/), [V597](https://pvs-studio.ru/ru/docs/warnings/v597/), [V598](https://pvs-studio.ru/ru/docs/warnings/v598/)\)\. Однако постепенно я изменил своё мнение\. Большое количество ошибок, обнаруживаемых PVS\-Studio, связано не с какими\-то особенностями языка программирования, а с невнимательностью программистов\. Я имею ввиду опечатки и неудачное изменение кода после copy\-paste\. Именно в этом силён анализатор PVS\-Studio для C\+\+, и мы решили, что эти наработки можно применить и для C\#\. Язык C\# не защищает от того, чтобы перепутать имя переменной или от "[эффекта последней строки](https://pvs-studio.ru/ru/blog/posts/cpp/0260/)", связанного с утратой внимания\. Ещё одним важным фактором, который подтолкнул решиться на создание C\# анализатора, является появление Roslyn\. Без него работа по созданию анализатора была бы слишком дорогой для нас\. [Roslyn](https://github.com/dotnet/roslyn) \- это открытая платформа компиляции C\# и Visual Basic\. Roslyn выполняет два основных действия: строит синтаксическое дерево \(парсинг\) и компилирует его\. Дополнительно позволяет анализировать исходный код, рекурсивно обходить его, работать с проектами Visual Studio, выполнять код на лету\. ## Что нашлось интересного? Для C\+\+ у меня есть любимая диагностика [V501](https://pvs-studio.ru/ru/blog/examples/v501/)\. Теперь есть её аналог и для C\#: V3001\. Начнем именно с этой диагностики\. **Фрагмент кода N1** В коде есть свойство "фокальная точка": ```cpp [DataMember(Name = "focalPoint")] public ImageCropFocalPoint FocalPoint { get; set; } ``` Данное свойство имеет тип 'ImageCropFocalPoint', определение которого приведено ниже: ```cpp public class ImageCropFocalPoint { [DataMember(Name = "left")] public decimal Left { get; set; } [DataMember(Name = "top")] public decimal Top { get; set; } } ``` Казалось бы, при работе с таким свойством невозможно ошибиться\. Ан нет\. В методе HasFocalPoint\(\) допущена досадная опечатка: ```cpp 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** ```cpp 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** ```cpp 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 именованными константами: ```cpp public enum DBTypes { Integer, Date, Nvarchar, Ntext } ``` А вот метод SetProperty\(\) почему\-то рассматривает только 3 варианта\. Ещё раз повторюсь, я не говорю, что это ошибка\. Однако анализатор предлагает обратить внимание на этот фрагмент кода и я с ним полностью согласен\. ```cpp 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** ```cpp 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\# не защищает от "[эффекта последней строки](https://pvs-studio.ru/ru/blog/posts/cpp/0260/)"\. И вот мы добрались до соответствующего примера: ```cpp 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** ```cpp 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** Предлагаю читателям проверить внимательность и найти ошибку в следующем фрагменте кода\. Для этого изучите метод, но не читайте текст ниже\. Чтобы не сделать это случайно, я вставил разделитель \(единорога :\)\. ```cpp public static string GetTreePathFromFilePath(string filePath) { List treePath = new List(); 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](https://import.viva64.com/docx/blog/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\.Reverse](https://msdn.microsoft.com/ru-ru/library/b0axc2h2%28v=vs.110%29.aspx)\)\. Однако применительно к массивам, метод Reverse\(\) не меняет исходный массив\. Для массивов этот метод реализуется за счёт метода\-расширения [Reverse\(\)](https://msdn.microsoft.com/en-us/library/vstudio/bb358497%28v=vs.110%29.aspx) класса 'Enumerable'\. Этот метод не производит перестановку на месте, а возвращает изменённую коллекцию\. Правильно будет написать: ```cpp string[] pathPaths = filePath.Split('/'); pathPaths = pathPaths.Reverse().ToArray(); ``` Или даже так: ```cpp string[] pathPaths = filePath.Split('/').Reverse().ToArray(); ``` **Фрагмент кода N10** Анализатор PVS\-Studio выдал несколько предупреждений V3013, о подозрительном совпадении тел некоторых методов\. На мой взгляд, все эти предупреждения ложные\. Как мне кажется, внимания заслуживает только одно из этих предупреждений: ```cpp 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\(\), нужно использовать не ```cpp source.GetSafeAbsolutePathDecoded() ``` а ```cpp source. GetAbsolutePathDecoded() ``` Я не уверен, что прав, но это место заслуживает проверки\. ## Ответы на вопросы Статья рассчитана на новую аудиторию\. Поэтому я заранее предвижу ряд вопросов, которые могут возникнуть\. Попробую заранее ответить на часть из них\. **Вы уведомили разработчиков проекта о найденных дефектах?** Да, мы всегда стараемся это сделать\. **Вы используете PVS\-Studio для проверки кода самого PVS\-Studio?** Да\. **PVS\-Studio поддерживает Mono?** Нет\. Более подробно ответы на эти и другие вопросы даны в заметке: "[Ответы на вопросы читателей статей про PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/a0085/)"\. ## Заключение В проекте нашлось не так много ошибок\. Наши читатели из C\+\+ аудитории уже знают, почему так происходит\. Но поскольку покорять и соблазнять C\# программистов нам только предстоит, перечислю ряд важных моментов: 1. Статический анализатор — это инструмент регулярного использования\. Его смысл находить ошибки на самом раннем этапе\. Нет смысла в разовых проверках проекта\. Как правило так выявляются ошибки, не оказывающие существенного влияния на работу программы или расположенные в редко используемых участках кода\. Причина в том, что настоящие ошибки всё это время исправлялись потом и кровью\. Их находили программисты, [часами отлаживая код](https://pvs-studio.ru/ru/blog/posts/cpp/0221/), их находили тестеры и что ещё хуже \- пользователи\. Многие из этих ошибок можно было бы сразу исправить, если регулярно использовать статический анализатор кода\. Рассматривайте PVS\-Studio как расширение предупреждений C\# компилятора\. Вы ведь, надеюсь, смотрите список предупреждений, выданных компилятором, не раз в год? Более подробно всё это разобрано в статье: "[Лев Толстой и статический анализ кода](https://pvs-studio.ru/ru/blog/posts/0105/)"\. 1. В статьях мы упоминаем только о тех фрагментах кода, которые показались нам интересными\. Как правило мы не пишем о коде, который анализатор совершенно честно посчитал подозрительным, но при этом видно, что настоящей ошибки нет\. Мы называем этот "код с запахом"\. Когда вы используете PVS\-Studio, этот код стоит проверить\. Но рассказывать про такие места в статье неуместно\. 1. Этого пункта нет для C\+\+, но он существует пока для C\#\. Мы реализовали ещё не так много диагностик, но быстро движемся\. Дайте нашему C\#\-единорогу немного подрасти\. Уж он вам тогда покажет\! Спасибо всем за внимание и желаю всем безбажных программ\.