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.

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

Support omit and emit of field default values in struct constants #507

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,8 @@ class ThriftyCodeGenerator {
constant.type.trueType,
cve,
needsDeclaration = false)

hasStaticInit.set(true)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,30 @@ class ThriftyCodeGeneratorTest {
java shouldContain "public Builder(@NonNull Foo struct)"
}

@Test
fun structConstWithDefaultValueInField() {
val thrift = """
|namespace java test.struct_const.default_fields
|
|struct Foo {
| 1: required string text = "FOO";
| 2: required i32 number;
|}
|
|const Foo THE_FOO = {"number": 42}
""".trimMargin()

val file = compile("consts.thrift", thrift).single { it.typeSpec.name == "Constants" }

file.toString() shouldContain """
| static {
| Foo.Builder fooBuilder0 = new Foo.Builder();
| fooBuilder0.number(42);
| THE_FOO = fooBuilder0.build();
| }
""".trimMargin()
}

private fun compile(filename: String, text: String): List<JavaFile> {
val schema = parse(filename, text)
val gen = ThriftyCodeGenerator(schema).emitFileComment(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1888,14 +1888,13 @@ class KotlinCodeGenerator(
block.add("%T(\n⇥", className)

for (field in structType.fields) {
val fieldValue = fieldValues[field.name]
val fieldValue = fieldValues[field.name] ?: field.defaultValue
if (fieldValue != null) {
block.add("%L = ", names[field])
recursivelyRenderConstValue(block, field.type, fieldValue)
block.add(",\n")
} else {
check(!field.required) { "Missing value for required field '${field.name}'" }
// TODO: if there's a default value, support it
block.add("%L = null,\n", names[field])
}
}
Expand All @@ -1908,7 +1907,9 @@ class KotlinCodeGenerator(
for (field in structType.fields) {
val fieldValue = fieldValues[field.name]
if (fieldValue == null) {
check(!field.required) { "Missing value for required field '${field.name}'" }
check(!field.required || field.defaultValue != null) {
"Missing value for required field '${field.name}'"
}
continue
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1369,6 +1369,54 @@ class KotlinCodeGeneratorTest {
lines[1] shouldContain "\\/\\/ Generated on: [0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\\.\\S+".toRegex()
}

@Test
fun `struct const with default field value using builders`() {
val thrift = """
|namespace kt test.struct_const.default_fields
|
|struct Foo {
| 1: required string text = "FOO";
| 2: required i32 number;
|}
|
|const Foo THE_FOO = {"number": 42}
""".trimMargin()

val file = generate(thrift) { withDataClassBuilders() }

file.toString() shouldContain """
|public val THE_FOO: Foo = Foo.Builder().let {
| it.number(42)
| it.build()
| }
""".trimMargin()
}

@Test
fun `struct const with default field value using data classes`() {
val thrift = """
|namespace kt test.struct_const.default_fields
|
|struct Foo {
| 1: required string text = "FOO";
| 2: required i32 number;
|}
|
|const Foo THE_FOO = {"number": 42}
""".trimMargin()

val file = generate(thrift)

println(file)

file.toString() shouldContain """
|public val THE_FOO: Foo = Foo(
| text = "FOO",
| number = 42,
| )
""".trimMargin()
}

@Test
fun `constant reference`() {
val thrift = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,9 @@ class Constant private constructor (
Constant.validate(symbolTable, value, field.type)
}

check(allFields.none { it.value.required }) {
val missingRequiredFieldNames = allFields.filter { it.value.required }.map { it.key }.joinToString(", ")
val missingFields = allFields.values.filter { it.required && it.defaultValue == null }
check(missingFields.isEmpty()) {
val missingRequiredFieldNames = missingFields.joinToString(", ") { it.name }
"Some required fields are unset: $missingRequiredFieldNames"
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package com.microsoft.thrifty.schema

import com.microsoft.thrifty.schema.parser.*
import io.kotest.assertions.throwables.shouldNotThrowAny
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.assertions.throwables.shouldThrowExactly
import io.kotest.assertions.throwables.shouldThrowMessage
Expand Down Expand Up @@ -1292,6 +1293,49 @@ class LoaderTest {
strs.referencedConstants shouldBe listOf(str)
}

@Test
fun `struct constants can omit fields with default values`() {
val baseThrift = """
|namespace java foo;
|
|struct Foo {
| 1: required string TEXT;
| 2: required string TEXT_WITH_DEFAULT = "foo";
| 3: optional string OPTIONAL_TEXT;
|}
""".trimMargin()

val goodThrift = """
|namespace java bar;
|
|include "foo.thrift"
|
|const foo.Foo THE_FOO = {"TEXT": "some text"}
""".trimMargin()

val badThrift = """
|namespace java baz;
|
|include "foo.thrift"
|
|const foo.Foo NOT_THE_FOO = {"OPTIONAL_TEXT": "t"}
""".trimMargin()

val baseFile = File(tempDir, "foo.thrift")
val goodFile = File(tempDir, "good.thrift")
val badFile = File(tempDir, "bad.thrift")

baseFile.writeText(baseThrift)
goodFile.writeText(goodThrift)
badFile.writeText(badThrift)

val err = shouldThrow<LoadFailedException> { load(baseFile, badFile) }
err.message shouldContain "Some required fields are unset"


shouldNotThrowAny { load(baseFile, goodFile) }
}

private fun load(thrift: String): Schema {
val f = File.createTempFile("test", ".thrift", tempDir)
f.writeText(thrift)
Expand Down