Skip to content
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
6 changes: 6 additions & 0 deletions coil-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,9 @@ baselineProfile {
}
}
}

dependencies {
lintPublish(projects.coilLint) {
isTransitive = false
}
}
9 changes: 2 additions & 7 deletions coil-lint/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import coil3.setupPublishing
import com.vanniktech.maven.publish.JavaLibrary
import com.vanniktech.maven.publish.JavadocJar
import org.gradle.api.attributes.java.TargetJvmVersion

plugins {
Expand All @@ -9,11 +8,7 @@ plugins {
}

setupPublishing {
configure(JavaLibrary(javadocJar = JavadocJar.Empty()))
}

kotlin {
jvmToolchain(17)
configure(JavaLibrary())
}

// Lint dependencies require JVM 11+, so we must request JVM 17 compatible dependencies.
Expand All @@ -29,9 +24,9 @@ dependencies {
compileOnly(libs.lint.api)
compileOnly(libs.lint.checks)

testImplementation(libs.junit)
testImplementation(libs.lint.core)
testImplementation(libs.lint.tests)
testImplementation(libs.junit)
}

tasks.jar {
Expand Down
29 changes: 16 additions & 13 deletions coil-lint/src/main/kotlin/coil3/lint/ErrorFunctionDetector.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class ErrorFunctionDetector : Detector(), SourceCodeScanner {
scope = node,
location = context.getLocation(node),
message = "Using `kotlin.error()` inside `ImageRequest.Builder`. " +
"Did you mean to use Coil's `error()` extension function to set an error drawable? " +
"Consider importing `coil3.request.error` instead.",
"Did you mean to use Coil's `error()` extension function to set an error image? " +
"If yes, import `coil3.request.error` instead.",
)
}

Expand Down Expand Up @@ -72,23 +72,25 @@ class ErrorFunctionDetector : Detector(), SourceCodeScanner {
val receiverType = lambdaParent.receiverType?.canonicalText
val methodName = lambdaParent.methodName

// Check for ImageView.load { } or AsyncImage calls
if (methodName == "load" || methodName == "AsyncImage" || methodName == "SubcomposeAsyncImage") {
// Check for ImageView.load { }
if (receiverType?.contains("ImageView") == true && methodName == "load") {
return true
}

// Check for ImageRequest.Builder { } or ImageRequest.Builder().apply { }
// Check for ImageRequest.Builder { }
if (receiverType?.contains("ImageRequest.Builder") == true) {
return true
}

// Check for ImageRequest.Builder().apply { }
if (receiverType?.contains("ImageRequest\$Builder") == true) {
return true
}

// Check if the lambda is the trailing lambda of a function that returns/uses ImageRequest.Builder
val resolvedMethod = lambdaParent.resolve()
if (resolvedMethod != null) {
val returnType = resolvedMethod.returnType?.canonicalText ?: ""
val returnType = resolvedMethod.returnType?.canonicalText.orEmpty()
if (returnType.contains("ImageRequest.Builder") || returnType.contains("ImageRequest\$Builder")) {
return true
}
Expand All @@ -113,17 +115,18 @@ class ErrorFunctionDetector : Detector(), SourceCodeScanner {
id = "CoilErrorFunction",
briefDescription = "Kotlin stdlib `error()` used inside ImageRequest.Builder",
explanation = """
Using Kotlin's stdlib `error()` function inside an `ImageRequest.Builder` lambda \
(such as in `imageView.load { }` or `AsyncImage`) will throw an `IllegalStateException` \
at runtime instead of setting an error drawable.
Using Kotlin's stdlib `error()` function inside an `ImageRequest.Builder` lambda
(such as in `ImageView.load { }`) will throw an `IllegalStateException`
at runtime instead of setting an error image.

This is likely a mistake caused by IDE auto-import selecting the wrong function. \
Use Coil's `error(drawable)` extension function instead to set an error placeholder.
This is likely a mistake caused by not importing `coil3.request.error`.
By default, the compiler will resolve the `error()`function call to
`kotlin.error()` if `coil3.request.error` is not imported.

**Wrong:**
```kotlin
imageView.load(url) {
error(R.drawable.error) // This is kotlin.error() - throws exception!
error(R.drawable.error) // This is kotlin.error() - throws an exception!
}
```

Expand All @@ -132,7 +135,7 @@ class ErrorFunctionDetector : Detector(), SourceCodeScanner {
import coil3.request.error

imageView.load(url) {
error(R.drawable.error) // This is coil3.request.error() - sets error drawable
error(R.drawable.error) // This is coil3.request.error() - sets the error image.
}
```
""".trimIndent(),
Expand Down
60 changes: 49 additions & 11 deletions coil-lint/src/test/kotlin/coil3/lint/ErrorFunctionDetectorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.android.tools.lint.checks.infrastructure.TestFile
import com.android.tools.lint.checks.infrastructure.TestMode
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
import org.junit.Test

class ErrorFunctionDetectorTest : LintDetectorTest() {

Expand Down Expand Up @@ -50,7 +49,6 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
""",
).indented()

@Test
fun testStdlibErrorInsideLoadBlockTriggersWarning() {
lint()
.allowMissingSdk()
Expand Down Expand Up @@ -80,7 +78,34 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
.expectContains("Using kotlin.error() inside ImageRequest.Builder")
}

@Test
fun testStdlibErrorInsideLoadBlockNoImportTriggersWarning() {
lint()
.allowMissingSdk()
.testModes(TestMode.JVM_OVERLOADS)
.files(
imageViewStub,
coilExtensionsStub,
kotlinErrorStub,
kotlin(
"""
package test

import android.widget.ImageView
import coil3.load

fun test(imageView: ImageView) {
imageView.load("https://example.com/image.jpg") {
error("Failed to load") // Wrong: This is kotlin.error()
}
}
""",
).indented(),
)
.run()
.expectWarningCount(1)
.expectContains("Using kotlin.error() inside ImageRequest.Builder")
}

fun testCoilErrorExtensionDoesNotTriggerWarning() {
lint()
.allowMissingSdk()
Expand Down Expand Up @@ -108,7 +133,6 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
.expectClean()
}

@Test
fun testStdlibErrorOutsideCoilContextDoesNotTriggerWarning() {
lint()
.allowMissingSdk()
Expand All @@ -118,11 +142,9 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
"""
package test

import kotlin.error

fun validateInput(input: String) {
if (input.isEmpty()) {
error("Input cannot be empty") // This is fine - not in Coil context
error("Input cannot be empty") // This is fine - not in ImageView.load
}
}
""",
Expand All @@ -132,7 +154,6 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
.expectClean()
}

@Test
fun testStdlibErrorInRegularLambdaDoesNotTriggerWarning() {
lint()
.allowMissingSdk()
Expand All @@ -142,12 +163,10 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
"""
package test

import kotlin.error

fun process(items: List<String>) {
items.forEach { item ->
if (item.isEmpty()) {
error("Empty item found") // This is fine - not in Coil context
error("Empty item found") // This is fine - not in ImageView.load
}
}
}
Expand All @@ -157,4 +176,23 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
.run()
.expectClean()
}

fun testStdlibErrorInLoadFunctionDoesNotTriggerWarning() {
lint()
.allowMissingSdk()
.files(
kotlinErrorStub,
kotlin(
"""
package test

fun load() {
error("Nothing") // This is fine - not in ImageView.load
}
""",
).indented(),
)
.run()
.expectClean()
}
}