Skip to content

Commit

Permalink
[gradle] Add validation checks on invalid xml or item type. (#4680)
Browse files Browse the repository at this point in the history 
If a project has invalid value xml files (empty/broken content or
duplicated keys) then gradle will show an error.

fixes #4663
  • Loading branch information
@terrakok
terrakok committed Apr 24, 2024
1 parent 1bc3353 commit 644c7c3
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 94 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/gradle-plugin.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion components/gradle.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ...ins/compose/src/main/kotlin/org/jetbrains/compose/resources/ComposeResourcesGeneration.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}
Expand Down
62 changes: 38 additions & 24 deletions ...e-plugins/compose/src/main/kotlin/org/jetbrains/compose/resources/GenerateResClassTask.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

internal abstract class GenerateResClassTask : CodeGenerationTask() {
abstract fun safeAction()
}

internal abstract class GenerateResClassTask : IdeaImportTask() {
companion object {
private const val RES_FILE_NAME = "Res"
}
Expand All @@ -34,26 +54,20 @@ internal abstract class GenerateResClassTask : CodeGenerationTask() {
@get:OutputDirectory
abstract val codeDir: DirectoryProperty

@TaskAction
fun generate() {
try {
val dir = codeDir.get().asFile
dir.deleteRecursively()
dir.mkdirs()

if (shouldGenerateCode.get()) {
logger.info("Generate $RES_FILE_NAME.kt")

val pkgName = packageName.get()
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
val isPublic = makeAccessorsPublic.get()
getResFileSpec(pkgName, RES_FILE_NAME, moduleDirectory, isPublic).writeTo(dir)
} 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)
override fun safeAction() {
val dir = codeDir.get().asFile
dir.deleteRecursively()
dir.mkdirs()

if (shouldGenerateCode.get()) {
logger.info("Generate $RES_FILE_NAME.kt")

val pkgName = packageName.get()
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
val isPublic = makeAccessorsPublic.get()
getResFileSpec(pkgName, RES_FILE_NAME, moduleDirectory, isPublic).writeTo(dir)
} else {
logger.info("Generation Res class is disabled")
}
}
}
74 changes: 34 additions & 40 deletions .../compose/src/main/kotlin/org/jetbrains/compose/resources/GenerateResourceAccessorsTask.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>

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

@TaskAction
fun generate() {
try {
val kotlinDir = codeDir.get().asFile
val rootResDir = resDir.get()
val sourceSet = sourceSetName.get()
override fun safeAction() {
val kotlinDir = codeDir.get().asFile
val rootResDir = resDir.get()
val sourceSet = sourceSetName.get()

logger.info("Clean directory $kotlinDir")
kotlinDir.deleteRecursively()
kotlinDir.mkdirs()
logger.info("Clean directory $kotlinDir")
kotlinDir.deleteRecursively()
kotlinDir.mkdirs()

if (shouldGenerateCode.get()) {
logger.info("Generate accessors for $rootResDir")
if (shouldGenerateCode.get()) {
logger.info("Generate accessors for $rootResDir")

//get first level dirs
val dirs = rootResDir.listNotHiddenFiles()
//get first level dirs
val dirs = rootResDir.listNotHiddenFiles()

dirs.forEach { f ->
if (!f.isDirectory) {
error("${f.name} is not directory! Raw files should be placed in '${rootResDir.name}/files' directory.")
}
dirs.forEach { f ->
if (!f.isDirectory) {
error("${f.name} is not directory! Raw files should be placed in '${rootResDir.name}/files' directory.")
}

//type -> id -> resource item
val resources: Map<ResourceType, Map<String, List<ResourceItem>>> = dirs
.flatMap { dir ->
dir.listNotHiddenFiles()
.mapNotNull { it.fileToResourceItems(rootResDir.toPath()) }
.flatten()
}
.groupBy { it.type }
.mapValues { (_, items) -> items.groupBy { it.name } }

val pkgName = packageName.get()
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
val isPublic = makeAccessorsPublic.get()
getAccessorsSpecs(
resources, pkgName, sourceSet, moduleDirectory, isPublic
).forEach { it.writeTo(kotlinDir) }
} 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)

//type -> id -> resource item
val resources: Map<ResourceType, Map<String, List<ResourceItem>>> = dirs
.flatMap { dir ->
dir.listNotHiddenFiles()
.mapNotNull { it.fileToResourceItems(rootResDir.toPath()) }
.flatten()
}
.groupBy { it.type }
.mapValues { (_, items) -> items.groupBy { it.name } }

val pkgName = packageName.get()
val moduleDirectory = packagingDir.getOrNull()?.let { it.invariantSeparatorsPath + "/" } ?: ""
val isPublic = makeAccessorsPublic.get()
getAccessorsSpecs(
resources, pkgName, sourceSet, moduleDirectory, isPublic
).forEach { it.writeTo(kotlinDir) }
} else {
logger.info("Generation accessors for $rootResDir is disabled")
}
}

Expand Down
61 changes: 42 additions & 19 deletions ...lugins/compose/src/main/kotlin/org/jetbrains/compose/resources/PrepareComposeResources.kt
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

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

@TaskAction
fun run() {
override fun safeAction() {
realOutputFiles.get().forEach { f -> f.delete() }
fileSystem.copy { copy ->
copy.includeEmptyDirs = false
Expand All @@ -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
Expand All @@ -109,8 +117,7 @@ internal abstract class PrepareComposeResourcesTask : DefaultTask() {
@get:OutputDirectory
abstract val outputDir: DirectoryProperty

@TaskAction
fun run() = Unit
override fun safeAction() = Unit
}

internal data class ValueResourceRecord(
Expand All @@ -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
Expand Down Expand Up @@ -163,8 +170,7 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
dir.asFileTree.matching { it.include("values*/*.$suffix.$CONVERTED_RESOURCE_EXT") }
}

@TaskAction
fun run() {
override fun safeAction() {
val outDir = outputDir.get().asFile
val suffix = fileSuffix.get()
realOutputFiles.get().forEach { f -> f.delete() }
Expand All @@ -176,7 +182,13 @@ internal abstract class XmlValuesConverterTask : DefaultTask() {
.resolve(f.parentFile.name)
.resolve(f.nameWithoutExtension + ".$suffix.$CONVERTED_RESOURCE_EXT")
output.parentFile.mkdirs()
convert(f, output)
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}")
}
}
}
}
Expand All @@ -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? {
val type = ResourceType.fromString(node.nodeName) ?: return null
val key = node.attributes.getNamedItem("name").nodeValue
private fun getItemRecord(node: Node): ValueResourceRecord {
val type = ResourceType.fromString(node.nodeName) ?: error("Unknown resource type: '${node.nodeName}'.")
val key = node.attributes.getNamedItem("name")?.nodeValue ?: error("Attribute 'name' not found.")
val value: String
when (type) {
ResourceType.STRING -> {
Expand Down Expand Up @@ -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)
}
Expand Down
Loading

0 comments on commit 644c7c3

Please sign in to comment.