Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added an Android Lint check that ensures the correct Log class is used #83

Merged
merged 4 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ tasks.dokkaHtmlMultiModule.configure {

apiValidation {
nonPublicMarkers.add("com.juul.khronicle.KhronicleInternal")
ignoredProjects += "khronicle-android-lint"
}

fun Project.withPluginWhenEvaluated(plugin: String, action: Project.() -> Unit) {
Expand Down
7 changes: 6 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
[versions]
android-compile = "34"
android-agp = "8.7.2"
android-tools = "31.7.2" # If the AGP version is X.Y.Z, then the Lint library version is X+23.Y.Z.
jacoco = "0.8.7"
jvm-toolchain = "8"
kotlin = "2.0.21"
ktor = "3.0.1"

[libraries]
android-lint-api = { module = "com.android.tools.lint:lint-api", version.ref = "android-tools" }
android-lint-test = { module = "com.android.tools.lint:lint-tests", version.ref = "android-tools" }
junit = "junit:junit:4.13.2"
kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version = "1.9.0" }
ktor-core = { module = "io.ktor:ktor-client-core", version.ref = "ktor" }
ktor-logging = { module = "io.ktor:ktor-client-logging", version.ref = "ktor" }
ktor-mock = { module = "io.ktor:ktor-client-mock", version.ref = "ktor" }

[plugins]
android-library = { id = "com.android.library", version = "8.7.2" }
android-library = { id = "com.android.library", version.ref = "android-agp" }
android-publish = { id = "com.vanniktech.maven.publish", version = "0.30.0" }
api = { id = "org.jetbrains.kotlinx.binary-compatibility-validator", version = "0.16.3" }
atomicfu = { id = "org.jetbrains.kotlinx.atomicfu", version = "0.26.0" }
Expand Down
13 changes: 13 additions & 0 deletions khronicle-android-lint/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
plugins {
id("com.android.lint")
kotlin("jvm")
id("org.jmailen.kotlinter")
}

dependencies {
compileOnly(libs.android.lint.api)

testImplementation(libs.android.lint.api)
testImplementation(libs.android.lint.test)
testImplementation(libs.junit)
}
3 changes: 3 additions & 0 deletions khronicle-android-lint/gradle.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# needed so that prepareLintJarForPublish can succeed
# See https://issuetracker.google.com/issues/161727305
kotlin.stdlib.default.dependency=false
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.juul.khronicle

import com.android.tools.lint.client.api.IssueRegistry
import com.android.tools.lint.client.api.Vendor
import com.android.tools.lint.detector.api.CURRENT_API
import com.android.tools.lint.detector.api.Issue

class KhronicleIssueRegistry : IssueRegistry() {
override val issues: List<Issue>
get() = listOf(WrongLogUsageDetector.ISSUE_LOG)

override val api: Int
get() = CURRENT_API

/**
* works with Studio 4.0 or later; see
* [com.android.tools.lint.detector.api.describeApi]
*/
override val minApi: Int
get() = 7

override val vendor = Vendor(
vendorName = "JuulLabs/Khronicle",
identifier = "com.juul.khronicle:khronicle:{version}",
feedbackUrl = "https://github.com/JuulLabs/khronicle/issues",
)
}
138 changes: 138 additions & 0 deletions khronicle-android-lint/src/main/kotlin/WrongLogUsageDetector.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package com.juul.khronicle

import com.android.tools.lint.detector.api.Category.Companion.MESSAGES
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Implementation
import com.android.tools.lint.detector.api.Incident
import com.android.tools.lint.detector.api.Issue
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.LintFix
import com.android.tools.lint.detector.api.Scope.Companion.JAVA_FILE_SCOPE
import com.android.tools.lint.detector.api.Severity
import com.android.tools.lint.detector.api.SourceCodeScanner
import com.intellij.psi.PsiMethod
import com.intellij.psi.PsiType
import org.jetbrains.uast.UCallExpression

class WrongLogUsageDetector : Detector(), SourceCodeScanner {
private val androidLogMethodToKhronicleLogMethod = mapOf(
"v" to "verbose",
"d" to "debug",
"i" to "info",
"w" to "warn",
"e" to "error",
"wtf" to "assert",
)

override fun getApplicableMethodNames() = listOf("v", "d", "i", "w", "e", "wtf")

override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
val evaluator = context.evaluator

if (evaluator.isMemberInClass(method, "android.util.Log")) {
context.report(
Incident(
issue = ISSUE_LOG,
scope = node,
location = context.getLocation(node),
message = "Using 'android.util.Log' instead of 'com.juul.khronicle.Log'",
fix = quickFixIssueLog(node),
),
)
return
}
}

private fun quickFixIssueLog(logCall: UCallExpression): LintFix {
val arguments = logCall.valueArguments
val methodName = logCall.methodName

val tag = arguments[0].asSourceString()
val message: String
val throwable: String?

when (arguments.size) {
2 -> {
val msgOrThrowable = arguments[1]
val isString = msgOrThrowable.getExpressionType()?.isString() ?: true

message = when {
isString -> msgOrThrowable.asSourceString()
else -> ""
}

throwable = when {
isString -> null
else -> msgOrThrowable.sourcePsi?.text
}
}

3 -> {
message = arguments[1].asSourceString()
throwable = arguments[2].sourcePsi?.text
}

else -> {
throw IllegalStateException("android.util.Log overloads should have 2 or 3 arguments")
}
}

val khronicleLogMethodName = androidLogMethodToKhronicleLogMethod[methodName]

val fixSource1 = buildString {
append("com.juul.khronicle.Log.$khronicleLogMethodName(tag = $tag")
if (throwable == null) {
append(")")
} else {
append(", throwable = $throwable)")
}

append(" { $message }")
}

val fixSource2 = buildString {
append("com.juul.khronicle.Log.$khronicleLogMethodName")
if (throwable != null) {
append("(throwable = $throwable)")
}

append(" { $message }")
}

val logCallSource = logCall.uastParent!!.sourcePsi?.text
return fix()
.group()
.add(
fix()
.replace()
.text(logCallSource)
.shortenNames()
.reformat(true)
.with(fixSource1)
.build(),
).add(
fix()
.replace()
.text(logCallSource)
.shortenNames()
.reformat(true)
.with(fixSource2)
.build(),
).build()
twyatt marked this conversation as resolved.
Show resolved Hide resolved
}

companion object {
@Suppress("ktlint:standard:max-line-length")
val ISSUE_LOG = Issue.create(
id = "LogNotKhronicle",
briefDescription = "Logging call to android.util.Log instead of Khronicle",
explanation = "Since Khronicle is included in the project, it is likely that calls to android.util.Log should instead be going to Khronicle.",
category = MESSAGES,
priority = 5,
severity = Severity.WARNING,
implementation = Implementation(WrongLogUsageDetector::class.java, JAVA_FILE_SCOPE),
)

private fun PsiType.isString() = canonicalText == "java.lang.String"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.juul.khronicle.KhronicleIssueRegistry
Loading