Enforcing Best Practices on Android with Lint

Driving Developers Towards Better Code with Build-Time Enforcement

It seems like every year the landscape of Android best practice changes drastically, and our code tends to follow the trend. However, each change leaves in its wake a vast amount of code that you might call “the old way of doing things”, or even worse, the “bad way of doing things.” Without a doubt, there will be scenarios where the temptation to keep doing things the old way will strike; as humans we tend to take the path of least resistance and stick to things we already know rather than learning something new.

So how do we encourage developers to do things the right way? At the very least, how do we prevent developers from doing things the wrong way? You might propose simply deprecating the component or class that we should no longer use, which makes using it unattractive in the IDE.

The problem with that approach is that it isn’t enforced by the compiler. If I don’t care that it looks ugly to use, and if my code reviewer doesn’t care either (or if they simply miss the deprecation in the review), that new and bad code is shipping to production. And even if there were a way to prohibit this, I would only want to stop the deprecated code from being used in new code, not old code. You can see a discussion here as to why that wouldn’t be a good idea.

Android Lint helps us solve all of these headaches described above because it comes equipped with the ability to scan your code and report custom errors (or warnings) against your code, while also being able to upgrade, downgrade, ignore, and baseline the issues reported against your code.

In order to understand this more clearly, we’ll take a simple example inspired by a rule I wrote for our Android flagship app codebase here at Capital One. This rule prohibits the use of the configChanges attribute on activity elements within the AndroidManifest.xml. Why? This attribute can be used incorrectly to avoid having to deal with configuration changes in Activities, which can lead to poor user experience. Because fixing this after the fact is often non-trivial, we wanted to prevent it from being done in the future while back-burnering the existing uses until the team could address them.

It is worth noting that this use case is even harder to solve without lint than the one introduced above, as deprecation isn’t an option since we do not own the android:configChanges attribute.

Writing the rule: PreventingActivityRestart

Intention: Discourage the use of the android:configChanges attribute in the AndroidManifest.xml and force developers to handle configuration changes the “right” way.

There is a Github repository that can be found here that contains all the code for the rule we’ll write below.

To set the scene, here is the code we are looking to prohibit:

When a developer adds a class to the manifest that has this attribute, we will want to show them an error and explain why that practice is discouraged, and what they should do to fix the issue.

Let’s get started by adding the below dependencies to a new java library module (we’ll use Kotlin though 🎉 ) so we can use the lint APIs. Take note of the comment regarding the versioning of your lint dependencies.

In order to write our lint rule, we need to code up a few things:

• Detector: This is what does the “checking” for violations of our rule. Methods in the Detector are invoked as lint scans the project. We’ll call ours PreventingActivityRestartDetector.
• Issue: This is the Issue we’re trying to check for. In our case, our Issue will be called PreventingActivityRestart and be reported on activity elements in the AndroidManifest.xml that use the android:configChanges attribute.
• IssueRegistry: This class supplies lint with the Issues we want to scan the project for. In this example, we would add the PreventingActivityRestart Issue to the IssueRegistry to tell lint we want to check for violations of that Issue.

First, let’s write the Detector that will be responsible for reporting that Issue.

Our PreventingActivityRestartDetector extends from Detector so that we can pass it to lint and be notified when the manifest is scanned (how that happens is described below). It also implements XmlScanner, which is an interface that contains the methods we need to override in order to make our Detector work on XML files. The Detector class we extend from is abstract and contains the functions we would be forced to implement by XmlScanner. Because we extend Detector, we can pick and choose what pieces of the code we are interested in visiting by implementing the appropriate methods.

In this case, we are interested in visiting the XML element activity, so we return that from getApplicableElements(). When an element with that name is found, we want to be notified so we can do something about it, so we override visitElement(). In that method, we’ll decide if we want to report the Issue (defined below). In this case, we always will, but in more complex rules you can decide not to if the surrounding source indicates you shouldn’t based on your business logic.

Next, let’s write out the Issue we want to report:

Most of the fields on the Issue are self-explanatory. In most cases, a glance at the source code for the field type will clear up any questions you may have.

That said, one of the fields I don’t love is Priority. The spectrum is 1–10, and that leaves a really wide, arbitrary range of values that can be used. This field will really only be used when addressing your lint baseline (a file you can generate to tell lint to ignore existing issues, useful for starting with a clean slate), so the best question to ask yourself is: “When I have time to clean up existing issues, how important will this Issue be?”

The one field that is critical to get right is the Implementation field, as it takes in the Class of your Detector and a Scope. That Scope is what lint uses to apply your Detector to certain types of files in your project, and if you don’t get that right, the visitX() methods on your Detector will never be called. If you see that behavior when debugging your tests, double-check your Scope! For our custom rule, we are using Scope.MANIFEST_SCOPE which, according to the Scope source code is a:

Scope-set used for detectors which are affected by the manifest only

In other words, using this Scope ensures our Detector will be able to visit the manifest file within the project.

Now that the Issue is defined, we can report it in visitElement()

You may notice that the report() method takes in a message, and I’ve chosen to simply re-use the brief description field of the Issue. After reviewing the source code for the built-in rules published with the Android Gradle Plugin, it seems that this message field is a good place to communicate something that is specific to the instance of the violation you are reporting. This is helpful when the Issue can show up in more than one way.

Our Issue is pretty cut and dry, so I have chosen to re-use our generic messaging.

Finally, let’s add our Issue to an IssueRegistry.

The IssueRegistry is where lint will pick up our custom rule. We won’t use this class in our code, but we’ll use the qualified path to the class when defining the jar task in our build.gradle below.

By overriding the api val here and returning CURRENT_API, we are indicating that our rule has been tested on the current lint API.

Before we can be confident that our rule works, we need to test it. Testing a lint rule is done exactly the way you might imagine. We write code that violates the rule and assert the warnings that we expect to be reported by lint.

Pro tip: Use the indented() method on your TestFile for syntax highlighting.

We’ll also want to be sure to test negative and edge cases in order to make sure our lint rule is reliable.

Publishing the rule

To allow our new rule to be a part of the lint analysis, we need to publish it to the lint.jar that is used when the lint task is run.

Note, this is only a snippet of the lint rules module build.gradle, the rest of the build.gradle lives in the Github repository linked above.

Importing the rule

Our rule is now working (we’re sure of it because we tested!), so let’s get it into the project. There are a few ways of doing this, but what I recommend is having our lint rules live in a module alongside the other modules in our codebase.

Project structure with lint rules in their own module

With this setup, we can simply build our lint rules module and use the lintChecks configuration to indicate where we want to apply our rules.

In order to be sure our new rules are actually being used, we can run ./gradlew -q app:dependencies --configuration lintChecks and ensure the lintChecks configuration includes our custom lint rules.

This indicates that our project is being used by lint to analyze the modules that declare that configuration. For more visual evidence that the rule we wrote is being published via the IssueRegistry, we can decompile the lint.jar found at /app/build/intermediates/lint_jar/global/prepareLintJar using a tool like JD-GUI. We should see our IssueRegistry containing the rule we added.

Enforcing best practices

Now that our rule has been written, published and imported, we need to fix all the violations of the rule or create a baseline for the rule. Lint will ignore violations in the baseline until we have time to address them. This is easily done by defining a baseline file in the lintOptions block of our build.gradle.

The next time lint is run, it will create this baseline file for us and all the violations from that run will be placed inside the file.

Note that the creation of the baseline will result in a failed build to ensure that the baseline wasn’t created accidentally (otherwise you’d be unintentionally ignoring violations).

After the baseline is created, lint will use that file to avoid reporting baselined Issues. If we only want to baseline our custom rule, we can add check "PreventingActivityRestart" to the lintOptions block prior to the baseline being created. Lint will then only look for that specific Issue and place violations of it in the baseline. But we will need to remember to remove that line after the initial baseline is created or lint will only ever check for that Issue.

Hopefully this article has given you the confidence you need to start your own custom lint rule journey towards more hygienic code!

DISCLOSURE STATEMENT: © 2019 Capital One. Opinions are those of the individual author. Unless noted otherwise in this post, Capital One is not affiliated with, nor endorsed by, any of the companies mentioned. All trademarks and other intellectual property used or displayed are property of their respective owners.