Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code cleanup for Kotlin-based TypeScript generator (Comments from code review 21-07-29) #444

Merged
merged 9 commits into from
Aug 2, 2021
Merged

Code cleanup for Kotlin-based TypeScript generator (Comments from code review 21-07-29) #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
85c94a4
adding comments from todays code review
cmnrd Jul 29, 2021
3ab49f3
Address comments in TypeScript code generation from the code review, …
hokeun Jul 30, 2021
834e9fe
Remove """ when unnecessary.
hokeun Jul 30, 2021
6cfbc78
Change ===, !== to == and != in kotlin.
hokeun Jul 30, 2021
2806256
More code cleanup for TypeScript generator in Kotlin.
hokeun Jul 30, 2021
ac9e06f
Merge branch 'master' into code-review-21-07-29
hokeun Jul 31, 2021
0119cd9
Fix errors after merging with master.
hokeun Jul 31, 2021
d660d67
Use Class.method formats for Class to String conversion methods.
hokeun Jul 31, 2021
11ba609
Remove default from switch statement.
hokeun Aug 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions org.lflang/src/org/lflang/Target.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public enum Target {
)
),
CCPP("CCpp", true, Target.C.keywords),
CPP("Cpp", true, Arrays.asList(
CPP("Cpp", true, "cpp", "Cpp", Arrays.asList(
// List via: https://en.cppreference.com/w/cpp/keyword
"alignas", // (since C++11)
"alignof", // (since C++11)
Expand Down Expand Up @@ -190,7 +190,7 @@ public enum Target {
"xor_eq"
)
),
TS("TypeScript", false, Arrays.asList(
TS("TypeScript", false, "ts", "TS", Arrays.asList(
// List via: https://github.com/Microsoft/TypeScript/issues/2536
// Reserved words
"break",
Expand Down Expand Up @@ -349,6 +349,16 @@ public enum Target {
*/
private final String description;

/**
* Name of package containing Kotlin classes for the target language.
*/
public final String packageName;

/**
* Prefix of names of Kotlin classes for the target language.
*/
public final String classNamePrefix;

/**
* Whether or not this target requires types.
*/
Expand All @@ -364,17 +374,31 @@ public enum Target {
*/
public final static Target[] ALL = Target.values();


/**
* Private constructor for targets.
*
* @param name String representation of this target.
* @param description String representation of this target.
* @param requiresTypes Types Whether this target requires type annotations or not.
* @param packageName Name of package containing Kotlin classes for the target language.
* @param classNamePrefix Prefix of names of Kotlin classes for the target language.
* @param keywords List of reserved strings in the target language.
*/
Target(String description, boolean requiresTypes, List<String> keywords) {
Target(String description, boolean requiresTypes, String packageName,
String classNamePrefix, List<String> keywords) {
this.description = description;
this.requiresTypes = requiresTypes;
this.keywords = keywords;
this.packageName = packageName;
this.classNamePrefix = classNamePrefix;
}


/**
* Private constructor for targets without pakcageName and classNamePrefix.
*/
Target(String description, boolean requiresTypes, List<String> keywords) {
this(description, requiresTypes, "N/A", "N/A", keywords);
}


Expand Down
42 changes: 11 additions & 31 deletions org.lflang/src/org/lflang/generator/LFGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,6 @@ public class LFGenerator extends AbstractGenerator {
// Indicator of whether generator errors occurred.
protected boolean generatorErrorsOccurred = false;


private String getPackageName(Target target) {
switch (target) {
case CPP: return "cpp";
case TS: return "ts";
default:
throw new IllegalArgumentException("Unexpected target '" + target + "'");
}
}

private String getClassNamePrefix(Target target) {
switch (target) {
case CPP: return "Cpp";
case TS: return "TS";
default:
throw new IllegalArgumentException("Unexpected target '" + target + "'");
}
}

/**
* Create a target-specific FileConfig object in Kotlin
*
Expand Down Expand Up @@ -79,16 +60,11 @@ private FileConfig createFileConfig(final Target target,
switch (target) {
case CPP:
case TS: {
// These targets use kotlin
String packageName = getPackageName(target);
String classNamePrefix = getClassNamePrefix(target);
String className = "org.lflang.generator." + packageName + "." + classNamePrefix + "FileConfig";
String className = "org.lflang.generator." + target.packageName + "." + target.classNamePrefix + "FileConfig";
try {

return (FileConfig) Class.forName(className)
.getDeclaredConstructor(Resource.class, IFileSystemAccess2.class, IGeneratorContext.class)
.newInstance(resource, fsa, context);

} catch (InvocationTargetException e) {
throw new RuntimeException("Exception instantiating " + className, e.getTargetException());
} catch (ReflectiveOperationException e) {
Expand All @@ -108,9 +84,12 @@ private GeneratorBase createGenerator(Target target, FileConfig fileConfig,
case C: return new CGenerator(fileConfig, errorReporter);
case CCPP: return new CCppGenerator(fileConfig, errorReporter);
case Python: return new PythonGenerator(fileConfig, errorReporter);
default:
return createKotlinGenerator(target, fileConfig, errorReporter);
case CPP:
case TS:
return createKotlinBaseGenerator(target, fileConfig, errorReporter);
}
// If no case matched, then throw a runtime exception.
throw new RuntimeException("Unexpected target!");
}

/**
Expand All @@ -125,13 +104,13 @@ private GeneratorBase createGenerator(Target target, FileConfig fileConfig,
*
* @return A Kotlin Generator object if the class can be found
*/
private GeneratorBase createKotlinGenerator(Target target, FileConfig fileConfig,
private GeneratorBase createKotlinBaseGenerator(Target target, FileConfig fileConfig,
ErrorReporter errorReporter) {
// Since our Eclipse Plugin uses code injection via guice, we need to
// play a few tricks here so that Kotlin FileConfig and
// Kotlin Generator do not appear as an import. Instead we look the
// class up at runtime and instantiate it if found.
String classPrefix = "org.lflang.generator." + getPackageName(target) + "." + getClassNamePrefix(target);
String classPrefix = "org.lflang.generator." + target.packageName + "." + target.classNamePrefix;
try {
Class<?> generatorClass = Class.forName(classPrefix + "Generator");
Class<?> fileConfigClass = Class.forName(classPrefix + "FileConfig");
Expand All @@ -151,8 +130,9 @@ private GeneratorBase createKotlinGenerator(Target target, FileConfig fileConfig
+ "Eclipse. The " + target + " code generator is written in Kotlin"
+ "and, unfortunately, the Eclipse Kotlin plugin is "
+ "broken, preventing us from loading the generator"
+ "properly. Please consider building the RCA via Maven.");
// FIXME: Add a link to the wiki with more information.
+ "properly. Please consider building the RCA via Maven.\n"
+ "For instructions building RCA via Maven, see: "
+ "https://github.com/icyphy/lingua-franca/wiki/Running-Lingua-Franca-IDE-%28Epoch%29-with-Kotlin-based-Code-Generators-Enabled-%28without-Eclipse-Environment%29");
return null;
}
}
Expand Down
2 changes: 1 addition & 1 deletion org.lflang/src/org/lflang/generator/cpp/CppExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ val TriggerRef.name: String
/** Return a comment to be inserted at the top of generated files. */
fun fileComment(r: Resource) = """
/*
* This file was autogenerated by the Lingua Franca Compiler
* This file was autogenerated by the Lingua Franca Compiler.
*
* Source: ${r.uri}
* Date: ${LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"))}
Expand Down
55 changes: 28 additions & 27 deletions org.lflang/src/org/lflang/generator/ts/TSGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class TSGenerator(
tsCode.append(preambleGenerator.generatePreamble())

val parameterGenerator = TSParameterGenerator(this, fileConfig, targetConfig, reactors)
val (mainParameters, parameterCode) = parameterGenerator.generatePrameters()
val (mainParameters, parameterCode) = parameterGenerator.generateParameters()
tsCode.append(parameterCode)

val reactorGenerator = TSReactorGenerator(this, errorReporter)
Expand All @@ -189,31 +189,31 @@ class TSGenerator(
"pnpm",
listOf("install"),
fileConfig.srcGenPkgPath,
false // only produce a warning if command is not found
false // only produce a warning if command is not found
)

// Attempt to use pnpm, but fall back on npm if it is not available.
if (pnpmInstall !== null) {
if (pnpmInstall != null) {
val ret = pnpmInstall.run()
if (ret !== 0) {
if (ret != 0) {
errorReporter.reportError(findTarget(resource),
"ERROR: pnpm install command failed: " + pnpmInstall.errors.toString())
}
} else {
errorReporter.reportWarning(
"Falling back on npm. To prevent an accumulation of replicated dependencies, " +
"it is highly recommended to install pnpm globally (npm install -g pnpm).")
val npmInstall = commandFactory.createCommand("npm", listOf("install"), fileConfig.getSrcGenPkgPath())
val npmInstall = commandFactory.createCommand("npm", listOf("install"), fileConfig.srcGenPkgPath)

if (npmInstall === null) {
if (npmInstall == null) {
errorReporter.reportError(
"The TypeScript target requires npm >= 6.14.4. " +
"For installation instructions, see: https://www.npmjs.com/get-npm. \n" +
"Auto-compiling can be disabled using the \"no-compile: true\" target property.")
return
}

if (npmInstall.run() !== 0) {
if (npmInstall.run() != 0) {
errorReporter.reportError(findTarget(resource),
"ERROR: npm install command failed: " + npmInstall.errors.toString())
errorReporter.reportError(findTarget(resource), "ERROR: npm install command failed." +
Expand All @@ -237,14 +237,15 @@ class TSGenerator(

protocArgs.addAll(
listOf(
"--plugin=protoc-gen-ts=" + tsFileConfig.getSrcGenPkgPath().resolve("node_modules").resolve(".bin").resolve("protoc-gen-ts"),
"--js_out=import_style=commonjs,binary:"+tsOutPath,
"--ts_out=" + tsOutPath)
"--plugin=protoc-gen-ts=" + tsFileConfig.srcGenPkgPath.resolve("node_modules").resolve(".bin").resolve("protoc-gen-ts"),
"--js_out=import_style=commonjs,binary:$tsOutPath",
"--ts_out=$tsOutPath"
)
)
protocArgs.addAll(targetConfig.protoFiles)
val protoc = commandFactory.createCommand("protoc", protocArgs, tsFileConfig.srcPath)

if (protoc === null) {
if (protoc == null) {
errorReporter.reportError("Processing .proto files requires libprotoc >= 3.6.1")
return
}
Expand All @@ -260,7 +261,7 @@ class TSGenerator(
// targetConfig.compileLibraries.add('-l')
// targetConfig.compileLibraries.add('protobuf-c')
} else {
errorReporter.reportError("protoc returns error code " + returnCode)
errorReporter.reportError("protoc returns error code $returnCode")
}
// FIXME: report errors from this command.
} else {
Expand All @@ -272,8 +273,8 @@ class TSGenerator(

// Invoke the compiler on the generated code.
println("Type Checking")
val tsc = commandFactory.createCommand("npm", listOf("run", "check-types"), fileConfig.getSrcGenPkgPath())
if (tsc === null) {
val tsc = commandFactory.createCommand("npm", listOf("run", "check-types"), fileConfig.srcGenPkgPath)
if (tsc == null) {
errorReporter.reportError(errorMessage);
return
}
Expand All @@ -284,9 +285,9 @@ class TSGenerator(
//val babelPath = codeGenConfig.outPath + File.separator + "node_modules" + File.separator + ".bin" + File.separator + "babel"
// Working command $./node_modules/.bin/babel src-gen --out-dir js --extensions '.ts,.tsx'
println("Compiling")
val babel = commandFactory.createCommand("npm", listOf("run", "build"), fileConfig.getSrcGenPkgPath())
val babel = commandFactory.createCommand("npm", listOf("run", "build"), fileConfig.srcGenPkgPath)

if (babel === null) {
if (babel == null) {
errorReporter.reportError(errorMessage);
return
}
Expand Down Expand Up @@ -325,10 +326,10 @@ class TSGenerator(
* @return The TS type.
*/
private fun getActionType(action: Action): String {
if (action.type !== null) {
return getTargetType(action.type)
return if (action.type != null) {
getTargetType(action.type)
} else {
return "Present"
"Present"
}
}

Expand All @@ -339,7 +340,7 @@ class TSGenerator(
*/
override fun timeInTargetLanguage(value: TimeValue): String {
return if (value.unit != TimeUnit.NONE) {
"""TimeValue.${value.unit}(${value.time})"""
"TimeValue.${value.unit}(${value.time})"
} else {
// The value must be zero.
"TimeValue.zero()"
Expand All @@ -349,14 +350,14 @@ class TSGenerator(
override fun getTargetType(s: StateVar): String {
val type = super.getTargetType(s)
return if (!isInitialized(s)) {
type + " | undefined"
"$type | undefined"
} else {
type
}
}

override fun getTargetReference(param: Parameter): String {
return """this.${param.name}.get()"""
return "this.${param.name}.get()"
}

/**
Expand Down Expand Up @@ -398,15 +399,15 @@ class TSGenerator(

// Virtual methods.
override fun generateDelayBody(action: Action, port: VarRef): String {
return """actions.${action.name}.schedule(0, ${generateVarRef(port)} as ${getActionType(action)});"""
return "actions.${action.name}.schedule(0, ${generateVarRef(port)} as ${getActionType(action)});"
}

override fun generateForwardBody(action: Action, port: VarRef): String {
return """${generateVarRef(port)} = ${action.name} as ${getActionType(action)};"""
return "${generateVarRef(port)} = ${action.name} as ${getActionType(action)};"
}

override fun generateDelayGeneric(): String {
return """T extends Present"""
return "T extends Present"
}

override fun supportsGenerics(): Boolean {
Expand All @@ -430,11 +431,11 @@ class TSGenerator(
}

override fun getTargetFixedSizeListType(baseType: String, size: Int): String {
return """Array(${size})<${baseType}>"""
return "Array(${size})<${baseType}>"
}

override fun getTargetVariableSizeListType(baseType: String): String {
return """Array<${baseType}>"""
return "Array<${baseType}>"
}

override fun getTarget(): Target {
Expand Down
Loading