search

casid / jte

Secure and speedy templates for Java and Kotlin.
https://jte.gg
Apache License 2.0
819 stars 59 forks source link

jte-models: Add support to generate Kotlin code #282

Closed marcospereira closed 1 year ago

marcospereira commented 1 year ago

What?

This adds support to generate Kotlin code (instead of Java) when using the jte-models extension. There are a few inconveniences when mixing both languages (clashing names such as List, which are different for both, not having to import standard-lib classes in Kotlin that would break the Java generate code), and having this will solve that.

How

Simply by providing a language configuration to the extension. The default is Java, so people upgrading won't have to change anything in their build files. An example:

jte {
    generate()
    binaryStaticContent = true
    jteExtension('gg.jte.models.generator.ModelExtension') {
        language = 'kotlin'
    }
}

Generate code

The generated code looks like this:

@file:Suppress("ktlint")
package test.mytemplates

import gg.jte.models.runtime.*

interface Templates {

    @JteView("hello.jte")
    fun hello(message: java.lang.String): JteModel

}

@file:Suppress("ktlint")
package test.mytemplates

import gg.jte.TemplateEngine
import gg.jte.models.runtime.*

class DynamicTemplates(private val engine: TemplateEngine) : Templates {

    override fun hello(message: java.lang.String): JteModel {
        val paramMap = mapOf(

            "message" to message,
        )
        return DynamicJteModel(engine, "hello.jte", paramMap);
    }

}

@file:Suppress("ktlint")
package test.mytemplates

import gg.jte.models.runtime.*
import gg.jte.ContentType
import gg.jte.TemplateOutput
import gg.jte.html.HtmlInterceptor
import gg.jte.html.HtmlTemplateOutput

class StaticTemplates : Templates {

    override fun hello(message: java.lang.String): JteModel {
        return StaticJteModel<TemplateOutput>(
            ContentType.Plain,
            { output, interceptor -> test.mytemplates.JtehelloGenerated.render(output, interceptor, message) },
            "hello.jte",
            "test.mytemplates",
            test.mytemplates.JtehelloGenerated.JTE_LINE_INFO
        );
    }

}
edward3h commented 1 year ago

Looks good to me. Just one question: since Kotlin can interoperate with Java classes, what is the benefit of generating Kotlin versions of the classes? (I'm not much of a Kotlin user)

marcospereira commented 1 year ago

Looks good to me. Just one question: since Kotlin can interoperate with Java classes, what is the benefit of generating Kotlin versions of the classes? (I'm not much of a Kotlin user)

Interoperability is quite good, but there are places where it fails. For example, if you have a Kte template such as:

@param messages: List<String>

<html lang="pt">
    <head>
        <title>First Page</title>
    </head>
    <body>
        <h1>message</h1>
    </body>
</html>

List is a kotlin.collections.List, and users don't need to import manually. The generated (Java) interface misses the import and is then broken:

package br.ufpe.liber;
import gg.jte.models.runtime.*;

public interface Templates {

    @JteView("index.kte")
    JteModel index(List<String> messages);

}

Compilation error:

Templates.java:8: error: cannot find symbol
    JteModel index(List<String> messages);
                   ^
  symbol:   class List
  location: interface Templates

There are workarounds, but then the user needs to use them constantly, which impacts the developer experience.

marcospereira commented 1 year ago

Thanks for reviewing, @edward3h. I pushed a couple of commits after your feedback. :-)

edward3h commented 1 year ago

Looks good. Thank you for your contribution.

codecov[bot] commented 1 year ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (9230876) 91.09% compared to head (1888f90) 91.13%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #282 +/- ## ============================================ + Coverage 91.09% 91.13% +0.03% - Complexity 1181 1186 +5 ============================================ Files 75 76 +1 Lines 3100 3113 +13 Branches 479 480 +1 ============================================ + Hits 2824 2837 +13 + Misses 169 168 -1 - Partials 107 108 +1 ``` | [Files](https://app.codecov.io/gh/casid/jte/pull/282?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager) | Coverage Δ | | |---|---|---| | [...rc/main/java/gg/jte/models/generator/Language.java](https://app.codecov.io/gh/casid/jte/pull/282?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLW1vZGVscy9zcmMvbWFpbi9qYXZhL2dnL2p0ZS9tb2RlbHMvZ2VuZXJhdG9yL0xhbmd1YWdlLmphdmE=) | `100.00% <100.00%> (ø)` | | | [...main/java/gg/jte/models/generator/ModelConfig.java](https://app.codecov.io/gh/casid/jte/pull/282?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLW1vZGVscy9zcmMvbWFpbi9qYXZhL2dnL2p0ZS9tb2RlbHMvZ2VuZXJhdG9yL01vZGVsQ29uZmlnLmphdmE=) | `78.94% <100.00%> (+9.71%)` | :arrow_up: | | [...n/java/gg/jte/models/generator/ModelGenerator.java](https://app.codecov.io/gh/casid/jte/pull/282?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLW1vZGVscy9zcmMvbWFpbi9qYXZhL2dnL2p0ZS9tb2RlbHMvZ2VuZXJhdG9yL01vZGVsR2VuZXJhdG9yLmphdmE=) | `93.10% <100.00%> (+0.79%)` | :arrow_up: | | [...ls/src/main/java/gg/jte/models/generator/Util.java](https://app.codecov.io/gh/casid/jte/pull/282?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLW1vZGVscy9zcmMvbWFpbi9qYXZhL2dnL2p0ZS9tb2RlbHMvZ2VuZXJhdG9yL1V0aWwuamF2YQ==) | `61.90% <100.00%> (+1.90%)` | :arrow_up: | | [...n/java/gg/jte/models/generator/ModelExtension.java](https://app.codecov.io/gh/casid/jte/pull/282?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager#diff-anRlLW1vZGVscy9zcmMvbWFpbi9qYXZhL2dnL2p0ZS9tb2RlbHMvZ2VuZXJhdG9yL01vZGVsRXh0ZW5zaW9uLmphdmE=) | `81.25% <50.00%> (+14.58%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/casid/jte/pull/282/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andreas+Hager)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

casid commented 1 year ago

Thanks for the contribution @marcospereira and thanks for the review @edward3h.

I know it is against the Java conventions, but so far enum constants in the jte project have been camel case. Could we rename the Language enum values to Java and Kotlin?

I think it is also okay to be case sensitive here when we parse the setting and do a Language.valueOf(configuredLanguage); as you had it initially. This will always be an error message at build time, never at runtime. We probably do not want to support users entering jAvA and the like :-)

Other than that, this looks fantastic!

marcospereira commented 1 year ago

Thanks, @casid.

I know it is against the Java conventions, but so far enum constants in the jte project have been camel case. Could we rename the Language enum values to Java and Kotlin?

Done in a059cd3.

I think it is also okay to be case sensitive here when we parse the setting and do a Language.valueOf(configuredLanguage); as you had it initially. This will always be an error message at build time, never at runtime. We probably do not want to support users entering jAvA and the like :-)

Done in 1888f90.

This should be good to go now. :-)

casid commented 1 year ago

Wow, that was quick! I'll merge this as soon as the CI build is done.

marcospereira commented 1 year ago

Wow, that was quick! I'll merge this as soon as the CI build is done.

Thanks! Also, when can we have a release with those changes?

casid commented 1 year ago

There haven't been any changes since the last release, so there's nothing blocking us from releasing this asap. Will have a look at it this weekend.

marcospereira commented 1 year ago

There haven't been any changes since the last release, so there's nothing blocking us from releasing this asap. Will have a look at it this weekend.

Thank you. No rush here, though. :-)

By the way, I just submitted a follow-up pr to update the docs. See #283. :-)

casid commented 1 year ago

@marcospereira I published version 3.1.2 which contains this feature!