Skip to content

Commit 644c7c3

Browse files
terrakokterrakok
authored
[gradle] Add validation checks on invalid xml or item type. (JetBrains#4680)
If a project has invalid value xml files (empty/broken content or duplicated keys) then gradle will show an error. fixes JetBrains#4663
1 parent 1bc3353 commit 644c7c3

File tree

7 files changed

+195
-94
lines changed

7 files changed

+195
-94
lines changed

‎.github/workflows/gradle-plugin.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
strategy:
1717
fail-fast: false
1818
matrix:
19-
os: [ubuntu-20.04, macos-12, windows-2022]
19+
os: [ubuntu-20.04, macos-14, windows-2022]
2020
gradle: [7.4, 8.7]
2121
agp: [7.3.1, 8.3.1]
2222
runs-on: ${{ matrix.os }}

‎components/gradle.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ android.useAndroidX=true
88

99
#Versions
1010
kotlin.version=1.9.23
11-
compose.version=1.6.10-dev1596
11+
compose.version=1.6.10-beta02
1212
agp.version=8.2.2
1313

1414
#Compose

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ internal fun Project.configureComposeResourcesGeneration(
6868
//setup task execution during IDE import
6969
tasks.configureEach { importTask ->
7070
if (importTask.name == "prepareKotlinIdeaImport") {
71-
importTask.dependsOn(tasks.withType(CodeGenerationTask::class.java))
71+
importTask.dependsOn(tasks.withType(IdeaImportTask::class.java))
7272
}
7373
}
7474
}

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

+38-24
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,38 @@ package org.jetbrains.compose.resources
22

33
import org.gradle.api.DefaultTask
44
import org.gradle.api.file.DirectoryProperty
5-
import org.gradle.api.file.RegularFile
65
import org.gradle.api.provider.Property
76
import org.gradle.api.provider.Provider
8-
import org.gradle.api.tasks.*
7+
import org.gradle.api.tasks.Input
8+
import org.gradle.api.tasks.Optional
9+
import org.gradle.api.tasks.OutputDirectory
10+
import org.gradle.api.tasks.TaskAction
911
import java.io.File
1012

1113
/**
1214
* This task should be FAST and SAFE! Because it is being run during IDE import.
1315
*/
14-
internal abstract class CodeGenerationTask : DefaultTask()
16+
internal abstract class IdeaImportTask : DefaultTask() {
17+
@get:Input
18+
val ideaIsInSync: Provider<Boolean> = project.provider {
19+
System.getProperty("idea.sync.active", "false").toBoolean()
20+
}
21+
22+
@TaskAction
23+
fun run() {
24+
try {
25+
safeAction()
26+
} catch (e: Exception) {
27+
//message must contain two ':' symbols to be parsed by IDE UI!
28+
logger.error("e: $name task was failed:", e)
29+
if (!ideaIsInSync.get()) throw e
30+
}
31+
}
1532

16-
internal abstract class GenerateResClassTask : CodeGenerationTask() {
33+
abstract fun safeAction()
34+
}
35+
36+
internal abstract class GenerateResClassTask : IdeaImportTask() {
1737
companion object {
1838
private const val RES_FILE_NAME = "Res"
1939
}
@@ -34,26 +54,20 @@ internal abstract class GenerateResClassTask : CodeGenerationTask() {
3454
@get:OutputDirectory
3555
abstract val codeDir: DirectoryProperty
3656

37-
@TaskAction
38-
fun generate() {
39-
try {
40-
val dir = codeDir.get().asFile
41-
dir.deleteRecursively()
42-
dir.mkdirs()
43-
44-
if (shouldGenerateCode.get()) {
45-
logger.info("Generate $RES_FILE_NAME.kt")
46-
47-
val pkgName = packageName.get()
48-
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
49-
val isPublic = makeAccessorsPublic.get()
50-
getResFileSpec(pkgName, RES_FILE_NAME, moduleDirectory, isPublic).writeTo(dir)
51-
} else {
52-
logger.info("Generation Res class is disabled")
53-
}
54-
} catch (e: Exception) {
55-
//message must contain two ':' symbols to be parsed by IDE UI!
56-
logger.error("e: $name task was failed:", e)
57+
override fun safeAction() {
58+
val dir = codeDir.get().asFile
59+
dir.deleteRecursively()
60+
dir.mkdirs()
61+
62+
if (shouldGenerateCode.get()) {
63+
logger.info("Generate $RES_FILE_NAME.kt")
64+
65+
val pkgName = packageName.get()
66+
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
67+
val isPublic = makeAccessorsPublic.get()
68+
getResFileSpec(pkgName, RES_FILE_NAME, moduleDirectory, isPublic).writeTo(dir)
69+
} else {
70+
logger.info("Generation Res class is disabled")
5771
}
5872
}
5973
}

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

+34-40
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import java.io.File
1414
import java.nio.file.Path
1515
import kotlin.io.path.relativeTo
1616

17-
internal abstract class GenerateResourceAccessorsTask : CodeGenerationTask() {
17+
internal abstract class GenerateResourceAccessorsTask : IdeaImportTask() {
1818
@get:Input
1919
abstract val packageName: Property<String>
2020

@@ -39,51 +39,45 @@ internal abstract class GenerateResourceAccessorsTask : CodeGenerationTask() {
3939
@get:OutputDirectory
4040
abstract val codeDir: DirectoryProperty
4141

42-
@TaskAction
43-
fun generate() {
44-
try {
45-
val kotlinDir = codeDir.get().asFile
46-
val rootResDir = resDir.get()
47-
val sourceSet = sourceSetName.get()
42+
override fun safeAction() {
43+
val kotlinDir = codeDir.get().asFile
44+
val rootResDir = resDir.get()
45+
val sourceSet = sourceSetName.get()
4846

49-
logger.info("Clean directory $kotlinDir")
50-
kotlinDir.deleteRecursively()
51-
kotlinDir.mkdirs()
47+
logger.info("Clean directory $kotlinDir")
48+
kotlinDir.deleteRecursively()
49+
kotlinDir.mkdirs()
5250

53-
if (shouldGenerateCode.get()) {
54-
logger.info("Generate accessors for $rootResDir")
51+
if (shouldGenerateCode.get()) {
52+
logger.info("Generate accessors for $rootResDir")
5553

56-
//get first level dirs
57-
val dirs = rootResDir.listNotHiddenFiles()
54+
//get first level dirs
55+
val dirs = rootResDir.listNotHiddenFiles()
5856

59-
dirs.forEach { f ->
60-
if (!f.isDirectory) {
61-
error("${f.name} is not directory! Raw files should be placed in '${rootResDir.name}/files' directory.")
62-
}
57+
dirs.forEach { f ->
58+
if (!f.isDirectory) {
59+
error("${f.name} is not directory! Raw files should be placed in '${rootResDir.name}/files' directory.")
6360
}
64-
65-
//type -> id -> resource item
66-
val resources: Map<ResourceType, Map<String, List<ResourceItem>>> = dirs
67-
.flatMap { dir ->
68-
dir.listNotHiddenFiles()
69-
.mapNotNull { it.fileToResourceItems(rootResDir.toPath()) }
70-
.flatten()
71-
}
72-
.groupBy { it.type }
73-
.mapValues { (_, items) -> items.groupBy { it.name } }
74-
75-
val pkgName = packageName.get()
76-
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
77-
val isPublic = makeAccessorsPublic.get()
78-
getAccessorsSpecs(
79-
resources, pkgName, sourceSet, moduleDirectory, isPublic
80-
).forEach { it.writeTo(kotlinDir) }
81-
} else {
82-
logger.info("Generation accessors for $rootResDir is disabled")
8361
}
84-
} catch (e: Exception) {
85-
//message must contain two ':' symbols to be parsed by IDE UI!
86-
logger.error("e: $name task was failed:", e)
62+
63+
//type -> id -> resource item
64+
val resources: Map<ResourceType, Map<String, List<ResourceItem>>> = dirs
65+
.flatMap { dir ->
66+
dir.listNotHiddenFiles()
67+
.mapNotNull { it.fileToResourceItems(rootResDir.toPath()) }
68+
.flatten()
69+
}
70+
.groupBy { it.type }
71+
.mapValues { (_, items) -> items.groupBy { it.name } }
72+
73+
val pkgName = packageName.get()
74+
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
75+
val isPublic = makeAccessorsPublic.get()
76+
getAccessorsSpecs(
77+
resources, pkgName, sourceSet, moduleDirectory, isPublic
78+
).forEach { it.writeTo(kotlinDir) }
79+
} else {
80+
logger.info("Generation accessors for $rootResDir is disabled")
8781
}
8882
}
8983

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

+42-19
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,23 @@
11
package org.jetbrains.compose.resources
22

3-
import org.gradle.api.DefaultTask
43
import org.gradle.api.Project
5-
import org.gradle.api.file.*
4+
import org.gradle.api.file.DirectoryProperty
5+
import org.gradle.api.file.FileSystemOperations
6+
import org.gradle.api.file.FileTree
67
import org.gradle.api.provider.Property
78
import org.gradle.api.provider.Provider
8-
import org.gradle.api.tasks.*
9+
import org.gradle.api.tasks.IgnoreEmptyDirectories
10+
import org.gradle.api.tasks.Input
11+
import org.gradle.api.tasks.InputFiles
12+
import org.gradle.api.tasks.Internal
13+
import org.gradle.api.tasks.OutputDirectory
14+
import org.gradle.api.tasks.OutputFiles
15+
import org.gradle.api.tasks.SkipWhenEmpty
16+
import org.gradle.api.tasks.TaskProvider
917
import org.jetbrains.compose.internal.utils.uppercaseFirstChar
1018
import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet
1119
import org.w3c.dom.Node
20+
import org.xml.sax.SAXParseException
1221
import java.io.File
1322
import java.util.*
1423
import javax.inject.Inject
@@ -60,7 +69,7 @@ internal fun Project.getPreparedComposeResourcesDir(sourceSet: KotlinSourceSet):
6069
private fun getPrepareComposeResourcesTaskName(sourceSet: KotlinSourceSet) =
6170
"prepareComposeResourcesTaskFor${sourceSet.name.uppercaseFirstChar()}"
6271

63-
internal abstract class CopyNonXmlValueResourcesTask : DefaultTask() {
72+
internal abstract class CopyNonXmlValueResourcesTask : IdeaImportTask() {
6473
@get:Inject
6574
abstract val fileSystem: FileSystemOperations
6675

@@ -82,8 +91,7 @@ internal abstract class CopyNonXmlValueResourcesTask : DefaultTask() {
8291
dir.asFileTree.matching { it.exclude("values*/*.${XmlValuesConverterTask.CONVERTED_RESOURCE_EXT}") }
8392
}
8493

85-
@TaskAction
86-
fun run() {
94+
override fun safeAction() {
8795
realOutputFiles.get().forEach { f -> f.delete() }
8896
fileSystem.copy { copy ->
8997
copy.includeEmptyDirs = false
@@ -95,7 +103,7 @@ internal abstract class CopyNonXmlValueResourcesTask : DefaultTask() {
95103
}
96104
}
97105

98-
internal abstract class PrepareComposeResourcesTask : DefaultTask() {
106+
internal abstract class PrepareComposeResourcesTask : IdeaImportTask() {
99107
@get:InputFiles
100108
@get:SkipWhenEmpty
101109
@get:IgnoreEmptyDirectories
@@ -109,8 +117,7 @@ internal abstract class PrepareComposeResourcesTask : DefaultTask() {
109117
@get:OutputDirectory
110118
abstract val outputDir: DirectoryProperty
111119

112-
@TaskAction
113-
fun run() = Unit
120+
override fun safeAction() = Unit
114121
}
115122

116123
internal data class ValueResourceRecord(
@@ -135,7 +142,7 @@ internal data class ValueResourceRecord(
135142
}
136143
}
137144

138-
internal abstract class XmlValuesConverterTask : DefaultTask() {
145+
internal abstract class XmlValuesConverterTask : IdeaImportTask() {
139146
companion object {
140147
const val CONVERTED_RESOURCE_EXT = "cvr" //Compose Value Resource
141148
private const val FORMAT_VERSION = 0
@@ -163,8 +170,7 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
163170
dir.asFileTree.matching { it.include("values*/*.$suffix.$CONVERTED_RESOURCE_EXT") }
164171
}
165172

166-
@TaskAction
167-
fun run() {
173+
override fun safeAction() {
168174
val outDir = outputDir.get().asFile
169175
val suffix = fileSuffix.get()
170176
realOutputFiles.get().forEach { f -> f.delete() }
@@ -176,7 +182,13 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
176182
.resolve(f.parentFile.name)
177183
.resolve(f.nameWithoutExtension + ".$suffix.$CONVERTED_RESOURCE_EXT")
178184
output.parentFile.mkdirs()
179-
convert(f, output)
185+
try {
186+
convert(f, output)
187+
} catch (e: SAXParseException) {
188+
error("XML file ${f.absolutePath} is not valid. Check the file content.")
189+
} catch (e: Exception) {
190+
error("XML file ${f.absolutePath} is not valid. ${e.message}")
191+
}
180192
}
181193
}
182194
}
@@ -186,17 +198,28 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
186198
private fun convert(original: File, converted: File) {
187199
val doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(original)
188200
val items = doc.getElementsByTagName("resources").item(0).childNodes
189-
val records = List(items.length) { items.item(it) }.mapNotNull { getItemRecord(it)?.getAsString() }
201+
val records = List(items.length) { items.item(it) }
202+
.filter { it.hasAttributes() }
203+
.map { getItemRecord(it) }
204+
205+
//check there are no duplicates type + key
206+
records.groupBy { it.key }
207+
.filter { it.value.size > 1 }
208+
.forEach { (key, records) ->
209+
val allTypes = records.map { it.type }
210+
require(allTypes.size == allTypes.toSet().size) { "Duplicated key '$key'." }
211+
}
212+
190213
val fileContent = buildString {
191214
appendLine("version:$FORMAT_VERSION")
192-
records.sorted().forEach { appendLine(it) }
215+
records.map { it.getAsString() }.sorted().forEach { appendLine(it) }
193216
}
194217
converted.writeText(fileContent)
195218
}
196219

197-
private fun getItemRecord(node: Node): ValueResourceRecord? {
198-
val type = ResourceType.fromString(node.nodeName) ?: return null
199-
val key = node.attributes.getNamedItem("name").nodeValue
220+
private fun getItemRecord(node: Node): ValueResourceRecord {
221+
val type = ResourceType.fromString(node.nodeName) ?: error("Unknown resource type: '${node.nodeName}'.")
222+
val key = node.attributes.getNamedItem("name")?.nodeValue ?: error("Attribute 'name' not found.")
200223
val value: String
201224
when (type) {
202225
ResourceType.STRING -> {
@@ -225,7 +248,7 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
225248
}
226249
}
227250

228-
else -> error("Unknown string resource type: '$type'")
251+
else -> error("Unknown string resource type: '$type'.")
229252
}
230253
return ValueResourceRecord(type, key, value)
231254
}

0 commit comments

Comments
 (0)