Разрабатывая статический анализатор PVS-Studio, мы стараемся развивать его в различных направлениях. Так, наша команда работает над плагинами для IDE (Visual Studio, Rider), улучшением интеграции с CI и т. д. Увеличение эффективности анализа проектов под Unity также является одной из наших приоритетных целей. Мы считаем, что статический анализ позволит программистам, использующим этот игровой движок, повысить качество своего исходного кода и упростить работу над любыми проектами. Поэтому хотелось бы увеличить популярность PVS-Studio среди компаний, занимающихся разработкой под Unity. Одним из первых шагов в реализации данной задумки стало написание нами аннотаций для методов, определённых в движке. Это позволяет контролировать корректность кода, связанного с вызовами аннотируемых методов.
Введение
Аннотации являются одним из важнейших механизмов анализатора. Они предоставляют различную информацию об аргументах, возвращаемом значении и внутренних особенностях методов, которые не могут быть выяснены в автоматическом режиме. В то же время, программист, производящий аннотирование, может предполагать примерную внутреннюю структуру метода и особенности его работы, основываясь на документации и здравом смысле.
Например, вызов метода GetComponent выглядит несколько странным, если значение, возвращаемое им, не используется. Ерундовая ошибка? Отнюдь. Конечно, это может быть попросту лишний вызов, забытый и покинутый всеми. А может быть так, что пропущено какое-то важное присваивание. Аннотации могут помочь анализатору найти подобные и многие другие ошибки.
Конечно же, нами уже написано множество аннотаций для анализатора. Например, проаннотированы методы классов из пространства имён System. Кроме того, существует и механизм автоматического аннотирования некоторых методов. Подробнее об этом можно узнать здесь. Отмечу, что указанная статья больше рассказывает про часть PVS-Studio, отвечающую за анализ проектов на C++. Тем не менее, ощутимой разницы в принципе работы аннотаций для C# и C++ нет.
Написание аннотаций для методов Unity
Мы стремимся повысить качество проверки кода проектов, использующих Unity, поэтому и было принято решение проаннотировать методы данного движка.
Изначальная идея состояла в том, чтобы покрыть аннотациями вообще все методы Unity, однако, их оказалось очень много. Вследствие этого, мы решили для начала проаннотировать методы из наиболее часто используемых классов.
Сбор информации
Сначала необходимо было выяснить, какие же именно классы используются чаще других. Кроме того, важным аспектом является обеспечение возможности сбора результатов аннотирования – новых ошибок, которые анализатор найдёт в реальных проектах благодаря написанным аннотациям. Поэтому первым шагом был поиск соответствующих open-source проектов. Однако сделать это оказалось не так уж просто.
Проблема в том, что многие проекты из найденных были достаточно небольшими по объёму исходного кода. В таких если и есть ошибки, то их не очень много, а уж найти там какие-то срабатывания, связанные именно с методами из Unity, вероятность ещё меньше. Порой попадались проекты, которые практически не использовали (или не использовали совсем) Unity-специфичные классы, хотя по описанию были так или иначе связаны с движком. Подобные находки совершенно не подходили для поставленной задачи.
Конечно, иногда везло. Например, жемчужиной в этой коллекции является MixedRealityToolkit. Кода в нём уже прилично, а значит, и собранная статистика использования Unity-методов в таком проекте будет более полной.
Таким образом, было набрано 20 проектов, использующих возможности движка. Для того, чтобы найти наиболее часто используемые классы, была написана основанная на Roslyn утилита, производящая подсчёт вызовов методов из Unity. Такую программу, кстати, тоже вполне можно назвать статическим анализатором. Ведь если задуматься, она действительно анализирует исходники, не прибегая к запуску самого проекта.
Написанный "анализатор" позволил найти классы, средняя частота использования которых в найденных проектах наиболее высока:
- UnityEngine.Vector3
- UnityEngine.Mathf
- UnityEngine.Debug
- UnityEngine.GameObject
- UnityEngine.Material
- UnityEditor.EditorGUILayout
- UnityEngine.Component
- UnityEngine.Object
- UnityEngine.GUILayout
- UnityEngine.Quaternion
- И т.д.
Конечно, это вовсе не означает, что эти классы действительно так уж часто используются разработчиками – всё-таки статистика, основанная на такой небольшой выборке проектов, не особо заслуживает доверия. Однако для начала этой информации было достаточно, чтобы быть уверенным в том, что аннотируются методы классов, которые хоть где-то используются.
Аннотирование
После получения необходимой информации пришло время заняться собственно аннотированием. Верными помощниками в этом деле были документация и редактор Unity, где был создан тестовый проект. Это было необходимо для проверки некоторых моментов, не уточнённых в документации. К примеру, далеко не всегда было ясно, приведёт ли передача null в каком-либо аргументе к ошибке, или же выполнение программы продолжится без проблем. Конечно, передача null, как правило, это не совсем хорошо, но в этом случае мы считали ошибками лишь то, что прерывало поток выполнения, либо же логировалось редактором Unity как ошибка.
Во время проведения таких проверок были найдены интересные особенности работы некоторых методов. К примеру, запуск кода
MeshRenderer renderer = cube.GetComponent<MeshRenderer>();
Material m = renderer.material;
List<int> outNames = null;
m.GetTexturePropertyNameIDs(outNames);приводит к тому, что крашится сам редактор Unity, хотя обычно в подобных случаях происходит прерывание выполнения текущего скрипта и логирование соответствующей ошибки. Конечно, вряд ли разработчики часто пишут подобное, но сам факт того, что редактор Unity можно довести до падения запуском обычных скриптов – не очень хорошо. Подобное же происходит ещё как минимум в одном случае:
MeshRenderer renderer = cube.GetComponent<MeshRenderer>();
Material m = renderer.material;
string keyWord = null;
bool isEnabled = m.IsKeywordEnabled(keyWord);Указанные проблемы актуальны для редактора Unity 2019.3.10f1.
Сбор результатов
После того, как аннотирование выполнено, необходимо проверить, как это повлияет на выдаваемые предупреждения. Перед добавлением аннотаций для каждого из подобранных проектов формируется лог с ошибками, который мы называем эталонным. Затем новые аннотации встраиваются в анализатор и производится повторная проверка проектов. Сформированные списки предупреждений, благодаря аннотациям, будут отличаться от эталонных.
Процедура тестирования аннотаций выполняется в автоматическом режиме с помощью специально написанной под эти нужды программы CSharpAnalyserTester. Он производит запуск анализа на проектах, после чего сравнивает получаемые логи с эталонными и формирует файлы, содержащие информацию о различиях.
Описанный подход также используется и для того, чтобы выяснить, какие изменения в логах появляются при добавлении новой или изменении существующей диагностики.
Как уже отмечалось ранее, с поиском больших открытых проектов под Unity возникли сложности. Это неприятно, ведь для них анализатор смог бы выдать больше интересных срабатываний. В то же время, было бы намного больше различий между эталонными логами и логами, сформированными после проведения аннотирования.
Тем не менее, написанные аннотации помогли выявить несколько подозрительных моментов в рассматриваемых проектах, что тоже является приятным результатом работы.
Например, был найден несколько странный вызов GetComponent:
void OnEnable()
{
GameObject uiManager = GameObject.Find("UIRoot");
if (uiManager)
{
uiManager.GetComponent<UIManager>();
}
}Предупреждение анализатора: V3010 The return value of function 'GetComponent' is required to be utilized. — ADDITIONAL IN CURRENT UIEditorWindow.cs 22
Опираясь на документацию, логично сделать вывод, что значение, возвращаемое этим методом, должно как-то использоваться. Поэтому при аннотировании он был отмечен соответствующим образом. Тут же результат вызова ничему не присваивается, что выглядит немного странно.
А вот ещё один пример дополнительных срабатываний анализатора:
public void ChangeLocalID(int newID)
{
if (this.LocalPlayer == null) // <=
{
this.DebugReturn(
DebugLevel.WARNING,
string.Format(
....,
this.LocalPlayer,
this.CurrentRoom.Players == null, // <=
newID
)
);
}
if (this.CurrentRoom == null) // <=
{
this.LocalPlayer.ChangeLocalID(newID); // <=
this.LocalPlayer.RoomReference = null;
}
else
{
// remove old actorId from actor list
this.CurrentRoom.RemovePlayer(this.LocalPlayer);
// change to new actor/player ID
this.LocalPlayer.ChangeLocalID(newID);
// update the room's list with the new reference
this.CurrentRoom.StorePlayer(this.LocalPlayer);
}
}Предупреждения анализатора:
- V3095 The 'this.CurrentRoom' object was used before it was verified against null. Check lines: 1709, 1712. — ADDITIONAL IN CURRENT LoadBalancingClient.cs 1709
- V3125 The 'this.LocalPlayer' object was used after it was verified against null. Check lines: 1715, 1707. — ADDITIONAL IN CURRENT LoadBalancingClient.cs 1715
Отметим, что PVS-Studio не обращает внимания на передачу LocalPlayer в string.Format, так как это не приведёт к ошибке. Да и выглядит код так, будто написано это намеренно.
В данном случае влияние аннотаций не столь очевидно. Тем не менее, именно они стали причиной появления этих срабатываний. Возникает вопрос – почему этих предупреждений не было раньше?
Дело в том, что в методе DebugReturn производится несколько вызовов, которые в теории могли бы повлиять на значение свойства CurrentRoom:
public virtual void DebugReturn(DebugLevel level, string message)
{
#if !SUPPORTED_UNITY
Debug.WriteLine(message);
#else
if (level == DebugLevel.ERROR)
{
Debug.LogError(message);
}
else if (level == DebugLevel.WARNING)
{
Debug.LogWarning(message);
}
else if (level == DebugLevel.INFO)
{
Debug.Log(message);
}
else if (level == DebugLevel.ALL)
{
Debug.Log(message);
}
#endif
}Анализатору неизвестны особенности работы вызываемых методов, а значит, и неизвестно, как они будут влиять на ситуацию. Так, PVS-Studio предполагает, что значение this.CurrentRoom могло измениться во время работы метода DebugReturn, поэтому далее и производится проверка.
Аннотации же дали информацию о том, что методы, вызываемые внутри DebugReturn, не повлияют на значения других переменных. Следовательно, использование переменной перед её проверкой на равенство null можно считать подозрительным.
Заключение
Подводя итог, стоит сказать, что аннотирование Unity-специфичных методов, вне всяких сомнений, позволит находить больше различных ошибок в проектах, использующих этот движок. Тем не менее, покрытие аннотациями всех доступных методов потребует достаточно много времени. Более эффективным будет аннотировать в первую очередь наиболее часто используемые. Однако, чтобы понять, какие именно классы используются чаще, нужны подходящие проекты с большой кодовой базой. Кроме того, крупные проекты позволяют гораздо лучше контролировать результативность аннотирования. Всем этим мы продолжим заниматься в ближайшее время.
Анализатор постоянно развивается и дорабатывается. Добавление аннотаций для методов Unity лишь один из примеров расширения его возможностей. Таким образом, со временем эффективность работы PVS-Studio растёт. Поэтому, если вы ещё не пробовали PVS-Studio, самое время исправить это, загрузив его с соответствующей страницы. Там же можно получить триальный ключ для анализатора, чтобы ознакомиться с его возможностями, проверив различные проекты.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Nikita Lipilin. How the PVS-Studio analyzer began to find even more errors in Unity projects.
