Detekt Dangers in Android projects

Code quality is crucial. When you work in big teams on a huge projects it becomes more and more important.

Why do we need to care about code quality?

  • Keep common code style across the whole team
  • Not waste time on code reviews arguing about extra lines
  • Write code which is more clear and atomic in specified borders

Static analysis is a well-known approach to help with it. With just a bit of automation we could make developers' work more clear and predictable. Imagine you as a developer write your code, push it to the repo, make a pull request. Just everyday work, right? And this code is checked automatically with specific warnings reported to you in the same pull request.

There are bunch of tools to use for static analysis in Android projects. We could use ktlint, detekt, Android Lint, spotless and so on. Let's just stop on detekt for now and see how it works.

Detekt

detekt is a static analysis tool for Kotlin. It can be used as a standalone or via Gradle plugin.

Rules

If we want to check something and say is it a correct thing or not then we need some rules. Detekt ships with a lot of rules inside. By the way there's formatting rule set inside which is basically ktlint rules working inside of detekt.

Config

We can control enabled rules with yaml configuration file and providing it to detekt gradle plugin. For example you could just copy-paste default config into your project and change it later according your needs.

Custom rules

It is possible to write custom rules which opens the whole new level of creativity in checks automation. Almost every project has its own verbal conventions between developers. These are what could be a good use case for custom rules.

Baseline

You may ask how do you integrate detekt in huge-legacy project in which detekt rises thousands of warnings with this config. The answer is baseline. Baseline is basically just a file with a list of warnings which are ignored on consequential runs.

Setting up

Setting up detekt is as simple as applying plugin id("io.gitlab.arturbosch.detekt").version("1.20.0") in some gradle module's build.gradle.kts. But when you applying such plugin you also need to configure it somewhere in a single place to reuse it easily. Solution is to extract applying and configuration logic.

By the way you really should take care about build.gradle.kts configuration files in your projects. I saw too many projects with a complete mess inside of them. You should treat your configuration logic like any other code. With all these separation of concerns like principles and so on. Separate configuration logic if it makes sense. For example you may want to abstract any third party Gradle Plugin like this.

Most of projects I've seen used some allprojects or subprojects lambdas which is actually a bad practice. Good practice is Gradle Plugin. But writing a real Gradle Plugin is not easy. Here comes the convention plugins idea. You may know an approach to use buildSrc module for such use cases which is basically the same included build but more like magic under the Gradle's hood. I'd recommend to create standalone included builds for better control.

Gradle convention plugin

  • includeBuild new module (let's say it is called build-logic) in root settings.gradle.kts
includeBuild("build-logic")
  • Add Kotlin Gradle Plugin implementation dependency into this build-logic module
plugins {  
    base  
    id("io.gitlab.arturbosch.detekt").version("1.20.0")  
}  
  
buildscript {  
    dependencies {  
        classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.6.20")  
    }  
}
  • Create younameit.gradle.kts file inside
  • Now you can apply this plugin to any other module with just
plugins {
    id("younameit")
}
  • It's nice idea to prefix all your custom convention plugins. I like the idea to prefix them with convention word.

Detekt

Let's write detekt.convention.gradle.kts with some detekt configuration.

import io.gitlab.arturbosch.detekt.Detekt  
import io.gitlab.arturbosch.detekt.DetektPlugin  
import io.gitlab.arturbosch.detekt.report.ReportMergeTask  
  
plugins {  
    base  
    id("io.gitlab.arturbosch.detekt") // We need to apply this plugin with version in build.gradle.kts
}  
  
dependencies {  
    detektPlugins("io.gitlab.arturbosch.detekt:detekt-formatting:1.20.0") // This is formatting rule set
}  
  
val configFile = files("$rootDir/config/detekt/config.yml") // This is where our rules config will be stored
val baselineFile = file("config/detekt/baseline.xml") // This is the location of baseline files in each module, will be ignored if not found
val mergedReportFile = file("${rootProject.buildDir}/reports/detekt/report.xml") // Location of merged report. Basically a final result of detekt work
  
/**  
 * Location of single module report inside its build directory
 * Must be named detekt.xml 
 * Workaround for https://github.com/detekt/detekt/issues/4192#issuecomment-946325201
 */ 
val outputReportFile = file("$buildDir/reports/detekt/detekt.xml")  
  
detekt {  
    ignoreFailures = true // We do not want to crash if warnings found, cause they will be used later
  
    parallel = true // Runs detekt checks in parallel
  
    config.setFrom(configFile)  
    buildUponDefaultConfig = true // Tells the detekt to use default config if its settings are not overriden in explicit config
  
    baseline = baselineFile  // Baseline file location
}

val reportMerge: TaskProvider<ReportMergeTask> = rootProject.registerMaybe("reportMerge") {  
    description = "Runs merge of all detekt reports into single one"  
    output.set(mergedReportFile)  
}  

/**
 * Enable only XML reports output and set its location
 */
tasks.withType<Detekt>().configureEach {  
    reports {  
        html.required.set(false)  
        sarif.required.set(false)  
        txt.required.set(false)  
        xml.required.set(true)  
        xml.outputLocation.set(outputReportFile)  
    }  
}  
  
/**
 * Finalizes every detekt task with ReportMergeTask
 */
plugins.withType<DetektPlugin> {  
    tasks.withType<Detekt> {  
        finalizedBy(reportMerge)  
        reportMerge.configure {  
            input.from(xmlReportFile)  
        }  
    }
}  
  
/**  
 * Workaround to get [TaskProvider] by task name if it already exists in given [Project]  
 * or register it otherwise  
 * [Github - Introduce TaskContainer.maybeNamed](https://github.com/gradle/gradle/issues/8057) 
 */
inline fun <reified T : Task> Project.registerMaybe(  
    taskName: String,  
    configuration: Action<in T> = Action {},  
): TaskProvider<T> {  
    if (taskName in tasks.names) {  
        return tasks.named(taskName, T::class, configuration)  
    }  
    return tasks.register(taskName, T::class, configuration)  
}

You may note merge logic here. There is an official guide on this topic. I assume our project is multi-module, so we possibly want to see a single report (many modules - one report), not one per each. Official guide recommends to use submodules configuration of all detekt modules with reusing same merging task but this way we leak our detekt abstraction to root project. Instead of this we register reportMerge task in the rootProject only for the first module and reuse it in all other.

How to use

Console

This commands run Gradle tasks reporting every warning found to console and into final report file.

# To run on whole project
./gradle detekt

# To run on single module (app in this case)
./gradle :app:detekt

You could understand what's wrong just looking at messages reported and googling rules. Speaking about baselines - there's special task to create baselines

# To create baselines in all modules
./gradlew detektBaseline

# To create a baseline for single module (app in this case)
./gradle :app:detektBaseline
Autocorrect

Formatting issues could be corrected automatically by using --auto-correct parameter

./gradle detekt --auto-correct

IntelliJ Plugin

There is an Detekt Intellij Plugin. You may want to install it for faster feedback. Just install it from IntelliJ marketplace and set configuration files.

Unfortunately, I don't know any way to generate single baseline file for whole project for this moment. That's why you need to change baselines in plugin settings as you work in another module. That's a valid point only for projects with multiple baselines, of course.

Danger

Danger is basically a standalone tool to report something back to pull requests. There are well known Ruby and JS implementations with configs on these languages. And we got 1.0.0 release of Kotlin one not too long ago. It uses JS under the hood but provides a possibility to write Kotlin Script configs. We will use it.

Config

Configuring Danger is a real fun. We are free to use danger plugins and write our own rules. Let's see my simple example

@file:DependsOn("io.github.ackeecz:danger-kotlin-detekt:0.1.4")  
  
import io.github.ackeecz.danger.detekt.DetektPlugin  
import systems.danger.kotlin.*  
import java.io.File  
  
register.plugin(DetektPlugin)  
  
danger(args) {  
    warnDetekt()  
  
    onGitHub {  
        warnWorkInProgress()  
    }  
}  
  
fun warnDetekt() {  
    val detektReport = File("build/reports/detekt/report.xml")  
    if (!detektReport.exists()) {  
        warn(  
            "🙈 No detekt report found",  
        )  
        return  
    }  
    DetektPlugin.parseAndReport(detektReport)  
}  
  
fun GitHub.warnWorkInProgress() {  
    if ("WIP" in pullRequest.title) {  
        warn(  
            "🚧 PR is marked with Work in Progress (WIP)",  
        )  
    }  
}

I used AckeeCZ/danger-kotlin-detekt plugin here to parse and report detekt warnings from given report file. There are special constructions like onGit and onGithub for many tools. You can check something in git commits, Github PR's and so on. In my case I also check for WIP letters in pull request title and make a comment to highlight it. A lot of space for creativity, huh?

Keep in mind there's a symbol limit in Github comment API, so there could be issues with a lot of warnings printed by danger.

Github Actions

Github Actions is a go to solution for CI if your project is small and simple. To add Actions workflow we just need to create .github/workflows/pull-request.yml in our repo and push a repo to Github.

name: Pull Request  
  
on:  
  pull_request:  
    branches: [ main ]  
  
jobs:  
  pipeline:  
  
    runs-on: ubuntu-latest  
  
    permissions:  
      pull-requests: write  
  
    steps:  
      - name: Checkout  
        uses: actions/checkout@v2  
  
      - name: Set up environment  
        uses: actions/setup-java@v1  
        with:  
          java-version: 11  
  
      - name: Change wrapper permissions  
        run: chmod +x ./gradlew  
  
      - name: detekt # Run detekt
        run: ./gradlew detekt  
  
      - name: Danger # And Danger action
        uses: docker://ghcr.io/danger/danger-kotlin:1.0.0  
        with:  
          args: --dangerfile config/danger/config.df.kts # Note I have config located like this
        env:  
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Extracted automatically

This way we eliminate all work on setting up the bot account for comments. Github solves it on its own.

Other CI tool

Github Actions setup for public repo is easy, but setting up any other CI tool is not much harder. In most cases we need to generate an access token to the repo and set it as an DANGER_GITHUB_API_TOKEN environment variable inside the CI job hooked to PR. More information here in the Ruby version documentation. Danger should be installed on this machine before running, it could be installed from brew or from source code.

danger-kotlin ci --dangerfile config/danger/config.df.kts

Sample

I've created sample repository for this blog post so you could see basic implementation I've tried to describe here. You could also look at this pull request with Github Actions bot commit written by Danger.

Conclusion

Setting up static analysis on CI is one of the easiest things to make project development more productive. I'd recommend to set up CI as early as possible. It's a nice foundation to build more automatic checks in the future. Basically every discussion in code reviews could be ended with the written detekt or danger rule. Declare zero warning policy and do not merge any PR with warnings. It helps a lot and it's much simpler then fixing everything after years of uncontrolled development.