Publish coil-lint with coil-core. Improve coil-lint test coverage. (#3323)

Author: colinrtwhite

coil-core/build.gradle.kts

@@ -89,3 +89,9 @@ baselineProfile {
         }
     }
 }
+
+dependencies {
+    lintPublish(projects.coilLint) {
+        isTransitive = false
+    }
+}

coil-lint/build.gradle.kts

@@ -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 {
@@ -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.
@@ -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 {

coil-lint/src/main/kotlin/coil3/lint/ErrorFunctionDetector.kt

@@ -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.",
         )
     }
 
@@ -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
                         }
@@ -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!
                 }
                 ```
 
@@ -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(),

coil-lint/src/test/kotlin/coil3/lint/ErrorFunctionDetectorTest.kt

@@ -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() {
 
@@ -50,7 +49,6 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
         """,
     ).indented()
 
-    @Test
     fun testStdlibErrorInsideLoadBlockTriggersWarning() {
         lint()
             .allowMissingSdk()
@@ -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()
@@ -108,7 +133,6 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
             .expectClean()
     }
 
-    @Test
     fun testStdlibErrorOutsideCoilContextDoesNotTriggerWarning() {
         lint()
             .allowMissingSdk()
@@ -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
                         }
                     }
                     """,
@@ -132,7 +154,6 @@ class ErrorFunctionDetectorTest : LintDetectorTest() {
             .expectClean()
     }
 
-    @Test
     fun testStdlibErrorInRegularLambdaDoesNotTriggerWarning() {
         lint()
             .allowMissingSdk()
@@ -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
                             }
                         }
                     }
@@ -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()
+    }
 }