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

marcospereira · 2023-10-05T18:01:32Z

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
        );
    }

}

marcospereira

A few comments and questions.

marcospereira · 2023-10-05T18:03:17Z

jte-models/src/main/java/gg/jte/models/generator/ModelConfig.java

+        try {
+            language = Language.valueOf(configuredLanguage);
+        } catch (IllegalArgumentException ex) {
+            // how to report wrong conciguration? fail or default to Java?


@casid, I'm unsure how (or even if) to do this. It is an open question for this PR.

In my opinion you should fail the build in this case. Building Java when a different language was specified is not what the user wants.

I think throwing an exception with a good description would be good enough, but you should test that.

Done in 440c1f0.

marcospereira · 2023-10-05T18:03:48Z

jte-models/src/main/java/gg/jte/models/generator/ModelExtension.java

@@ -21,7 +21,7 @@ public class ModelExtension implements JteExtension {

    @Override
    public String name() {
-        return "Generate type-safe model facade for templates";
+        return "Generate type-safe model facade for templates in Java";


I'm also unsure about the impact of changing this.

The name isn't actually used anywhere at the moment.

marcospereira · 2023-10-05T18:04:47Z

jte-models/src/main/java/gg/jte/models/generator/ModelGenerator.java

@@ -23,17 +23,27 @@ public class ModelGenerator {
    private final String targetClassName;
    private final String interfaceName;

+    private final Language language;
+
    public ModelGenerator(TemplateEngine engine, String templateSubDirectory, String targetClassName, String interfaceName) {


I kept this constructor for compatibility reasons. But it may be unnecessary now since it is not used in the code base.

ModelGenerator is internal implementation, so it is not necessary to keep the constructor overload.

Removed in aac2a43.

edward3h · 2023-10-05T19:30:59Z

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 · 2023-10-06T01:40:37Z

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 · 2023-10-06T02:20:41Z

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

edward3h · 2023-10-06T12:42:06Z

Looks good. Thank you for your contribution.

codecov · 2023-10-06T18:07:48Z

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

@@             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	Coverage Δ
...rc/main/java/gg/jte/models/generator/Language.java	`100.00% <100.00%> (ø)`
...main/java/gg/jte/models/generator/ModelConfig.java	`78.94% <100.00%> (+9.71%)`	⬆️
...n/java/gg/jte/models/generator/ModelGenerator.java	`93.10% <100.00%> (+0.79%)`	⬆️
...ls/src/main/java/gg/jte/models/generator/Util.java	`61.90% <100.00%> (+1.90%)`	⬆️
...n/java/gg/jte/models/generator/ModelExtension.java	`81.25% <50.00%> (+14.58%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

casid · 2023-10-06T18:13:48Z

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 · 2023-10-06T18:35:16Z

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 · 2023-10-06T18:37:36Z

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

marcospereira · 2023-10-06T18:42:24Z

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 · 2023-10-06T18:49:38Z

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 · 2023-10-06T18:55:22Z

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 · 2023-10-07T04:48:28Z

@marcospereira I published version 3.1.2 which contains this feature!

jte-models: Add support to generate Kotlin code

4da6b5f

marcospereira commented Oct 5, 2023

View reviewed changes

marcospereira added 2 commits October 5, 2023 22:18

Remove unused constructor

aac2a43

Report configuration error when language is not supported

440c1f0

marcospereira added 2 commits October 6, 2023 14:32

Rename Language enum values to follow project camel case convention

a059cd3

Language configuration is now case sensitive

1888f90

casid merged commit 64786c0 into casid:main Oct 6, 2023

marcospereira mentioned this pull request Oct 6, 2023

jte-models: Update docs with correct language values #283

Merged

marcospereira mentioned this pull request Oct 10, 2023

KTE: Generate templates using default parameter values #285

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

marcospereira commented Oct 5, 2023 •

edited

Loading

marcospereira left a comment

marcospereira Oct 5, 2023

edward3h Oct 5, 2023

marcospereira Oct 6, 2023

marcospereira Oct 5, 2023

edward3h Oct 5, 2023

marcospereira Oct 5, 2023

edward3h Oct 5, 2023

marcospereira Oct 6, 2023

edward3h commented Oct 5, 2023

marcospereira commented Oct 6, 2023

marcospereira commented Oct 6, 2023

edward3h commented Oct 6, 2023

codecov bot commented Oct 6, 2023 •

edited

Loading

casid commented Oct 6, 2023

marcospereira commented Oct 6, 2023

casid commented Oct 6, 2023

marcospereira commented Oct 6, 2023

casid commented Oct 6, 2023

marcospereira commented Oct 6, 2023

casid commented Oct 7, 2023

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

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

Conversation

marcospereira commented Oct 5, 2023 • edited Loading

What?

How

Generate code

marcospereira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edward3h commented Oct 5, 2023

marcospereira commented Oct 6, 2023

marcospereira commented Oct 6, 2023

edward3h commented Oct 6, 2023

codecov bot commented Oct 6, 2023 • edited Loading

Codecov Report

casid commented Oct 6, 2023

marcospereira commented Oct 6, 2023

casid commented Oct 6, 2023

marcospereira commented Oct 6, 2023

casid commented Oct 6, 2023

marcospereira commented Oct 6, 2023

casid commented Oct 7, 2023

marcospereira commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 6, 2023 •

edited

Loading