redundent / kotlin-xml-builder

A lightweight type safe builder to build xml documents in Kotlin
Apache License 2.0
155 stars 19 forks source link

DSL generator generates incorrect code #55

Open asm0dey opened 1 year ago

asm0dey commented 1 year ago

Hi there!

the generator generates code like

open class Link(nodeName: String) : Node(nodeName) {
    init {
        xmlns = "http://www.w3.org/2005/Atom"
    }

    var `base`: String? by attributes

but attributes is a read-only map, so setters can't be delegated there. One possible solution is to rewrite it like this:

    var base: String?
        get() = get("base")
        set(value) = set("base", value)

but it is also flawed since does not support attribute namespaces. Probably there should be a map storing attribute-namespace relation and then it could be used like

var base: String?
get() = get("base")
set(value) = set("base", value, nss["base"])
asm0dey commented 1 year ago

I performed some best-effort migration from the plain-text generated code to KotlinPoet, which allowed me to avoid some annoying bugs (for example, in some schemas (atom), base and type were duplicated many-many times and also some functions were duplicated). Tests do not pass, but I believe that the result is equivalent. However, it's up to you to check the correctness. You can see results in my fork repository: https://github.com/asm0dey/kotlin-xml-builder/blob/main/kotlin-xml-dsl-generator/src/main/kotlin/org/redundent/kotlin/xml/gen/DslGenerator.kt

redundent commented 1 year ago

Hi, I've been meaning to switch to using KotlinPoet for a while so thanks for getting that started. I'll take a look at your fork and see how it goes

asm0dey commented 1 year ago

I've also added a dependency on jing because otherwise generate doesn't work with (some) RelaxNG schemas

redundent commented 1 year ago

This is proving to be more of a nightmare than I hoped. Turns out I never published the dsl builder library to Maven Central after jcenter went offline so I'm having to set all that up again.

asm0dey commented 1 year ago

Do you need help? I have experience with it, can help with GH Actions, for example, you'll only need to put your gpg private key, password and id into GH Secrets

redundent commented 1 year ago

No, I can handle it. Thanks for the offer though. I just need to find the time to deal with getting it all sorted. I'm hoping to have some time this weekend to get it going.

asm0dey commented 1 year ago

If anything, here is the example with all the modern best practices: https://github.com/asm0dey/dummylib-multiplatform/blob/main/convention-plugins/src/main/kotlin/module.publication.gradle.kts

Here it's used for a KMP project, but the same approach can be used for JVM.

And here is the deployment action: https://github.com/asm0dey/dummylib-multiplatform/blob/main/.github/workflows/deploy.yml

redundent commented 1 year ago

I just published v1.9.0 of the kotlin-xml-dsl-generator which contains the fix for incorrect code generation. I have a ton of stuff to clean up in the repo and I didn't touch the kotlinpoet changes yet. I will do that soon.

asm0dey commented 1 year ago

Awesome! If you want, I can submit a PR, just briefly describe your desired result

redundent commented 1 year ago

Thanks but I'll take care of it. While fixing the attribute issue, I noticed how much of an embarrassing mess that code was an now I just want to set it on fire and rewrite it all.

asm0dey commented 1 year ago

If you need any help - I'm here for you!