В этой статье я расскажу, как мы написали собственные анализаторы кода и чистим с их помощью нашу кодовую базу .net от наиболее острых / частых косяков. Главный посыл — сделать это довольно просто, не бойтесь писать свои анализаторы для борьбы с именно вашими багами. Вторичный посыл — попробуйте наши анализаторы и сообщите о результатах. Полное руководство я писать не буду, их довольно много в интернете, а вот небольшой обзор, что это как и с какими проблемами я столкнулся, надеюсь, окажется вам полезным.
В декабре 2016 года я сменил работу и перешел в новую компанию. Новая контора находилась в стадии перехода от «отдел в полтора программиста на коленке лабает небольшой сервис для внутренних нужд» в «ИТ-компания в 100 человек разрабатывает ERP систему для нужд группы компаний и готовит продукт для внешних пользователей».
Естественно, в процессе этого перехода многие наколеночные «так принято» вещи становились серьезными и соответствующими разнообразным best practices. В частности, в компании появилась за несколько месяцев до моего прихода полноценная иерархия тестовых сред и отдел тестирования неуклонно боролся за соответствие тестовых сред — среде боевой.
Авторы унаследованной архитектуры исповедовали микросервисный подход, сроки были жесткими, процент джуниоров — большой. Тестеры при развертывании на тестовую среду столкнулись с тем, что внезапно микросервис А, развернутый на тестовой среде, мог вызывать микросервис Б прямо на проде. Смотрим в код — и видим — URL микросервиса Б зашит внутрь кода микросервиса А.
В процессе работы помимо проблемы «захардкоженная ссылка на микросервис» выявились и другие проблемы кода. Code-review check list все рос, ругань становилась все злее. Необходимо было решение, которое выявит все проблемы и предотвратит появление таких ситуаций в будущем.
Новый компилятор языка C# — Roslyn — полностью open source и включает поддержку pluggable static analyzers. Анализаторы могут выдавать ошибки, предупреждения и хинты, работают как во время компиляции, так и в реальном времени в IDE.
Анализаторы имеют доступ к AST(абстрактному синтаксическому дереву) и могут подписываться на элементы определенного типа, например на числа или строки или на вызовы методов. В самом простом виде это выглядит как-то так:
(Это моя самая первая попытка написать анализатор для захардкоженных email).
По дереву можно ходить вверх, вниз и вбок, и нет необходимости отличать, скажем, комментарий от строкового литерала — парсер компилятора делает все это за нас. Впрочем, уровня синтаксиса может быть недостаточно. Например,
На синтаксическом уровне мы не можем понять, конструктор какого типа мы вызываем. Скорее всего, это System.Guid. А что, если нет? На уровне синтаксического дерева это просто «конструктор класса с идентификатором, равным «Guid»». Для более точной диагностики нам нужно вызвать так называемую семантическую модель и получить у нее информацию о том, какому символу соответствует элемент синтаксического дерева.
Семантическая модель — это промежуточные результаты компиляции. В отличие от синтаксиса, она может быть неполна (напоминаю, мы работаем в контексте IDE, программист может прямо сейчас писать анализируемый класс). Поэтому готовьтесь, методы семантической модели много и часто возвращают null.
Анализаторы ставятся в проект, как обычные NuGet пакеты. После перекомпиляции начинаются проявляться ошибки и предупреждения. Мы в конторе сделали внутренний NuGet пакет (у нас вообще вся сборка всего идет через пакеты), который ставит рекомендованную комбинацию анализаторов. Планируем еще туда дописать наши специфические конторские штуки, и туда же можно добавлять не наши пакеты (я например добавил AsyncFixer, очень удобная штука, всем рекомендую).
Вот так это выглядит в редакторе
А так в итогах компиляции
Самый важный для нас анализатор. Тупо ищет все литералы с http, https, ftp и так далее. Список исключений довольно большой — в основном разнообразный автогенеренный код, SOAP атрибуты, namespace и все такое.
Редко, но метко встречающаяся в нашем коде проблема. Иногда «сумма с НДС» высчитывается из «суммы без НДС» умножением на 1.18. Это сразу две проблемы. Во-первых, ставка НДС может измениться, и искать все места в коде — большая проблема. Во-вторых, ставка НДС вообще-то — не константа, а атрибут конкретного заказа — он может быть со ставкой НДС 0% (такие юрлица часто бывают у перевозчиков), может относится к другой юрисдикции, наконец, при изменении ставки НДС она изменится для новых заказов, а для старых окажется прежней.
Поэтому никакого захардкоженного НДС!
В исключения тут попали в основном функции вырезания подстрок (substring). Иногда там имеет смысл передавать жестко прописанное 18. Приходится ловить по именам параметров мет��да.
Мы часто из кода ссылаемся на id базы (у нас приняты Guid). Это плохо. Особенно мы страдаем из-за таблиц статусов, которым на самом деле стоило бы просто сделать enum (от них мы избавляемся, но они есть). Пока в качестве промежуточного решения мы договорились, что в коде могут встречать id, если они одинаковы во всех средах (значит, создаются миграциями) и если они располагаются непосредственно в коде соответствующей Entity (да, мы используем Entity Framework 6 и не стесняемся).
— анализатор захардкоженных email
— анализатор, обязывающий писать методы контроллера асинхронными
Естественным было разделить анализаторы по принципу 1 анализатор = 1 assembly = 1 nuget пакет. Дело в том, что все написанные нами анализаторы очень opinionated, и логично предоставить возможность подключать / отключать их по одному. В процесс написания анализаторов я стал активно выносить общий код из анализаторов в общую библиотеку и возникла проблема: если разные версии анализаторов загружены в студию — они не могут загрузить разные версии общей библиотеки. Самым простым решением оказалось сливать на этапе билда каждый анализатор с общей библиотекой в единый assembly. Для этого я воспользовался ILRepack, у которого есть удобный биндинг для MSBuild (ILRepack.MSBuild.Task). К сожалению, биндинг немного устарел, но это вроде бы пока не влияет.
Стандартные зависимости анализаторов (Microsoft.CodeAnalysis.*) таскать с собой не нужно, и зависеть от них NuGet тоже не должен. Visual Studio сама выдаст нам нужные библиотеки.
Я привык настраивать непрерывную сборку проектов с исходным кодом при помощи Appveyor. Собирать и публиковать NuGet он тоже умеет. Но анализаторы — особая магия. В частности, в плане структуры директорий внутри пакета, там все по-особому, нужно раскладывать в папку analyzers вместо стандартной lib. Я немного помучился и пришел к решению — в csproj как эта сборки прописан вызов Nuget.exe, который мы таскаем с собой в пакете. Некрасиво, как мне кажется, но более красивого решения я не нашел.
Юнит-тесты для анализаторов — это супер классно и удобно. Я использовал стандартную MS обертку для юнит-тестов анализатора, любой false позитив тут же оформлял юнит-тестом. Классно! Гораздо приятнее, чем возиться с нечетким человеческим ТЗ.
Перейти на .netstandart мне пока не удалось. Попытка с ходу оказалась неудачной, официальный template все еще генерит portable class библиотеки. Ждем обновленного template и попробуем сделать по нему. https://github.com/dotnet/roslyn/issues/18414
Буду рад пинкам по поводу моего подхода. Исходники можно посмотреть здесь.
Буду также рад, если кто-то посоветует хорошие опенсорсные C# проекты, на которых проверить анализаторы, или проверит у себя на своих (может даже не опенсорсных) и зарепортит баги.
Очень хотелось бы ссылок на другие полезные анализаторы, желательно не дублирующие функционал Resharper.
Проблема
В декабре 2016 года я сменил работу и перешел в новую компанию. Новая контора находилась в стадии перехода от «отдел в полтора программиста на коленке лабает небольшой сервис для внутренних нужд» в «ИТ-компания в 100 человек разрабатывает ERP систему для нужд группы компаний и готовит продукт для внешних пользователей».
Естественно, в процессе этого перехода многие наколеночные «так принято» вещи становились серьезными и соответствующими разнообразным best practices. В частности, в компании появилась за несколько месяцев до моего прихода полноценная иерархия тестовых сред и отдел тестирования неуклонно боролся за соответствие тестовых сред — среде боевой.
Авторы унаследованной архитектуры исповедовали микросервисный подход, сроки были жесткими, процент джуниоров — большой. Тестеры при развертывании на тестовую среду столкнулись с тем, что внезапно микросервис А, развернутый на тестовой среде, мог вызывать микросервис Б прямо на проде. Смотрим в код — и видим — URL микросервиса Б зашит внутрь кода микросервиса А.
В процессе работы помимо проблемы «захардкоженная ссылка на микросервис» выявились и другие проблемы кода. Code-review check list все рос, ругань становилась все злее. Необходимо было решение, которое выявит все проблемы и предотвратит появление таких ситуаций в будущем.
Статические анализаторы
Новый компилятор языка C# — Roslyn — полностью open source и включает поддержку pluggable static analyzers. Анализаторы могут выдавать ошибки, предупреждения и хинты, работают как во время компиляции, так и в реальном времени в IDE.
Анализаторы имеют доступ к AST(абстрактному синтаксическому дереву) и могут подписываться на элементы определенного типа, например на числа или строки или на вызовы методов. В самом простом виде это выглядит как-то так:
private static void AnalyzeLiteral(SyntaxNodeAnalysisContext context)
{
if (syntaxNode is LiteralExpressionSyntax literal)
{
var value = literal.Token.ValueText;
if (value != "@" && value.Contains("@"))
{
context.ReportDiagnostic( Diagnostic.Create(Rule, context.Node.GetLocation(), value));
}
}(Это моя самая первая попытка написать анализатор для захардкоженных email).
По дереву можно ходить вверх, вниз и вбок, и нет необходимости отличать, скажем, комментарий от строкового литерала — парсер компилятора делает все это за нас. Впрочем, уровня синтаксиса может быть недостаточно. Например,
var guid = new Guid("...");На синтаксическом уровне мы не можем понять, конструктор какого типа мы вызываем. Скорее всего, это System.Guid. А что, если нет? На уровне синтаксического дерева это просто «конструктор класса с идентификатором, равным «Guid»». Для более точной диагностики нам нужно вызвать так называемую семантическую модель и получить у нее информацию о том, какому символу соответствует элемент синтаксического дерева.
private static void AnalyzeCtor(SyntaxNodeAnalysisContext context)
{
if (context.Node.GetContainingClass().IsProbablyMigration())
{
return;
}
if (context.Node is ObjectCreationExpressionSyntax createNode)
{
var type = context.SemanticModel.GetTypeInfo(createNode).Type as INamedTypeSymbol;
var guidType = context.SemanticModel.Compilation.GetTypeByMetadataName(typeof(System.Guid).FullName);
if (Equals(type, guidType))
{
..
}
}
}Семантическая модель — это промежуточные результаты компиляции. В отличие от синтаксиса, она может быть неполна (напоминаю, мы работаем в контексте IDE, программист может прямо сейчас писать анализируемый класс). Поэтому готовьтесь, методы семантической модели много и часто возвращают null.
Анализаторы ставятся в проект, как обычные NuGet пакеты. После перекомпиляции начинаются проявляться ошибки и предупреждения. Мы в конторе сделали внутренний NuGet пакет (у нас вообще вся сборка всего идет через пакеты), который ставит рекомендованную комбинацию анализаторов. Планируем еще туда дописать наши специфические конторские штуки, и туда же можно добавлять не наши пакеты (я например добавил AsyncFixer, очень удобная штука, всем рекомендую).
Вот так это выглядит в редакторе
А так в итогах компиляции
Какие анализаторы мы сделали
Анализатор захардкоженных URL
Самый важный для нас анализатор. Тупо ищет все литералы с http, https, ftp и так далее. Список исключений довольно большой — в основном разнообразный автогенеренный код, SOAP атрибуты, namespace и все такое.
Анализатор захардкоженной ставки НДС
Редко, но метко встречающаяся в нашем коде проблема. Иногда «сумма с НДС» высчитывается из «суммы без НДС» умножением на 1.18. Это сразу две проблемы. Во-первых, ставка НДС может измениться, и искать все места в коде — большая проблема. Во-вторых, ставка НДС вообще-то — не константа, а атрибут конкретного заказа — он может быть со ставкой НДС 0% (такие юрлица часто бывают у перевозчиков), может относится к другой юрисдикции, наконец, при изменении ставки НДС она изменится для новых заказов, а для старых окажется прежней.
Поэтому никакого захардкоженного НДС!
В исключения тут попали в основном функции вырезания подстрок (substring). Иногда там имеет смысл передавать жестко прописанное 18. Приходится ловить по именам параметров мет��да.
Анализатор захардкоженного Guid
Мы часто из кода ссылаемся на id базы (у нас приняты Guid). Это плохо. Особенно мы страдаем из-за таблиц статусов, которым на самом деле стоило бы просто сделать enum (от них мы избавляемся, но они есть). Пока в качестве промежуточного решения мы договорились, что в коде могут встречать id, если они одинаковы во всех средах (значит, создаются миграциями) и если они располагаются непосредственно в коде соответствующей Entity (да, мы используем Entity Framework 6 и не стесняемся).
Что у нас в планах
— анализатор захардкоженных email
— анализатор, обязывающий писать методы контроллера асинхронными
Возникшие трудности и странные моменты
Общая библиотека
Естественным было разделить анализаторы по принципу 1 анализатор = 1 assembly = 1 nuget пакет. Дело в том, что все написанные нами анализаторы очень opinionated, и логично предоставить возможность подключать / отключать их по одному. В процесс написания анализаторов я стал активно выносить общий код из анализаторов в общую библиотеку и возникла проблема: если разные версии анализаторов загружены в студию — они не могут загрузить разные версии общей библиотеки. Самым простым решением оказалось сливать на этапе билда каждый анализатор с общей библиотекой в единый assembly. Для этого я воспользовался ILRepack, у которого есть удобный биндинг для MSBuild (ILRepack.MSBuild.Task). К сожалению, биндинг немного устарел, но это вроде бы пока не влияет.
Стандартные зависимости анализаторов (Microsoft.CodeAnalysis.*) таскать с собой не нужно, и зависеть от них NuGet тоже не должен. Visual Studio сама выдаст нам нужные библиотеки.
Сборка при помощи Appveyor
Я привык настраивать непрерывную сборку проектов с исходным кодом при помощи Appveyor. Собирать и публиковать NuGet он тоже умеет. Но анализаторы — особая магия. В частности, в плане структуры директорий внутри пакета, там все по-особому, нужно раскладывать в папку analyzers вместо стандартной lib. Я немного помучился и пришел к решению — в csproj как эта сборки прописан вызов Nuget.exe, который мы таскаем с собой в пакете. Некрасиво, как мне кажется, но более красивого решения я не нашел.
Написание юнит-тестов
Юнит-тесты для анализаторов — это супер классно и удобно. Я использовал стандартную MS обертку для юнит-тестов анализатора, любой false позитив тут же оформлял юнит-тестом. Классно! Гораздо приятнее, чем возиться с нечетким человеческим ТЗ.
Переход на .netstandard
Перейти на .netstandart мне пока не удалось. Попытка с ходу оказалась неудачной, официальный template все еще генерит portable class библиотеки. Ждем обновленного template и попробуем сделать по нему. https://github.com/dotnet/roslyn/issues/18414
Заключение
Буду рад пинкам по поводу моего подхода. Исходники можно посмотреть здесь.
Буду также рад, если кто-то посоветует хорошие опенсорсные C# проекты, на которых проверить анализаторы, или проверит у себя на своих (может даже не опенсорсных) и зарепортит баги.
Очень хотелось бы ссылок на другие полезные анализаторы, желательно не дублирующие функционал Resharper.
