From d595d639bb82166a46797f2de600d09d3107a544 Mon Sep 17 00:00:00 2001 From: Ben Bader Date: Thu, 8 Dec 2022 12:07:45 -0800 Subject: [PATCH] Support omit and emit of field default values in struct constants (#507) --- .../ThriftyCodeGenerator.kt | 2 + .../thrifty/gen/ThriftyCodeGeneratorTest.kt | 24 ++++++++++ .../thrifty/kgen/KotlinCodeGenerator.kt | 7 +-- .../thrifty/kgen/KotlinCodeGeneratorTest.kt | 48 +++++++++++++++++++ .../com/microsoft/thrifty/schema/Constant.kt | 5 +- .../microsoft/thrifty/schema/LoaderTest.kt | 44 +++++++++++++++++ 6 files changed, 125 insertions(+), 5 deletions(-) diff --git a/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt b/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt index acffa04..86aa090 100644 --- a/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt +++ b/thrifty-java-codegen/src/main/kotlin/com/microsoft.thrifty.gen/ThriftyCodeGenerator.kt @@ -961,6 +961,8 @@ class ThriftyCodeGenerator { constant.type.trueType, cve, needsDeclaration = false) + + hasStaticInit.set(true) } } diff --git a/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt b/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt index 97c4986..61e1a55 100644 --- a/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt +++ b/thrifty-java-codegen/src/test/kotlin/com/microsoft/thrifty/gen/ThriftyCodeGeneratorTest.kt @@ -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 { val schema = parse(filename, text) val gen = ThriftyCodeGenerator(schema).emitFileComment(false) diff --git a/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt b/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt index e821da8..b1a557f 100644 --- a/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt +++ b/thrifty-kotlin-codegen/src/main/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt @@ -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]) } } @@ -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 } diff --git a/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt b/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt index a94ef72..6a6fe01 100644 --- a/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt +++ b/thrifty-kotlin-codegen/src/test/kotlin/com/microsoft/thrifty/kgen/KotlinCodeGeneratorTest.kt @@ -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 = """ diff --git a/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt b/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt index 7000cd7..eb79036 100644 --- a/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt +++ b/thrifty-schema/src/main/kotlin/com/microsoft/thrifty/schema/Constant.kt @@ -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 { diff --git a/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt b/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt index bca5b6e..227af13 100644 --- a/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt +++ b/thrifty-schema/src/test/kotlin/com/microsoft/thrifty/schema/LoaderTest.kt @@ -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 @@ -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 { 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)