Привет, Хабр!

Сегодня я хочу затронуть тему, которая будет полезна как Java-разработчикам, так и начинающим тех- и тимлидам. Я расскажу о том, как добиться высокого качества кода на вашем Java проекте и перестать волноваться о стилях кодирования.

Когда-то (очень давно, на четвёртом-пятом годах карьеры) я носил лычки «проектировщика программного обеспечения» и готовил многостраничные гайды по оформлению кода, правильных инструментах и библиотеках. Это были красивые всеобъемлющие документы, с прочтения которых начинался онбординг любого разработчика на проекте. А затем я тщательно следил за соблюдением этих правил на этапе code review. И знаете, сейчас я понимаю, что это было полное абсолютное стопроцентное дерьмо.

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

Далее я покажу своё видение того, какие инструменты и в какой конфигурации должны применяться на Java проектах (а особенно в микросервисах), но сначала немного вводной информации.

Зачем вообще следить за качеством кода?

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

Качество — комплексная оценка продукта. Это не только количество ошибок в продукте, но также скорость работы, возможность масштабирования под растущую нагрузку, скорость выведения на рынок новых возможностей и т.п.

Я глубоко убеждён, что качество программного продукта неразрывно связано с качеством программного кода: продукт не может быть качественным, если его код — отстой.

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

Зачем нужен архитектор или техлид?

На каждом проекте должен быть человек, играющий роль архитектора или технического лидера. Этот человек несёт персональную ответственность за технические решения, принимаемые на проекте, а также следит за качеством кода. Делает он это посредством CI пайплайна, который нужно настроить на максимально тщательную проверку программного кода. Весь код, который не соответствует принятым критериям качества, должен отвергаться и никогда не должен попасть в основную ветку разработки (master/development).

Какие проверки кода могут быть?

Проверяться могут любые аспекты кода. В целом, чем больше проверок, тем лучше.

Все проверки можно разделить на две категории — автоматические и экспертные. Экспертные — это code review. Это самые медленные, дорогие и ненадёжные проверки, которые только могут быть, но, тем не менее, они должны быть.

Автоматические проверки включают в себя:

• компиляцию кода;

• выполнение тестов;

• статический анализ;

• проверку на уязвимости и т.д.

Зачем нужен Checkstyle и EditorConfig?

Одна из распространенных проблем, встречающаяся особенно среди опытных разработчиков — это вкусовщина. Люди привыкают к определённым стилям написания и оформления кода и начинают переносить их с одного места работы на другое. Очень часто возникают разногласия между разработчиками по принципам форматирования кода.

Checkstyle позволяет зафиксировать и отслеживать во время сборки проекта соответствие кода принятым стандартам оформления. Больше не будет споров по типу: табуляция или пробелы; 2 или 4 пробела для отступа и т.д. За этим будет следить Checkstyle.

Чтобы разработчикам было проще следовать стандартам оформления кода, необходимо в каждом проекте иметь сконфигурированный файл EditorConfig. Современные IDE умеют его подтягивать и брать из него настройки.

Checkstyle работает на основе построения AST и содержит очень много проверок. Одни из моих любимых: RedundantModifier, SingleSpaceSeparator, EmptyLineSeparator и IllegalImport. Пример конфига, который я использую для своих проектов, можно найти на GitHub.

Основная рекомендация по внедрению: начинайте с небольшого количества проверок; подбирайте их под ваш проект и стиль, постепенно добавляя новые проверки, до тех пор, пока не попробуете все. Checkstyle должен обязательно проверять код ваших тестов и ронять сборку, если что-то нашёл.

Пример конфигурации для Gradle:

plugins {
    id 'checkstyle'
}
test {
    useJUnitPlatform()
    dependsOn checkstyleMain, checkstyleTest 
}
checkstyle {
    toolVersion '10.3.1'
    configFile file("config/checkstyle/checkstyle.xml")
    ignoreFailures = false
    maxWarnings = 0
    maxErrors = 0
}
checkstyleMain {
    source ='src/main/java'
}
checkstyleTest {
    source ='src/test/java'
}

Пример конфигурации для Maven:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-checkstyle-plugin</artifactId>
    <dependencies>
        <dependency>
            <groupId>com.puppycrawl.tools</groupId>
            <artifactId>checkstyle</artifactId>
            <version>10.3.1</version>
        </dependency>
        <dependency>
            <groupId>com.thomasjensen.checkstyle.addons</groupId>
            <artifactId>checkstyle-addons</artifactId>
            <version>6.0.1</version>
        </dependency>
    </dependencies>
    <configuration>
        <encoding>UTF-8</encoding>
        <consoleOutput>true</consoleOutput>
        <violationSeverity>warning</violationSeverity>
        <failOnViolation>true</failOnViolation>
        <failsOnError>true</failsOnError>
        <linkXRef>false</linkXRef>
        <includeTestSourceDirectory>true</includeTestSourceDirectory>
        <sourceDirectories>
            <directory>${project.build.sourceDirectory}</directory>
            <directory>${project.build.testSourceDirectory}</directory>
        </sourceDirectories>
        <checkstyleRules>
           ...
           <!-- Правила можно описать прямо в parent pom. Удобно для многомодульных проектов -->
        </checkstyleRules>
    </configuration>
    <executions>
        <execution>
            <id>check</id>
            <phase>validate</phase>
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Зачем нужен PMD?

PMD — это ещё один статический анализатор, гармонично дополняющий Checkstyle. Он также строит AST, но помимо этого использует байт-код, поэтому его нужно запускать после компиляции проекта. А ещё PMD позволяет достаточно легко добавлять свои собственные проверки. Подробнее об этом можно почитать в статье от Wrike.

Моя любимая проверка — JUnitTestsShouldIncludeAssert. Я о ней недавно рассказывал в статье про AssertJ; почитайте, если ещё нет.

В PMD все проверки объединены в группы, что на мой взгляд удобно, но некоторые вещи приходится сразу убирать. Пример моей конфигурации тут.

К Gradle проекту PMD можно подключить так:

plugins {
    id 'pmd'
}
test {
    useJUnitPlatform()
    dependsOn pmdMain, pmdTest
}
pmd {
    consoleOutput = true
    toolVersion = "6.47.0"
    ruleSetFiles = files("config/pmd/pmd.xml") // Исключения только через внешний файл
    ruleSets = []
}

Maven плагин также не позволяет настраивать исключения прямо в parent pom, поэтому приходится использовать файл с конфигом:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-pmd-plugin</artifactId>
    <configuration>
        <failOnViolation>true</failOnViolation>
        <includeTests>true</includeTests>
        <linkXRef>false</linkXRef>
        <printFailingErrors>true</printFailingErrors>
        <rulesets>
            <ruleset>config/pmd/pmd.xml</ruleset> <!-- Есть нюансы для многомодульных проектов -->
        </rulesets>
    </configuration>
    <executions>
        <execution>
            <id>pmd-check</id>
            <phase>test</phase> <!-- Желательно наличие байт-кода; см. https://maven.apache.org/plugins/maven-pmd-plugin/faq.html -->
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Зачем нужен SpotBugs?

SpotBugs — тоже статический анализатор, но ориентированный больше на поиск ошибок в байт-коде. Он является преемником FindBugs и имеет свыше 400 эвристик, на основании которых работает. Он может обнаруживать потенциальные NPE или игнорирование возвращаемого методом значения (смотрите статью про AssertJ, которую я упоминал ранее).

Из недостатков стоит отметить, что SpotBugs достаточно параноидален и часто жалуется на вещи, которые мы не хотим исправлять (много false positive по отдельным правилам). Такие эвристики можно проигнорировать, используя excludeFilterFile.

Основной секрет правильного использования SpotBugs на проекте состоит в том, что порог фильтрации ошибок должен быть выставлен в Low: так вы не пропустите действительно важных предупреждений.

Конфигурация для Gradle:

plugins {
    id 'com.github.spotbugs' version '5.0.9'
}
spotbugs {
    showProgress = true
    effort = 'max'
    reportLevel = 'low'
    excludeFilter = file("config/spotbugs/exclude.xml")
}

Maven вариант конфига:

<plugin>
    <groupId>com.github.spotbugs</groupId>
    <artifactId>spotbugs-maven-plugin</artifactId>
    <version>4.5.3</version>
    <configuration>
        <includeTests>true</includeTests>
        <effort>Max</effort>
        <threshold>Low</threshold>
        <xmlOutput>false</xmlOutput>
        <excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
    </configuration>
    <executions>
        <execution>
            <id>check</id>
            <phase>test</phase>
            <goals>
                <goal>check</goal>
            </goals>
        </execution>
    </executions>
</plugin>

Почему бы не использовать только SonarQube?

На многих проектах (особенно в «кровавом энтерпрайзе») уже может использоваться SonarQube. И тут возникает резонный вопрос: не достаточно ли только его?

На мой взгляд, нет. Не существует никакой серебряной пули в разработке ПО, которая бы решила все проблемы с качеством кода. Sonar не исключение: он большой и сложный; проверки выполняются долго, а ещё конфигурация правил находится вне репозитория проекта.

Признаюсь честно, я не поклонник Sonar'а. И тем не менее, на всех своих проектах я стараюсь использовать SonarQube, но при этом предпочитаю рассматривать его как последнюю линию обороны.

Зачем JaCoCo в проекте?

Помимо статического анализа кода в каждом проекте должны быть модульные и интеграционные тесты. Они также должны запускаться в CI пайплайне и ронять сборку при обнаружении проблем. Важно понимать, что тестируемый код значительно отличается от «просто хорошего кода, который работает».

Критически важно фиксировать и контролировать минимальное покрытие кода тестами.
 Я для этого предпочитаю использовать библиотеку JaCoCo: она уронит сборку, если покрытие кода станет ниже зафиксированного порога.

Пример настройки правил для Gradle:

plugins {
    id 'jacoco'
}
test {
    useJUnitPlatform()
    finalizedBy jacocoTestReport
    finalizedBy jacocoTestCoverageVerification
}
jacocoTestReport {
    reports {
        html.enabled true
    }
}
jacocoTestCoverageVerification {
    dependsOn test
    violationRules {
        rule {
            limit {
                counter = 'CLASS'
                value = 'MISSEDCOUNT'
                maximum = 0
            }
        }
        rule {
            limit {
                counter = 'METHOD'
                value = 'MISSEDCOUNT'
                maximum = 0
            }
        }
        rule {
            limit {
                counter = 'LINE'
                value = 'MISSEDCOUNT'
                maximum = 0
            }
        }
        rule {
            limit {
                counter = 'INSTRUCTION'
                value = 'COVEREDRATIO'
                minimum = 1.0
            }
        }
    }
}
check.dependsOn jacocoTestReport, jacocoTestCoverageVerification

И аналогичные правила для Maven:

<plugin>
    <groupId>org.jacoco</groupId>
    <artifactId>jacoco-maven-plugin</artifactId>
    <version>0.8.8</version>
    <configuration>
        <excludes>
            <!-- excludes generated code -->
            <exclude>**/*MapperImpl.class</exclude>
        </excludes>
    </configuration>
    <executions>
        <execution>
            <id>prepare-agent</id>
            <goals>
                <goal>prepare-agent</goal>
            </goals>
        </execution>
        <execution>
            <id>report</id>
            <phase>test</phase>
            <goals>
                <goal>report</goal>
            </goals>
        </execution>
        <execution>
            <id>check-minimal</id>
            <phase>package</phase>
            <goals>
                <goal>check</goal>
            </goals>
            <configuration>
                <rules>
                    <rule>
                        <element>BUNDLE</element>
                        <limits>
                            <limit>
                                <counter>INSTRUCTION</counter>
                                <value>COVEREDRATIO</value>
                                <minimum>0.80</minimum>
                            </limit>
                            <limit>
                                <counter>CLASS</counter>
                                <value>MISSEDCOUNT</value>
                                <maximum>0</maximum>
                            </limit>
                            <limit>
                                <counter>METHOD</counter>
                                <value>MISSEDCOUNT</value>
                                <maximum>0</maximum>
                            </limit>
                            <limit>
                                <counter>LINE</counter>
                                <value>MISSEDCOUNT</value>
                                <maximum>0</maximum>
                            </limit>
                        </limits>
                    </rule>
                </rules>
            </configuration>
        </execution>
    </executions>
</plugin>

А какие проверки можно использовать ещё?

CI пайплайн не ограничивается только статическим анализом кода и тестами. Можно сканировать зависимости на предмет уязвимостей. Если вы собираете Docker-образы, то можно проверять их, используя разнообразные линтеры.

Если вы используете БД, то можно (и нужно!) проверять её структуру и код ваших миграций на соответствие лучшим практикам и типовые ошибки. У меня есть такой инструмент — pg-index-health — набор проверок для PostgreSQL. Очень рекомендую.

Ну, и напоследок

При внедрении статического анализа на проекте ни в коем случае не забывайте про элементарную психологию. Какие бы классные ребята с вами не работали, они в первую очередь люди, а большинство людей негативно воспринимает нововведения; особенно те, которые их как-то ограничивают. Дайте вашей команде привыкнуть. Внедряйте проверки постепенно, демонстрируя их полезность. И помните: негативная реакция и естественное сопротивление команды не должны вас ни в коем случае останавливать.