[gradle] Add validation checks on invalid xml or item type. (#4680)

If a project has invalid value xml files (empty/broken content or
duplicated keys) then gradle will show an error.

fixes https://github.com/JetBrains/compose-multiplatform/issues/4663

.github/workflows/gradle-plugin.yml

@@ -16,7 +16,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-20.04, macos-12, windows-2022]
+        os: [ubuntu-20.04, macos-14, windows-2022]
         gradle: [7.4, 8.7]
         agp: [7.3.1, 8.3.1]
     runs-on: ${{ matrix.os }}

components/gradle.properties

@@ -8,7 +8,7 @@ android.useAndroidX=true
 
 #Versions
 kotlin.version=1.9.23
-compose.version=1.6.10-dev1596
+compose.version=1.6.10-beta02
 agp.version=8.2.2
 
 #Compose

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/resources/ComposeResourcesGeneration.kt

@@ -68,7 +68,7 @@ internal fun Project.configureComposeResourcesGeneration(
     //setup task execution during IDE import
     tasks.configureEach { importTask ->
         if (importTask.name == "prepareKotlinIdeaImport") {
-            importTask.dependsOn(tasks.withType(CodeGenerationTask::class.java))
+            importTask.dependsOn(tasks.withType(IdeaImportTask::class.java))
         }
     }
 }

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/resources/GenerateResClassTask.kt

@@ -2,18 +2,38 @@ package org.jetbrains.compose.resources
 
 import org.gradle.api.DefaultTask
 import org.gradle.api.file.DirectoryProperty
-import org.gradle.api.file.RegularFile
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
-import org.gradle.api.tasks.*
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.Optional
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.TaskAction
 import java.io.File
 
 /**
  * This task should be FAST and SAFE! Because it is being run during IDE import.
  */
-internal abstract class CodeGenerationTask : DefaultTask()
+internal abstract class IdeaImportTask : DefaultTask() {
+    @get:Input
+    val ideaIsInSync: Provider<Boolean> = project.provider {
+        System.getProperty("idea.sync.active", "false").toBoolean()
+    }
+
+    @TaskAction
+    fun run() {
+        try {
+            safeAction()
+        } catch (e: Exception) {
+            //message must contain two ':' symbols to be parsed by IDE UI!
+            logger.error("e: $name task was failed:", e)
+            if (!ideaIsInSync.get()) throw e
+        }
+    }
+
+    abstract fun safeAction()
+}
 
-internal abstract class GenerateResClassTask : CodeGenerationTask() {
+internal abstract class GenerateResClassTask : IdeaImportTask() {
     companion object {
         private const val RES_FILE_NAME = "Res"
     }
@@ -34,9 +54,7 @@ internal abstract class GenerateResClassTask : CodeGenerationTask() {
     @get:OutputDirectory
     abstract val codeDir: DirectoryProperty
 
-    @TaskAction
+    override fun safeAction() {
-    fun generate() {
-        try {
         val dir = codeDir.get().asFile
         dir.deleteRecursively()
         dir.mkdirs()
@@ -51,9 +69,5 @@ internal abstract class GenerateResClassTask : CodeGenerationTask() {
         } else {
             logger.info("Generation Res class is disabled")
         }
-        } catch (e: Exception) {
-            //message must contain two ':' symbols to be parsed by IDE UI!
-            logger.error("e: $name task was failed:", e)
-        }
     }
 }

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/resources/GenerateResourceAccessorsTask.kt

@@ -14,7 +14,7 @@ import java.io.File
 import java.nio.file.Path
 import kotlin.io.path.relativeTo
 
-internal abstract class GenerateResourceAccessorsTask : CodeGenerationTask() {
+internal abstract class GenerateResourceAccessorsTask : IdeaImportTask() {
     @get:Input
     abstract val packageName: Property<String>
 
@@ -39,9 +39,7 @@ internal abstract class GenerateResourceAccessorsTask : CodeGenerationTask() {
     @get:OutputDirectory
     abstract val codeDir: DirectoryProperty
 
-    @TaskAction
+    override fun safeAction() {
-    fun generate() {
-        try {
         val kotlinDir = codeDir.get().asFile
         val rootResDir = resDir.get()
         val sourceSet = sourceSetName.get()
@@ -81,10 +79,6 @@ internal abstract class GenerateResourceAccessorsTask : CodeGenerationTask() {
         } else {
             logger.info("Generation accessors for $rootResDir is disabled")
         }
-        } catch (e: Exception) {
-            //message must contain two ':' symbols to be parsed by IDE UI!
-            logger.error("e: $name task was failed:", e)
-        }
     }
 
     private fun File.fileToResourceItems(

gradle-plugins/compose/src/main/kotlin/org/jetbrains/compose/resources/PrepareComposeResources.kt

@@ -1,14 +1,23 @@
 package org.jetbrains.compose.resources
 
-import org.gradle.api.DefaultTask
 import org.gradle.api.Project
-import org.gradle.api.file.*
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.FileSystemOperations
+import org.gradle.api.file.FileTree
 import org.gradle.api.provider.Property
 import org.gradle.api.provider.Provider
-import org.gradle.api.tasks.*
+import org.gradle.api.tasks.IgnoreEmptyDirectories
+import org.gradle.api.tasks.Input
+import org.gradle.api.tasks.InputFiles
+import org.gradle.api.tasks.Internal
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.OutputFiles
+import org.gradle.api.tasks.SkipWhenEmpty
+import org.gradle.api.tasks.TaskProvider
 import org.jetbrains.compose.internal.utils.uppercaseFirstChar
 import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet
 import org.w3c.dom.Node
+import org.xml.sax.SAXParseException
 import java.io.File
 import java.util.*
 import javax.inject.Inject
@@ -60,7 +69,7 @@ internal fun Project.getPreparedComposeResourcesDir(sourceSet: KotlinSourceSet):
 private fun getPrepareComposeResourcesTaskName(sourceSet: KotlinSourceSet) =
     "prepareComposeResourcesTaskFor${sourceSet.name.uppercaseFirstChar()}"
 
-internal abstract class CopyNonXmlValueResourcesTask : DefaultTask() {
+internal abstract class CopyNonXmlValueResourcesTask : IdeaImportTask() {
     @get:Inject
     abstract val fileSystem: FileSystemOperations
 
@@ -82,8 +91,7 @@ internal abstract class CopyNonXmlValueResourcesTask : DefaultTask() {
         dir.asFileTree.matching { it.exclude("values*/*.${XmlValuesConverterTask.CONVERTED_RESOURCE_EXT}") }
     }
 
-    @TaskAction
+    override fun safeAction() {
-    fun run() {
         realOutputFiles.get().forEach { f -> f.delete() }
         fileSystem.copy { copy ->
             copy.includeEmptyDirs = false
@@ -95,7 +103,7 @@ internal abstract class CopyNonXmlValueResourcesTask : DefaultTask() {
     }
 }
 
-internal abstract class PrepareComposeResourcesTask : DefaultTask() {
+internal abstract class PrepareComposeResourcesTask : IdeaImportTask() {
     @get:InputFiles
     @get:SkipWhenEmpty
     @get:IgnoreEmptyDirectories
@@ -109,8 +117,7 @@ internal abstract class PrepareComposeResourcesTask : DefaultTask() {
     @get:OutputDirectory
     abstract val outputDir: DirectoryProperty
 
-    @TaskAction
+    override fun safeAction() = Unit
-    fun run() = Unit
 }
 
 internal data class ValueResourceRecord(
@@ -135,7 +142,7 @@ internal data class ValueResourceRecord(
     }
 }
 
-internal abstract class XmlValuesConverterTask : DefaultTask() {
+internal abstract class XmlValuesConverterTask : IdeaImportTask() {
     companion object {
         const val CONVERTED_RESOURCE_EXT = "cvr" //Compose Value Resource
         private const val FORMAT_VERSION = 0
@@ -163,8 +170,7 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
         dir.asFileTree.matching { it.include("values*/*.$suffix.$CONVERTED_RESOURCE_EXT") }
     }
 
-    @TaskAction
+    override fun safeAction() {
-    fun run() {
         val outDir = outputDir.get().asFile
         val suffix = fileSuffix.get()
         realOutputFiles.get().forEach { f -> f.delete() }
@@ -176,7 +182,13 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
                             .resolve(f.parentFile.name)
                             .resolve(f.nameWithoutExtension + ".$suffix.$CONVERTED_RESOURCE_EXT")
                         output.parentFile.mkdirs()
+                        try {
                             convert(f, output)
+                        } catch (e: SAXParseException) {
+                            error("XML file ${f.absolutePath} is not valid. Check the file content.")
+                        } catch (e: Exception) {
+                            error("XML file ${f.absolutePath} is not valid. ${e.message}")
+                        }
                     }
                 }
             }
@@ -186,17 +198,28 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
     private fun convert(original: File, converted: File) {
         val doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(original)
         val items = doc.getElementsByTagName("resources").item(0).childNodes
-        val records = List(items.length) { items.item(it) }.mapNotNull { getItemRecord(it)?.getAsString() }
+        val records = List(items.length) { items.item(it) }
+            .filter { it.hasAttributes() }
+            .map { getItemRecord(it) }
+
+        //check there are no duplicates type + key
+        records.groupBy { it.key }
+            .filter { it.value.size > 1 }
+            .forEach { (key, records) ->
+                val allTypes = records.map { it.type }
+                require(allTypes.size == allTypes.toSet().size) { "Duplicated key '$key'." }
+            }
+
         val fileContent = buildString {
             appendLine("version:$FORMAT_VERSION")
-            records.sorted().forEach { appendLine(it) }
+            records.map { it.getAsString() }.sorted().forEach { appendLine(it) }
         }
         converted.writeText(fileContent)
     }
 
-    private fun getItemRecord(node: Node): ValueResourceRecord? {
+    private fun getItemRecord(node: Node): ValueResourceRecord {
-        val type = ResourceType.fromString(node.nodeName) ?: return null
+        val type = ResourceType.fromString(node.nodeName) ?: error("Unknown resource type: '${node.nodeName}'.")
-        val key = node.attributes.getNamedItem("name").nodeValue
+        val key = node.attributes.getNamedItem("name")?.nodeValue ?: error("Attribute 'name' not found.")
         val value: String
         when (type) {
             ResourceType.STRING -> {
@@ -225,7 +248,7 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
                     }
             }
 
-            else -> error("Unknown string resource type: '$type'")
+            else -> error("Unknown string resource type: '$type'.")
         }
         return ValueResourceRecord(type, key, value)
     }

gradle-plugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/ResourcesTest.kt

@@ -12,6 +12,24 @@ import kotlin.io.path.relativeTo
 import kotlin.test.*
 
 class ResourcesTest : GradlePluginTestBase() {
+    @Test
+    fun testSafeImport() {
+        with(testProject("misc/commonResources")) {
+            file("src/commonMain/composeResources/drawable-en").renameTo(
+                file("src/commonMain/composeResources/drawable-rent")
+            )
+            gradleFailure("prepareKotlinIdeaImport").checks {
+                check.logContains("e: generateResourceAccessorsForCommonMain task was failed:")
+                check.logContains("contains unknown qualifier: 'rent'.")
+            }
+
+            gradle("prepareKotlinIdeaImport", "-Didea.sync.active=true").checks {
+                check.logContains("e: generateResourceAccessorsForCommonMain task was failed:")
+                check.logContains("contains unknown qualifier: 'rent'.")
+            }
+        }
+    }
+
     @Test
     fun testGeneratedAccessors(): Unit = with(testProject("misc/commonResources")) {
         //check generated resource's accessors
@@ -36,7 +54,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/drawable-en").renameTo(
             file("src/commonMain/composeResources/drawable-rent")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 contains unknown qualifier: 'rent'.
@@ -47,7 +65,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/drawable-rent").renameTo(
             file("src/commonMain/composeResources/drawable-rUS-en")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 Region qualifier must be declared after language: 'en-rUS'.
@@ -58,7 +76,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/drawable-rUS-en").renameTo(
             file("src/commonMain/composeResources/drawable-rUS")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 Region qualifier must be used only with language.
@@ -69,7 +87,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/drawable-rUS").renameTo(
             file("src/commonMain/composeResources/drawable-en-fr")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 contains repetitive qualifiers: 'en' and 'fr'.
@@ -80,7 +98,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/drawable-en-fr").renameTo(
             file("src/commonMain/composeResources/image")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 Unknown resource type: 'image'
@@ -91,7 +109,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/image").renameTo(
             file("src/commonMain/composeResources/files-de")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 The 'files' directory doesn't support qualifiers: 'files-de'.
@@ -102,7 +120,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/files-de").renameTo(
             file("src/commonMain/composeResources/strings")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 Unknown resource type: 'strings'.
@@ -113,7 +131,7 @@ class ResourcesTest : GradlePluginTestBase() {
         file("src/commonMain/composeResources/strings").renameTo(
             file("src/commonMain/composeResources/string-us")
         )
-        gradle("prepareKotlinIdeaImport").checks {
+        gradleFailure("prepareKotlinIdeaImport").checks {
             check.logContains(
                 """
                 Forbidden directory name 'string-us'! String resources should be declared in 'values/strings.xml'.
@@ -129,6 +147,58 @@ class ResourcesTest : GradlePluginTestBase() {
             file("src/commonMain/composeResources/drawable/vector_2.xml")
         )
 
+        val testXml = file("src/commonMain/composeResources/values/test.xml")
+        testXml.writeText("")
+        gradleFailure("prepareKotlinIdeaImport").checks {
+            check.logContains("${testXml.name} is not valid. Check the file content.")
+        }
+
+        testXml.writeText("invalid")
+        gradleFailure("prepareKotlinIdeaImport").checks {
+            check.logContains("${testXml.name} is not valid. Check the file content.")
+        }
+
+        testXml.writeText("""
+            <resources>
+                <aaa name="v">aaa</aaa>
+            </resources>
+        """.trimIndent())
+        gradleFailure("prepareKotlinIdeaImport").checks {
+            check.logContains("${testXml.name} is not valid. Unknown resource type: 'aaa'.")
+        }
+
+        testXml.writeText("""
+            <resources>
+                <drawable name="v">aaa</drawable>
+            </resources>
+        """.trimIndent())
+        gradleFailure("prepareKotlinIdeaImport").checks {
+            check.logContains("${testXml.name} is not valid. Unknown string resource type: 'drawable'.")
+        }
+
+        testXml.writeText("""
+            <resources>
+                <string name="v1">aaa</string>
+                <string name="v2">aaa</string>
+                <string name="v3">aaa</string>
+                <string name="v1">aaa</string>
+            </resources>
+        """.trimIndent())
+        gradleFailure("prepareKotlinIdeaImport").checks {
+            check.logContains("${testXml.name} is not valid. Duplicated key 'v1'.")
+        }
+
+        testXml.writeText("""
+            <resources>
+                <string name="v1">aaa</string>
+                <string foo="v2">aaa</string>
+            </resources>
+        """.trimIndent())
+        gradleFailure("prepareKotlinIdeaImport").checks {
+            check.logContains("${testXml.name} is not valid. Attribute 'name' not found.")
+        }
+        testXml.delete()
+
         file("build.gradle.kts").modify { txt ->
             txt + """
                 compose.resources {