Port TypeScript code generator to Kotlin. #431

hokeun · 2021-07-26T05:00:39Z

No description provided.

…ator to Kotlin

… Kotlin

…java.

> Task :org.lflang:compileKotlin e: /Users/hokeunkim/Development/lingua-franca-intellij/org.lflang/src/org/lflang/generator/ts/TsGenerator.kt: (223, 30): Unresolved reference: LinkedList

… __GenDelay. (with prefix "<", separator ", ", suffix ">") Also fix off indentation for reactEpilogue.

… location, proto file import, and compilation.

lhstrh

Great progress!

org.lflang/src/org/lflang/generator/LFGenerator.java

org.lflang/src/org/lflang/generator/ts/TsFileConfig.kt

org.lflang/src/org/lflang/generator/ts/TsGenerator.kt

org.lflang/src/org/lflang/generator/ts/TsReactorGenerator.kt

Soroosh129 · 2021-07-26T05:33:01Z

org.lflang/src/org/lflang/generator/ts/TsGenerator.kt

+        for (federate in federates) {
+            var tsFileName = fileConfig.name
+            if (isFederated) {
+                tsFileName += '_' + federate.name


There is a FedFileConfig class that might be useful here. Here is an example of its use in the C target. The behavior in C target is to put each federate into its own folder.

Thanks for the suggestion. I believe the federated execution is not supported yet in TypeScript and I'm planning to fix that later. I added a TODO to follow up. Would this be ok with you for now?

1) Merge create methods for FileConfigs and Generators 2) Fix comment formatting 3) Add TODOs for the comments to be followed up.

lhstrh · 2021-07-26T06:16:35Z

org.lflang/src/org/lflang/generator/LFGenerator.java

+        String classNamePrefix;
+        String targetName;
+        switch (target) {
+            case CPP: {


If I'm not mistaken this info can be read straight from the Target enum. If not, it probably makes sense to store it there.

I removed targetName, which is for the error messages.

I could toLowerCase and some string operations for package name and class name prefix; however, I think explicitly assigning the values makes the code more readable, and I'm afraid that for other languages, the relationship between enum and these names can be arbitrary.

Would this make sense to you?

I think it's better to have fewer strings floating around and strive for uniformity. For the package name, toLower of the name() sounds appropriate. When it comes to the class name prefix, I guess the toString() could work? We have both TypeScript and TS floating around in the code base, and the Kotlin classes now use Ts; this is a perfect example of lack of uniformity that we can solve by adopting some conventions; the Target enum is the place to encode them.

Got it. I did that. Thanks for the explanation.

…, so it will be "CPP" and "TS" for CPP and TS respectively.

… Kotlin from the target enum.

…tlin from the target enum as well.

hokeun · 2021-07-26T19:43:15Z

Thanks, Marten and Soroush, for reviewing my pull request! I'll be merging this pull request to master as it seems that your comments are all either addressed or marked to be followed up. Let me know if you still have comments. I'll be happy to address them soon!

oowekyala

Just a few minor comments about Kotlin code style and idioms. Most of my comments can be fixed automatically in Intellij, for instance:

This is why I think it's cheap to fix them to have a more idiomatic code style.

org.lflang/src/org/lflang/generator/ts/TSReactorGenerator.kt

oowekyala · 2021-07-27T12:41:30Z

org.lflang/src/org/lflang/generator/ts/TSReactorGenerator.kt

+        // Next handle timers.
+        for (timer in reactor.timers) {
+            var timerPeriod: String
+            if (timer.period === null) {


This is a weird way to put it. You don't need a var. You could initialize the variable like so:

val timerPeriod: String = if (timer.period == null) "0" else getTargetValue(timer.period)

or equivalently

val timerPeriod = timer.period?.let { getTargetValue(it) } ?: "0"

Same for the other vars.

oowekyala · 2021-07-27T12:44:11Z

org.lflang/src/org/lflang/generator/ts/TSReactorGenerator.kt

+    private fun pr(builder: StringBuilder, text: Any) = tsGenerator.prw(builder, text)
+    private fun pr(text: Any) = tsGenerator.prw(code, text)
+
+    private fun getTargetValue(v: Value): String = tsGenerator.getTargetValueW(v)


It would be more idiomatic to use extensions for those methods. You can write

private fun Value.getTargetValue(): String = tsGenerator.getTargetValueW(this)

With this style, in my above comment,

val timerPeriod = timer.period?.let { getTargetValue(it) } ?: "0"

can be rewritten to

val timerPeriod = timer.period?.getTargetValue() ?: "0"

I tried as you commented, but it doesn't compile. Maybe I'm missing something?

Look at the edit to my comment. If that doesn't work please post the compiler error

Thanks, done in #444

oowekyala · 2021-10-12T19:30:54Z

I think since this PR, the old TypeScriptGenerator.xtend has never been deleted. @hokeun can we delete it or is it still used somewhere?

https://github.com/lf-lang/lingua-franca/blob/master/org.lflang/src/org/lflang/generator/TypeScriptGenerator.xtend

hokeun · 2021-10-12T20:51:14Z

I think since this PR, the old TypeScriptGenerator.xtend has never been deleted. @hokeun can we delete it or is it still used somewhere?

https://github.com/lf-lang/lingua-franca/blob/master/org.lflang/src/org/lflang/generator/TypeScriptGenerator.xtend

Thanks, Clément. I postponed the deletion for a bit until we feel confident that we don't really need it anymore. I think it's a good time to actually delete this. I'll create a PR for deleting these.

hokeun added 12 commits July 24, 2021 17:22

Port preamble and parameter generation parts of TypeScript code gener…

512d568

…ator to Kotlin

Port reactor skeleton generation part of TypeScript code generator to…

6833440

… Kotlin

Port reaction generation part of TypeScript code generator to Kotlin.

f22b4e0

Port runtime part of TypeScript code generation to Kotlin.

e2cde0d

Fix a few bugs in TypeScript reactor generation.

661032b

Kotlin code clean up.

032400a

Add build and compile comments for Kotlin TypeScript generator.

acf6bd9

Switch TypeScript code generator from xtext to Kotlin in LFGenerator.…

89955f6

…java.

Fix compile error:

7e35870

> Task :org.lflang:compileKotlin e: /Users/hokeunkim/Development/lingua-franca-intellij/org.lflang/src/org/lflang/generator/ts/TsGenerator.kt: (223, 30): Unresolved reference: LinkedList

Fix error in code generation for reactor's typeParams for example, in…

d8e5b5d

… __GenDelay. (with prefix "<", separator ", ", suffix ">") Also fix off indentation for reactEpilogue.

Fix errors in list and reference type code generation.

04f8c9a

Fix errors in ProtoNoPacking.lf test, including proto file generation…

6afcb59

… location, proto file import, and compilation.

lhstrh approved these changes Jul 26, 2021

View reviewed changes

Soroosh129 reviewed Jul 26, 2021

View reviewed changes

hokeun self-assigned this Jul 26, 2021

Address reviewers' comments:

9f3114a

1) Merge create methods for FileConfigs and Generators 2) Fix comment formatting 3) Add TODOs for the comments to be followed up.

lhstrh reviewed Jul 26, 2021

View reviewed changes

hokeun added 2 commits July 25, 2021 23:21

Fix error in createFileConfig for targets other than CPP and TS.

7ea1942

Let error messages read the enum directly. (toString() will be called…

bd6d6db

…, so it will be "CPP" and "TS" for CPP and TS respectively.

hokeun changed the title ~~Ts kotlin~~ Port TypeScript code generator to Kotlin. Jul 26, 2021

hokeun added 3 commits July 26, 2021 00:16

Retrieve package name and class name prefix for the code generator in…

2c50c49

… Kotlin from the target enum.

Retrieve package name and class name prefix for the file config in Ko…

69778b4

…tlin from the target enum as well.

Change class name prefixes for TypeScript target from Ts to TS.

c77beff

hokeun closed this Jul 26, 2021

hokeun reopened this Jul 26, 2021

hokeun merged commit a5da2a4 into master Jul 26, 2021

oowekyala reviewed Jul 27, 2021

View reviewed changes

hokeun mentioned this pull request Oct 12, 2021

Delete TypeScript Generator in xtend as it's fully migrated to Kotlin and not used anymore. #601

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port TypeScript code generator to Kotlin. #431

Port TypeScript code generator to Kotlin. #431

hokeun commented Jul 26, 2021

lhstrh left a comment

Soroosh129 Jul 26, 2021

hokeun Jul 26, 2021

lhstrh Jul 26, 2021

hokeun Jul 26, 2021

lhstrh Jul 26, 2021

hokeun Jul 26, 2021

hokeun commented Jul 26, 2021

oowekyala left a comment

oowekyala Jul 27, 2021

oowekyala Jul 27, 2021 •

edited

hokeun Jul 30, 2021

oowekyala Jul 30, 2021

hokeun Jul 31, 2021

oowekyala commented Oct 12, 2021

hokeun commented Oct 12, 2021

Port TypeScript code generator to Kotlin. #431

Port TypeScript code generator to Kotlin. #431

Conversation

hokeun commented Jul 26, 2021

lhstrh 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

hokeun commented Jul 26, 2021

oowekyala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oowekyala Jul 27, 2021 • edited

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

oowekyala commented Oct 12, 2021

hokeun commented Oct 12, 2021

oowekyala Jul 27, 2021 •

edited