Static code analysis Lint

Formatting Code Analysis Rule with Android Lint - For Kotlin and Java at once

Posted on by Balázs Ruda

Screen-Shot-2019-08-04-at-12.27.48-PM

TL;DR

In this article, primarily, I want to show that it’s worth the time to write a custom code analysis rule to enforce code conventions. Secondly, I want to demonstrate that in addition to Checkstyle and ktlint, Android Lint should also be considered when creating a formatting-related code analysis rule, despite that not being its basic purpose. I’ll explain its main advantage in my case against the other tools, and describe the inner workings that provide this benefit. For inspiration, I’ll also show the steps I took to make my formatting rule work with Android Lint.

Then, I’ll detail how to use my rule in any Android projects, and how to fix the already existing errors in our codebase. I’ll also show what testing library is provided by the tool and how I used it for testing and debugging my rule. Finally, I’ll write a few words about how Android Lint can be applied outside Android projects as well.

Motivation

I get frustrated when I get formatting-related comments on my merge requests or when I have to add such comments to others’. Sometimes, we have to change branches in order to fix a single formatting error in our MR, which is quite demotivating. That’s why I started thinking about ways we could automate fixing this minor (yet important) problem, so we could focus on more complex issues during code reviews.

At my company – Supercharge – we use several lint tools, such as ktlint or Checkstyle, for enforcing global formatting conventions (variable order, line length, etc.), but these don’t include certain company-wide rules by default. For those cases, the above-mentioned tools support adding custom rules as well. I decided to write one for the most common formatting mistakes we make:


fun main() {
    for (j in 0..2) {
        println(j)
    }
    var i = 2
    println(++i)
    if (i == 2) {
        println("2")
    } else {
        ++i
    }
}


fun main() {
    for (j in 0..2) {
        println(j)
    }
    
var i = 2 println(++i)
if (i == 2) { println("2") } else { ++i } }

We always require double line breaks before and after “block statements”, which can be if, switch (when in Kotlin), for, try, or while blocks, unless they are exactly at the beginning or the end of a method or another block. We believe this makes our code more readable, that’s why we always leave a comment during code review if someone violates this rule (and if we happen to notice it).

What is the best tool for this problem

We can say that ktlint for Kotlin and Checkstyle for Java are the main formatting code analysis tools, but now I’ll pick another tool yet: Android Lint (not only for Android projects), because it also supports writing custom rules that can be applied to Kotlin and Java at the same time. It seems a more logical choice, because we use both languages in our Android projects, and I wanted neither to write the same rule twice nor maintain them concurrently. Not to mention the integration of the rule, which would also need to be done two times.

Why Java is still important at all

As an Android developer, I have to write code in both Java and Kotlin. However, we can say that it's not worth the time to focus on Java anymore, because as Kotlin is coming up, we use it more and more instead of Java in our codebase. On the other hand, I believe the transition isn't happening that quickly in big projects already in production. So, as we want to see nicely formatted code in Java as well, it's important to have this rule for both languages.

How to write custom lint rules

There are many tutorials and articles about writing custom code analysis rules, so I won’t detail it too much in this post. But if you’re interested, here are a couple of links I can recommend for the different tools: To Checkstyle, the official doc, to ktlint and Android Lint, Niklas Baudy’s great medium articles.

How Android Lint and other static code analysis tools work

while b ≠ 0
  if a > b
    a := a − b
  else
    b := b − a
return a
An abstract syntax tree for the following code for the Euclidean algorithm:
Example for AST (source: Wikipedia about Abstract Syntax Tree)

Initially, Android Lint was created for finding mainly Android specific issues in Java code. In order to analyze the Java code, Android Lint used to create a Java-specific Abstract Syntax Tree (or just AST, learn more on Wikipedia), which is a tree representation of the source code. Other static code analysis tools also use a language specific AST for analysis;
Checkstyle: Java-, ktlint: Kotlin-, detekt: Kotlin-specific. The tools traverse this AST and find errors by checking its nodes and their attributes.

How Android Lint can check two languages at once

The difference between Android Lint and other tools is that as Kotlin also became a supported language for Android, Android Lint started to support Kotlin as well regarding the rules we already had to Java. For this, they introduced the so called Universal Abstract Syntax Tree (developed by JetBrains) which provides a tree representation of the code that can be applied for both Kotlin and Java languages at the same time, so it’s on a higher abstraction level than a language specific AST. For more information, I recommend this talk part from Tor Norbye — the creator and maintainer of Android Lint.

Both UAST and AST provide high level details about the source code. They don’t contain information about whitespaces or braces, but it’s usually enough for the Android specific rules. See a UAST example below:

UAST example for a certain code snippet
By calling the asRecursiveLogString() method on a certain UAST node in the Android Lint API, we can print out the UAST.

The UAST library includes all the language-specific expressions, but also provides common interfaces for them. Example for the if expression:

UAST expression example
UIfExpression common interface is implemented by the JavaUIfExpression, JavaUTernaryExpression, and KotlinUIfExpression.

PSI Tree (Program Structure Interface Tree)

The PSI tree is built on the UAST in case of Android Lint (in case of other tools, it’s built on the language specific AST), and this is the tree that contains more details about the structure of the code, such as whitespaces, braces, and similar elements.

In Android Lint, PSI Expressions are included in the implementation of the UAST Expressions, which are the nodes of the UAST. Eg. org.jetbrains.kotlin.psi.KtIfExpression can be accessed from KotlinUIfExpression.

There is even a handy intelliJ plugin: PsiViewer that can ease the debugging of PSI tree-based code analysis rules. See the example below with the same code snippet, where we can see that the two languages have different language-specific tokens in the trees:

PSI Tree representation of a method in Java and Kotlin
PSI Tree representation of a method in Java and Kotlin

Using both UAST and PSI Tree

I need both UAST and PSI Tree to create my formatting rule, because UAST is great to filter and visit nodes that I’m interested in for both languages at once, but as I mentioned it doesn’t provide information about formatting, eg. whitespace information that is essential for me. UAST rather focuses on a higher abstraction level, so primarily it isn’t for creating formatting rules.

Since the Gradle plugin 3.4 and the related Android Lint version 26.4.0, we can get the PSI tree representation of each node and its surroundings, not only in Java, but Kotlin as well. This makes it possible to use Android Lint for my formatting rule.

How the rule is implemented

First, I need to create my issue, where I set the scope to JAVA_FILE, which means both Java and Kotlin in Android Lint currently. (Here is where we can set other kinds of files as well such as XML or Gradle files, because Android Lint can check them as well.)


val ISSUE_MISSING_EMPTY_LINES_AROUND_BLOCK_STATEMENTS = Issue.create(
    "MissingEmptyLinesAroundBlockStatements",
    "Marks missing empty lines around blocks statements.",
    "Block statements (such as 'if', 'for', 'while', 'switch', 'when') should always surrounded by empty lines.\n" +
            "Exception if they are placed exactly at the end or the beginning of an enclosing block.\n" +
            "Applying this rule we can get more readable code.",
    Category.CORRECTNESS,
    DEFAULT_PRIORITY,
    Severity.WARNING,
    Implementation(MissingEmptyLinesAroundBlockStatementsDetector::class.java, EnumSet.of(Scope.JAVA_FILE))
)

Initialization of a custom Issue class

Then, in my detector class, I enumerate the nodes I’m interested in inside getApplicableUastTypes function, so Android Lint will know that it should call the related overridden visit methods such as visitForEachExpression. In each visit method, I just call my checker method.


class MissingEmptyLinesAroundBlockStatementsDetector : Detector(), Detector.UastScanner {
    override fun getApplicableUastTypes() = listOf(
        UIfExpression::class.java,
        USwitchExpression::class.java,
        UForExpression::class.java,
        UForEachExpression::class.java,
        UWhileExpression::class.java,
        UDoWhileExpression::class.java,
        UTryExpression::class.java
    )
override fun createUastHandler(context: JavaContext) = MethodBlocksHandler(context)

class MethodBlocksHandler(private val context: JavaContext) : UElementHandler() {

    override fun visitIfExpression(node: UIfExpression) {
        if (node.thenExpression !is UBlockExpression) {
            return
        }

        checkWhiteSpaceAroundBlock(node)
    }

    override fun visitSwitchExpression(node: USwitchExpression) =
        checkWhiteSpaceAroundBlock(node)

    override fun visitForExpression(node: UForExpression) =
        checkWhiteSpaceAroundBlock(node)

    override fun visitForEachExpression(node: UForEachExpression) =
        checkWhiteSpaceAroundBlock(node)

    override fun visitWhileExpression(node: UWhileExpression) =
        checkWhiteSpaceAroundBlock(node)

    override fun visitDoWhileExpression(node: UDoWhileExpression) =
        checkWhiteSpaceAroundBlock(node)

    override fun visitTryExpression(node: UTryExpression) =
        checkWhiteSpaceAroundBlock(node)

    private fun checkWhiteSpaceAroundBlock(node: UElement) {
        checkWhiteSpaceAroundBlock(node, forward = true)
        checkWhiteSpaceAroundBlock(node, forward = false)
    }
    
    ...
How to tell Android Lint what nodes we want to check

In my checker method, the forward parameter describes if I want to check the line break before or after the “block statement”. In the method body, I investigate the line break number of whitespaces around the block statement I’m visiting. For that, I do three main steps:


...

private fun checkWhitespaceAroundBlock(node: UElement, forward: Boolean) { val firstBlockLevelPsiNode = firstBlockLevelNode(node)?.sourcePsi ?: return
val firstRelevantWhiteSpaceNode = firstRelevantWhiteSpaceNode(firstBlockLevelPsiNode, forward) ?: return
if (firstRelevantWhiteSpaceNode.text.count { p -> p == '\n' } > 1) return
context.report( ISSUE_MISSING_EMPTY_LINES_AROUND_BLOCK_STATEMENTS, context.getLocation(firstRelevantWhiteSpaceNode), "Block statement isn't ${if (forward) "followed" else "preceded"} by empty line." ) }
...
  • First, by firstBlockLevelNode method, I check what’s the first block level node, around which I want to check the whitespaces, because if I use an if block for assigning a value to a variable like this,

val max = if (a > b) {
    print("Choose a")
    a
} else {
    print("Choose b")
    b
}

“Block statement” in value assignment
Lint would investigate the whitespace just before the if keyword, but I am interested in the whitespace before the val keyword. So, in this case, the first block level statement is the value assignment that wraps my if statement.
  • Second, in the firstRelevantWhiteSpaceNode method, I check what the first relevant whitespace node is, where we should count the line breaks. Sometimes there is no relevant whitespace to check, because if my block is at the beginning or end of a method or any other blocks, then that’s fine and I can forego further investigation. See:

fun foo() {
   for (i in 0..2) {
       bar = true
   }
}

Single block statement in a method”
At this point I’m already using the PSI nodes, because I want to check whitespace information that is not provided in UAST. For that, I need to get the sourcePsi property of the block level UAST node.

fun foo() {
   val i = 0
   // Beginning of block
   for (i in 0..0) {
       bar = true
   }
}

Having a comment before a “block statement”
An edge case is if there’s a comment just above my block statement. Here, I want to check the whitespace above the comment, so the first relevant whitespace is above the comment statement.
  • Finally, I count the number of line breaks in the relevant whitespace; If it’s not bigger than 1, I report an issue.

The result

As a result, I expect the following warnings in the IDE, so it’ll be clear for each developer that an additional line has to be added. I also can get these warnings in the Lint report if I run lint gradle task. Furthermore, I can even raise the severity of the issue to error, if I want to block the merge request job.

IDE warning before and after block statement IDE warning before and after block statement
IDE warning before and after block statement

How to integrate a rule

In this tutorial, we can see the steps to do that using an extra module in our project. The difference here is that I pushed my compiled version of the rule to a Maven repository, so it can be integrated with just one line in any Android project, without any git submodules or similar solutions.


repositories {
    mavenCentral()
}

dependencies { implementation "io.supercharge:lint-checks-common-rules:0.2.0" }
Use deployed custom lint checks library as dependency

How to fix the already existing issues

Even though we always wanted to filter out this formatting issue by code review, sometimes we missed it, so we accumulated several issues in our codebases so far. It is quite boring to fix all the issues by hand, but fortunately, Android Lint gives support for auto fixing (see detailed in this article), which can be applied in two separate cases:

  • By hitting the alt + enter keyboard combination
IDE warning before and after block statement
QuickFix in practice by the IDE
  • By running the lintFix gradle task that automatically fix all the Android Lint issues that the quick fix feature is configured for:
IDE warning before and after block statement
QuickFix in practice — diff after lintFix gradle task run

The second option is more relevant for fixing the already existing issues, because it’s completely automatic. To configure the quick fix feature for my rule, I just need to pass a LintFix object when I invoke the report method. This quick fix appends one more line break to the single whitespace:


...

private fun checkWhiteSpaceAroundBlock(node: UElement, forward: Boolean) { ...
context.report( ISSUE_MISSING_EMPTY_LINES_AROUND_BLOCK_STATEMENTS, context.getLocation(firstRelevantWhiteSpaceNode), "Block statement isn't ${if (forward) "followed" else "preceded"} by empty line.", createQuickFix() ) }
private fun createQuickFix() = LintFix.create() .name("Add extra line") .replace() .text("\n") .with("\n\n") .autoFix() .build()
Reporting an issue with LintFix object

How to test a custom rule

Android Lint provides a testing framework as well, with which we can unit test our custom checks very comfortably.


@Test
fun `missing empty line before variable assignment if block statement in Kotlin`() {
    lint()
        .files(
            TestFiles.kt(
                """
        package foo
        class Example {
            fun foo() {
                val i = 0
                val k = when (i) {
                    0 -> println("Foo")
                    else -> println("Bar")
                }
            }
        }"""
            ).indented()
        )
        .issues(ISSUE_MISSING_EMPTY_LINES_AROUND_BLOCK_STATEMENTS)
        .run()
        .expect(
            """
        |src/foo/Example.kt:6: Warning: Block statement isn't preceded by empty line. [MissingEmptyLinesAroundBlockStatements]
        |        val i = 0
        |                 ^
        |0 errors, 1 warnings
        |""".trimMargin()
        )
        .expectFixDiffs(
            """
        |Fix for src/foo/Example.kt line 6: Add extra line:
        |@@ -7 +7
        |+
        |""".trimMargin()
        )
}

Checking false negative case: Unit test for testing custom Android Lint check
  • First, I need to write a snippet of code about a scenario, in which case I would expect an issue, if I want to check false negativity.
  • Second, I have to point to the Issue object I expect in the issues method.
  • Third, in the expect method, I can describe what errors I expect and where.
  • Finally, I have a chance to test the quick fix mechanism as well by describing the expected diff after the fix in the expectFixDiffs method.

The steps are very similar for checking false positive cases; the only difference is that we use the expectClean method at the end.


@Test
fun `no missing empty line around block statement in Kotlin`() {
    lint()
        .files(
            TestFiles.kt(
                """
        package foo
        class Example {
            fun foo() {
                for (i in 0..0) {
                    bar = true
                }
            }
        }"""
            ).indented()
        )
        .issues(ISSUE_MISSING_EMPTY_LINES_AROUND_BLOCK_STATEMENTS)
        .run()
        .expectClean()
}

Checking false positive case: Unit test for testing custom Android Lint check

Another great thing about unit testing is that syntax highlighting is supported if you use Android Studio, which minimizes the chance to write faulty code inside the test. This is a feature that is not available in other code analysis tools (such as Checkstyle, ktlint, or detekt).

IDE warning before and after block statement
Code syntax highlighting inside unit test by Android Studio IDE

Besides Kotlin, we can also write Java language specific tests. For that, we have to use the TestFiles.java(…) static method, and then the IDE can provide Java language syntax highlighting as well.

Debugging with unit tests

Using tests are currently the only way to debug a rule step by step, because otherwise the program runs in the IDE that you can’t launch in debug mode. By using tests, the debugging method is the same as debugging a unit test of any application; you can use breakpoints and evaluate the state of the program at any stage if you run it in debug mode.

Android Lint outside Android

Since Gradle 3.1.0, the tool is available for pure Java and Kotlin projects as well. See more information about the configuration in this tutorial. In a former Android Lint version (26.3) there was a bug as the article mentions, which made impossible to use custom rules with non-Android gradle modules. However, it was fixed in version 26.4, so we can leverage our Android Lint rules even on backend Kotlin/Java projects as well.

Conclusion

After integrating the custom rule, we don’t have to focus on this frustrating issue anymore; instead, we can spend our brain power on finding more complex problems when we review each other’s code. Even better, we can 100% guarantee that this mistake won’t get in our codebase accidentally, because we can configure our MR job to fail when someone misses to fix this issue.

Though Android Lint isn’t primarily for formatting rules, in my case it was quite practical, because it can be used for both Java and Kotlin: no double rule writing, maintaining and integration needed.

On the other hand, we have to notice that this rule is a very simple one in that I only have to check whitespaces and curly braces on PSI level, which are the same in the two languages. That’s why I don’t have to write any language specific code. However, if I should write some language specific code yet (eg. handling Elvis operator in Kotlin or Ternary operator in Java), I would still consider to prefer Android Lint against writing one-one ktlint and a Checkstyle rules, because I would still have much less work probably.

References

If you are interested in Android Lint even more, I can really recommend this three talks:

Source code

If you’re interested in my code more thoroughly, please use this Github link: https://github.com/team-supercharge/lint-checks.


This article was originally published on Supercharge’s blog, you can find the original version here.