Skip to content

Commit

Permalink
Add HttpTransport, async clients to iOS (#543)
Browse files Browse the repository at this point in the history
This PR makes the newly-added HttpTransport into expect/actual types, with a new implementation for iOS. This in turn required quite a bit of work to set up testing, and uncovered what seems to be a kotlinc type-inference bug that I've worked around here by changing the kotlin code generator a little bit.
  • Loading branch information
benjamin-bader authored Nov 1, 2023
1 parent 188faba commit ee13d65
Show file tree
Hide file tree
Showing 15 changed files with 943 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pre-merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:

- name: Build and test
shell: bash
run: ./gradlew check codeCoverageReport --parallel --no-daemon
run: ./gradlew check codeCoverageReport --parallel --no-daemon -x iosX64Test

- name: Boot simulator
if: matrix.os == 'macos-13'
Expand Down
5 changes: 3 additions & 2 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[versions]
dokka = "1.7.20"
junit = "5.10.0"
kotest = "5.5.4"
kotlin = "1.9.10"
Expand All @@ -8,7 +9,7 @@ okio = "3.3.0"
antlr = "org.antlr:antlr4:4.9.3"
apacheThrift = "org.apache.thrift:libthrift:0.19.0"
clikt = "com.github.ajalt.clikt:clikt:3.1.0"
dokka = "org.jetbrains.dokka:dokka-gradle-plugin:1.7.20"
dokka = { module = "org.jetbrains.dokka:dokka-gradle-plugin", version.ref = "dokka" }
guava = "com.google.guava:guava:31.1-jre"
javaPoet = "com.squareup:javapoet:1.13.0"
kotlin-bom = { module = "org.jetbrains.kotlin:kotlin-bom", version.ref = "kotlin" }
Expand Down Expand Up @@ -37,7 +38,7 @@ kotlin = [ "kotlin-stdlib", "kotlin-reflect" ]
testing = ["junit", "hamcrest", "kotest-assertions-core", "kotest-assertions-coreJvm"]

[plugins]
dokka = "org.jetbrains.dokka:1.7.20"
dokka = { id = "org.jetbrains.dokka", version.ref = "dokka" }
gradlePluginPublish = "com.gradle.plugin-publish:1.2.1"
kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" }
kotlin-mpp = { id = "org.jetbrains.kotlin.multiplatform", version.ref = "kotlin" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,7 @@ class KotlinCodeGenerator(
}

val structTypeName = ClassName(struct.kotlinNamespace, struct.name)
val resultVarName = nameAllocator.newName("resultVar", "resultVar")
val spec = TypeSpec.classBuilder("Builder")
.addSuperinterface(StructBuilder::class.asTypeName().parameterizedBy(structTypeName))
.addProperty(PropertySpec.builder(builderVarName, structTypeName.copy(nullable = true), KModifier.PRIVATE)
Expand All @@ -961,11 +962,20 @@ class KotlinCodeGenerator(
.addFunction(FunSpec.builder("build")
.addModifiers(KModifier.OVERRIDE)
.returns(structTypeName)
.addStatement(
"return %N ?: %M(%S)",
builderVarName,
MemberName("kotlin", "error"),
"Invalid union; at least one value is required")
// We're doing some convoluted stuff to work around an apparent kotlinc bug
// where type inference and/or smart-casting fails. iOS tests got a compiler
// error about the builder var being of type "UnionType.Builder" which clearly
// it isn't. This reformulation seems to work.
.addStatement("val %N = %N", resultVarName, builderVarName)
.beginControlFlow("if (%N == null)", resultVarName)
.addStatement("%M(%S)", MemberName("kotlin", "error"), "Invalid union; at least one value is required")
.endControlFlow()
.addStatement("return %N!!", resultVarName)
// .addStatement(
// "return %N ?: %M(%S)",
// builderVarName,
// MemberName("kotlin", "error"),
// "Invalid union; at least one value is required")
.build())
.addFunction(FunSpec.builder("reset")
.addModifiers(KModifier.OVERRIDE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,13 @@ class KotlinCodeGeneratorTest {
| this.value_ = source
| }
|
| public override fun build(): Union = value_ ?:
| public override fun build(): Union {
| val resultVar = value_
| if (resultVar == null) {
| error("Invalid union; at least one value is required")
| }
| return resultVar!!
| }
|
| public override fun reset(): Unit {
| value_ = null
Expand Down Expand Up @@ -1488,6 +1493,27 @@ class KotlinCodeGeneratorTest {
files.shouldCompile()
}

@Test
fun `union with builder should compile`() {
val thrift = """
|namespace kt test.union
|
|union Union {
| 1: i32 result;
| 2: i64 bigResult;
| 3: string error;
|}
""".trimMargin()

val files = generate(thrift) {
withDataClassBuilders()
coroutineServiceClients()
}
files.shouldCompile()

println(files)
}

private fun generate(thrift: String, config: (KotlinCodeGenerator.() -> KotlinCodeGenerator)? = null): List<FileSpec> {
val configOrDefault = config ?: { emitFileComment(false) }
return KotlinCodeGenerator()
Expand Down
Loading

0 comments on commit ee13d65

Please sign in to comment.