Site icon AppTractor

Прекратите спорить в Code Review — начните внедрять с правилами линтера

AppTractor

Для начала давайте рассмотрим следующие комментарии в обзоре кода:

«Эй, Джейн, отступы в этой строке сделаны неправильно».

«Привет, Джон, я вижу, что ViewModel зависит от Repository напрямую. Вместо этого лучше зависеть от UseCase».

«Разве мы недавно не договорились, что UseCase должны открывать только одну функцию?»

«Подождите, я запутался, мы что, должны обращаться к ресурсам String непосредственно из ViewModel?»

И, конечно, самое важное:

«Здесь две пустые строки 😱».

Знаете, что это такое? Это все запахи code review! И да, такой термин существует, но, к сожалению, не я его придумал.

Инженеры не созданы для запоминания вещей. Но мы определенно можем создать инструменты, которые будут делать это за нас.

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

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

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

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

Почему именно Konsist? Разве у нас уже нет линтеров?

Это правда, у нас уже много лет есть линтеры! Даже Android Studio поставляется со встроенным. Но попробовав несколько альтернатив в наших Android-приложениях в PSS, мы поняли одну вещь — все они требовали сложного обучения, и ни один из них не мог обеспечить соблюдение архитектурных правил удобным для сопровождения способом, поэтому нам приходилось быть очень бдительными, чтобы обнаружить эти нарушения во время проверки кода. Код было трудно читать, интеграция с нашей системой CI/CD была нетривиальной задачей, и, в конце концов, никто не хотел писать собственные правила линтинга.

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

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

Написание правил линтинга в Konsist

Поскольку правила линтеинга в Konsist — это юнит-тесты, мы можем писать их аналогично написанию тестов с помощью JUnit 5, JUnit 4 или Kotest.

В наших проектах мы предпочитаем структурировать наши тесты в стиле Given-When-Then, поэтому мы сделаем то же самое для наших правил, используя Behavior Spec из Kotest:

  1. Given: Это настройка нашего теста. Мы укажем файлы или классы, к которым должно применяться наше правило.
  2. When: В модульных тестах это обычно действие, которое выполняется до того, как мы оценим результат. Поскольку правила линтинга не связаны с конкретным действием, этот шаг необязателен. Однако мы все равно можем использовать его для дополнительной фильтрации файлов, которые мы указали на предыдущем шаге (например, фильтрация свойств класса, конструкторов, функций и т. д.).
  3. Then: Это наше утверждение. Мы будем утверждать, что файлы или классы, которые мы указали на предыдущих шагах, не нарушают правило, которое мы хотим применить.

Вот пример правила, которое обязывает все ViewModel расширять нашу BaseViewModel:

dependencies {
    testImplementation("com.lemonappdev:konsist:$konsistVersion")
    testImplementation("io.kotest:kotest-runner-junit5:$kotestVersion")
}
class ViewModelsExtendBaseViewModel : BehaviorSpec() {

    init {
        Given("All classes in production code") {
            val scope = Konsist.scopeFromProduction().classes()

            When("There is a ViewModel") {
                val viewModels = scope.withNameEndingWith("ViewModel")

                Then("It extends the BaseViewModel") {
                    viewModels.assertTrue {
                        it.hasParentWithName("BaseViewModel")
                    }
                }
            }
        }
    }
}

Вот описание того, что делает этот код:

  1. Given: Сначала мы определяем область видимости (скоуп), которая представляет собой набор файлов или классов, на которые мы хотим нацелиться. В данном случае мы включаем все классы в производственном коде (исключая тестовые классы). Мы также можем выбрать конкретные модули, наборы исходных текстов, пакеты или каталоги. Обо всех доступных опциях вы можете узнать из документации.
  2. When: Мы фильтруем эти классы, чтобы получить только те, имена которых заканчиваются на ViewModel.
  3. Then: Мы утверждаем, что эти отфильтрованные классы расширяют BaseViewModel.

И все! Довольно просто, правда? Теперь у нас есть правило линтера, которое мы можем запустить так же, как и юнит-тест с помощью Gradle или Android Studio. И самое приятное, что мы даже можем следовать Test-Driven Development. Выявив плохой запах кода, мы можем написать неудачный тест, затем исправить нарушение и подтвердить, что тест (правило линтера) проходит.

Добавление кастомного сообщения

Чтобы сделать сообщение об ошибке в правиле более удобным для разработчиков, мы рекомендуем добавить пользовательское сообщение, которое объясняет, почему это правило линтера должно выполняться, используя параметр additionalMessage функции assertTrue:

/* ... */

viewModels.assertTrue(additionalMessage = MESSAGE) {
    it.hasParentWithName("BaseViewModel")
}

/* ... */

private companion object {
    private const val MESSAGE =
        "Always extend the BaseViewModel when creating a new ViewModel, to take advantage of the lifecycle events and other features it provides."
}

Указание базы

Давайте попробуем запустить тест (правило lint), который мы только что написали:

Упс, не получилось! Очевидно, в нашем проекте уже есть несколько нарушений этого правила. Это означает, что у нас есть два варианта:

  1. Либо исправить нарушения путем рефакторинга наших ViewModel, чтобы они расширяли BaseViewModel.
  2. Либо добавить классы-нарушители в базу (baseline). База — это список файлов или классов, которые мы намеренно исключаем из правила линтера. Это может быть как унаследованный код, который мы не хотим рефакторить в данный момент, так и допустимые случаи, когда правило линтинга не применяется.

Чтобы применить базу, мы можем создать массив BASELINE в companion object и задать его в качестве параметра в функции withoutName, чтобы отфильтровать эти классы. В нашем случае мы добавим BaseViewModel в базу, поскольку очевидно, что она не может расширять себя:

/* ... */

val viewModels = scope.withNameEndingWith("ViewModel").withoutName(*BASELINE)

/* ... */

private companion object {
    private const val MESSAGE =
        "Always extend the BaseViewModel when creating a new ViewModel, to take advantage of the lifecycle events and other features it provides."

    private val BASELINE = arrayOf("BaseViewModel")
}

Альтернативным способом исключения классов из правил линтинга является использование аннотации @Suppress в каждом файле или классе, который мы хотим исключить, однако мы рекомендуем использовать массив BASELINE непосредственно в тесте, так как это сделает правило линтера более самодостаточным и позволит нам сразу увидеть, какие исключения из этого правила существуют.

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

Преимущество № 1: Соблюдение архитектуры

Замечания по архитектуре в code review — одни из самых важных, но и самых разрушительных. На переделку архитектуры у человека могут уйти часы, не говоря уже о том, что на такие замечания часто отвечают: «Может, мы исправим это позже?» (и это так и остается невыполненным).

Именно здесь Konsist дает больше всего преимуществ. Он может обеспечить соблюдение наших архитектурных правил, особенно в многомодульном проекте. Давайте рассмотрим несколько примеров:

class ViewModelsDoNotInjectRepositories : BehaviorSpec() {

    init {
        Given("All classes in production code") {
            val scope = Konsist.scopeFromProduction().classes()

            When("There is a ViewModel") {
                val viewModels = scope.withNameEndingWith("ViewModel")

                Then("No Repository is listed in the constructor parameters") {
                    viewModels.withConstructor {
                        it.hasParameter { param ->
                            param.type.name.endsWith("Repository")
                        }
                    }.assertEmpty()
                }
            }
        }
    }
}
class RepositoriesResideInRepositoriesModule : BehaviorSpec() {

    init {
        Given("All classes in production code") {
            val scope = Konsist.scopeFromProduction().classes()

            When("There is a Repository") {
                val repositories = scope.withNameEndingWith("Repository")

                Then("It resides in the repositories module") {
                    repositories.assertTrue {
                        it.resideInModule("data/repositories")
                    }
                }
            }
        }
    }
}
class DomainLayerDoesNotImportDTOs : BehaviorSpec() {

    init {
        Given("The domain layer module") {
            val scope = Konsist.scopeFromDirectory("domain").files

            Then("It does not import DTOs") {
                scope.assertFalse {
                    it.text.contains("com.perrystreet.dto")
                }
            }
        }
    }
}
class DesignSystemDoesNotImportDomainModels : BehaviorSpec() {

    init {
        Given("The design system module") {
            val scope = Konsist.scopeFromDirectory("design-system").files

            Then("It does not import domain models") {
                scope.assertFalse {
                    it.text.contains("com.perrystreet.domain.models")
                }
            }
        }
    }
}

Преимущество № 2: Предотвращение ошибок

Обычно, когда мы сталкиваемся с ошибкой в нашей кодовой базе, мы должны задать себе следующий вопрос:

Можем ли мы написать юнит-тест, чтобы предотвратить эту ошибку?

Знаете, что еще лучше, чем юнит-тест? Правило линтинга! Потому что правило линтинга может предотвратить ошибку на еще более ранней стадии разработки. Итак, первый вопрос, который мы должны задать, должен быть перефразирован:

Можем ли мы написать правило линтинга, чтобы предотвратить эту ошибку?

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

Расширенная пирамида тестирования с добавлением слоя правил линтера

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

Давайте рассмотрим правило, которое защищает нас от ошибки, с которой мы недавно столкнулись в наших проектах.

Рассмотрим следующую функцию в Retrofit, которая выполняет POST-запрос к серверу, отправляя id и name в качестве параметров @Field:

@POST(PATH)
fun postProfile(@Field("id") id: Long, @Field("name") name: String)

Однако, как только вы попробуете выполнить его, приложение завершится со следующей ошибкой:

java.lang.IllegalArgumentException: @Field parameters can only be used with form encoding. (parameter #1)
    for method postProfile

Аннотация @Field отправляет данные в кодировке form-URL, и это требует наличия в функции аннотации @FormUrlEncoded, иначе произойдет сбой:

@POST(PATH)
@FormUrlEncoded
fun postProfile(@Field("id") id: Long, @Field("name") name: String)

Идеальный случай для использования правила линтинга, чтобы предотвратить эту проблему:

class RetrofitFieldParamsUseFormUrlEncoded : BehaviorSpec() {

    init {
        Given("All functions in production code") {
            val scope = Konsist.scopeFromProduction().functions()

            When("There is a function with the @POST annotation") {
                val functions = scope.withAnnotationNamed("POST")

                And("It has at least one @Field parameter") {
                    val functionsWithFieldParams = functions.withParameter {
                        it.hasAnnotationWithName("Field")
                    }

                    Then("It has the @FormUrlEncoded annotation") {
                        functionsWithFieldParams.assertTrue {
                            it.hasAnnotationWithName("FormUrlEncoded")
                        }
                    }
                }
            }
        }
    }
}

А вот и разбивка кода:

  1. Given: Мы берем все функции в производственном коде.
  2. When: Мы фильтруем их, чтобы получить те, которые имеют аннотацию @POST.
  3. And: Применяем дополнительный фильтр, чтобы получить функции, у которых хотя бы один параметр имеет аннотацию @Field.
  4. Then: Мы утверждаем, что эти отфильтрованные функции также имеют аннотацию @FormUrlEncoded.

Структура проекта в Konsist

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

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

Вот так будет выглядеть структура модуля и файл build.gradle.kts:

plugins {
    id("kotlin")
}

java {
    sourceCompatibility = JavaVersion.VERSION_17
    targetCompatibility = JavaVersion.VERSION_17
}

tasks.test {
    useJUnitPlatform()
}

dependencies {
    testImplementation("com.lemonappdev:konsist:$konsistVersion")
    testImplementation("io.kotest:kotest-runner-junit5:$kotestVersion")
}

Примечание: Вам не нужно добавлять здесь остальные модули в качестве зависимостей. Konsist по умолчанию будет иметь доступ ко всему проекту и не требует явных зависимостей.

Еще одно преимущество такого подхода — мы можем запускать наши правила lint отдельно от остальных юнит-тестов с помощью Gradle-задачи :{module}:test:

./gradlew :konsist:test

Наконец, чтобы сократить количество дублирующегося кода при написании правил линтера, мы рекомендуем создать файл KonsistUtils.kt с часто используемыми скоупами:

object KonsistUtils {

    val productionCode
        get() = Konsist.scopeFromProduction()

    val classes
        get() = productionCode.classes()

    val interfaces
        get() = productionCode.interfaces()

    val viewModels
        get() = classes.withNameEndingWith("ViewModel")

    val useCases
        get() = classes.withNameEndingWith("UseCase")

    val repositories
        get() = classes.withNameEndingWith("Repository")

    val domainModule
        get() = Konsist.scopeFromDirectory("domain")

    val designSystemModule
        get() = Konsist.scopeFromDirectory("design-system")

    /* ... */
}

Запуск правил линтера на CI/CD

Итак, после всей тяжелой работы по настройке модуля Konsist и написанию собственных правил осталось одно — обеспечить их выполнение в нашей CI/CD-системе, чтобы блокировать пул-реквест при нарушении правил.

Если вы уже настроили конвейер для своих юнит-тестов, то это будет очень просто. Единственное, что нужно, — это дополнительный шаг, который будет запускать юнит-тесты в модуле konsist, который мы создали ранее.

Давайте рассмотрим пример конфигурации с использованием GitHub Actions или Bitrise.

Конфигурация GitHub Action

Следующий GitHub Action проверяет репозиторий, устанавливает Java и Gradle, а затем выполняет задачу Gradle, которая запускает все модульные тесты (правила lint) в модуле konsist. Мы запускаем это действие, когда открывается пул-реквест:

name: Run Konsist lint rules
on:
  pull_request:
    types: [opened, synchronize]

jobs:
  run-lint-rules:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - uses: actions/setup-java@v4
        with:
          distribution: temurin
          java-version: 17

      - name: Setup Gradle
        uses: gradle/actions/setup-gradle@v3

      - name: Run Konsist lint rules Gradle task
        run: ./gradlew :konsist:test

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

Конфигурация Bitrise

Если вы используете Bitrise, вам нужно будет просто добавить один дополнительный шаг в ваш рабочий процесс, который запускает Gradle-задачу :konsist:test:

Наконец, если вы хотите параллельно выполнять все модульные тесты во всех модулях, включая правила линтинга Konsist, вы можете объединить их в одну задачу Gradle, которую можно определить в файле build.gradle.kts верхнего уровня:

tasks.register("runAllTests") {
    dependsOn(
        ":app:test",
        ":domain:test",
        ":other-module:test",
        // ...
        ":konsist:test"
    )
}

Резюме

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

Благодаря Konsist мы на Android наконец-то получили долгожданный инструмент, который делает написание правил линтинга простым и доступным, не требующим сложного обучения. Инструмент, который, безусловно, является важной вехой в написании пользовательских правил, благодаря своей гениальной идее переосмыслить их как юнит-тесты.

Пожалуйста, обязательно ознакомьтесь с руководством по внесению вклада в Konsist, чтобы узнать, как вы можете поддержать проект.

А что насчет iOS?

Наша команда использует SwiftLint на iOS — инструмент, который может писать правила на основе regexp, используя сопоставление шаблонов имен файлов, что позволяет описать большинство, но не все архитектурные правила. Мы надеемся подробнее рассказать об этом в одном из ближайших постов в блоге.

Источник

Exit mobile version