diff --git a/src/main/kotlin/no/item/xp/plugin/DuplicateFieldNameException.kt b/src/main/kotlin/no/item/xp/plugin/DuplicateFieldNameException.kt new file mode 100644 index 0000000..658e773 --- /dev/null +++ b/src/main/kotlin/no/item/xp/plugin/DuplicateFieldNameException.kt @@ -0,0 +1,25 @@ +package no.item.xp.plugin + +import org.gradle.api.GradleException + +class DuplicateFieldNameException( + val fieldNames: List, + val source: String? = null, +) : GradleException(createMessage(fieldNames, source)) { + /** + * Adds the file the duplicates was found in, unless it has already been added by a more specific caller + */ + fun withSource(source: String): DuplicateFieldNameException = + if (this.source == null) DuplicateFieldNameException(fieldNames, source) else this +} + +private fun createMessage( + fieldNames: List, + source: String?, +): String { + val label = if (fieldNames.size == 1) "Duplicate field name" else "Duplicate field names" + val names = fieldNames.joinToString(", ") { "\"$it\"" } + val location = source?.let { " in \"$it\"" } ?: "" + + return "$label $names$location. A field name can only be used once in the same object." +} diff --git a/src/main/kotlin/no/item/xp/plugin/GenerateCodeTask.kt b/src/main/kotlin/no/item/xp/plugin/GenerateCodeTask.kt index 1c922b8..a49f244 100644 --- a/src/main/kotlin/no/item/xp/plugin/GenerateCodeTask.kt +++ b/src/main/kotlin/no/item/xp/plugin/GenerateCodeTask.kt @@ -122,7 +122,11 @@ open class GenerateCodeTask ObjectTypeModel(Paths.get(fileInJar.entry.name).fileName.nameWithoutExtension, emptyList()).right() }, { - parseObjectTypeModel(it, Paths.get(fileInJar.entry.name).fileName.nameWithoutExtension, mixins) + try { + parseObjectTypeModel(it, Paths.get(fileInJar.entry.name).fileName.nameWithoutExtension, mixins) + } catch (e: DuplicateFieldNameException) { + throw e.withSource(fileInJar.entry.name) + } }, ).fold( { left -> diff --git a/src/main/kotlin/no/item/xp/plugin/GenerateTypeScriptWorkAction.kt b/src/main/kotlin/no/item/xp/plugin/GenerateTypeScriptWorkAction.kt index d309515..e1666ca 100644 --- a/src/main/kotlin/no/item/xp/plugin/GenerateTypeScriptWorkAction.kt +++ b/src/main/kotlin/no/item/xp/plugin/GenerateTypeScriptWorkAction.kt @@ -71,6 +71,8 @@ abstract class GenerateTypeScriptWorkAction : WorkAction logger.lifecycle("Updated file: ${Path.of(targetFile.absoluteFile.toURI()).toUri()}") }, ) + } catch (e: DuplicateFieldNameException) { + throw e.withSource(simpleFilePath(parameters.getXmlFile().get().asFile)) } catch (e: Exception) { logger.error("Can't parse file", e) } diff --git a/src/main/kotlin/no/item/xp/plugin/parser/ParseInterfaceModel.kt b/src/main/kotlin/no/item/xp/plugin/parser/ParseInterfaceModel.kt index 1cb191f..1dd91a7 100644 --- a/src/main/kotlin/no/item/xp/plugin/parser/ParseInterfaceModel.kt +++ b/src/main/kotlin/no/item/xp/plugin/parser/ParseInterfaceModel.kt @@ -1,6 +1,7 @@ package no.item.xp.plugin.parser import arrow.core.Either +import no.item.xp.plugin.DuplicateFieldNameException import no.item.xp.plugin.extensions.getChildNodesAtXPath import no.item.xp.plugin.extensions.getChildNodesAtXPathAsEither import no.item.xp.plugin.extensions.getNodeAttribute @@ -37,17 +38,33 @@ fun parseInputTypeList( nodes: Collection, mixins: List, ): List { - return nodes - .flatMap { node -> - when (node.nodeName) { - "input" -> listOfNotNull(parseInput(node)) - "option-set" -> listOfNotNull(parseOptionSet(node, mixins)) - "item-set" -> listOfNotNull(parseItemSet(node, mixins)) - "field-set" -> parseFieldSet(node, mixins) - "mixin" -> findMixinFields(mixins, node) - else -> emptyList() + val fields = + nodes + .flatMap { node -> + when (node.nodeName) { + "input" -> listOfNotNull(parseInput(node)) + "option-set" -> listOfNotNull(parseOptionSet(node, mixins)) + "item-set" -> listOfNotNull(parseItemSet(node, mixins)) + "field-set" -> parseFieldSet(node, mixins) + "mixin" -> findMixinFields(mixins, node) + else -> emptyList() + } } - } + + // Fields from mixins and field-sets are added to the same object, so their names can collide + val duplicateFieldNames = + fields + .groupingBy { it.name } + .eachCount() + .filterValues { it > 1 } + .keys + .toList() + + if (duplicateFieldNames.isNotEmpty()) { + throw DuplicateFieldNameException(duplicateFieldNames) + } + + return fields } private fun findMixinFields( diff --git a/src/main/kotlin/no/item/xp/plugin/parser/ParseMixin.kt b/src/main/kotlin/no/item/xp/plugin/parser/ParseMixin.kt index eb2237a..dbd13b9 100644 --- a/src/main/kotlin/no/item/xp/plugin/parser/ParseMixin.kt +++ b/src/main/kotlin/no/item/xp/plugin/parser/ParseMixin.kt @@ -3,6 +3,7 @@ package no.item.xp.plugin.parser import arrow.core.Either import arrow.core.flatMap import no.item.xp.plugin.CyclicDependenciesException +import no.item.xp.plugin.DuplicateFieldNameException import no.item.xp.plugin.extensions.getChildNodesAtXPath import no.item.xp.plugin.extensions.getFormNode import no.item.xp.plugin.models.MixinDependencyModel @@ -82,7 +83,11 @@ private fun walkMixinGraph( interfaceModel } - return parseObjectTypeModel(mixin.node, mixin.name, dependentOnMixins).getOrNull() + try { + return parseObjectTypeModel(mixin.node, mixin.name, dependentOnMixins).getOrNull() + } catch (e: DuplicateFieldNameException) { + throw e.withSource("site/mixins/${mixin.name}/${mixin.name}.xml") + } } fun parseMixinDependencyModel( diff --git a/src/test/kotlin/no/item/xp/plugin/parser/ParseDuplicateFieldNameTest.kt b/src/test/kotlin/no/item/xp/plugin/parser/ParseDuplicateFieldNameTest.kt new file mode 100644 index 0000000..c84fb1c --- /dev/null +++ b/src/test/kotlin/no/item/xp/plugin/parser/ParseDuplicateFieldNameTest.kt @@ -0,0 +1,129 @@ +package no.item.xp.plugin.parser + +import no.item.xp.plugin.DuplicateFieldNameException +import no.item.xp.plugin.extensions.getChildNodeAtXPath +import no.item.xp.plugin.models.ObjectTypeModel +import no.item.xp.plugin.models.StringField +import no.item.xp.plugin.stringToXMLDocument +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class ParseDuplicateFieldNameTest { + private val mixins = listOf(ObjectTypeModel("intro", listOf(StringField("intro", "Intro", true, false)))) + + private fun parseForm(xml: String) = + parseObjectTypeModel(stringToXMLDocument(xml).getChildNodeAtXPath("content-type/form")!!, "article", mixins) + + @Test + fun `fail if mixin contains field with same name`() { + // language=XML + val xml = + """ + +
+ + + + + +
+ """ + + val exception = assertFailsWith { parseForm(xml) } + + assertEquals(listOf("intro"), exception.fieldNames) + } + + @Test + fun `fail if field-set contains field with same name`() { + // language=XML + val xml = + """ + +
+ + + + + + + + + + +
+
+ """ + + val exception = assertFailsWith { parseForm(xml) } + + assertEquals(listOf("title"), exception.fieldNames) + } + + @Test + fun `allow same field name in different objects`() { + // language=XML + val xml = + """ + +
+ + + + + + + + + + + +
+
+ """ + + assertEquals(3, parseForm(xml).getOrNull()?.fields?.size) + } + + @Test + fun `fail with name of mixin that contains duplicates`() { + // language=XML + val xml = + """ + +
+ + + +
+ """ + + // language=XML + val xml2 = + """ + +
+ +
+
+ """ + + val mixinDependencies = + mapOf("aa" to xml, "bb" to xml2) + .map { (name, xml) -> parseMixinDependencyModel(stringToXMLDocument(xml).getChildNodeAtXPath("mixin/form")!!, name) } + + val exception = assertFailsWith { parseMixin(mixinDependencies.first(), mixinDependencies) } + + assertEquals( + "Duplicate field name \"intro\" in \"site/mixins/aa/aa.xml\". A field name can only be used once in the same object.", + exception.message, + ) + } +}