Back in November 2019 I wrote about an issue that can cause problems when serialising Kotlin objects. It is one thing to know about this issue, but quite another to find all instances of it in your code and, as importantly, to remember it in code you and the rest of your team add to the codebase in the future. Wouldn’t it be great if we could enforce this in our build process? Or if some of the static analysis tools we use could enforce it for us? Sadly, at the time of writing, none of the static analysis tools that I use (Android lint, ktlint, and detekt) have a check for this potential issue.
To address this I am happy to release ReadResolveCheck a lint check which anyone can easily add to their project.
ReadResolveCheck is available from jCenter and, assuming that you have that repository configured in your build script you can add the lint check by adding a single dependency to your module build script:
dependencies {
.
.
.
lintChecks "com.stylingandroid:read-resolve-check:1.0.0"
}
That’s all you need to do – this check will be added to your lint check suite and run every time the suite is run. There’s a little more information in the readme on the Github repo, but essentially that’s it.
Although it seems like this should be the shortest post in Styling Android history, let’s actually take a look under the covers to see what’s happening internally. Writing custom lint checks is a really useful tool to have at your disposal because it enables you to add checks which may be specific to your project .
The class that performs all of the work has this framework:
internal class SerializableObjectMissingReadResolveDetector : Detector(), Detector.UastScanner {
companion object {
.
.
.
}
override fun getApplicableUastTypes() =
listOf>(UClass::class.java)
override fun createUastHandler(context: JavaContext): UElementHandler? =
object : UElementHandler() {
.
.
.
}
}
The companion object contains a number of constants which are used throughout this class. For conciseness I won’t bother including them here, but you can see them in the source. The class itself extends Detector() which is part of the lint API, and also extends Detector.UastScanner. Universal Abstract Syntax Tree (UAST) is a JetBrains tookit which provides a tree representation of the JVM bytecode and works for both Java & Kotlin code as a result. This custom lint check will use this UAST representation to analyse the code.
The getApplicableUastTypes() method provides a list of UAST types that we’re interested in. In this case we’re only interested in Class types because that’s what we’ll apply the check to.
The createUastHandler() method is overridden from Detector.UastScanner and is returns a UElementHandler which is responsible for performing the check.
override fun createUastHandler(context: JavaContext): UElementHandler? =
object : UElementHandler() {
override fun visitClass(node: UClass) {
if (
node is KotlinUClass &&
node.ktClass is KtObjectDeclaration &&
isSerializable(node)
) {
if (node.doesNotHaveReadResolveMethod()) {
context.report(
MISSING_READ_RESOLVE_ISSUE,
node,
context.getLocation(node as PsiElement),
MISSING_READ_RESOLVE_ISSUE_EXPLANATION,
node.createMissingReadResolveLintFix()
)
} else {
node.methods
.filterIsInstance()
.filter { it.name == "readResolve" && !it.hasCorrectReadResolveSignature() }
.forEach { method: KotlinUMethod ->
node.methodSignatureCheck(method)
}
}
}
}
fun UClass.methodSignatureCheck(method: KotlinUMethod) {
name?.let { name ->
context.report(
MISSING_READ_RESOLVE_ISSUE,
context.getLocation(method),
INCORRECT_READ_RESOLVE_SIGNATURE_EXPLANATION,
method.createIncorrectMethodSignatureLintFix(name)
)
}
}
fun isSerializable(node: UClass) =
context.evaluator.implementsInterface(
node,
Serializable::class.java.canonicalName,
false
)
fun KotlinUClass.doesNotHaveReadResolveMethod(): Boolean =
allMethods.none { it.name == "readResolve" }
fun KotlinUMethod.hasCorrectReadResolveSignature(): Boolean =
returnType?.canonicalText == JAVA_OBJECT && parameters.isEmpty()
.
.
.
}
}
The visitClass() method is a visitor which is called for every class found. It first performs a check that the class matches certain criteria: Firstly that it is a Kotlin class; Secondly that it is a Kotlin object declaration; and thirdly that it is Serialisable. Only if all of those conditions are met will the rest of the check be performed.
If these are met we then check whether the class being visited has a readResolve() method defined. If it does not then we report an issue using context.report(). This mainly uses some of the constants defined in the companion object, and a mix of attributes from the UClass of the node in the UAST tree that we’re visiting. The final argument provides a method to perform a fix, and we’ll look at this momentarily.
If the class implements a method named readResolve() we check that the method signature is correct, and, if not, we report a slightly different warning and provide a slightly different fix.
So that’s the check logic implemented, so let’s now look at those auto fixes:
override fun createUastHandler(context: JavaContext): UElementHandler? =
object : UElementHandler() {
.
.
.
fun KotlinUClass.createMissingReadResolveLintFix(): LintFix {
val objectSource = sourceElement.text
val hasClassBody = objectSource.trimEnd().endsWith("}")
return if (hasClassBody) {
LintFix.create()
.name("Add readResolve() method")
.replace()
.pattern("\\}\\s*$")
.with("\n@Suppress(\"UnusedPrivateMember\")\nprivate fun readResolve(): Any = $name\n}")
.reformat(true)
.autoFix()
.build()
} else {
LintFix.create()
.name("Add readResolve() method")
.replace()
.text(LintFix.ReplaceString.INSERT_END)
.with(" {\n@Suppress(\"UnusedPrivateMember\")\nprivate fun readResolve(): Any = $name\n}")
.reformat(true)
.autoFix()
.build()
}
}
fun KotlinUMethod.createIncorrectMethodSignatureLintFix(objectName: String): LintFix {
return LintFix.create()
.name("Fix incorrect readResolve() method signature")
.replace()
.text(sourceElement.text)
.with("@Suppress(\"UnusedPrivateMember\")\nprivate fun readResolve(): Any = $objectName")
.reformat(true)
.autoFix()
.build()
}
}
}
The createMissingReadResolveLintFix() method has two separate branches depending on whether the class has an existing body or not. The reason for this is that the fix will need to include a class body if there is not already one defined.
The fix is implemented through a LintFix builder which gets created for both branches, and also for the createIncorrectMethodSignatureLintFix() method. We first call the name() method which sets the text that will be displayed to the user in the context menu. We then specify the text that we want to replace, and this is slightly different in the various fixes for different cases – in two cases we change text matching a literal string, and in another we match a regex pattern. We then provide the text that we with to replace the previously defined text with. Once again this varies slightly for the different cases, but essentially adds a readResolve() method. Next we specify reformat(true) which will apply the formatting rules defined within the IDE settings – so we please both tab / space indent camps provided they have their preference correctly configured in the IDE. We next call autoFix() which will automatically apply this fix without human intervention. Finally we call build() to create the LintFix instance from the builder.
That’s the basic check & fix logic in place, but we now need to add some support code to make this discoverable by the lint tool. First we need to create an IssueRegistry instance:
class LibraryIssueRegistry : IssueRegistry() {
override val issues = listOf(
SerializableObjectMissingReadResolveDetector.MISSING_READ_RESOLVE_ISSUE
)
override val api = com.android.tools.lint.detector.api.CURRENT_API
}
The issues list is a list of issues defined in this library, and in this case we include the one we have just defined.
We now need to declare this class within the manifest of our JAR. We could either do this with a static file, or we can actually do this in our build script:
.
.
.
jar {
manifest {
attributes("Lint-Registry-v2": "com.stylingandroid.readresolve.lint.LibraryIssueRegistry")
}
}
.
.
.
This creates an entry in the JAR manifest named Lint-Registry-v2 (which is a name looked for by the lint tool), and we give the fully qualified name of our IssueRegistry class.
That’s pretty much it for this implementation, but there is actually slightly more to this overall story. At the start I mentioned ktlint and detekt as other static analysis tools that I commonly use, and it’s worth a quick explanation of why I opted for lint over those tools.
Firstly, lint provides the richest experience for a developer because it is well integrated with the IDE and also provides the auto fix options which is certainly helpful to developers who are unfamiliar with the issue getting a quick resolution.
That said, I actually attempted to create checks for all three tools so that folks could choose the one which worked best for them. Unfortunately I was unable to get robust enough checks working for both ktlint and detekt. The reason for this is that they both use Program Structure Interface (PSI) another JetBrains toolkit to analyse the class structure instead of UAST. While PSI does have the tools to do what we need, neither ktlint nor detekt provides a BindingContext which is necessary to reliably determine whether any given class implements Serializable. Although detekt has this as an experimental feature, I was only getting a BindingContext.EMPTY being passed in to my custom check.
Although I was unable to get these PSI-based checks working, I am deeply indebted to Vladimir Dolzhenko who has huge knowledge of PSI and was incredibly helpful. Also to Hadi Hariri for putting me in contact with Vladimir. Last but by no means least, I also had some very though provoking discussions with Matt Dolan when I first started on this, and those discussions certainly helped to steer my efforts in the right direction.
The source code for this article is available here and is available to use from jCenter.
© 2020, Mark Allison. All rights reserved.
Copyright © 2020 Styling Android. All Rights Reserved.
Information about how to reuse or republish this work may be available at http://blog.stylingandroid.com/license-information.
